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

*-wrapper; Switch from `infixSalt` to `suffixSalt` #86166

Merged
merged 1 commit into from May 12, 2020

Conversation

@Ericson2314
Copy link
Member

Ericson2314 commented Apr 28, 2020

Motivation for this change

I hate the thing too even though I made it, and rather just get rid of
it. But we can't do that yet. In the meantime, this brings us more
inline with autoconf and will make it slightly easier for me to write a
pkg-config wrapper, which we need.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built stdenv 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"
  • 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.
@Ericson2314 Ericson2314 changed the base branch from master to staging Apr 28, 2020
@Ericson2314 Ericson2314 force-pushed the Ericson2314:suffix-salt branch 4 times, most recently from 6e937f0 to 48263a1 Apr 28, 2020
@ofborg ofborg bot added the 6.topic: stdenv label May 1, 2020
@Ericson2314 Ericson2314 force-pushed the Ericson2314:suffix-salt branch 2 times, most recently from 04ae4ad to 6765930 May 11, 2020
@Ericson2314
Copy link
Member Author

Ericson2314 commented May 11, 2020

OK Linux stdenv builds, but on Darwin building Perl package Locale-gettext fails with:

t/bind.t ....... Can't load '/private/tmp/nix-build-perl5.30.2-gettext-1.07.drv-1/Locale-gettext-1.07/blib/arch/auto/Locale/gettext/gettext.bundle' for module Locale::gettext: dlopen(/private/tmp/nix-build-perl5.30.2-gettext-1.07.drv-1/Locale-gettext-1.07/blib/arch/auto/Locale/gettext/gettext.bundle, 2): Symbol not found: _libintl_bind_textdomain_codeset
  Referenced from: /private/tmp/nix-build-perl5.30.2-gettext-1.07.drv-1/Locale-gettext-1.07/blib/arch/auto/Locale/gettext/gettext.bundle
  Expected in: flat namespace
 in /private/tmp/nix-build-perl5.30.2-gettext-1.07.drv-1/Locale-gettext-1.07/blib/arch/auto/Locale/gettext/gettext.bundle at /nix/store/qjr1xvby7aa4mvy4vqr1lyr8hiwy41b6-perl-5.30.2/lib/perl5/5.30.2/darwin-thread-multi-2level/DynaLoader.pm line 197.
 at t/bind.t line 6.
@Ericson2314 Ericson2314 force-pushed the Ericson2314:suffix-salt branch 3 times, most recently from 1c7a499 to 5a306aa May 12, 2020
I hate the thing too even though I made it, and rather just get rid of
it. But we can't do that yet. In the meantime, this brings us more
inline with autoconf and will make it slightly easier for me to write a
pkg-config wrapper, which we need.
<listitem>
<para>
The cc- and binutils-wrapper's "infix salt" and <literal>_BUILD_</literal> and <literal>_TARGET_</literal> user infixes have been replaced with with a "suffix salt" and suffixes and <literal>_FOR_BUILD</literal> and <literal>_FOR_TARGET</literal>.
This matches the autotools convention for env vars which standard for these things, making interfacing with other tools easier.

This comment has been minimized.

Copy link
@Ericson2314

Ericson2314 May 12, 2020

Author Member

In particular I am about to write a pkg-config wrapper for #86077

configureFlags="--parallel=''${NIX_BUILD_CORES:-1} CC=$BUILD_CC CXX=$BUILD_CXX $configureFlags"
''
# CC_FOR_BUILD and CXX_FOR_BUILD are used to bootstrap cmake
+ ''

This comment has been minimized.

Copy link
@matthewbauer

matthewbauer May 12, 2020

Member

This doesn't need to be split

This comment has been minimized.

Copy link
@Ericson2314

Ericson2314 May 12, 2020

Author Member

No, but I like making the comments not change the hashes, so there's no perverse incentive to not keep the docs up to date.

@Ericson2314 Ericson2314 merged commit a0c003e into NixOS:staging May 12, 2020
14 checks passed
14 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="1ac5398"; rev="1ac5398589916a6a433e845342c9b85c4c52f5dc"; } ./pkgs/t
Details
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="1ac5398"; rev="1ac5398589916a6a433e845342c9b85c4c52f5dc"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="1ac5398"; rev="1ac5398589916a6a433e845342c9b85c4c52f5dc"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="1ac5398"; rev="1ac5398589916a6a433e845342c9b85c4c52f5dc"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="1ac5398"; rev="1ac5398589916a6a433e845342c9b85c4c52f5dc"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="1ac5398"; rev="1ac5398589916a6a433e845342c9b85c4c52f5dc"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="1ac5398"; rev="1ac5398589916a6a433e845342c9b85c4c52f5dc"; } ./pkgs/t
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@Ericson2314 Ericson2314 deleted the Ericson2314:suffix-salt branch May 12, 2020
@Gaelan
Copy link
Contributor

Gaelan commented May 22, 2020

@Ericson2314 this PR broke bind, because its build system expects BUILD_CC to be set when cross compiling. Should we fix that by re-adding BUILD_CC to the cc-wrapper setup hook, or just by manually setting BUILD_CC=$CC_FOR_BUILD in the bind derivation?

@Ericson2314
Copy link
Member Author

Ericson2314 commented May 23, 2020

@Gaelan Thanks for noticing that! Yes do BUILD_CC=$CC_FOR_BUILD, as BUILD_CC is a less-/non-standard thing.

@Gaelan
Copy link
Contributor

Gaelan commented May 24, 2020

Will do. Also, this seems to make cross-compiled texinfoInteractive fail with a patch rejection; I haven’t figured out why this causes that. I’ll investigate more when I’m in front of my computer.

@erictapen
Copy link
Contributor

erictapen commented Jun 15, 2020

This also broke libgcc, but I have no clue why. @Ericson2314 do you have an idea?

erictapen added a commit to erictapen/nixpkgs that referenced this pull request Jun 15, 2020
This is due to a hint by @Ericson2314 in
NixOS#86166 (comment)
@erictapen erictapen mentioned this pull request Jun 15, 2020
2 of 10 tasks complete
puzzlewolf added a commit to puzzlewolf/nixpkgs that referenced this pull request Jun 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.