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

ed: fix docs location. Fixes #32150. Supersedes #32315. #32322

Merged
merged 3 commits into from
Dec 4, 2017
Merged

ed: fix docs location. Fixes #32150. Supersedes #32315. #32322

merged 3 commits into from
Dec 4, 2017
+9 −19

Conversation

berce
Copy link
Contributor

@berce berce commented Dec 4, 2017

Motivation for this change

DESTDIR was set manually, but other installFlags were missing. Nix could fill the gaps, but that resulted in some wrong directories: man pages ended up in /nix/store/<hash>-ed/nix/store/<hash>-ed/share/.
The solution is simple: let nix figure it all out.

#32315 caused mass rebuilding. As suggested by @orivej, I rebased the commit on staging.
I now also removed the configureFlags.

Please review the entire default.nix file for further improvements. I 'd be happy to include more 'modernizations' and avoid extra mass rebuilding.

There's no maintainer set. Any volonteers?

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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.

Sorry, something went wrong.

@orivej orivej changed the title ed: fix output directories. Fixes #32150. Supersedes #32315. ed: fix docs location. Fixes #32150. Supersedes #32315. Dec 4, 2017
@orivej orivej merged commit a9b5900 into NixOS:staging Dec 4, 2017
@orivej
Copy link
Contributor

orivej commented Dec 4, 2017

Thanks! Fixed the issues you outlined.

@orivej orivej mentioned this pull request Dec 4, 2017
Closed
@berce berce deleted the staging branch December 5, 2017 06:56
@Ericson2314
Copy link
Member

The lack of the CC will probably break cross though.

@berce
Copy link
Contributor Author

berce commented Dec 6, 2017

After @orivej suggested it could be omitted, I confirmed that it made no difference on my system, so we removed it.
Could you test cross? The magic powers of nix may be strong enough :-)
Let's now test and/or wait for input and confirmation from others before changing it again.

@orivej
Copy link
Contributor

orivej commented Dec 6, 2017

Cross exports CC, and ed looks like a normal package, why does it need any special treatment?

@Ericson2314
Copy link
Member

I'd love to be wrong, but I recall the configure script was not influenced by the environment, falling back on its own definition of CC which override the environment for make.

@Ericson2314
Copy link
Member

Ericson2314 commented Dec 8, 2017 •
edited
Loading

https://hydra.nixos.org/eval/1416753 yeah, ed fails now.

@orivej orivej mentioned this pull request Dec 8, 2017
Merged
8 tasks
@orivej
Copy link
Contributor

orivej commented Dec 8, 2017

#32489 fixes cross ed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
10.rebuild-darwin: 501+ 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild 10.rebuild-linux: 501+ 10.rebuild-linux-stdenv This PR causes stdenv to rebuild
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@berce @orivej @Ericson2314 @GrahamcOfBorg