Repository: conan-io/conan
Pull request #12517 (2022-11-11 19:05:01)

allowing placeholders in EnvVars
Changelog: Feature: Allow placeholders in EnvVars
Docs: https://github.com/conan-io/docs/pull/XXXX

Close https://github.com/conan-io/conan/issues/12427



Patch:
diff --git a/conan/tools/_compilers.py b/conan/tools/_compilers.py
index a3ab6bc86..a4f6f9da2 100644
--- a/conan/tools/_compilers.py
+++ b/conan/tools/_compilers.py
@@ -150,11 +150,8 @@ def build_type_flags(settings):
         # clang include the gnu (overriding some things, but not build type) and apple clang
         # overrides clang but it doesn't touch clang either
         if str(compiler) in ["clang", "gcc", "apple-clang", "qcc", "mcst-lcc"]:
-            # FIXME: It is not clear that the "-s" is something related with the build type
-            # cmake is not adjusting it
-            # -s: Remove all symbol table and relocation information from the executable.
             flags = {"Debug": ["-g"],
-                     "Release": ["-O3", "-s"] if str(compiler) == "gcc" else ["-O3"],
+                     "Release": ["-O3"],
                      "RelWithDebInfo": ["-O2", "-g"],
                      "MinSizeRel": ["-Os"],
                      }.get(build_type, [])
diff --git a/conan/tools/cmake/cmakedeps/templates/target_data.py b/conan/tools/cmake/cmakedeps/templates/target_data.py
index 61011bb0c..cb4b733b0 100644
--- a/conan/tools/cmake/cmakedeps/templates/target_data.py
+++ b/conan/tools/cmake/cmakedeps/templates/target_data.py
@@ -44,15 +44,13 @@ class ConfigDataTemplate(CMakeDepsFileTemplate):
         dependency_filenames = self._get_dependency_filenames()
         # Get the nodes that have the property cmake_find_mode=None (no files to generate)
         dependency_find_modes = self._get_dependencies_find_modes()
-        # package_folder might not be defined if Editable and layout()
-        package_folder = self.conanfile.package_folder or ""
-        package_folder = package_folder.replace('\\', '/').replace('$', '\\$').replace('"', '\\"')
+        root_folder = self._root_folder.replace('\\', '/').replace('$', '\\$').replace('"', '\\"')
 
         return {"global_cpp": global_cpp,
                 "has_components": self.conanfile.cpp_info.has_components,
                 "pkg_name": self.pkg_name,
                 "file_name": self.file_name,
-                "package_folder": package_folder,
+                "package_folder": root_folder,
                 "config_suffix": self.config_suffix,
                 "components_names": components_names,
                 "components_cpp": components_cpp,
@@ -139,7 +137,12 @@ class ConfigDataTemplate(CMakeDepsFileTemplate):
     def _get_global_cpp_cmake(self):
         global_cppinfo = self.conanfile.cpp_info.aggregated_components()
         pfolder_var_name = "{}_PACKAGE_FOLDER{}".format(self.pkg_name, self.config_suffix)
-        return _TargetDataContext(global_cppinfo, pfolder_var_name, self.conanfile.package_folder)
+        return _TargetDataContext(global_cppinfo, pfolder_var_name, self._root_folder)
+
+    @property
+    def _root_folder(self):
+        return self.conanfile.recipe_folder if self.conanfile.package_folder is None \
+            else self.conanfile.package_folder
 
     def _get_required_components_cpp(self):
         """Returns a list of (component_name, DepsCppCMake)"""
@@ -149,8 +152,7 @@ class ConfigDataTemplate(CMakeDepsFileTemplate):
         direct_visible_host = self.conanfile.dependencies.filter({"build": False, "visible": True,
                                                                   "direct": True})
         for comp_name, comp in sorted_comps.items():
-            deps_cpp_cmake = _TargetDataContext(comp, pfolder_var_name,
-                                                self.conanfile.package_folder)
+            deps_cpp_cmake = _TargetDataContext(comp, pfolder_var_name, self._root_folder)
             public_comp_deps = []
             for require in comp.requires:
                 if "::" in require:  # Points to a component of a different package
@@ -216,9 +218,7 @@ class _TargetDataContext(object):
                 assert os.path.isabs(p), "{} is not absolute".format(p)
 
                 # Trying to use a ${mypkg_PACKAGE_FOLDER}/include path instead of full
-                # TODO: We could do something similar for some build_folder for editables
-                # package_folder can be None if editable and layout()
-                if package_folder is not None and p.startswith(package_folder):
+                if p.startswith(package_folder):
                     # Prepend the {{ pkg_name }}_PACKAGE_FOLDER{{ config_suffix }}
                     rel = p[len(package_folder):]
                     rel = rel.replace('\\', '/').replace('$', '\\$').replace('"', '\\"').lstrip("/")
@@ -241,13 +241,6 @@ class _TargetDataContext(object):
                                    replace('"', '\\"'))
                                    for v in values)
 
