Always remove package folder in conan create (#4918)

* Always remove package folder in conan create

* remove

* fix if-else

* Remove package folder earlier

* review with new test

conans/client/installer.py

@@ -77,17 +77,12 @@ def _get_build_folder(self, conanfile, package_layout, pref, keep_build, recorde
         return build_folder, skip_build
 
     def _prepare_sources(self, conanfile, pref, package_layout, conanfile_path, source_folder,
-                         build_folder, package_folder, remotes):
+                         build_folder, remotes):
         export_folder = package_layout.export()
         export_source_folder = package_layout.export_sources()
 
         complete_recipe_sources(self._remote_manager, self._cache, conanfile, pref.ref, remotes)
-        try:
-            rmdir(build_folder)
-            rmdir(package_folder)
-        except OSError as e:
-            raise ConanException("%s\n\nCouldn't remove folder, might be busy or open\n"
-                                 "Close any app using it, and retry" % str(e))
+        _remove_folder_raising(build_folder)
 
         config_source(export_folder, export_source_folder, source_folder,
                       conanfile, self._output, conanfile_path, pref.ref,
@@ -140,6 +135,7 @@ def _build(self, conanfile, pref, build_folder):
 
     def _package(self, conanfile, pref, package_layout, conanfile_path, build_folder,
                  package_folder):
+
         # FIXME: Is weak to assign here the recipe_hash
         manifest = package_layout.recipe_manifest()
         conanfile.info.recipe_hash = manifest.summary_hash
@@ -190,10 +186,11 @@ def build_package(self, node, keep_build, recorder, remotes):
             with package_layout.conanfile_write_lock(self._output):
                 set_dirty(build_folder)
                 self._prepare_sources(conanfile, pref, package_layout, conanfile_path, source_folder,
-                                      build_folder, package_folder, remotes)
+                                      build_folder, remotes)
 
         # BUILD & PACKAGE
         with package_layout.conanfile_read_lock(self._output):
+            _remove_folder_raising(package_folder)
             mkdir(build_folder)
             os.chdir(build_folder)
             self._output.info('Building your package in %s' % build_folder)
@@ -227,6 +224,14 @@ def build_package(self, node, keep_build, recorder, remotes):
             return node.pref
 
 
+def _remove_folder_raising(folder):
+    try:
+        rmdir(folder)
+    except OSError as e:
+        raise ConanException("%s\n\nCouldn't remove folder, might be busy or open\n"
+                             "Close any app using it, and retry" % str(e))
+
+
 def _handle_system_requirements(conan_file, pref, cache, out):
     """ check first the system_reqs/system_requirements.txt existence, if not existing
     check package/sha1/

conans/test/functional/command/create_test.py

@@ -1,10 +1,12 @@
 import os
+import textwrap
 import unittest
 
 from parameterized.parameterized import parameterized
 
 from conans.client import tools
-from conans.test.utils.tools import TestClient
+from conans.model.ref import ConanFileReference, PackageReference
+from conans.test.utils.tools import TestClient, NO_SETTINGS_PACKAGE_ID
 from conans.util.files import load
 
 
@@ -68,29 +70,34 @@ def test(self):
     @parameterized.expand([(True, ), (False, )])
     def keep_build_test(self, with_test):
         client = TestClient()
-        conanfile = """from conans import ConanFile
-class MyPkg(ConanFile):
-    exports_sources = "*.h"
-    def source(self):
-        self.output.info("mysource!!")
-    def build(self):
-        self.output.info("mybuild!!")
-    def package(self):
-        self.output.info("mypackage!!")
-        self.copy("*.h")
-"""
+        conanfile = textwrap.dedent("""
+            from conans import ConanFile
+            
+            class MyPkg(ConanFile):
+                exports_sources = "*.h"
+                def source(self):
+                    self.output.info("mysource!!")
+                def build(self):
+                    self.output.info("mybuild!!")
+                def package(self):
+                    self.output.info("mypackage!!")
+                    self.copy("*.h")
+            """)
         if with_test:
-            client.save({"conanfile.py": conanfile,
-                         "header.h": ""})
-        else:
-            test_conanfile = """from conans import ConanFile
-class MyPkg(ConanFile):
-    def test(self):
-        pass
-"""
+            test_conanfile = textwrap.dedent("""
+            from conans import ConanFile
+
+            class MyPkg(ConanFile):
+                def test(self):
+                    pass
+            """)
             client.save({"conanfile.py": conanfile,
                          "header.h": "",
                          "test_package/conanfile.py": test_conanfile})
+        else:
+            client.save({"conanfile.py": conanfile,
+                         "header.h": ""})
+
         client.run("create . Pkg/0.1@lasote/testing")
         self.assertIn("Pkg/0.1@lasote/testing: mysource!!", client.out)
         self.assertIn("Pkg/0.1@lasote/testing: mybuild!!", client.out)
@@ -135,6 +142,36 @@ class MyPkg(ConanFile):
         client.run("create . Pkg/0.1@lasote/testing --keep-build", assert_error=True)
         self.assertIn("ERROR: --keep-build specified, but build folder not found", client.out)
 
+    def keep_build_package_folder_test(self):
+        """
+        Package folder should be deleted always before a new conan create command, even with
+        --keep-build
+        """
+        client = TestClient()
+        conanfile = textwrap.dedent("""
+            from conans import ConanFile
+
+            class MyPkg(ConanFile):
+                exports_sources = "*.h", "*.cpp"
+                def package(self):
+                    self.copy("*.h")
+            """)
+        client.save({"conanfile.py": conanfile,
+                     "header.h": "",
+                     "source.cpp": ""})
+        client.run("create . pkg/0.1@danimtb/testing")
+        ref = ConanFileReference("pkg", "0.1", "danimtb", "testing")
+        pref = PackageReference(ref, NO_SETTINGS_PACKAGE_ID)
+        package_files = os.listdir(client.cache.package(pref))
+        self.assertIn("header.h", package_files)
+        self.assertNotIn("source.cpp", package_files)
+        client.save({"conanfile.py": conanfile.replace("self.copy(\"*.h\")",
+                                                       "self.copy(\"*.cpp\")")})
+        client.run("create . pkg/0.1@danimtb/testing -kb")
+        package_files = os.listdir(client.cache.package(pref))
+        self.assertNotIn("header.h", package_files)
+        self.assertIn("source.cpp", package_files)
+
     def create_test(self):
         client = TestClient()
         client.save({"conanfile.py": """from conans import ConanFile
@@ -454,3 +491,24 @@ def test(self):
         self.assertTrue(os.path.exists(os.path.join(client.current_folder, "test_package",
                                                     "build_folder")))
         self.assertFalse(os.path.exists(default_build_dir))
+
+    def package_folder_build_error_test(self):
+        """
+        Check package folder is not created if the build step fails
+        """
+        client = TestClient()
+        conanfile = textwrap.dedent("""
+        from conans import ConanFile
+
+        class MyPkg(ConanFile):
+
+            def build(self):
+                raise ConanException("Build error")
+        """)
+        client.save({"conanfile.py": conanfile})
+        ref = ConanFileReference("pkg", "0.1", "danimtb", "testing")
+        pref = PackageReference(ref, NO_SETTINGS_PACKAGE_ID, None)
+        client.run("create . %s" % ref.full_repr(), assert_error=True)
+        self.assertIn("Build error", client.out)
+        package_folder = client.cache.package(pref)
+        self.assertFalse(os.path.exists(package_folder))