Skip to content

Commit

Permalink
Clean repeated entries in PATH after vcvars call (#3598)
Browse files Browse the repository at this point in the history
* Clean repeated

* only the path

* fixed test

* Simplified path dedup
  • Loading branch information
lasote authored Sep 27, 2018
1 parent 2b3e0e3 commit eb6226d
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 6 deletions.
14 changes: 8 additions & 6 deletions conans/client/tools/win.py
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ def vcvars_command(settings, arch=None, compiler_version=None, force=False, vcva

def vcvars_dict(settings, arch=None, compiler_version=None, force=False, filter_known_paths=False,
vcvars_ver=None, winsdk_version=None, only_diff=True):
known_path_lists = ("INCLUDE", "LIB", "LIBPATH", "PATH")
known_path_lists = ("include", "lib", "libpath", "path")
cmd = vcvars_command(settings, arch=arch,
compiler_version=compiler_version, force=force,
vcvars_ver=vcvars_ver, winsdk_version=winsdk_version) + " && echo __BEGINS__ && set"
Expand All @@ -407,14 +407,17 @@ def vcvars_dict(settings, arch=None, compiler_version=None, force=False, filter_
continue
try:
name_var, value = line.split("=", 1)
new_value = value.split(os.pathsep) if name_var in known_path_lists else value
new_value = value.split(os.pathsep) if name_var.lower() in known_path_lists else value
# Return only new vars & changed ones, but only with the changed elements if the var is
# a list
if only_diff:
old_value = os.environ.get(name_var)
# The new value ends with separator and the old value, is a list, get only the
# new elements
if old_value and value.endswith(os.pathsep + old_value):
if name_var.lower() == "path":
old_values_lower = [v.lower() for v in old_value.split(os.pathsep)]
# Clean all repeated entries, not append if the element was already there
new_env[name_var] = [v for v in new_value if v.lower() not in old_values_lower]
elif old_value and value.endswith(os.pathsep + old_value):
# The new value ends with separator and the old value, is a list, get only the new elements
new_env[name_var] = value[:-(len(old_value) + 1)].split(os.pathsep)
elif value != old_value:
# Only if the vcvars changed something, we return the variable,
Expand All @@ -438,7 +441,6 @@ def relevant_path(path):

return new_env


@contextmanager
def vcvars(*args, **kwargs):
if platform.system() == "Windows":
Expand Down
25 changes: 25 additions & 0 deletions conans/test/util/tools_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -712,6 +712,31 @@ def vcvars_echo_test(self):
self.assertIn("Conan:vcvars already set", str(output))
self.assertIn("VS140COMNTOOLS=", str(output))

@unittest.skipUnless(platform.system() == "Windows", "Requires Windows")
def vcvars_env_not_duplicated_path_test(self):
"""vcvars is not looking at the current values of the env vars, with PATH it is a problem because you
can already have set some of the vars and accumulate unnecessary entries."""
settings = Settings.loads(default_settings_yml)
settings.os = "Windows"
settings.compiler = "Visual Studio"
settings.compiler.version = "15"
settings.arch = "x86"
settings.arch_build = "x86_64"

# Set the env with a PATH containing the vcvars paths
tmp = tools.vcvars_dict(settings, only_diff=False)
tmp = {key.lower(): value for key, value in tmp.items()}
with tools.environment_append({"path": tmp["path"]}):
previous_path = os.environ["PATH"].split(";")
# Duplicate the path, inside the tools.vcvars shouldn't have repeated entries in PATH
with tools.vcvars(settings):
path = os.environ["PATH"].split(";")
values_count = {value: path.count(value) for value in path}
for value, counter in values_count.items():
if value and counter > 1 and previous_path.count(value) != counter:
# If the entry was already repeated before calling "tools.vcvars" we keep it
self.fail("The key '%s' has been repeated" % value)

@unittest.skipUnless(platform.system() == "Windows", "Requires Windows")
def vcvars_amd64_32_cross_building_support_test(self):
# amd64_x86 crossbuilder
Expand Down

0 comments on commit eb6226d

Please sign in to comment.