stdenv: Store one package per line in nix-support/propagated-* #27284
+31
−22
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Ericson2314
added
1.severity: mass-rebuild
|
Spaces in store paths can never possibly work since you can't escape a space in a |
Ericson2314
force-pushed
the
prop-lines
branch
3 times, most recently
from
64d2858 to
fb0e153
Compare
|
@dezgeg That's true. I guess take this PR as a matter of style. |
This makes those files a bit easier to read. Also, for what it's worth, it brings us one baby step closer to handling spaces in store paths. Also, I optimized handling of many transitive deps with read. Probably, not very beneficial, but nice to enforce the pkg-per-line structure. Doing so let me find much dubious code and fix it. Two misc notes: - `propagated-user-env-packages` also needed to be adjusted as sometimes it is copied to/from the propagated input files. - `local fd` should ensure that file descriptors aren't clobbered during recursion.
|
OK, this is tested. |
|
Seems fine to me, though I don't see a huge advantage - these files are not intended for human consumption so the readability is not really important, and as @dezgeg mentioned, whitespace is unlikely to ever work well (also this solution wouldn't support newlines in filenames). |
|
@edolstra Thanks for the verdict. TBH this was basically me getting side-tracked when working on that previous PR, but I do occasionally read those files by hand---at least when debugging. |
|
`printLines` broke package propagation. Please, revert.
See #27427 for discussion.
|
8 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation for this change
This makes those files a bit easier to read. Also, for what it's worth, it brings us one tiny step closer to handling spaces in store paths.
Also, I optimized handling of many transitive deps with read. Probably, not very beneficial, but nice to enforce the pkg-per-line structure. Doing so let me find much dubious code and fix it.
Two misc notes:
propagated-user-env-packagesalso needed to be adjusted as sometimes it is copied to/from the propagated input files.local fdshould ensure that file descriptors aren't clobbered during recursion.Things done
Built stdenv on Darwin and Linux