-        def join_paths_single_var(values):
-            """
-            semicolon-separated list of dirs:
-            e.g: set(LIBFOO_INCLUDE_DIR "/path/to/included/dir;/path/to/included/dir2")
-            """
-            return '"%s"' % ";".join(p.replace('\\', '/').replace('$', '\\$') for p in values)
-
         self.include_paths = join_paths(cpp_info.includedirs)
         self.lib_paths = join_paths(cpp_info.libdirs)
         self.res_paths = join_paths(cpp_info.resdirs)
diff --git a/conan/tools/env/environment.py b/conan/tools/env/environment.py
index 4d43ed19a..a4b96ea9f 100644
--- a/conan/tools/env/environment.py
+++ b/conan/tools/env/environment.py
@@ -251,16 +251,34 @@ class EnvVars:
     def keys(self):
         return self._values.keys()
 
-    def get(self, name, default=None):
+    def get(self, name, default=None, variable_reference=None):
+        """
+        :param name: The name of the environment variable
+        :param default: The returned value if the variable doesn't exist, by default None
+        :param variable_reference: if specified, use a variable reference instead of the
+        pre-existing value of environment variable, where {name} can be used to refer to the
+        name of the variable.
+        """
         v = self._values.get(name)
         if v is None:
             return default
-        return v.get_value(self._subsystem, self._pathsep)
+        if variable_reference:
+            return v.get_str(variable_reference, self._subsystem, self._pathsep)
+        else:
+            return v.get_value(self._subsystem, self._pathsep)
 
-    def items(self):
-        """returns {str: str} (varname: value)"""
-        return {k: v.get_value(self._subsystem, self._pathsep)
-                for k, v in self._values.items()}.items()
+    def items(self, variable_reference=None):
+        """returns {str: str} (varname: value)
+         :param variable_reference: if specified, use a variable reference instead of the
+        pre-existing value of environment variable, where {name} can be used to refer to the
+        name of the variable.
+        """
+        if variable_reference:
+            return {k: v.get_str(variable_reference, self._subsystem, self._pathsep)
+                    for k, v in self._values.items()}.items()
+        else:
+            return {k: v.get_value(self._subsystem, self._pathsep)
+                    for k, v in self._values.items()}.items()
 
     @contextmanager
     def apply(self):
diff --git a/conan/tools/gnu/pkgconfigdeps.py b/conan/tools/gnu/pkgconfigdeps.py
index 38a7737d6..122e667b1 100644
--- a/conan/tools/gnu/pkgconfigdeps.py
+++ b/conan/tools/gnu/pkgconfigdeps.py
@@ -157,10 +157,12 @@ class _PCContentGenerator:
     def content(self, info):
         assert isinstance(info, _PCInfo) and info.cpp_info is not None
 
-        package_folder = self._dep.package_folder or ""  # If editable, package_folder can be None
+        # If editable, package_folder can be None
+        root_folder = self._dep.recipe_folder if self._dep.package_folder is None \
+            else self._dep.package_folder
         version = info.cpp_info.get_property("component_version") or self._dep.ref.version
 
-        prefix_path = package_folder.replace("\\", "/")
+        prefix_path = root_folder.replace("\\", "/")
         libdirs = _get_formatted_dirs(info.cpp_info.libdirs, prefix_path)
         includedirs = _get_formatted_dirs(info.cpp_info.includedirs, prefix_path)
         custom_content = info.cpp_info.get_property("pkg_config_custom_content")
diff --git a/conan/tools/microsoft/msbuilddeps.py b/conan/tools/microsoft/msbuilddeps.py
index 3c3d2739c..2053d38a0 100644
--- a/conan/tools/microsoft/msbuilddeps.py
+++ b/conan/tools/microsoft/msbuilddeps.py
@@ -158,17 +158,18 @@ class MSBuildDeps(object):
             for p in paths:
                 assert os.path.isabs(p), "{} is not absolute".format(p)
                 full_path = escape_path(p)
