Yacc tool initialization on windows adding spurious paths to PATH causing build failure. #3336
Comments
|
Can you add a SConstruct to repro this? |
|
could this be related: #3287 |
|
I don't know if this helps, the following SConstruct file: Produces this output: Path (the paths marked do not exist): The marked builders are new in 3.0.5 and did not exist in 3.0.4: |
|
Perhaps coming from the lex tool? |
|
It's got to come from the stanza in |
|
So a quick guess is this might help: |
|
The following SConstruct behaves as expected: Output: Path: Explicitly listing the msvc/msvs tools results in the expected path construction. I'm guessing the reason that the tools were not specified explicitly was that the default construction (up until 3.0.5) was what was expected (with the addition of the JDK). The JDK addition now makes sense in terms of the default environment. It appears that I'm the tool in this case. |
|
The default tool set is intended to make things simpler, you don't have to figure out all the tools your project needs (if what you need is included in the defaults). Bill has concluded that it's quite a bit faster to not use the default set, and just list the ones you want, particularly on windows platforms. For what it's worth... |
|
The irony is that for my builds the tools are passed for the mingw-w64 builds and explicit folders were being appended to the path for msvc builds. It was purely an oversight that the tools were not passed for msvc builds. In almost all cases I do not want cygwin silently added to the path. I learned something valuable moving forward. |
|
If you apply the patch that Mats posted without listing the tools
explicitly, does it fix your issue?
…On Wed, Mar 27, 2019 at 11:37 AM jcbrill ***@***.***> wrote:
The irony is that for my builds the tools are passed for the mingw-w64
builds and explicit folders were being appended to the path for msvc
builds. It was purely an oversight that the tools were not passed for msvc
builds. In almost all cases I do not want cygwin silently added to the
path. I learned something valuable moving forward.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#3336 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAFBNGXOzBuk8VjZqEVbt58a8GzP5puzks5va7pvgaJpZM4cNxvA>
.
|
|
I will try to apply the patch later today. The patch will not change the build failure that I experienced. My build script must be changed to accommodate the default SCons 3.0.5 behavior in one of two ways: (1) explicitly pass the tools for msvc to the environment and use AppendENVPath for dlltool or (2) make no changes to the environment and change AppendENVPath for dlltool to PrependENVTool. I have elected the first option. To recap: there are paths added to the default environment ENV[PATH] that do not exist on my computer. I suppose that is a bug. The two line SConstruct file attached above demonstrates and simply reproduces this behavior. Any version of MSVC could be passed to the environment (or probably even none at all). Paths that don't exist really have no effect other that polluting the path. The build failure that I experienced is because one of the additional paths added to the default environment ENV[PATH] constuction does exist (cygwin/bin). Cygwin/bin contains flex.exe (which is what I suspect SCons was looking for) and also a variety of gcc/mingw tools. The path location that I eventually add manually to the end of the ENV[PATH] using AppendENVPath for the desired dlltool executable is meaningless since there is already a dlltool in the path. This is not a bug but an unintended consequence of the construction of the default environment. If I understand the patch correctly, paths that do not exist will not be added to the path. That's all well and good. However my problem was with an unintended consequence of an added path that does exist conflicting with my expectations of behavior based on SCons 3.0.4 and earlier. The only solution to my build problem is to change my script as described earlier. I fear that adding cygwin to the path may break a number of scripts that are using AppendENVPath to explicitly add tools to the default environment. |
That was the idea of the tiny patch: right now all the extra "possible places something could be found" are added as they are tried, and as long as the process ended with a "hit", they're kept; instead I try to keep just the "hit". |
|
I just tried the first test. Good news: folders that don't exist are not added to the path. |
|
Oh, I was afraid of that, wasn't sure if it came back as a directory or as a full path. More thinking then - thanks for the quick update! |
|
I don't think you need to add the path when it exists as the calling procedure likely will. The following seems to work: Original 3.0.5 code: Modified 3.0.5 code: |
|
The implications of the following excerpt from the 3.0.5 release email now makes a lot more sense: |
When tool modules initialize, they check paths to decide if the underlying tool is actually present. The current checker adds any "default paths" the caller may have supplied (locations like mingw, cygwin, chocolatey install locations, etc.); if there is match from this list, any previous default paths are also kept. To avoid keeping these non-matching paths, restore the original PATH; the caller is responsible for adding to PATH if necessary. Docstring now says so. Note lex and yacc tool modules seem to expect the path-modifying behavior that's being gotten rid of - so they preseve the path first and restore it after. The change here won't break those, but makes the extra behavior unneeded - could remove it. Signed-off-by: Mats Wichmann <[email protected]>
When tool modules initialize, they check paths to decide if the underlying tool is actually present. The current checker adds any "default paths" the caller may have supplied (locations like mingw, cygwin, chocolatey install locations, etc.); if there is match from this list, any previous default paths are also kept. To avoid keeping these non-matching paths, restore the original PATH; the caller is responsible for adding to PATH if necessary. Docstring now says so. Note lex and yacc tool modules seem to expect the path-modifying behavior that's being gotten rid of - so they preseve the path first and restore it after. The change here won't break those, but makes the extra behavior unneeded - could remove it. Signed-off-by: Mats Wichmann <[email protected]>
bdbaddog
changed the title
When tool modules initialize, they check paths to decide if the underlying tool is actually present. The current checker adds any "default paths" the caller may have supplied (locations like mingw, cygwin, chocolatey install locations, etc.); if there is match from this list, any previous default paths are also kept. To avoid keeping these non-matching paths, restore the original PATH; the caller is responsible for adding to PATH if necessary. Docstring now says so. Note lex and yacc tool modules seem to expect the path-modifying behavior that's being gotten rid of - so they preseve the path first and restore it after. The change here won't break those, but makes the extra behavior unneeded - could remove it. Signed-off-by: Mats Wichmann <[email protected]>
[#3336] do not add not-found tool paths
|
Should be resolved in master now. |
Additional paths (possibly extraneous) are being added to the msvc environment path for SCons 3.0.5 as compared to SCons 3.0.4.
The following paths are added to the ENV['PATH'] after the msvc configuration in SCons 3.0.5:
The following paths are added to the ENV['PATH'] after the msvc configuration in SCons 3.0.4:
When configured from the command line using vcvarsall.bat, the JDK is not added to the path.
If nothing else, this appears to violate The Law of Least Astonishment.
Configuration:
scons-users message: https://pairlist4.pair.net/pipermail/scons-users/2019-March/007658.html
The additional SCons added paths cause build failures when user-specified mingw-w64 tools are added to the path using AppendENVPath. In the test case, the cygwin/bin folder precedes the user desired mingw-w64 path that was manually added.
Due to the additional paths, PrependENVPath would have to be used instead to guarantee the desired tools would likely be invoked.
An annotated comparison of configurations of msvc 2005 and mingw-w64 from the command line, SCons 3.0.4 and SCons 3.0.5 is shown below. The same holds for all tested versions of MSVC.
Command-line msvs 2005 environment:
SCons 3.0.4 mingw build ENV{}:
SCons 3.0.4 msvs 2005 build ENV{}:
SCons 3.0.5 mingw build ENV{}:
SCons 3.0.5 msvs 2005 build ENV{}:
The text was updated successfully, but these errors were encountered: