Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#3336] do not add not-found tool paths #3337

Merged
merged 7 commits into from
Apr 24, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER
- Use importlib to dynamically load tool and platform modules instead of imp module
- sconsign: default to .sconsign.dblite if no filename is specified.
Be more informative in case of unsupported pickle protocol (py2 only).
- Fix issue #3336 - on Windows, paths were being added to PATH even if
tools were not found in those paths.

From John Doe:

Expand Down
25 changes: 14 additions & 11 deletions src/engine/SCons/Tool/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1318,28 +1318,31 @@ def tool_list(platform, env):

def find_program_path(env, key_program, default_paths=[]):
"""
Find the location of key_program and then return the path it was located at.
Checking the default install locations.
Mainly for windows where tools aren't all installed in /usr/bin,etc
:param env: Current Environment()
:param key_program: Program we're using to locate the directory to add to PATH.
Find the location of a tool using various means.

Mainly for windows where tools aren't all installed in /usr/bin, etc.

:param env: Current Construction Environment.
:param key_program: Tool to locate.
:param default_paths: List of additional paths this tool might be found in.
"""
# First search in the SCons path
path = env.WhereIs(key_program)
if (path):
if path:
return path
# then the OS path:

# Then in the OS path
path = SCons.Util.WhereIs(key_program)
if (path):
if path:
return path

# If that doesn't work try default location for mingw
# Finally, add the defaults and check again. Do not change
# ['ENV']['PATH'] permananetly, the caller can do that if needed.
save_path = env['ENV']['PATH']
for p in default_paths:
env.AppendENVPath('PATH', p)
path = env.WhereIs(key_program)
if not path:
env['ENV']['PATH'] = save_path
env['ENV']['PATH'] = save_path
return path

# Local Variables:
Expand Down
19 changes: 11 additions & 8 deletions src/engine/SCons/Tool/lex.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,15 @@ def lexEmitter(target, source, env):

def get_lex_path(env, append_paths=False):
"""
Find the a path containing the lex or flex binaries. If a construction
environment is passed in then append the path to the ENV PATH.
Find the path to the lex tool, searching several possible names

Only called in the Windows case, so the default_path
can be Windows-specific

:param env: current construction environment
:param append_paths: if set, add the path to the tool to PATH
:return: path to lex tool, if found
"""
# save existing path to reset if we don't want to append any paths
envPath = env['ENV']['PATH']
bins = ['flex', 'lex', 'win_flex']

for prog in bins:
Expand All @@ -83,9 +87,7 @@ def get_lex_path(env, append_paths=False):
prog,
default_paths=CHOCO_DEFAULT_PATH + MINGW_DEFAULT_PATHS + CYGWIN_DEFAULT_PATHS )
if bin_path:
if not append_paths:
env['ENV']['PATH'] = envPath
else:
if append_paths:
env.AppendENVPath('PATH', os.path.dirname(bin_path))
return bin_path
SCons.Warnings.Warning('lex tool requested, but lex or flex binary not found in ENV PATH')
Expand Down Expand Up @@ -113,7 +115,8 @@ def generate(env):
env["LEXFLAGS"] = SCons.Util.CLVar("")

if sys.platform == 'win32':
get_lex_path(env, append_paths=True)
# ignore the return - we do not need the full path here
_ = get_lex_path(env, append_paths=True)
env["LEX"] = env.Detect(['flex', 'lex', 'win_flex'])
if not env.get("LEXUNISTD"):
env["LEXUNISTD"] = SCons.Util.CLVar("")
Expand Down
3 changes: 2 additions & 1 deletion src/engine/SCons/Tool/swig.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,10 @@ def generate(env):

from SCons.Platform.mingw import MINGW_DEFAULT_PATHS
from SCons.Platform.cygwin import CYGWIN_DEFAULT_PATHS
from SCons.Platform.win32 import CHOCO_DEFAULT_PATH

if sys.platform == 'win32':
swig = SCons.Tool.find_program_path(env, 'swig', default_paths=MINGW_DEFAULT_PATHS + CYGWIN_DEFAULT_PATHS + [r'C:\ProgramData\chocolatey\bin'] )
swig = SCons.Tool.find_program_path(env, 'swig', default_paths=MINGW_DEFAULT_PATHS + CYGWIN_DEFAULT_PATHS + CHOCO_DEFAULT_PATH)
if swig:
swig_bin_dir = os.path.dirname(swig)
env.AppendENVPath('PATH', swig_bin_dir)
Expand Down
27 changes: 17 additions & 10 deletions src/engine/SCons/Tool/yacc.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,15 @@ def yyEmitter(target, source, env):

def get_yacc_path(env, append_paths=False):
"""
Find the a path containing the lex or flex binaries. If a construction
environment is passed in then append the path to the ENV PATH.
Find the path to the yacc tool, searching several possible names

Only called in the Windows case, so the default_path
can be Windows-specific

:param env: current construction environment
:param append_paths: if set, add the path to the tool to PATH
:return: path to yacc tool, if found
"""
# save existing path to reset if we don't want to append any paths
envPath = env['ENV']['PATH']
bins = ['bison', 'yacc', 'win_bison']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move this list to the top, name win_bins and have a non-win bins and then use that everywhere instead of having several lists scattered throughout the module?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no problem, I've got that ready to push... but while I'm at it, the block in yacc.py from lines 145-151 seems like it is redundant now - like the "old version" was still left around when the get_yacc_path function was added. Should this be dropped?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.


for prog in bins:
Expand All @@ -113,12 +117,11 @@ def get_yacc_path(env, append_paths=False):
prog,
default_paths=CHOCO_DEFAULT_PATH + MINGW_DEFAULT_PATHS + CYGWIN_DEFAULT_PATHS )
if bin_path:
if not append_paths:
env['ENV']['PATH'] = envPath
else:
if append_paths:
env.AppendENVPath('PATH', os.path.dirname(bin_path))
return bin_path
SCons.Warnings.Warning('lex tool requested, but lex or flex binary not found in ENV PATH')
SCons.Warnings.Warning('yacc tool requested, but yacc or bison binary not found in ENV PATH')


def generate(env):
"""Add Builders and construction variables for yacc to an Environment."""
Expand Down Expand Up @@ -148,7 +151,8 @@ def generate(env):
SCons.Warnings.Warning('yacc tool requested, but bison binary not found in ENV PATH')

if sys.platform == 'win32':
get_yacc_path(env, append_paths=True)
# ignore the return - we do not need the full path here
_ = get_yacc_path(env, append_paths=True)
env["YACC"] = env.Detect(['bison', 'yacc', 'win_bison'])
else:
env["YACC"] = env.Detect(["bison", "yacc"])
Expand All @@ -162,7 +166,10 @@ def generate(env):
env['YACCVCGFILESUFFIX'] = '.vcg'

def exists(env):
return env.Detect(['bison', 'yacc'])
if sys.platform == 'win32':
return get_yacc_path(env)
else:
return env.Detect(['bison', 'yacc'])

# Local Variables:
# tab-width:4
Expand Down
2 changes: 1 addition & 1 deletion test/LEX/live_mingw.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
test.skip_test('Not windows environment; skipping test.\n')

if not test.where_is('gcc'):
test.skip_test('No mingw or cygwin on windows; skipping test.\n')
test.skip_test('No mingw or cygwin build environment found; skipping test.\n')

lex = test.where_is('lex') or test.where_is('flex')

Expand Down