Repository: conan-io/conan
Pull request: Remove double application of env vars from profile
Changelog: BugFix: Prepend environment variables are applied twice in conanfile
Docs: omit

- [x] Refer to the issue that supports this Pull Request: closes #4385 
- [x] If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
- [x] I've read the [Contributing guide](https://github.com/conan-io/conan/blob/develop/.github/CONTRIBUTING.md).
- [x] I've followed the PEP8 style guides for Python code.
- [ ] I've opened another PR in the Conan docs repo to the ``develop`` branch, documenting this one. 

<sup>**Note:** By default this PR will skip the slower tests and will use a limited set of python versions. Check [here](https://github.com/conan-io/conan/blob/develop/.github/PR_INCREASE_TESTING.md) how to increase the testing level by writing some tags in the current PR body text.</sup>


Problem statement:
Virtualenv generator prepends duplicated values in env vars from profile
With a *conanfile.txt* empty and a profile with:

```
[env]
PREPEND_VAR = ['kk','pp']
```

Commands:

```
$ conan install . -g virtualenv
```

In activate.sh there is:
```
PREPEND_VAR="kk":"pp":"kk":"pp"
```
and in *activate.bat* you alaso have:
```
SET PREPEND_VAR=kk;pp;kk;pp
```

So at least variables that should be pretended are applied twice.

Hints from issue discussion:

Commit message: make test conditional

Patch to review:
diff --git a/conans/client/loader.py b/conans/client/loader.py
index 525ef71b8ca..9bd73cab09c 100644
--- a/conans/client/loader.py
+++ b/conans/client/loader.py
@@ -174,7 +174,6 @@ def _parse_conan_txt(self, contents, path, display_name, processed_profile):
 
         # imports method
         conanfile.imports = parser.imports_method(conanfile)
-        conanfile._conan_env_values.update(processed_profile._env_values)
         return conanfile
 
     def load_virtual(self, references, processed_profile, scope_options=True,
diff --git a/conans/test/functional/configuration/profile_test.py b/conans/test/functional/configuration/profile_test.py
index e1ce9776577..133c34ff104 100644
--- a/conans/test/functional/configuration/profile_test.py
+++ b/conans/test/functional/configuration/profile_test.py
@@ -1,4 +1,5 @@
 import os
+import platform
 import unittest
 from collections import OrderedDict
 from textwrap import dedent
@@ -27,7 +28,6 @@ def build(self):
             self.run("SET")
         else:
             self.run("env")
-
 """
 
 
@@ -44,23 +44,27 @@ class ProfileTest(unittest.TestCase):
     def setUp(self):
         self.client = TestClient()
 
+    def profile_conanfile_txt_test(self):
+        self.client.save({"conanfile.txt": ""})
+        create_profile(self.client.cache.profiles_path, "envs", settings={},
+                       env=[("A_VAR", "A_VALUE"), ("PREPEND_VAR", ["new_path", "other_path"])],
+                       package_env={"Hello0": [("OTHER_VAR", "2")]})
+        self.client.run("install . -pr envs -g virtualenv")
+
     def test_profile_relative_cwd(self):
-        client = TestClient()
-        client.save({"conanfile.txt": "",
-                     "sub/sub/profile": ""})
-        client.current_folder = os.path.join(client.current_folder, "sub")
-        client.run("install .. -pr=sub/profile2", assert_error=True)
-        self.assertIn("ERROR: Profile not found: sub/profile2", client.out)
-        client.run("install .. -pr=sub/profile")
-        self.assertIn("conanfile.txt: Installing package", client.out)
+        self.client.save({"conanfile.txt": "", "sub/sub/profile": ""})
+        self.client.current_folder = os.path.join(self.client.current_folder, "sub")
+        self.client.run("install .. -pr=sub/profile2", assert_error=True)
+        self.assertIn("ERROR: Profile not found: sub/profile2", self.client.out)
+        self.client.run("install .. -pr=sub/profile")
+        self.assertIn("conanfile.txt: Installing package", self.client.out)
 
     def base_profile_generated_test(self):
         """we are testing that the default profile is created (when not existing, fresh install)
          even when you run a create with a profile"""
-        client = TestClient()
-        client.save({CONANFILE: conanfile_scope_env,
-                     "myprofile": "include(default)\n[settings]\nbuild_type=Debug"})
-        client.run("create . conan/testing --profile myprofile")
+        self.client.save({CONANFILE: conanfile_scope_env,
+                          "myprofile": "include(default)\n[settings]\nbuild_type=Debug"})
+        self.client.run("create . conan/testing --profile myprofile")
 
     def bad_syntax_test(self):
         self.client.save({CONANFILE: conanfile_scope_env})
@@ -148,11 +152,19 @@ def install_profile_env_test(self):
         files["conanfile.py"] = conanfile_scope_env
 
         create_profile(self.client.cache.profiles_path, "envs", settings={},
-                       env=[("A_VAR", "A_VALUE")], package_env={"Hello0": [("OTHER_VAR", "2")]})
+                       env=[("A_VAR", "A_VALUE"),
+                            ("PREPEND_VAR", ["new_path", "other_path"])],
+                       package_env={"Hello0": [("OTHER_VAR", "2")]})
 
         self.client.save(files)
         self.client.run("export . lasote/stable")
         self.client.run("install Hello0/0.1@lasote/stable --build missing -pr envs")
+        if platform.system() == "Windows":
+            self._assert_env_variable_printed("PREPEND_VAR", "new_path;other_path")
+        else:
+            self._assert_env_variable_printed("PREPEND_VAR", "new_path:other_path")
+        self.assertNotIn("PREPEND_VAR=new_path;other_path;new_path;other_path", self.client.out)
+        self.assertNotIn("PREPEND_VAR=new_path:other_path:new_path:other_path", self.client.out)
         self._assert_env_variable_printed("A_VAR", "A_VALUE")
         self._assert_env_variable_printed("OTHER_VAR", "2")
 
diff --git a/conans/test/unittests/client/generators/virtualenv_test.py b/conans/test/unittests/client/generators/virtualenv_test.py
new file mode 100644
index 00000000000..777a9c658d8
--- /dev/null
+++ b/conans/test/unittests/client/generators/virtualenv_test.py
@@ -0,0 +1,19 @@
+import platform
+import unittest
+
+from conans import ConanFile, Settings
+from conans.client.generators.virtualenv import VirtualEnvGenerator
+from conans.model.env_info import EnvValues
+from conans.test.utils.tools import TestBufferConanOutput
+
+
+@unittest.skipUnless(platform.system() == "Windows", "Test both .sh and .bat files")
+class VirtualenvGeneratorTest(unittest.TestCase):
+
+    def prepend_values_test(self):
+        conanfile = ConanFile(TestBufferConanOutput(), None)
+        conanfile.initialize(Settings({}), EnvValues.loads("PATH=[1,2,three]"))
+        gen = VirtualEnvGenerator(conanfile)
+        content = gen.content
+        self.assertIn("PATH=\"1\":\"2\":\"three\":$PATH", content["activate.sh"])
+        self.assertIn("PATH=1;2;three;%PATH%", content["activate.bat"])
diff --git a/conans/test/unittests/client/loader_test.py b/conans/test/unittests/client/loader_test.py
new file mode 100644
index 00000000000..5ae0a4d96ee
--- /dev/null
+++ b/conans/test/unittests/client/loader_test.py
@@ -0,0 +1,69 @@
+import os
+import textwrap
+import unittest
+
+from conans import Settings
+from conans.client.graph.python_requires import ConanPythonRequire
+from conans.client.loader import ConanFileLoader
+from conans.model.env_info import EnvValues
+from conans.model.profile import Profile
+from conans.model.ref import ConanFileReference
+from conans.test.utils.conanfile import MockSettings
+from conans.test.utils.runner import TestRunner
+from conans.test.utils.test_files import temp_folder
+from conans.test.utils.tools import TestBufferConanOutput
+from conans.util.files import save
+
+
+class LoadConanfileTxtTest(unittest.TestCase):
+
+    def setUp(self):
+        settings = Settings()
+        self.profile = Profile()
+        self.profile._settings = settings
+        self.profile._user_options = None
+        self.profile._env_values = None
+        self.conanfile_txt_path = os.path.join(temp_folder(), "conanfile.txt")
+        output = TestBufferConanOutput()
+        self.loader = ConanFileLoader(TestRunner(output), output, None)
+
+    def env_test(self):
+        env_values = EnvValues()
+        env_values.add("PREPEND_PATH", ["hello", "bye"])
+        env_values.add("VAR", ["var_value"])
+        self.profile._env_values = env_values
+        save(self.conanfile_txt_path, "")
+        conanfile = self.loader.load_conanfile_txt(self.conanfile_txt_path, self.profile)
+        self.assertEquals(conanfile.env, {"PREPEND_PATH": ["hello", "bye"], "VAR": ["var_value"]})
+
+
+class LoadConanfileTest(unittest.TestCase):
+
+    def setUp(self):
+        settings = Settings()
+        self.profile = Profile()
+        self.profile._settings = settings
+        self.profile._user_options = None
+        self.profile._env_values = None
+        self.profile._dev_reference = None
+        self.profile._package_settings = None
+        self.conanfile_path = os.path.join(temp_folder(), "conanfile.py")
+        output = TestBufferConanOutput()
+        self.loader = ConanFileLoader(TestRunner(output), output, ConanPythonRequire(None, None))
+
+    def env_test(self):
+        env_values = EnvValues()
+        env_values.add("PREPEND_PATH", ["hello", "bye"])
+        env_values.add("VAR", ["var_value"])
+        self.profile._env_values = env_values
+        save(self.conanfile_path,
+             textwrap.dedent("""
+                from conans import ConanFile
+                
+                class TestConan(ConanFile):
+                    name = "hello"
+                    version = "1.0"
+             """))
+        ref = ConanFileReference("hello", "1.0", "user", "channel")
+        conanfile = self.loader.load_conanfile(self.conanfile_path, self.profile, ref)
+        self.assertEquals(conanfile.env, {"PREPEND_PATH": ["hello", "bye"], "VAR": ["var_value"]})


Code review comments:
diff_hunk: @@ -0,0 +1,19 @@
+import platform
+import unittest
+
+from conans import ConanFile, Settings
+from conans.client.generators.virtualenv import VirtualEnvGenerator
+from conans.model.env_info import EnvValues
+from conans.test.utils.tools import TestBufferConanOutput
+
+
+@unittest.skipUnless(platform.system() == "Windows", "Test both .sh and .bat files")
path: conans/test/unittests/client/generators/virtualenv_test.py
text: @user1:
why not mock `platform.system()`?

@author:
I have moved this check inside the test so it is not skipped when out of windows

diff_hunk: @@ -148,11 +152,19 @@ def install_profile_env_test(self):
         files["conanfile.py"] = conanfile_scope_env
 
         create_profile(self.client.cache.profiles_path, "envs", settings={},
-                       env=[("A_VAR", "A_VALUE")], package_env={"Hello0": [("OTHER_VAR", "2")]})
+                       env=[("A_VAR", "A_VALUE"),
+                            ("PREPEND_VAR", ["new_path", "other_path"])],
+                       package_env={"Hello0": [("OTHER_VAR", "2")]})
 
         self.client.save(files)
         self.client.run("export . lasote/stable")
         self.client.run("install Hello0/0.1@lasote/stable --build missing -pr envs")
+        if platform.system() == "Windows":
+            self._assert_env_variable_printed("PREPEND_VAR", "new_path;other_path")
+        else:
+            self._assert_env_variable_printed("PREPEND_VAR", "new_path:other_path")
+        self.assertNotIn("PREPEND_VAR=new_path;other_path;new_path;other_path", self.client.out)
path: conans/test/functional/configuration/profile_test.py
text: @user1:
Do not check this way. For example, when using Linux, it will never happen, because ``os.pathsep``.
The correct way to robustly check this is something like:

``self.assertEqual(1, str(self.client.out).count("new_path")``

diff_hunk: @@ -148,11 +152,19 @@ def install_profile_env_test(self):
         files["conanfile.py"] = conanfile_scope_env
 
         create_profile(self.client.cache.profiles_path, "envs", settings={},
-                       env=[("A_VAR", "A_VALUE")], package_env={"Hello0": [("OTHER_VAR", "2")]})
+                       env=[("A_VAR", "A_VALUE"),
+                            ("PREPEND_VAR", ["new_path", "other_path"])],
+                       package_env={"Hello0": [("OTHER_VAR", "2")]})
 
         self.client.save(files)
         self.client.run("export . lasote/stable")
         self.client.run("install Hello0/0.1@lasote/stable --build missing -pr envs")
+        if platform.system() == "Windows":
+            self._assert_env_variable_printed("PREPEND_VAR", "new_path;other_path")
path: conans/test/functional/configuration/profile_test.py
text: @user1:
You can use ``os.pathsep`` to check this in 1 line