source.py: Added `primary` argument to URL marking APIs

The Source must now mention whether the marked or translated
URL is "primary" or not. Even when a Source may have multiple
URLs, the auxilliary URLs are derived from the primary one, not
only is this true for git, but it is mandated by our tracking
API which assumes there is a primary URL.

This adjusts the `git` source and the test `fetch_source.py` source
to behave properly and advertize it's primary URL properly.

This is a part of #620

3 files changed, 52 insertions, 19 deletions

diff --git a/buildstream/plugins/sources/git.py b/buildstream/plugins/sources/git.py
index da5563782..f45a23bfc 100644
--- a/buildstream/plugins/sources/git.py
+++ b/buildstream/plugins/sources/git.py
@@ -100,13 +100,14 @@ INCONSISTENT_SUBMODULE = "inconsistent-submodules"
 #
 class GitMirror(SourceFetcher):
 
-    def __init__(self, source, path, url, ref):
+    def __init__(self, source, path, url, ref, *, primary=False):
 
         super().__init__()
         self.source = source
         self.path = path
         self.url = url
         self.ref = ref
+        self.primary = primary
         self.mirror = os.path.join(source.get_mirror_directory(), utils.url_directory_name(url))
         self.mark_download_url(url)
 
@@ -124,7 +125,8 @@ class GitMirror(SourceFetcher):
             # system configured tmpdir is not on the same partition.
             #
             with self.source.tempdir() as tmpdir:
-                url = self.source.translate_url(self.url, alias_override=alias_override)
+                url = self.source.translate_url(self.url, alias_override=alias_override,
+                                                primary=self.primary)
                 self.source.call([self.source.host_git, 'clone', '--mirror', '-n', url, tmpdir],
                                  fail="Failed to clone git repository {}".format(url),
                                  fail_temporarily=True)
@@ -146,7 +148,9 @@ class GitMirror(SourceFetcher):
                                           .format(self.source, url, tmpdir, self.mirror, e)) from e
 
     def _fetch(self, alias_override=None):
-        url = self.source.translate_url(self.url, alias_override=alias_override)
+        url = self.source.translate_url(self.url,
+                                        alias_override=alias_override,
+                                        primary=self.primary)
 
         if alias_override:
             remote_name = utils.url_directory_name(alias_override)
@@ -307,7 +311,7 @@ class GitSource(Source):
         self.node_validate(node, config_keys + Source.COMMON_CONFIG_KEYS)
 
         self.original_url = self.node_get_member(node, str, 'url')
-        self.mirror = GitMirror(self, '', self.original_url, ref)
+        self.mirror = GitMirror(self, '', self.original_url, ref, primary=True)
         self.tracking = self.node_get_member(node, str, 'track', None)
 
         # At this point we now know if the source has a ref and/or a track.
@@ -327,6 +331,11 @@ class GitSource(Source):
         for path, _ in self.node_items(modules):
             submodule = self.node_get_member(modules, Mapping, path)
             url = self.node_get_member(submodule, str, 'url', None)
+
+            # Make sure to mark all URLs that are specified in the configuration
+            if url:
+                self.mark_download_url(url, primary=False)
+
             self.submodule_overrides[path] = url
             if 'checkout' in submodule:
                 checkout = self.node_get_member(submodule, bool, 'checkout')
diff --git a/buildstream/source.py b/buildstream/source.py
index 412970ae4..f9f306904 100644
--- a/buildstream/source.py
+++ b/buildstream/source.py
@@ -296,8 +296,10 @@ class Source(Plugin):
         self.__element_kind = meta.element_kind         # The kind of the element owning this source
         self.__directory = meta.directory               # Staging relative directory
         self.__consistency = Consistency.INCONSISTENT   # Cached consistency state
+
+        # The alias_override is only set on a re-instantiated Source
         self.__alias_override = alias_override          # Tuple of alias and its override to use instead
-        self.__expected_alias = None                    # A hacky way to store the first alias used
+        self.__expected_alias = None                    # The primary alias
 
         # FIXME: Reconstruct a MetaSource from a Source instead of storing it.
         self.__meta = meta                              # MetaSource stored so we can copy this source later.
@@ -503,17 +505,17 @@ class Source(Plugin):
         os.makedirs(directory, exist_ok=True)
         return directory
 
