buildenv: Read muliple lines in propagated-user-env-packages#1480
buildenv: Read muliple lines in propagated-user-env-packages#1480Ericson2314 wants to merge 1 commit intoNixOS:masterfrom
Conversation
| } | ||
| for (const auto & p : tokenizeString<std::vector<string>>(propagated, " ")) | ||
| if (done.find(p) == done.end()) | ||
| postponed.insert(p); |
There was a problem hiding this comment.
I don't get what all this readUntil stuff is needed for (or why the original used readLine in a loop). Wouldn't something like
tokenizeString<Strings>(readFile(propagatedFN), " \n"))
work?
There was a problem hiding this comment.
Oh does tokenizeString conceptually take a list of deliminators (disjunction) rather than single deliminating string (conjunction)? Fine with me then.
There was a problem hiding this comment.
Btw, what does if (done.find(p) == done.end()) do re empty entries? Want to make sure split and this coincide?
Ericson2314
force-pushed
the
fix-propagated-user-env-packages
branch
from
2703257 to
7da117c
Compare
|
@edolstra Made your simplification. |
src/buildenv/buildenv.cc
Outdated
| propagated = readLine(fd.get()); | ||
| propagated = readFile(fd.get()); | ||
| } | ||
| for (const auto & p : tokenizeString<std::vector<string>>(propagated, " ")) |
There was a problem hiding this comment.
Now it only tokenizes on spaces right? Dropping the " " will cause it to split on any whitespace.
There was a problem hiding this comment.
Ah sorry. I went to shrink my diff and did so too much.
For compatibility with old and new nixpkgs, either ' ' or '\n' can serve as a delimiter between entries.
Ericson2314
force-pushed
the
fix-propagated-user-env-packages
branch
from
7da117c to
bfac92a
Compare
|
See NixOS/nixpkgs#27859 for talk of doing the same thing in nixpkgs. |
|
I marked this as stale due to inactivity. → More info |
|
I closed this issue due to inactivity. → More info |
5 participants
For comparability with old and new nixpkgs, either ' ' or '\n' can serve as a delimiter between entries. I'll rewrite for the existing
11.xperl version next.See NixOS/nixpkgs#27427 for background.
@ttuegel I wrote it this way not because I disagree with making Nix and Nixpkgs share a buildenv, but because this is the simpler change. Also, it is unclear to me whether the
nix-support/*files are a Nix interface or Nixpkgs interface---seems to very case to case.Do not merge just yet---I only built it and did not test it yet.