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

cc-wrapper: Fix standalone gcc #28021

Merged
merged 3 commits into from
Aug 8, 2017
Merged

cc-wrapper: Fix standalone gcc #28021

merged 3 commits into from
Aug 8, 2017

Conversation

orivej
Copy link
Contributor

@orivej orivej commented Aug 8, 2017

This refactors changes to add-flags.sh introduced in #27672 to ensure that all salted variables are defined even if the wrapped program is invoked outside nix-build environment to address #27672 (comment) .

This pull request incorporates #28012 .

Currently I'm testing if this can build kdeFrameworks.kidletime on Linux.

Ericson2314
Ericson2314 approved these changes Aug 8, 2017
View reviewed changes
Copy link
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thanks for doing this :)

@orivej
cc-wrapper: Fix adding directories to rpath
b3e4f45 
This fixes a bug introduced in NixOS#27831: `for path in "$dir"/lib*.so` assumed that
all libs match `lib*.so`, but 0767478 started
adding libs that match `*.so` and `*.so.*`.
@orivej
cc-wrapper: Fix standalone gcc
9c1eb20 
This ensures that all salted variables are defined even if the wrapped program
is invoked outside nix-build environment.
@orivej
cc-wrapper: Fix standalone ld
75fb30a
globin added a commit that referenced this pull request Aug 8, 2017
@globin
Revert "cc-wrapper: fix set -u errors"
3e981b9 
This reverts commit 67a41ea.

see #28021 for discussion
@globin
Copy link
Member

globin commented Aug 8, 2017

Please don't merge yet.. this won't fix it btw.. but no idea what the issue is..

@orivej
Copy link
Contributor Author

orivej commented Aug 8, 2017

@globin Have you tested with the latest commit that fixed ld? My current build in progress has advanced far beyond building llvm.

@orivej
Copy link
Contributor Author

orivej commented Aug 8, 2017

I have just built kdeFrameworks.kidletime with this pull request.

@globin globin merged commit a6e7dff into NixOS:staging Aug 8, 2017
@orivej orivej mentioned this pull request Aug 8, 2017
Closed
@globin
Copy link
Member

globin commented Aug 8, 2017

doesn't build: https://hydra.mayflower.de/log/lfx45ymp527msaaxz8w0ib9yic61d5fh-qjackctl-0.4.5.drv

@orivej orivej mentioned this pull request Aug 9, 2017
Merged
8 tasks
@orivej
Copy link
Contributor Author

orivej commented Aug 9, 2017 •
edited

I do not understand this fully yet, but this failure is caused by #27657 (comment) . I assumed that libraries used from /nix/store during compilation do not disappear after compilation, but this is not the case when building qt5.qttools: it seems that there is /nix/store/…-qttools-5.9.1/lib/libQt5Xml.so during compilation of lupdate (and other binaries), so ld-wrapper is content to add an rpath for qttools without adding an rpath for qtbase, then this symlink disappears, and finally patchelf --shrink-rpath deletes now useless rpath for qttools…

@orivej
Copy link
Contributor Author

orivej commented Aug 9, 2017

Currently (at least when qt5.qttools is already built) nix-shell '<nixpkgs>' -A qt5.qttools fails with mkdir: cannot create directory ‘/nix/store/…-qttools-…/bin’: Read-only file system here. If this could be fixed by not using /nix/store for temporary files (deleted here), this will also fix building qjackctl.

Nevertheless, should I revert the aforementioned change (#27657 (comment)), or shall we fulfill its assumption that used libraries are not deleted from /nix/store, or both?

@orivej orivej mentioned this pull request Aug 9, 2017
Merged
orivej added a commit to orivej/nixpkgs that referenced this pull request Aug 9, 2017
@orivej
cc-wrapper: Fix support for qtbase-setup-hook
a27c0ae 
Revert NixOS#27657 (comment)
due to NixOS#28021 (comment)
globin pushed a commit that referenced this pull request Aug 9, 2017
@orivej @globin
cc-wrapper: Fix support for qtbase-setup-hook
771f28b 
Revert #27657 (comment)
due to #28021 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ericson2314 Ericson2314 Ericson2314 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@orivej @globin @Ericson2314