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

setup-hooks: do not pass missing dirs to find #15405

Closed
wants to merge 1 commit into from

Conversation

layus
Copy link
Member

@layus layus commented May 12, 2016

find fails when called with an inexistent search path.
That situation may arise when the output is created after by a postFixup hook.

I discovered the bug while building the nss package for #15339.
In that case, the "tools" output is only creted in the postFixup hook by the moveToOutput bin "$tools" here.

More generally, the multiple output features increases the risk to encounter empty or missing prefixes during the build.

The issue was introduced by a refactoring in d71a485.
This PR fixes my comment on that commit. (here)

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @edolstra to be a potential reviewer

@vcunat vcunat added the 1.severity: mass-rebuild This PR causes a large number of packages to rebuild label May 12, 2016
@vcunat
Copy link
Member

vcunat commented May 12, 2016

Alternatively we might opt not to even run fixupOutputHooks on outputs that don't exist at that point, but I think this PR is a better way.

@vcunat
Copy link
Member

vcunat commented May 20, 2016

These returns are bad, as the exit code of the last command is used which means failure. We need return 0.

Stripping (with flags -S) the file /nix/store/y2zcjk85rdfvcrbv06s73k9sn75adiw8-linux-config-4.4.10.
+++ stripAll -S
+++ local stripFlags=-S
+++ local _ls f stripOutput
+++ IFS=
+++ read -r -d '' f
+++ stopNest
+++ nestingLevel=1
+++ echo -en '\033[q'
+++ '[' -d /nix/store/y2zcjk85rdfvcrbv06s73k9sn75adiw8-linux-config-4.4.10 ']'
+++ return
+ exitHandler
+ exitCode=1

@vcunat
Copy link
Member

vcunat commented May 20, 2016

Well, in this particular case the problem is return in #15339 (the first one in stripDirs).

find fails when called with an inexistent search path.
That situation may arise when the output is created after by a postFixup hook.
@layus layus force-pushed the fix-empty-find branch from 8676a46 to 02c202d Compare May 20, 2016 12:27
@layus
Copy link
Member Author

layus commented May 20, 2016

Updated this PR to fix the calls to return. Same restrictions as the update in #15339.

vcunat pushed a commit that referenced this pull request May 22, 2016
find fails when called with an inexistent search path.
That situation may arise when the output is created after by a postFixup hook.
vcunat amended the PR by clarifying one more `return` to `return 0`.
@vcunat
Copy link
Member

vcunat commented May 22, 2016

Thanks. Staged with clarifying one more return. (That one seemed correct, but still.)

@vcunat vcunat closed this May 22, 2016
@layus
Copy link
Member Author

layus commented May 22, 2016

Thanks for following and improving this PR :-)

Le 22 mai 2016 12:18:26 UTC+02:00, "Vladimír Čunát" [email protected] a écrit :

Thanks. Staged with clarifying one more return. (That one seemed
correct, but still.)


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#15405 (comment)

@vcunat
Copy link
Member

vcunat commented May 23, 2016

:-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.severity: mass-rebuild This PR causes a large number of packages to rebuild
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants