Feature/profile env roundtrip (#9320)

* wip

* wip

* renaming methods to dissambiguate

* fix tests

* fix tests

* minor fixes and comments

* Update conans/test/unittests/tools/env/test_env.py

Co-authored-by: Luis Martinez de Bartolome Izquierdo <lasote@gmail.com>

Co-authored-by: Luis Martinez de Bartolome Izquierdo <lasote@gmail.com>

conan/tools/env/environment.py

@@ -53,13 +53,30 @@ def environment_wrap_command(conanfile, env_filenames, cmd, cwd=None):
 
 
 class _EnvValue:
-    def __init__(self, name, value=_EnvVarPlaceHolder, separator=" ",
-                 path=False):
+    def __init__(self, name, value=_EnvVarPlaceHolder, separator=" ", path=False):
         self._name = name
         self._values = [] if value is None else value if isinstance(value, list) else [value]
         self._path = path
         self._sep = separator
 
+    def dumps(self):
+        result = []
+        path = "(path)" if self._path else ""
+        if not self._values:  # Empty means unset
+            result.append("{}=!".format(self._name))
+        elif _EnvVarPlaceHolder in self._values:
+            index = self._values.index(_EnvVarPlaceHolder)
+            for v in self._values[:index]:
+                result.append("{}=+{}{}".format(self._name, path, v))
+            for v in self._values[index+1:]:
+                result.append("{}+={}{}".format(self._name, path, v))
+        else:
+            append = ""
+            for v in self._values:
+                result.append("{}{}={}{}".format(self._name, append, path, v))
+                append = "+"
+        return "\n".join(result)
+
     def copy(self):
         return _EnvValue(self._name, self._values, self._sep, self._path)
 
@@ -86,7 +103,7 @@ def prepend(self, value, separator=None):
         else:
             self._values.insert(0, value)
 
-    def compose(self, other):
+    def compose_env_value(self, other):
         """
         :type other: _EnvValue
         """
@@ -101,6 +118,7 @@ def compose(self, other):
 
     def get_str(self, conanfile, placeholder, pathsep=os.pathsep):
         """
+        :param conanfile: The conanfile is necessary to get win_bash, path separator, etc.
         :param placeholder: a OS dependant string pattern of the previous env-var value like
         $PATH, %PATH%, et
         :param pathsep: The path separator, typically ; or :
@@ -138,9 +156,17 @@ def __bool__(self):
 
     __nonzero__ = __bool__
 
+    def copy(self):
+        e = Environment(self._conanfile)
+        e._values = self._values.copy()
+        return e
+
     def __repr__(self):
         return repr(self._values)
 
+    def dumps(self):
+        return "\n".join([v.dumps() for v in reversed(self._values.values())])
+
     def define(self, name, value, separator=" "):
         self._values[name] = _EnvValue(name, value, separator, path=False)
 
@@ -259,19 +285,20 @@ def save_script(self, name, auto_activate=True):
         if auto_activate:
             register_environment_script(self._conanfile, path)
 
-    def compose(self, other):
+    def compose_env(self, other):
         """
         self has precedence, the "other" will add/append if possible and not conflicting, but
-        self mandates what to do
+        self mandates what to do. If self has define(), without placeholder, that will remain
         :type other: Environment
         """
         for k, v in other._values.items():
             existing = self._values.get(k)
             if existing is None:
                 self._values[k] = v.copy()
             else:
-                existing.compose(v)
+                existing.compose_env_value(v)
 
+        self._conanfile = self._conanfile or other._conanfile
         return self
 
     # Methods to user access to the environment object as a dict
@@ -319,6 +346,11 @@ def __init__(self):
     def __repr__(self):
         return repr(self._environments)
 
+    def __bool__(self):
+        return bool(self._environments)
+
+    __nonzero__ = __bool__
+
     def get_env(self, conanfile, ref):
         """ computes package-specific Environment
         it is only called when conanfile.buildenv is called
@@ -327,24 +359,34 @@ def get_env(self, conanfile, ref):
         result = Environment(conanfile)
         for pattern, env in self._environments.items():
             if pattern is None or fnmatch.fnmatch(str(ref), pattern):
-                result = env.compose(result)
-
-        # FIXME: Needed to assign _conanfile here too because in the env.compose returns env and it
-        #        hasn't conanfile
-        result._conanfile = conanfile
+                # Latest declared has priority, copy() necessary to not destroy data
+                result = env.copy().compose_env(result)
         return result
 
-    def compose(self, other):
+    def update_profile_env(self, other):
         """
         :type other: ProfileEnvironment
+        :param other: The argument profile has priority/precedence over the current one.
         """
         for pattern, environment in other._environments.items():
             existing = self._environments.get(pattern)
             if existing is not None:
-                self._environments[pattern] = environment.compose(existing)
+                self._environments[pattern] = environment.compose_env(existing)
             else:
                 self._environments[pattern] = environment
 
+    def dumps(self):
+        result = []
+        for pattern, env in self._environments.items():
+            if pattern is None:
+                result.append(env.dumps())
+            else:
+                result.append("\n".join("{}:{}".format(pattern, line) if line else ""
+                                        for line in env.dumps().splitlines()))
+        if result:
+            result.append("")
+        return "\n".join(result)
+
     @staticmethod
     def loads(text):
         result = ProfileEnvironment()
@@ -364,6 +406,7 @@ def loads(text):
                 else:
                     pattern, name = None, pattern_name[0]
 
+                # When loading from profile file, latest line has priority
                 env = Environment(conanfile=None)
                 if method == "unset":
                     env.unset(name)
@@ -377,10 +420,10 @@ def loads(text):
                 if existing is None:
                     result._environments[pattern] = env
                 else:
-                    result._environments[pattern] = env.compose(existing)
+                    result._environments[pattern] = env.compose_env(existing)
                 break
             else:
-                raise ConanException("Bad env defintion: {}".format(line))
+                raise ConanException("Bad env definition: {}".format(line))
         return result
 
 

conan/tools/env/virtualbuildenv.py

@@ -20,23 +20,23 @@ def environment(self):
 
         # Top priority: profile
         profile_env = self._conanfile.buildenv
-        build_env.compose(profile_env)
+        build_env.compose_env(profile_env)
 
         for require, build_require in self._conanfile.dependencies.build.items():
             if require.direct:
                 # higher priority, explicit buildenv_info
                 if build_require.buildenv_info:
-                    build_env.compose(build_require.buildenv_info)
+                    build_env.compose_env(build_require.buildenv_info)
             # Lower priority, the runenv of all transitive "requires" of the build requires
             if build_require.runenv_info:
-                build_env.compose(build_require.runenv_info)
+                build_env.compose_env(build_require.runenv_info)
             # Then the implicit
-            build_env.compose(runenv_from_cpp_info(self._conanfile, build_require.cpp_info))
+            build_env.compose_env(runenv_from_cpp_info(self._conanfile, build_require.cpp_info))
 
         # Requires in host context can also bring some direct buildenv_info
         for require in self._conanfile.dependencies.host.values():
             if require.buildenv_info:
-                build_env.compose(require.buildenv_info)
+                build_env.compose_env(require.buildenv_info)
 
         return build_env
 

conan/tools/env/virtualrunenv.py

@@ -38,9 +38,8 @@ def environment(self):
         test_req = self._conanfile.dependencies.test
         for _, dep in list(host_req.items()) + list(test_req.items()):
             if dep.runenv_info:
-                runenv.compose(dep.runenv_info)
-            runenv.compose(runenv_from_cpp_info(self._conanfile, dep.cpp_info))
-
+                runenv.compose_env(dep.runenv_info)
+            runenv.compose_env(runenv_from_cpp_info(self._conanfile, dep.cpp_info))
 
         return runenv
 

conans/client/profile_loader.py

@@ -149,7 +149,7 @@ def _load_profile(text, profile_path, default_folder):
         for include in profile_parser.get_includes():
             # Recursion !!
             profile, included_vars = read_profile(include, cwd, default_folder)
-            inherited_profile.compose(profile)
+            inherited_profile.compose_profile(profile)
             profile_parser.update_vars(included_vars)
 
         # Apply the automatic PROFILE_DIR variable
@@ -239,7 +239,7 @@ def get_package_name_value(item):
 
     if doc.buildenv:
         buildenv = ProfileEnvironment.loads(doc.buildenv)
-        base_profile.buildenv.compose(buildenv)
+        base_profile.buildenv.update_profile_env(buildenv)
 
 
 def profile_from_args(profiles, settings, options, env, conf, cwd, cache):
@@ -253,12 +253,12 @@ def profile_from_args(profiles, settings, options, env, conf, cwd, cache):
         result = Profile()
         for p in profiles:
             tmp, _ = read_profile(p, cwd, cache.profiles_path)
-            result.compose(tmp)
+            result.compose_profile(tmp)
 
     args_profile = _profile_parse_args(settings, options, env, conf)
 
     if result:
-        result.compose(args_profile)
+        result.compose_profile(args_profile)
     else:
         result = args_profile
     return result

conans/model/profile.py

@@ -90,9 +90,13 @@ def dumps(self):
             result.append("[conf]")
             result.append(self.conf.dumps())
 
+        if self.buildenv:
+            result.append("[buildenv]")
+            result.append(self.buildenv.dumps())
+
         return "\n".join(result).replace("\n\n", "\n")
 
-    def compose(self, other):
+    def compose_profile(self, other):
         self.update_settings(other.settings)
         self.update_package_settings(other.package_settings)
         # this is the opposite
@@ -116,7 +120,7 @@ def compose(self, other):
             self.build_requires[pattern] = list(existing.values())
 
         self.conf.update_conf_definition(other.conf)
-        self.buildenv.compose(other.buildenv)
+        self.buildenv.update_profile_env(other.buildenv)  # Profile composition, last has priority
 
     def update_settings(self, new_settings):
         """Mix the specified settings with the current profile.

conans/test/unittests/client/generators/generator_filter_error_test.py

@@ -2,7 +2,6 @@
 import unittest
 
 from conans.test.utils.tools import NO_SETTINGS_PACKAGE_ID, TestClient
-from conans.util.files import load
 
 
 class MultiGeneratorFilterErrorTest(unittest.TestCase):

conans/test/unittests/client/profile_loader/profile_loader_test.py

@@ -7,6 +7,7 @@
 from conans.model.env_info import EnvValues
 from conans.model.profile import Profile
 from conans.model.ref import ConanFileReference
+from conans.test.utils.mocks import ConanFileMock
 from conans.test.utils.profiles import create_profile as _create_profile
 from conans.test.utils.test_files import temp_folder
 from conans.util.files import load, save
@@ -448,3 +449,20 @@ def test_profile_load_relative_path_pardir():
                                       default_profile_folder)
     assert ({"BORSCHT": "RUSSIAN SOUP"}, {}) == profile.env_values.env_dicts("")
     assert current_profile_folder.replace("\\", "/") == variables["PROFILE_DIR"]
+
+
+def test_profile_buildenv():
+    tmp = temp_folder()
+    txt = textwrap.dedent("""
+        [buildenv]
+        MyVar1=My Value; 11
+        MyVar1+=MyValue12
+        MyPath1=(path)/some/path11
+        MyPath1+=(path)/other path/path12
+        """)
+    profile, _ = get_profile(tmp, txt)
+    buildenv = profile.buildenv
+    env = buildenv.get_env(ConanFileMock(), None)
+    assert env.get("MyVar1") == "My Value; 11 MyValue12"
+
+    assert env.get("MyPath1") == "/some/path11{}/other path/path12".format(os.pathsep)

conans/test/unittests/model/profile_test.py

@@ -119,17 +119,17 @@ def test_update_build_requires():
     profile2 = Profile()
     profile2.build_requires["*"] = ["zlib/1.2.8"]
 
-    profile.compose(profile2)
+    profile.compose_profile(profile2)
     assert profile.build_requires["*"] == ["zlib/1.2.8"]
 
     profile3 = Profile()
     profile3.build_requires["*"] = ["zlib/1.2.11"]
 
-    profile.compose(profile3)
+    profile.compose_profile(profile3)
     assert profile.build_requires["*"] == ["zlib/1.2.11"]
 
     profile4 = Profile()
     profile4.build_requires["*"] = ["cmake/2.7"]
 
-    profile.compose(profile4)
+    profile.compose_profile(profile4)
     assert profile.build_requires["*"] == ["zlib/1.2.11", "cmake/2.7"]