Skip to content

Conversation

@gnprice
Copy link
Contributor

@gnprice gnprice commented Apr 12, 2020

Motivation for this change

This caused none of these flags to have any effect. That's because the configure command looked like this:

./configure --prefix=/nix/store/svhl0fjdj1jl2sqcppy5vnzpfi4gj3d3-gbsplay-2016-12-17 \
    --without-test\ --without-contrib\ --disable-devdsp\ --enable-pulse\ --disable-alsa\ --disable-midi\ --disable-nas\ --disable-dsound\ --disable-i18n

with one giant flag '--without-test --without-contrib...', containing internal spaces.

This can be seen in nix log nixpkgs.gbsplay, in this line:

configure flags: --prefix=/nix/store/svhl0fjdj1jl2sqcppy5vnzpfi4gj3d3-gbsplay-2016-12-17 --without-test\ --without-contrib\ --disable-devdsp\ --enable-pulse\ --disable-alsa\ --disable-midi\ --disable-nas\ --disable-dsound\ --disable-i18n

and then in the fact that features like "devdsp" and "midi" are listed as enabled in later output, and source files like plugout_midi.c are included in the build.

I don't have a real opinion on whether it's better to have these flags or not, but it's clear the author's intention was to pass them. So, fix the attr name so they get passed.

(Found this in the course of testing #85042: enabling structured attrs for this package has the same effect as this fix.)

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"
  • Tested execution of all binary files (usually in ./result/bin/)
    (They successfully execute and print usage messages.)
  • 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.

This caused none of these flags to have any effect.  That's because
the configure command looked like this:

    ./configure --prefix=/nix/store/svhl0fjdj1jl2sqcppy5vnzpfi4gj3d3-gbsplay-2016-12-17 \
        --without-test\ --without-contrib\ --disable-devdsp\ --enable-pulse\ --disable-alsa\ --disable-midi\ --disable-nas\ --disable-dsound\ --disable-i18n

with one giant flag '--without-test --without-contrib...', containing
internal spaces.

This can be seen in `nix log nixpkgs.gbsplay`, in this line:

    configure flags: --prefix=/nix/store/svhl0fjdj1jl2sqcppy5vnzpfi4gj3d3-gbsplay-2016-12-17 --without-test\ --without-contrib\ --disable-devdsp\ --enable-pulse\ --disable-alsa\ --disable-midi\ --disable-nas\ --disable-dsound\ --disable-i18n

and then in the fact that features like "devdsp" and "midi" are listed
as enabled in later output, and source files like plugout_midi.c are
included in the build.

I don't have a real opinion on whether it's better to have these flags
or not, but it's clear the author's intention was to pass them.  So,
fix the attr name so they get passed.
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Apr 12, 2020
@FRidh FRidh merged commit 45a04ed into NixOS:master Apr 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants