docs: incorrect/misleading instructions on PATH importing

[mwichmann]

This came up during a Discord discussion on #scons-help

https://discord.com/channels/571796279483564041/571796280146133047/1006057851858452510

Both the man page and User Guide give examples of importing the external PATH variable into the SCons build, but the way it is demonstrated is not ideal. This is the first example in both places:

import os
env = Environment(ENV={'PATH': os.environ['PATH']})

The problem with this and the other examples is they overwrite the platform-default value of the ENV dictionary, rather than adding to or updating it. It turns out this is actually potentially fatal on modern Windows, where a variety of things don't work if %USERPROFILE% is not set, and this clobbers it, as well as %SystemRoot% (apparently needed for socket operations). On a Mac, this would clobber the setting of PATHOSX (which SCons itself uses, though possibly only in the TeX tool).

In the case of the Discord discussion, following this recommendation did in fact break a build on Windows.

It seems we should not make this approach a primary recommendation, or at the very least, include a warning. Explaining how to do it "correctly" may be a little tricky, though.... the basic model could be something like:

env.AppendENVPath('PATH', os.environ['PATH'])

But because that has to happen after the Environment creation, it may run into problems of tools needing the imported path to initialize themselves.

[bdbaddog]

Something like += type functionality would be ideal. ENV_PATH_REPLACE ENV_PATH_PREPEND

[mwichmann]

Well, we do have AppendENVPath and PrependENVPath. Not a replace function. But as mentioned above, you have to call that after you've already made the env, and if something in the initialization depended on paths, then it's "too late" at that point.

It also looks like you can't do env = Environment(ENV={'PATH': "${PATH}:/some/other/bin"}): apparently, in this context, the elements of ENV don't get substituted. Maybe that wouldn't even make sense.

[bdbaddog]

I was thinking those vars would be specifyable in the Environment() call and modify the envvars before tool initilization.

[mwichmann]

Was thinking about this just now, and it wouldn't be hard to add a kwarg that lets you specify a dict of variables that are to be appended to, rather than assigned to. In fact, it took just a few minutes to prototype. Of course, doing so leads to more questions.

First, specifically for a path, just appending doesn't work, as the execution environment is a special case: the construction variable is ENV, which is a dict whose path element PATH is the one you want to append to. Which suggests we also need a way to update that dict, not just self._dict but self._dict['ENV'] - or perhaps more appropriately, an arbitrary dict. That's not hard to do, either.

Second, when/how should these additional things be applied? The current flow in environment is like (summarized):

# apply platform

# set all the standard args outright (saving keys):
self.Replace(**kw)

# apply vars from variables (saving those keys too):
if variables:
    variables.Update(self)

# stash the things that were set:
save = {}
for k in keys:
    save[k] = self._dict[k]

# apply tools
apply_tools(self, tools, toolpath)

# Restore the "set" values that were saved off,
# so user-set values win:
for key, val in save.items():
    self._dict[key] = val

So should consvars.append happen up where the variables.Update happens? Or at the end?

The end bit of that flow is a little dodgy anyway, even ignoring any further additions: it's no good if tools clobber user settings, but it also could be bad if user settings clobber necessary things set up by tools. Seems like the end up that simplified sequence should be not replace but merge...

I guess this drifted off from being a doc bug a bit...