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.

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

gnat-wrapper/cc-wrapper cleanup and fixes. #98369

Closed
wants to merge 7 commits into from

Conversation

kquick
Copy link
Contributor

@kquick kquick commented Sep 21, 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.

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.

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.
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 kquick changed the title Gnatfix upd gnat-wrapper/cc-wrapper cleanup and fixes. Sep 21, 2020
@kquick kquick requested a review from Lucus16 September 21, 2020 04:51
@Lucus16
Copy link
Contributor

Lucus16 commented Sep 22, 2020

I must admit my understanding of these scripts do not go beyond "it works or it doesn't". I don't see anything problematic. Given you're changing cc-wrapper though, I believe this PR should target staging rather than master.

@kquick
Copy link
Contributor Author

kquick commented Sep 28, 2020

Good point. I've opened #99027 against staging to replace this PR.

@kquick kquick closed this Sep 28, 2020
@Lucus16
Copy link
Contributor

Lucus16 commented Sep 28, 2020 via email

@kquick
Copy link
Contributor Author

kquick commented Sep 28, 2020

How? I looked for a way to do that and didn't find anything, but I would have liked to do that.

@Lucus16
Copy link
Contributor

Lucus16 commented Sep 30, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants