Repository: conan-io/conan

Issue:
Move manifest calculation after 'post_export' hook
One of the provided hooks is the `post_export` one, it allows the user to affect the conanfile and the export folder, but it is being executed after computing an storing the digest/manifest, so those changes will break the computed hashes.

Hash must be computed after returning from the `post_export` hook.

Discussion:

Patch:
diff --git a/conans/client/cmd/export.py b/conans/client/cmd/export.py
index 31addfda9..44110243a 100644
--- a/conans/client/cmd/export.py
+++ b/conans/client/cmd/export.py
@@ -10,7 +10,7 @@ from conans.client.output import ScopedOutput
 from conans.errors import ConanException
 from conans.model.manifest import FileTreeManifest
 from conans.model.scm import SCM, get_scm_data
-from conans.paths import CONANFILE, CONAN_MANIFEST
+from conans.paths import CONANFILE
 from conans.search.search import search_recipes
 from conans.util.files import is_dirty, load, mkdir, rmdir, save, set_dirty, remove
 from conans.util.log import logger
@@ -42,9 +42,11 @@ def cmd_export(conanfile_path, conanfile, ref, keep_source, output, cache, hook_
     hook_manager.execute("pre_export", conanfile=conanfile, conanfile_path=conanfile_path,
                          reference=ref)
     logger.debug("EXPORT: %s" % conanfile_path)
-    output.highlight("Exporting package recipe")
 
+    output.highlight("Exporting package recipe")
     conan_linter(conanfile_path, output)
+
+    # Check for casing conflict
     # Maybe a platform check could be added, but depends on disk partition
     reference = str(ref)
     refs = search_recipes(cache, reference, ignorecase=True)
@@ -53,11 +55,65 @@ def cmd_export(conanfile_path, conanfile, ref, keep_source, output, cache, hook_
                              "You exported '%s' but already existing '%s'"
                              % (reference, " ".join(str(s) for s in refs)))
 
+    output = conanfile.output
+
+    # Get previous digest
+    package_layout = cache.package_layout(ref, conanfile.short_paths)
+    try:
+        previous_digest = FileTreeManifest.load(package_layout.export())
+    except IOError:
+        previous_digest = None
+    finally:
+        _recreate_folders(package_layout.export(), package_layout.export_sources())
+
+    # Copy sources to target folders
     with cache.conanfile_write_lock(ref):
-        _export_conanfile(conanfile_path, conanfile.output, cache, conanfile, ref, keep_source)
-    conanfile_cache_path = cache.conanfile(ref)
-    hook_manager.execute("post_export", conanfile=conanfile, conanfile_path=conanfile_cache_path,
-                         reference=ref)
+        origin_folder = os.path.dirname(conanfile_path)
+        export_recipe(conanfile, origin_folder, package_layout.export())
+        export_source(conanfile, origin_folder, package_layout.export_sources())
+        shutil.copy2(conanfile_path, package_layout.conanfile())
+
+        scm_data, captured_revision = _capture_export_scm_data(conanfile,
+                                                               os.path.dirname(conanfile_path),
+                                                               package_layout.export(),
+                                                               output, cache, ref)
+
+        # Execute post-export hook before computing the digest
+        hook_manager.execute("post_export", conanfile=conanfile, reference=ref,
+                             conanfile_path=package_layout.conanfile())
+
+        # Compute the new digest
+        digest = FileTreeManifest.create(package_layout.export(), package_layout.export_sources())
+        modified_recipe = not previous_digest or previous_digest != digest
+        if modified_recipe:
+            output.success('A new %s version was exported' % CONANFILE)
+            output.info('Folder: %s' % package_layout.export())
+        else:
+            output.info("The stored package has not changed")
+            digest = previous_digest  # Use the old one, keep old timestamp
+        digest.save(package_layout.export())
+
+    # Compute the revision for the recipe
+    revision = scm_data.revision if scm_data and captured_revision else digest.summary_hash
+    with package_layout.update_metadata() as metadata:
+        # Note that there is no time set, the time will come from the remote
+        metadata.recipe.revision = revision
+
+    # FIXME: Conan 2.0 Clear the registry entry if the recipe has changed
+    source_folder = package_layout.source()
+    if os.path.exists(source_folder):
+        try:
+            if is_dirty(source_folder):
+                output.info("Source folder is corrupted, forcing removal")
+                rmdir(source_folder)
+            elif modified_recipe and not keep_source:
+                output.info("Package recipe modified in export, forcing source folder removal")
+                output.info("Use the --keep-source, -k option to skip it")
+                rmdir(source_folder)
+        except BaseException as e:
+            output.error("Unable to delete source folder. Will be marked as corrupted for deletion")
+            output.warn(str(e))
+            set_dirty(source_folder)
 
 
 def _capture_export_scm_data(conanfile, conanfile_dir, destination_folder, output, paths, ref):
@@ -152,80 +208,21 @@ def _replace_scm_data_in_conanfile(conanfile_path, scm_data):
     save(conanfile_path, content)
 
 
-def _export_conanfile(conanfile_path, output, cache, conanfile, ref, keep_source):
-
-    exports_folder = cache.export(ref)
-    exports_source_folder = cache.export_sources(ref, conanfile.short_paths)
-
-    previous_digest = _init_export_folder(exports_folder, exports_source_folder)
-    origin_folder = os.path.dirname(conanfile_path)
-    export_recipe(conanfile, origin_folder, exports_folder)
-    export_source(conanfile, origin_folder, exports_source_folder)
-    shutil.copy2(conanfile_path, os.path.join(exports_folder, CONANFILE))
-
-    scm_data, captured_revision = _capture_export_scm_data(conanfile,
-                                                           os.path.dirname(conanfile_path),
-                                                           exports_folder,
-                                                           output, cache, ref)
-
-    digest = FileTreeManifest.create(exports_folder, exports_source_folder)
-
-    if previous_digest and previous_digest == digest:
-        output.info("The stored package has not changed")
-        modified_recipe = False
-        digest = previous_digest  # Use the old one, keep old timestamp
-    else:
-        output.success('A new %s version was exported' % CONANFILE)
-        output.info('Folder: %s' % exports_folder)
-        modified_recipe = True
-
-    digest.save(exports_folder)
-
-    revision = scm_data.revision if scm_data and captured_revision else digest.summary_hash
-    with cache.package_layout(ref).update_metadata() as metadata:
-        # Note that there is no time set, the time will come from the remote
-        metadata.recipe.revision = revision
-
-    # FIXME: Conan 2.0 Clear the registry entry if the recipe has changed
-    source = cache.source(ref, conanfile.short_paths)
-    remove = False
-    if is_dirty(source):
-        output.info("Source folder is corrupted, forcing removal")
-        remove = True
-    elif modified_recipe and not keep_source and os.path.exists(source):
-        output.info("Package recipe modified in export, forcing source folder removal")
-        output.info("Use the --keep-source, -k option to skip it")
-        remove = True
-    if remove:
-        output.info("Removing 'source' folder, this can take a while for big packages")
-        try:
-            # remove only the internal
-            rmdir(source)
-        except BaseException as e:
-            output.error("Unable to delete source folder. "
-                         "Will be marked as corrupted for deletion")
-            output.warn(str(e))
-            set_dirty(source)
-
-
-def _init_export_folder(destination_folder, destination_src_folder):
-    previous_digest = None
+def _recreate_folders(destination_folder, destination_src_folder):
     try:
         if os.path.exists(destination_folder):
-            if os.path.exists(os.path.join(destination_folder, CONAN_MANIFEST)):
-                previous_digest = FileTreeManifest.load(destination_folder)
             # Maybe here we want to invalidate cache
             rmdir(destination_folder)
         os.makedirs(destination_folder)
     except Exception as e:
         raise ConanException("Unable to create folder %s\n%s" % (destination_folder, str(e)))
+
     try:
         if os.path.exists(destination_src_folder):
             rmdir(destination_src_folder)
         os.makedirs(destination_src_folder)
     except Exception as e:
         raise ConanException("Unable to create folder %s\n%s" % (destination_src_folder, str(e)))
-    return previous_digest
 
 
 def _classify_patterns(patterns):
diff --git a/conans/client/graph/graph_manager.py b/conans/client/graph/graph_manager.py
index ffdc8710d..804200850 100644
--- a/conans/client/graph/graph_manager.py
+++ b/conans/client/graph/graph_manager.py
@@ -236,11 +236,10 @@ class GraphManager(object):
 
     def _load_graph(self, root_node, check_updates, update, build_mode, remote_name,
                     profile_build_requires, recorder, workspace, processed_profile):
+        assert isinstance(build_mode, BuildMode)
         builder = DepsGraphBuilder(self._proxy, self._output, self._loader, self._resolver,
                                    workspace, recorder)
         graph = builder.load_graph(root_node, check_updates, update, remote_name, processed_profile)
-        if build_mode is None:
-            return graph
         binaries_analyzer = GraphBinariesAnalyzer(self._cache, self._output,
                                                   self._remote_manager, workspace)
         binaries_analyzer.evaluate_graph(graph, build_mode, update, remote_name)
diff --git a/conans/util/files.py b/conans/util/files.py
index d2029f492..1046ea6b0 100644
--- a/conans/util/files.py
+++ b/conans/util/files.py
@@ -170,14 +170,14 @@ def save_files(path, files, only_if_modified=False):
 
 
 def load(path, binary=False):
-    '''Loads a file content'''
+    """ Loads a file content """
     with open(path, 'rb') as handle:
         tmp = handle.read()
         return tmp if binary else decode_text(tmp)
 
 
 def relative_dirs(path):
-    ''' Walks a dir and return a list with the relative paths '''
+    """ Walks a dir and return a list with the relative paths """
     ret = []
     for dirpath, _, fnames in walk(path):
         for filename in fnames:


Test patch:
diff --git a/conans/test/functional/hooks/__init__.py b/conans/test/functional/hooks/__init__.py
new file mode 100644
index 000000000..e69de29bb
diff --git a/conans/test/functional/hooks/test_post_export.py b/conans/test/functional/hooks/test_post_export.py
new file mode 100644
index 000000000..525129c5c
--- /dev/null
+++ b/conans/test/functional/hooks/test_post_export.py
@@ -0,0 +1,46 @@
+# coding=utf-8
+
+import os
+import textwrap
+import unittest
+
+from mock import patch
+
+from conans.client.hook_manager import HookManager
+from conans.model.manifest import FileTreeManifest
+from conans.model.ref import ConanFileReference
+from conans.paths import CONAN_MANIFEST
+from conans.test.utils.tools import TestClient
+
+
+class PostExportTestCase(unittest.TestCase):
+
+    def test_called_before_digest(self):
+        """ Test that 'post_export' hook is called before computing the digest of the
+            exported folders
+        """
+
+        ref = ConanFileReference.loads("name/version@user/channel")
+        conanfile = textwrap.dedent("""\
+            from conans import ConanFile
+            
+            class MyLib(ConanFile):
+                pass
+        """)
+
+        t = TestClient()
+        t.save({'conanfile.py': conanfile})
+        package_layout = t.cache.package_layout(ref)
+
+        def mocked_post_export(*args, **kwargs):
+            # There shouldn't be a digest yet
+            with self.assertRaisesRegexp(IOError, "No such file or directory"):
+                FileTreeManifest.load(package_layout.export())
+            self.assertFalse(os.path.exists(os.path.join(package_layout.export(), CONAN_MANIFEST)))
+
+        def mocked_load_hooks(hook_manager):
+            hook_manager.hooks["post_export"] = [("_", mocked_post_export)]
+
+        with patch.object(HookManager, "load_hooks", new=mocked_load_hooks):
+            t.run("export . {}".format(ref))
+        self.assertTrue(os.path.exists(os.path.join(package_layout.export(), CONAN_MANIFEST)))