diff options
author | Tristan Van Berkom <tristan.van.berkom@gmail.com> | 2018-09-03 06:23:38 +0000 |
---|---|---|
committer | Tristan Van Berkom <tristan.van.berkom@gmail.com> | 2018-09-03 06:23:38 +0000 |
commit | 88460cd26ebe4e0ab3f49648667ec7800f174d5d (patch) | |
tree | cfc19fda4367efc8365155d6f69038c6650b58fb | |
parent | 6805a2ab7192c56fa5a6141d0ad150d5dda67040 (diff) | |
parent | f9b2f1a93b8bffb56b4acaf11b77ee30cb79d822 (diff) | |
download | buildstream-88460cd26ebe4e0ab3f49648667ec7800f174d5d.tar.gz |
Merge branch 'tristan/source-fetcher-changes' into 'master'
Source fetcher changes
See merge request BuildStream/buildstream!772
-rw-r--r-- | buildstream/element.py | 2 | ||||
-rw-r--r-- | buildstream/plugin.py | 27 | ||||
-rw-r--r-- | buildstream/plugins/sources/git.py | 17 | ||||
-rw-r--r-- | buildstream/source.py | 117 | ||||
-rw-r--r-- | tests/frontend/project/sources/fetch_source.py | 18 |
5 files changed, 149 insertions, 32 deletions
diff --git a/buildstream/element.py b/buildstream/element.py index 0bc35ce38..ae8e10144 100644 --- a/buildstream/element.py +++ b/buildstream/element.py @@ -245,7 +245,7 @@ class Element(Plugin): # Collect the composited element configuration and # ask the element to configure itself. self.__config = self.__extract_config(meta) - self.configure(self.__config) + self._configure(self.__config) # Extract Sandbox config self.__sandbox_config = self.__extract_sandbox_config(meta) diff --git a/buildstream/plugin.py b/buildstream/plugin.py index a65db4d42..d8cca2751 100644 --- a/buildstream/plugin.py +++ b/buildstream/plugin.py @@ -179,6 +179,7 @@ class Plugin(): self.__provenance = provenance # The Provenance information self.__type_tag = type_tag # The type of plugin (element or source) self.__unique_id = _plugin_register(self) # Unique ID + self.__configuring = False # Whether we are currently configuring # Infer the kind identifier modulename = type(self).__module__ @@ -682,7 +683,32 @@ class Plugin(): else: yield log + # _configure(): + # + # Calls configure() for the plugin, this must be called by + # the core instead of configure() directly, so that the + # _get_configuring() state is up to date. + # + # Args: + # node (dict): The loaded configuration dictionary + # + def _configure(self, node): + self.__configuring = True + self.configure(node) + self.__configuring = False + + # _get_configuring(): + # + # Checks whether the plugin is in the middle of having + # its Plugin.configure() method called + # + # Returns: + # (bool): Whether we are currently configuring + def _get_configuring(self): + return self.__configuring + # _preflight(): + # # Calls preflight() for the plugin, and allows generic preflight # checks to be added # @@ -690,6 +716,7 @@ class Plugin(): # SourceError: If it's a Source implementation # ElementError: If it's an Element implementation # ProgramNotFoundError: If a required host tool is not found + # def _preflight(self): self.preflight() 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 b86f3fb16..e6350161f 100644 --- a/buildstream/source.py +++ b/buildstream/source.py @@ -28,6 +28,18 @@ Abstract Methods For loading and configuration purposes, Sources must implement the :ref:`Plugin base class abstract methods <core_plugin_abstract_methods>`. +.. attention:: + + In order to ensure that all configuration data is processed at + load time, it is important that all URLs have been processed during + :func:`Plugin.configure() <buildstream.plugin.Plugin.configure>`. + + Source implementations *must* either call + :func:`Source.translate_url() <buildstream.source.Source.translate_url>` or + :func:`Source.mark_download_url() <buildstream.source.Source.mark_download_url>` + for every URL that has been specified in the configuration during + :func:`Plugin.configure() <buildstream.plugin.Plugin.configure>` + Sources expose the following abstract methods. Unless explicitly mentioned, these methods are mandatory to implement. @@ -184,6 +196,13 @@ class SourceFetcher(): fetching and substituting aliases. *Since: 1.2* + + .. attention:: + + When implementing a SourceFetcher, remember to call + :func:`Source.mark_download_url() <buildstream.source.Source.mark_download_url>` + for every URL found in the configuration data at + :func:`Plugin.configure() <buildstream.plugin.Plugin.configure>` time. """ def __init__(self): self.__alias = None @@ -206,7 +225,7 @@ class SourceFetcher(): Implementors should raise :class:`.SourceError` if the there is some network error or if the source reference could not be matched. """ - raise ImplError("Source fetcher '{}' does not implement fetch()".format(type(self))) + raise ImplError("SourceFetcher '{}' does not implement fetch()".format(type(self))) ############################################################# # Public Methods # @@ -277,8 +296,11 @@ 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 + self.__marked_urls = set() # Set of marked download URLs # FIXME: Reconstruct a MetaSource from a Source instead of storing it. self.__meta = meta # MetaSource stored so we can copy this source later. @@ -289,7 +311,7 @@ class Source(Plugin): self.__config = self.__extract_config(meta) self.__first_pass = meta.first_pass - self.configure(self.__config) + self._configure(self.__config) COMMON_CONFIG_KEYS = ['kind', 'directory'] """Common source config keys @@ -351,10 +373,10 @@ class Source(Plugin): Args: ref (simple object): The internal source reference to set, or ``None`` node (dict): The same dictionary which was previously passed - to :func:`~buildstream.source.Source.configure` + to :func:`Plugin.configure() <buildstream.plugin.Plugin.configure>` - See :func:`~buildstream.source.Source.get_ref` for a discussion on - the *ref* parameter. + See :func:`Source.get_ref() <buildstream.source.Source.get_ref>` + for a discussion on the *ref* parameter. .. note:: @@ -384,8 +406,8 @@ class Source(Plugin): backend store allows one to query for a new ref from a symbolic tracking data without downloading then that is desirable. - See :func:`~buildstream.source.Source.get_ref` for a discussion on - the *ref* parameter. + See :func:`Source.get_ref() <buildstream.source.Source.get_ref>` + for a discussion on the *ref* parameter. """ # Allow a non implementation return None @@ -435,7 +457,7 @@ class Source(Plugin): :class:`.SourceError` Default implementation is to call - :func:`~buildstream.source.Source.stage`. + :func:`Source.stage() <buildstream.source.Source.stage>`. Implementors overriding this method should assume that *directory* already exists. @@ -453,8 +475,15 @@ class Source(Plugin): is recommended. Returns: - list: A list of SourceFetchers. If SourceFetchers are not supported, - this will be an empty list. + iterable: The Source's SourceFetchers, if any. + + .. note:: + + Implementors can implement this as a generator. + + The :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>` + method will be called on the returned fetchers one by one, + before consuming the next fetcher in the list. *Since: 1.2* """ @@ -477,17 +506,27 @@ 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 + :func:`Plugin.configure() <buildstream.plugin.Plugin.configure>` if + :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: @@ -506,25 +545,55 @@ 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 - This must be called during :func:`~buildstream.plugin.Plugin.configure` if - :func:`~buildstream.source.Source.translate_url` is not called. - 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:: + + This must be called for every URL in the configuration during + :func:`Plugin.configure() <buildstream.plugin.Plugin.configure>` if + :func:`Source.translate_url() <buildstream.source.Source.translate_url>` + is not called. *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 + + # Enforce proper behaviour of plugins by ensuring that all + # aliased URLs have been marked at Plugin.configure() time. + # + if self._get_configuring(): + # Record marked urls while configuring + # + self.__marked_urls.add(url) + else: + # If an unknown aliased URL is seen after configuring, + # this is an error. + # + # It is still possible that a URL that was not mentioned + # in the element configuration can be marked, this is + # the case for git submodules which might be automatically + # discovered. + # + assert (url in self.__marked_urls or not _extract_alias(url)), \ + "URL was not seen at configure time: {}".format(url) 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 |