Start factoring out Unix-assuming code#10364
Conversation
c2aecc9 to
edfc808
Compare
| unsetenv(name.first.c_str()); | ||
| } | ||
|
|
||
| void replaceEnv(const std::map<std::string, std::string> & newEnv) |
There was a problem hiding this comment.
I think this function is only used in processes.cc so I think it might even make more sense to just put it in that file.
There was a problem hiding this comment.
I think I want to keep it here because of where it is declared, for now, but yes it does seem that all the env var modification stuff (clearEnv too) is just used for process spawning. Once we get that working on Windows it would be good time to revisit this :).
20c3865 to
83571d3
Compare
83571d3 to
f42eca1
Compare
In the Nix commit, platform-specific sources will go here.
f42eca1 to
c9112b1
Compare
roberth
left a comment
There was a problem hiding this comment.
Should be fine, except for the abbreviated identifier.
This splits files and adds new identifiers in preperation for supporting windows, but no Windows-specific code is actually added yet. Co-authored-by: Robert Hensing <[email protected]>
5851a6c to
02fa206
Compare
|
Ehm, some of these moves don't make sense to me, in particular |
|
@edolstra Those are temporary moves until more is implemented, after which I'll move them back. (The next PR has comments on which |
Motivation
This splits files and adds new identifiers in preparation for supporting
windows, but no Windows-specific code is actually added yet.
Context
#8901 will add CPP for Windows, but not have to split up files because this PR does that. That will make that PR's diff easier to read.
Priorities and Process
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.