-                if full_path.startswith(package_folder):
-                    rel = full_path[len(package_folder)+1:]
+                if full_path.startswith(root_folder):
+                    rel = full_path[len(root_folder)+1:]
                     full_path = ("%s/%s" % (pkg_placeholder, rel))
                 ret.append(full_path)
             return "".join("{};".format(e) for e in ret)
 
-        package_folder = escape_path(dep.package_folder)
+        root_folder = dep.recipe_folder if dep.package_folder is None else dep.package_folder
+        root_folder = escape_path(root_folder)
 
         fields = {
             'name': name,
-            'root_folder': package_folder,
+            'root_folder': root_folder,
             'bin_dirs': join_paths(cpp_info.bindirs),
             'res_dirs': join_paths(cpp_info.resdirs),
             'include_dirs': join_paths(cpp_info.includedirs),
diff --git a/conans/client/graph/graph.py b/conans/client/graph/graph.py
index fbcfac5c5..0b40149f8 100644
--- a/conans/client/graph/graph.py
+++ b/conans/client/graph/graph.py
@@ -92,6 +92,7 @@ class Node(object):
         self.id_indirect_prefs = None
 
         self.cant_build = False  # It will set to a str with a reason if the validate_build() fails
+        self.should_build = False  # If the --build or policy wants to build this binary
 
     @property
     def id(self):
diff --git a/conans/client/graph/graph_binaries.py b/conans/client/graph/graph_binaries.py
index 4977e2d68..e94c59978 100644
--- a/conans/client/graph/graph_binaries.py
+++ b/conans/client/graph/graph_binaries.py
@@ -47,6 +47,7 @@ class GraphBinariesAnalyzer(object):
                     with_deps_to_build = True
                     break
         if build_mode.forced(conanfile, ref, with_deps_to_build):
+            node.should_build = True
             conanfile.output.info('Forced build from source')
             if node.cant_build:
                 node.binary = BINARY_INVALID
@@ -181,6 +182,7 @@ class GraphBinariesAnalyzer(object):
             pref = PackageReference(locked.ref, locked.package_id, locked.prev)  # Keep locked PREV
             self._process_node(node, pref, build_mode, update, remotes)
             if node.binary == BINARY_MISSING and build_mode.allowed(node.conanfile):
+                node.should_build = True
                 if node.cant_build:
                     node.binary = BINARY_INVALID
                 else:
@@ -206,7 +208,7 @@ class GraphBinariesAnalyzer(object):
             assert node.binary is None, "Node.binary should be None if not locked"
             pref = PackageReference(node.ref, node.package_id)
             self._process_node(node, pref, build_mode, update, remotes)
-            if node.binary in (BINARY_MISSING, BINARY_INVALID):
+            if node.binary in (BINARY_MISSING, BINARY_INVALID) and not node.should_build:
                 conanfile = node.conanfile
                 self._compatibility.compatibles(conanfile)
                 if node.conanfile.compatible_packages:
@@ -237,6 +239,7 @@ class GraphBinariesAnalyzer(object):
                     if node.binary == BINARY_MISSING and node.package_id == PACKAGE_ID_INVALID:
                         node.binary = BINARY_INVALID
                 if node.binary == BINARY_MISSING and build_mode.allowed(node.conanfile):
+                    node.should_build = True
                     if node.cant_build:
                         node.binary = BINARY_INVALID
                     else:
@@ -258,7 +261,8 @@ class GraphBinariesAnalyzer(object):
 
         if pref.id == PACKAGE_ID_INVALID:
             # annotate pattern, so unused patterns in --build are not displayed as errors
-            build_mode.forced(node.conanfile, node.ref)
+            if build_mode.forced(node.conanfile, node.ref):
+                node.should_build = True
             node.binary = BINARY_INVALID
             return
 
@@ -304,6 +308,7 @@ class GraphBinariesAnalyzer(object):
                 local_recipe_hash = package_layout.recipe_manifest().summary_hash
                 if local_recipe_hash != recipe_hash:
                     conanfile.output.info("Outdated package!")
+                    node.should_build = True
                     if node.cant_build:
                         node.binary = BINARY_INVALID
                     else:


Test patch:
diff --git a/conans/test/functional/layout/test_editable_msbuild.py b/conans/test/functional/layout/test_editable_msbuild.py
index a06d8779e..4773d72ae 100644
--- a/conans/test/functional/layout/test_editable_msbuild.py
+++ b/conans/test/functional/layout/test_editable_msbuild.py
@@ -1,8 +1,10 @@
 import os
 import platform
+import textwrap
 
 import pytest
 
+from conans.test.assets.genconanfile import GenConanfile
 from conans.test.assets.pkg_cmake import pkg_cmake_app
 from conans.test.utils.tools import TestClient
 
@@ -62,3 +64,25 @@ def test_editable_msbuild():
     c.run("create pkg")
     # print(c.out)
     assert "pkg/0.1: Created package" in c.out
+
+
+@pytest.mark.skipif(platform.system() != "Windows", reason="Only for windows")
+def test_editable_msbuilddeps():
+    # https://github.com/conan-io/conan/issues/12521
+    c = TestClient()
+    dep = textwrap.dedent("""
+        from conan import ConanFile
+        class Dep(ConanFile):
+            name = "dep"
+            version = "0.1"
+            def layout(self):
+                pass
+            """)
+    c.save({"dep/conanfile.py": dep,
+            "pkg/conanfile.py": GenConanfile("pkg", "0.1").with_settings("build_type", "arch")
+                                                          .with_requires("dep/0.1")
+                                                          .with_generator("MSBuildDeps")})
+    c.run("editable add dep dep/0.1")
+    c.run("install pkg")
+    # It doesn't crash anymore
+    assert "dep/0.1:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9 - Editable" in c.out
diff --git a/conans/test/integration/layout/test_layout_paths.py b/conans/test/integration/layout/test_layout_paths.py
new file mode 100644
index 000000000..5d863f411
--- /dev/null
+++ b/conans/test/integration/layout/test_layout_paths.py
@@ -0,0 +1,33 @@
+import textwrap
+
+from conans.test.assets.genconanfile import GenConanfile
+from conans.test.utils.tools import TestClient
+
+
+def test_editable_layout_paths():
+    # https://github.com/conan-io/conan/issues/12521
+    c = TestClient()
+    dep = textwrap.dedent("""
+        import os
+        from conan import ConanFile
+        from conan.tools.files import save
+        class Dep(ConanFile):
+            name = "dep"
+            version = "0.1"
+            def layout(self):
+                self.cpp.source.includedirs = ["include"]
+            """)
+    c.save({"dep/conanfile.py": dep,
+            "pkg/conanfile.py": GenConanfile("pkg", "0.1").with_settings("build_type", "arch")
+                                                          .with_requires("dep/0.1")
+                                                          .with_generator("CMakeDeps")
+                                                          .with_generator("PkgConfigDeps")})
+    c.run("editable add dep dep/0.1")
+    c.run("install pkg")
+    # It doesn't crash anymore
+    assert "dep/0.1:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9 - Editable" in c.out
+    arch = c.get_default_host_profile().settings['arch']
+    data = c.load(f"dep-release-{arch}-data.cmake")
+    assert 'set(dep_INCLUDE_DIRS_RELEASE "${dep_PACKAGE_FOLDER_RELEASE}/include")' in data
+    pc = c.load("dep.pc")
+    assert "includedir1=${prefix}/include" in pc
diff --git a/conans/test/integration/package_id/test_validate.py b/conans/test/integration/package_id/test_validate.py
index 60b644872..e77a007ce 100644
--- a/conans/test/integration/package_id/test_validate.py
+++ b/conans/test/integration/package_id/test_validate.py
@@ -118,17 +118,37 @@ class TestValidate(unittest.TestCase):
         self.assertIn("pkg/0.1: Package 'cb054d0b3e1ca595dc66bc2339d40f1f8f04ab31' created",
                       client.out)
 
-        client.run("create . pkg/0.1@ -s os=Windows")
+        # This is the main difference, building from source for the specified conf, fails
+        client.run("create . pkg/0.1@ -s os=Windows", assert_error=True)
+        self.assertIn("pkg/0.1:INVALID - Invalid", client.out)
+        self.assertIn("Windows not supported", client.out)
+
+        client.run("install pkg/0.1@ -s os=Windows --build=pkg", assert_error=True)
+        self.assertIn("pkg/0.1:INVALID - Invalid", client.out)
+        self.assertIn("Windows not supported", client.out)
+
+        client.run("install pkg/0.1@ -s os=Windows")
         self.assertIn("pkg/0.1: Main binary package 'INVALID' missing. "
                       "Using compatible package 'cb054d0b3e1ca595dc66bc2339d40f1f8f04ab31'",
                       client.out)
         self.assertIn("pkg/0.1:cb054d0b3e1ca595dc66bc2339d40f1f8f04ab31 - Cache", client.out)
+
+        # --build=missing means "use existing binary if possible", and compatibles are valid binaries
+        client.run("install pkg/0.1@ -s os=Windows --build=missing")
+        self.assertIn("pkg/0.1: Main binary package 'INVALID' missing. "
+                      "Using compatible package 'cb054d0b3e1ca595dc66bc2339d40f1f8f04ab31'",
+                      client.out)
+        self.assertIn("pkg/0.1:cb054d0b3e1ca595dc66bc2339d40f1f8f04ab31 - Cache", client.out)
+
         client.run("info pkg/0.1@ -s os=Windows")
         self.assertIn("pkg/0.1: Main binary package 'INVALID' missing. "
                       "Using compatible package 'cb054d0b3e1ca595dc66bc2339d40f1f8f04ab31'",
                       client.out)
         self.assertIn("ID: cb054d0b3e1ca595dc66bc2339d40f1f8f04ab31", client.out)
 
+        client.run("info pkg/0.1@ -s os=Windows --dry-build=pkg")
+        self.assertIn("Binary: Invalid", client.out)
+
     def test_validate_compatible_also_invalid(self):
         client = TestClient()
         conanfile = textwrap.dedent("""
diff --git a/conans/test/integration/toolchains/gnu/test_autotoolstoolchain.py b/conans/test/integration/toolchains/gnu/test_autotoolstoolchain.py
index 36ede715b..1b9b3269a 100644
--- a/conans/test/integration/toolchains/gnu/test_autotoolstoolchain.py
+++ b/conans/test/integration/toolchains/gnu/test_autotoolstoolchain.py
@@ -34,13 +34,13 @@ def test_extra_flags_via_conf():
     toolchain = client.load("conanautotoolstoolchain{}".format('.bat' if os_ == "Windows" else '.sh'))
     if os_ == "Windows":
         assert 'set "CPPFLAGS=%CPPFLAGS% -DNDEBUG -DDEF1 -DDEF2"' in toolchain
-        assert 'set "CXXFLAGS=%CXXFLAGS% -O3 -s --flag1 --flag2"' in toolchain
-        assert 'set "CFLAGS=%CFLAGS% -O3 -s --flag3 --flag4"' in toolchain
+        assert 'set "CXXFLAGS=%CXXFLAGS% -O3 --flag1 --flag2"' in toolchain
+        assert 'set "CFLAGS=%CFLAGS% -O3 --flag3 --flag4"' in toolchain
         assert 'set "LDFLAGS=%LDFLAGS% --flag5 --flag6"' in toolchain
     else:
         assert 'export CPPFLAGS="$CPPFLAGS -DNDEBUG -DDEF1 -DDEF2"' in toolchain
-        assert 'export CXXFLAGS="$CXXFLAGS -O3 -s --flag1 --flag2"' in toolchain
-        assert 'export CFLAGS="$CFLAGS -O3 -s --flag3 --flag4"' in toolchain
+        assert 'export CXXFLAGS="$CXXFLAGS -O3 --flag1 --flag2"' in toolchain
+        assert 'export CFLAGS="$CFLAGS -O3 --flag3 --flag4"' in toolchain
         assert 'export LDFLAGS="$LDFLAGS --flag5 --flag6"' in toolchain
 
 
diff --git a/conans/test/unittests/tools/env/test_env.py b/conans/test/unittests/tools/env/test_env.py
index 0545fd8ed..fb40a69be 100644
--- a/conans/test/unittests/tools/env/test_env.py
+++ b/conans/test/unittests/tools/env/test_env.py
@@ -410,3 +410,14 @@ class TestProfileEnvRoundTrip:
             MyPath1=+(path)/my/path12
             MyPath1+=(path)/my/path11
             """)
+
+
+def test_custom_placeholder():
+    # https://github.com/conan-io/conan/issues/12427
+    env = Environment()
+    env.append_path("MyVar", "MyValue")
+    env = env.vars(ConanFileMock())
+    assert env.get("MyVar", variable_reference="$penv{{{name}}}") == \
+           f"$penv{{MyVar}}{os.pathsep}MyValue"
+    items = {k: v for k, v in env.items(variable_reference="$penv{{{name}}}")}
+    assert  items == {"MyVar": f"$penv{{MyVar}}{os.pathsep}MyValue"}