diff options
author | Tristan Van Berkom <tristan.van.berkom@gmail.com> | 2018-09-03 06:50:11 +0000 |
---|---|---|
committer | Tristan Van Berkom <tristan.van.berkom@gmail.com> | 2018-09-03 06:50:11 +0000 |
commit | b422959fff1019952c0cf29d9882c9126c11b099 (patch) | |
tree | fcbafc55e845e7b973f31b062837056642070cfe | |
parent | b97a92b04324d2b021234c17f2e1f2a25dd32a51 (diff) | |
parent | 51eaa2c38fbeee6a34898bec3db048b0aa108696 (diff) | |
download | buildstream-b422959fff1019952c0cf29d9882c9126c11b099.tar.gz |
Merge branch 'tristan/source-fetcher-changes-1.2' into 'bst-1.2'
Source fetcher changes 1.2
See merge request BuildStream/buildstream!773
-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 70e12313c..6a6d1a9b8 100644 --- a/buildstream/element.py +++ b/buildstream/element.py @@ -262,7 +262,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 836b60834..f57c0e15c 100644 --- a/buildstream/plugin.py +++ b/buildstream/plugin.py @@ -162,6 +162,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__ @@ -651,7 +652,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 # @@ -659,6 +685,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 354f80b5d..065870169 100644 --- a/buildstream/plugins/sources/git.py +++ b/buildstream/plugins/sources/git.py @@ -90,13 +90,14 @@ GIT_MODULES = '.gitmodules' # 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) @@ -114,7 +115,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) @@ -136,7 +138,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) @@ -294,7 +298,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. @@ -314,6 +318,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 9f8f4ffdb..f546258e6 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. @@ -149,6 +161,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 @@ -171,7 +190,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 # @@ -216,8 +235,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. @@ -228,7 +250,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 @@ -290,10 +312,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:: @@ -317,8 +339,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 @@ -362,7 +384,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. @@ -380,8 +402,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* """ @@ -404,17 +433,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: @@ -433,25 +472,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 |