Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

gnat-wrapper/cc-wrapper cleanup and fixes #99027

Draft
wants to merge 7 commits into
base: staging
Choose a base branch
from
Draft

gnat-wrapper/cc-wrapper cleanup and fixes #99027

wants to merge 7 commits into from

Conversation

kquick
Copy link
Contributor

@kquick kquick commented Sep 28, 2020

Motivation for this change

Observation of #98274 which fixed the gnat issue I also encountered led to some additional cleanup and corner-case fixes in both the gnat-wrapper, the cc-wrapper, and the role.bash setup hook.

Re-submitted version of #98369 to build against staging instead of master.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
    • Since this changes the core stdenv wrappers, everything rebuilds. I've run several hundred package rebuilds, along with gcc/gnat builds, but don't have the machine power or time to wait for every package to rebuild. :-)
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@kquick
Quote comment reference to shellcheck to avoid shellcheck error.
e1e2cdc 
The shellcheck utility looks for lines starting with "# shellcheck"
and treats those as directives controlling it's use.  These lines
start that way, but are not actual shellcheck directives, causing it
to emit errors and fail.  This patch adjusts the comments so that
shellcheck does not perceive them as directives.
@kquick
Add additional shellcheck disable directives for valid cc-wrapper code.
8402414
@kquick
Use default value for comparison if variable not set.
df33f79
@kquick
Remove unneeded and unused copy-and-paste from gnat-wrapper.
0c45fb7
@kquick
Update gnat-wrapper determination of extra flags, ensuring defaults.
893651a
@kquick
GCC Ada compilation does not support format hardening flags.
ae5ea37 
These do not break the Ada compilations done with gcc, but they cause
three warnings about unused flags to be emitted with each compilation.
@kquick
Fix setup-hooks role.bash redirection to stderr, not a file named "2".
fd60dea
@kquick kquick mentioned this pull request Sep 28, 2020
Closed
10 tasks
@veprbl veprbl added this to WIP in Staging via automation Nov 22, 2020
@veprbl veprbl moved this from WIP to Needs review in Staging Nov 22, 2020
@FRidh FRidh added this to the 21.03 milestone Jan 4, 2021
@stale
Copy link

stale bot commented Jul 20, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 20, 2021
@Artturin Artturin modified the milestones: 21.05, 23.05 Dec 31, 2022
@stale stale bot removed 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md labels Dec 31, 2022
@stale stale bot removed 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md labels Dec 31, 2022
Artturin
Artturin reviewed Feb 17, 2023
View reviewed changes
pkgs/build-support/setup-hooks/role.bash
@@ -17,7 +17,7 @@ function getRole() {
role_post='_FOR_TARGET'
;;
*)
echo "@name@: used as improper sort of dependency" >2
echo "@name@: used as improper sort of dependency" >&2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

both changes to this file were fixed in 8ed1104

@Artturin Artturin removed this from the 23.05 milestone Feb 17, 2023
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 19, 2024
@wegank wegank marked this pull request as draft March 20, 2024 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Artturin Artturin Artturin left review comments

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314 Ericson2314 is a code owner

@veprbl veprbl Awaiting requested review from veprbl

@peti peti Awaiting requested review from peti

@davidtwco davidtwco Awaiting requested review from davidtwco

@Synthetica9 Synthetica9 Awaiting requested review from Synthetica9

@Lucus16 Lucus16 Awaiting requested review from Lucus16

Assignees
No one assigned
Labels
2.status: merge conflict 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-darwin-stdenv 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ 10.rebuild-linux-stdenv
Projects
Staging
  
Needs review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@kquick @Artturin @FRidh @SuperSandro2000 @wegank