Merge pull request #607 from ajkavanagh/more-reproducible-charms-fixes

Fixes for reproducible charms (bugs 606, 605, 603)

charmtools/build/builder.py

@@ -117,8 +117,14 @@ def fetch(self):
                 self.fetched_url = getattr(fetcher, "fetched_url", None)
                 self.vcs = getattr(fetcher, "vcs", None)
             self.revision = fetcher.get_revision(self.directory)
-            self.branch = fetcher.get_branch_for_revision(self.directory,
-                                                          self.revision)
+            # NOTE(ajkavanagh): Due to bug:#606 there isn't a good way to
+            # determine the branch for a revision as there isn't a one-to-one
+            # correspondence.  For the moment, work-around the bug by just
+            # using the revision as the branch so that things do at least
+            # build.
+            self.branch = self.revision
+            # self.branch = fetcher.get_branch_for_revision(self.directory,
+                                                          # self.revision)
 
         if not self.directory.exists():
             raise BuildError(
@@ -750,8 +756,8 @@ def generate_python_modules_from_lock_file(self):
                 continue
             vcs = data['vcs']
             if vcs == 'git':
-                if self.use_lock_file_branches:
-                    branch = data.get('branch', '')
+                branch = data.get('branch', '')
+                if self.use_lock_file_branches and branch:
                     if branch.startswith("refs/heads/"):
                         branch = branch[len("refs/heads/"):]
                     line = "{}@{}#egg={}".format(
@@ -978,11 +984,15 @@ def make_url_from_lock_for_layer(lock_spec, use_branches=False):
         url = lock_spec["url"]
     if use_branches:
         branch = lock_spec["branch"]
-        if branch.startswith("refs/heads/"):
-            branch = branch[len("refs/heads/"):]
-        return "{}@{}".format(url, branch)
-    else:
-        return "{}@{}".format(url, lock_spec["commit"])
+        # BUG: #603 - only format a branch if the branch is recorded.
+        if branch:
+            if branch.startswith("refs/heads/"):
+                branch = branch[len("refs/heads/"):]
+            # BUG: #605 - if branch is master or main, use the commit hash
+            # instead.
+            if branch not in ['master', 'main']:
+                return "{}@{}".format(url, branch)
+    return "{}@{}".format(url, lock_spec["commit"])
 
 
 def configLogging(build):

charmtools/build/tactics.py

@@ -1152,8 +1152,10 @@ def _add(self, wheelhouse, *reqs):
                         req = self.modules[package]
                         log.debug("module: %s - is vcs: %s", package, req.vcs)
                         if req.vcs:
-                            (branch, version) = self._extract_pkg_vcs(wheel,
-                                                                      req)
+                            version = self._extract_pkg_vcs(wheel, req)
+                            # Workaround bug:#603 and ensure that branch is
+                            # revision if none is named for the python module
+                            branch = req.revision if req.revision else version
                             log.debug("branch: %s, version=%s",
                                       branch, version)
                             self.lock_info.append({
@@ -1204,8 +1206,7 @@ def _extract_pkg_vcs(wheel, req):
             vcs_dir = dst_dir / req.name
             fetcher = fetchers.Fetcher(vcs_dir)
             revision = fetcher.get_revision(vcs_dir)
-            branch = fetcher.get_branch_for_revision(vcs_dir, revision)
-            return (branch, revision)
+            return revision
 
     def _run_in_venv(self, *args):
         assert self._venv is not None

charmtools/fetchers.py

@@ -137,7 +137,8 @@ def get_branch_for_revision(self, dir_, revision=None):
             try:
                 branch = check_output(cmd, cwd=dir_).decode('UTF-8').strip()
                 return branch
-            except FetchError:
+            except FetchError as e:
+                log.debug("get_branch_for_revision: got error: ", e)
                 continue
         return None
 

docs/reproducible-charms.rst

@@ -75,10 +75,13 @@ present:
 If the ``--ignore-lock-file`` option is used, then the charm is built as though
 there is no lock file.
 
-If the ``--use-lock-file-branches`` is used, then, for VCS items (layers,
-interfaces, and Python modules specified using a VCS string), then the branch
-(if there was one) will be used, rather than the commit version.  This can be
-used to track a branch in a layer or Python module.
+If the ``--use-lock-file-branches`` is used, then, for Python modules specified
+using a VCS string), the branch (if there was one) will be used, rather than
+the commit version.  This can be used to track a branch in a Python module.
+
+Note: as an individual commit be in multiple branches it's not possible to
+detect the branch for a layer.  Thus, this must be overriden manuall in the
+``build.lock`` file after it has been generated.
 
 Note: if ``--wheelhouse-overrides`` is used, then that wheelhouse will override
 the lock file.  i.e. the lock file overrides the layers' ``wheelhouse.txt``