Remove the identifier keyword since we are forced to use verbose_name if tags should work :/

Author: benjaoming

readthedocs/doc_builder/director.py

@@ -213,10 +213,10 @@ def build(self):
     def checkout(self):
         """Checkout Git repo and load build config file."""
 
-        identifier = self.data.build_commit or self.data.version.identifier
-        log.info("Cloning and fetching.", identifier=identifier)
-        self.vcs_repository.update(identifier=identifier)
+        log.info("Cloning and fetching.", identifier=self.data.version.verbose_name)
+        self.vcs_repository.update(identifier=self.data.version.verbose_name)
 
+        identifier = self.data.build_commit or self.data.version.identifier
         log.info("Checking out.", identifier=identifier)
         self.vcs_repository.checkout(identifier)
 

readthedocs/vcs_support/backends/bzr.py

@@ -16,8 +16,8 @@ class Backend(BaseVCS):
     supports_tags = True
     fallback_branch = ''
 
-    def update(self, identifier=None):
-        super().update(identifier=identifier)
+    def update(self):
+        super().update()
         if self.repo_exists():
             return self.up()
         return self.clone()

readthedocs/vcs_support/backends/git.py

@@ -55,15 +55,15 @@ def _get_clone_url(self):
     def set_remote_url(self, url):
         return self.run('git', 'remote', 'set-url', 'origin', url)
 
-    def update(self, identifier=None):
+    def update(self):
         """
         Clone or update the repository.
 
         :param identifier: This is the optional identifier for git fetch - a branch or tag name.
                            PR references are generated automatically for certain Git providers.
         :return:
         """
-        super().update(identifier=identifier)
+        super().update()
 
         if self.use_clone_fetch_checkout_pattern():
 
@@ -72,7 +72,11 @@ def update(self, identifier=None):
             self.clone_ng()
             # New behavior: No confusing return value. We are not using return values
             # in the callers.
-            self.fetch_ng(identifier=identifier)
+            # TODO: We are using self.verbose_name as the identifier. This is not a very
+            # clear understanding, but it's a continuation of how PR patterns are generating
+            # remote references already from verbose_name. We need to fix this by providing
+            # a targeted field on the Build model, for instance "git_reference".
+            self.fetch_ng(identifier=self.verbose_name)
 
         else:
             if self.repo_exists():

readthedocs/vcs_support/backends/hg.py

@@ -12,8 +12,8 @@ class Backend(BaseVCS):
     supports_branches = True
     fallback_branch = 'default'
 
-    def update(self, identifier=None):
-        super().update(identifier=identifier)
+    def update(self):
+        super().update()
         if self.repo_exists():
             return self.pull()
         return self.clone()

readthedocs/vcs_support/backends/svn.py

@@ -26,8 +26,8 @@ def __init__(self, *args, **kwargs):
         else:
             self.base_url = self.repo_url
 
-    def update(self, identifier=None):
-        super().update(identifier=identifier)
+    def update(self):
+        super().update()
         if self.repo_exists():
             return self.up()
         return self.co()

readthedocs/vcs_support/base.py

@@ -83,7 +83,7 @@ def make_clean_working_dir(self):
         safe_rmtree(self.working_dir, ignore_errors=True)
         self.check_working_dir()
 
-    def update(self, identifier=None):
+    def update(self):
         """
         Update a local copy of the repository in self.working_dir.