buildenv: Read muliple lines in propagated-user-env-packages #1480
Conversation
src/buildenv/buildenv.cc
Outdated
| } | ||
| 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
| @@ -114,7 +114,7 @@ static void addPkg(const Path & pkgDir, int priority) | |||
| return; | |||
| throw SysError(format("opening ‘%1%’") % propagatedFN); | |||
| } | |||
| 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 |
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.