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

Fix aliases.nix using the wrong self refs. #41810

Merged
merged 1 commit into from Jun 12, 2018
Merged

Fix aliases.nix using the wrong self refs. #41810

merged 1 commit into from Jun 12, 2018

Conversation

ElvishJerricco
Copy link
Contributor

Motivation for this change

Addresses this comment. For example, dbus_libs is frequently depended on instead of dbus, so the use of noXlibs (which overrides dbus to prevent X deps) did not properly change those dependencies.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@andir
Copy link
Member

andir commented Jun 10, 2018

Given the amount of rebuilds this should go into staging instead?

@ElvishJerricco
Copy link
Contributor Author

Yes. I'm surprised it's that many.

@ElvishJerricco ElvishJerricco changed the base branch from master to staging June 10, 2018 22:45
@FRidh FRidh requested a review from matthewbauer June 12, 2018 16:24
@orivej orivej changed the base branch from staging to master June 12, 2018 16:48
@orivej orivej force-pushed the fix-aliases-self-refs branch 2 times, most recently from a355d04 to c23000d Compare June 12, 2018 16:50
@orivej
Copy link
Contributor

orivej commented Jun 12, 2018

@GrahamcOfBorg eval

@orivej orivej changed the base branch from master to staging June 12, 2018 18:03
@orivej
Copy link
Contributor

orivej commented Jun 12, 2018

The mass rebuild seems correct. The difference is that after this PR xz and lzma (an alias for xz) are the same, but before this PR xz is built with bootstrap-stage4-stdenv-linux and lzma with the final stdenv-linux.

@ElvishJerricco
Copy link
Contributor Author

So is this good to merge?

@matthewbauer
Copy link
Member

I was hoping to get a response from @nbp just in case there was a real reason for doing it the original way. Otherwise it looks good though.

@matthewbauer matthewbauer merged commit 3320b5d into NixOS:staging Jun 12, 2018
@orivej
Copy link
Contributor

orivej commented Jun 12, 2018

The fact revealed by this PR that xz is from bootstrap-stage4-stdenv-linux on Linux is unexpected. (On Darwin it is from stdenv-darwin as expected.) You can test this with:

$ nix-instantiate --eval -A mc.builder -A mc.stdenv.name -A xz.builder -A xz.stdenv.name
# expected
"/nix/store/qckzjk3406va7h6s40cy9s75z2w715rq-bash-4.4-p19/bin/bash"
"stdenv-linux"
# unexpected
"/nix/store/d1prcspbh2qsviipvnaxizcj8l3g7fpw-bootstrap-tools/bin/bash"
"bootstrap-stage4-stdenv-linux"

@Ericson2314, could you look into this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthewbauer matthewbauer matthewbauer approved these changes

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

@nbp nbp Awaiting requested review from nbp

Assignees
No one assigned
Labels
10.rebuild-darwin: 501+ 10.rebuild-darwin-stdenv 10.rebuild-linux: 501+
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@ElvishJerricco @andir @orivej @matthewbauer @GrahamcOfBorg