-    def translate_url(self, url, *, alias_override=None):
+    def translate_url(self, url, *, alias_override=None, primary=True):
         """Translates the given url which may be specified with an alias
         into a fully qualified url.
 
         Args:
-           url (str): A url, which may be using an alias
+           url (str): A URL, which may be using an alias
            alias_override (str): Optionally, an URI to override the alias with. (*Since: 1.2*)
+           primary (bool): Whether this is the primary URL for the source. (*Since: 1.2*)
 
         Returns:
-           str: The fully qualified url, with aliases resolved
-
+           str: The fully qualified URL, with aliases resolved
         .. note::
 
            This must be called for every URL in the configuration during
@@ -521,6 +523,9 @@ class Source(Plugin):
            :func:`Source.mark_download_url() <buildstream.source.Source.mark_download_url>`
            is not called.
         """
+        # Ensure that the download URL is also marked
+        self.mark_download_url(url, primary=primary)
+
         # Alias overriding can happen explicitly (by command-line) or
         # implicitly (the Source being constructed with an __alias_override).
         if alias_override or self.__alias_override:
@@ -539,18 +544,15 @@ class Source(Plugin):
                         url = override_url + url_body
             return url
         else:
-            # Sneakily store the alias if it hasn't already been stored
-            if not self.__expected_alias and url and utils._ALIAS_SEPARATOR in url:
-                self.mark_download_url(url)
-
             project = self._get_project()
             return project.translate_url(url, first_pass=self.__first_pass)
 
-    def mark_download_url(self, url):
+    def mark_download_url(self, url, *, primary=True):
         """Identifies the URL that this Source uses to download
 
         Args:
-           url (str): The url used to download
+           url (str): The URL used to download
+           primary (bool): Whether this is the primary URL for the source
 
         .. note::
 
@@ -561,7 +563,17 @@ class Source(Plugin):
 
         *Since: 1.2*
         """
-        self.__expected_alias = _extract_alias(url)
+        # Only mark the Source level aliases on the main instance, not in
+        # a reinstantiated instance in mirroring.
+        if not self.__alias_override:
+            if primary:
+                expected_alias = _extract_alias(url)
+
+                assert (self.__expected_alias is None or
+                        self.__expected_alias == expected_alias), \
+                    "Primary URL marked twice with different URLs"
+
+                self.__expected_alias = expected_alias
 
     def get_project_directory(self):
         """Fetch the project base directory
diff --git a/tests/frontend/project/sources/fetch_source.py b/tests/frontend/project/sources/fetch_source.py
index ebd3fe757..10e89960c 100644
--- a/tests/frontend/project/sources/fetch_source.py
+++ b/tests/frontend/project/sources/fetch_source.py
@@ -15,14 +15,17 @@ from buildstream import Source, Consistency, SourceError, SourceFetcher
 
 
 class FetchFetcher(SourceFetcher):
-    def __init__(self, source, url):
+    def __init__(self, source, url, primary=False):
         super().__init__()
         self.source = source
         self.original_url = url
+        self.primary = primary
         self.mark_download_url(url)
 
     def fetch(self, alias_override=None):
-        url = self.source.translate_url(self.original_url, alias_override=alias_override)
+        url = self.source.translate_url(self.original_url,
+                                        alias_override=alias_override,
+                                        primary=self.primary)
         with open(self.source.output_file, "a") as f:
             success = url in self.source.fetch_succeeds and self.source.fetch_succeeds[url]
             message = "Fetch {} {} from {}\n".format(self.original_url,
@@ -37,12 +40,21 @@ class FetchSource(Source):
     # Read config to know which URLs to fetch
     def configure(self, node):
         self.original_urls = self.node_get_member(node, list, 'urls')
-        self.fetchers = [FetchFetcher(self, url) for url in self.original_urls]
         self.output_file = self.node_get_member(node, str, 'output-text')
         self.fetch_succeeds = {}
         if 'fetch-succeeds' in node:
             self.fetch_succeeds = {x[0]: x[1] for x in self.node_items(node['fetch-succeeds'])}
 
+        # First URL is the primary one for this test
+        #
+        primary = True
+        self.fetchers = []
+        for url in self.original_urls:
+            self.mark_download_url(url, primary=primary)
+            fetcher = FetchFetcher(self, url, primary=primary)
+            self.fetchers.append(fetcher)
+            primary = False
+
     def get_source_fetchers(self):
         return self.fetchers