From b1d2f001687a222893c45a9bf13af1705c9c483a Mon Sep 17 00:00:00 2001 From: Tristan Van Berkom Date: Thu, 6 Dec 2018 21:10:56 +0900 Subject: source.py: Add new delegate method validate_cache() This is guaranteed to be called only once for a given session once the sources are known to be Consistency.CACHED, if source tracking is enabled in the session for this source, then this will only be called if the sources become cached after tracking completes. --- buildstream/source.py | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) (limited to 'buildstream') diff --git a/buildstream/source.py b/buildstream/source.py index 5dc5abb63..bb54110ca 100644 --- a/buildstream/source.py +++ b/buildstream/source.py @@ -102,6 +102,11 @@ these methods are mandatory to implement. submodules). For details on how to define a SourceFetcher, see :ref:`SourceFetcher `. +* :func:`Source.validate_cache() ` + + Perform any validations which require the sources to be cached. + + **Optional**: This is completely optional and will do nothing if left unimplemented. Accessing previous sources -------------------------- @@ -480,9 +485,22 @@ class Source(Plugin): *Since: 1.2* """ - return [] + def validate_cache(self): + """Implement any validations once we know the sources are cached + + This is guaranteed to be called only once for a given session + once the sources are known to be + :attr:`Consistency.CACHED `, + if source tracking is enabled in the session for this source, + then this will only be called if the sources become cached after + tracking completes. + + *Since: 1.4* + """ + pass + ############################################################# # Public Methods # ############################################################# @@ -659,6 +677,11 @@ class Source(Plugin): with context.silence(): self.__consistency = self.get_consistency() # pylint: disable=assignment-from-no-return + # Give the Source an opportunity to validate the cached + # sources as soon as the Source becomes Consistency.CACHED. + if self.__consistency == Consistency.CACHED: + self.validate_cache() + # Return cached consistency # def _get_consistency(self): -- cgit v1.2.1 From ee7fc47fcb4a06398fb7460621df1257c1f0b4ed Mon Sep 17 00:00:00 2001 From: Tristan Van Berkom Date: Wed, 5 Dec 2018 15:01:27 +0900 Subject: git source plugin: Implementing submodule warnings o Unlisted submodule warning Now the git plugin will issue a configurable warning if a submodule exists and is used (checking out the submodule is not disabled), but is not specified in the source configuration. o Invalid submodule warning Now the git source plugin will issue a warning if the configuration specified a submodule which does not exist in the underlying git repository. As a side effect, this patch also changes the flow control of the git plugin such that submodules which are explicitly set to not be checked out, are also not fetched but instead ignored completely. --- buildstream/_versions.py | 2 +- buildstream/plugins/sources/git.py | 87 ++++++++++++++++++++++++++++++++------ 2 files changed, 76 insertions(+), 13 deletions(-) (limited to 'buildstream') diff --git a/buildstream/_versions.py b/buildstream/_versions.py index 42dcb1a31..8472a5b33 100644 --- a/buildstream/_versions.py +++ b/buildstream/_versions.py @@ -23,7 +23,7 @@ # This version is bumped whenever enhancements are made # to the `project.conf` format or the core element format. # -BST_FORMAT_VERSION = 19 +BST_FORMAT_VERSION = 20 # The base BuildStream artifact version diff --git a/buildstream/plugins/sources/git.py b/buildstream/plugins/sources/git.py index 7496f1f13..7c9675abf 100644 --- a/buildstream/plugins/sources/git.py +++ b/buildstream/plugins/sources/git.py @@ -133,7 +133,22 @@ details on common configuration options for sources. This plugin provides the following :ref:`configurable warnings `: -- ``git:inconsistent-submodule`` - A submodule was found to be missing from the underlying git repository. +- ``git:inconsistent-submodule`` - A submodule present in the git repository's .gitmodules was never + added with `git submodule add`. + +- ``git:unlisted-submodule`` - A submodule is present in the git repository but was not specified in + the source configuration and was not disabled for checkout. + + .. note:: + + The ``git:unlisted-submodule`` warning is available since :ref:`format version 20 ` + +- ``git:invalid-submodule`` - A submodule is specified in the source configuration but does not exist + in the repository. + + .. note:: + + The ``git:invalid-submodule`` warning is available since :ref:`format version 20 ` This plugin also utilises the following configurable :class:`core warnings `: @@ -158,6 +173,8 @@ GIT_MODULES = '.gitmodules' # Warnings WARN_INCONSISTENT_SUBMODULE = "inconsistent-submodule" +WARN_UNLISTED_SUBMODULE = "unlisted-submodule" +WARN_INVALID_SUBMODULE = "invalid-submodule" # Because of handling of submodules, we maintain a GitMirror @@ -680,13 +697,7 @@ class GitSource(Source): with self.timed_activity("Staging {}".format(self.mirror.url), silent_nested=True): self.mirror.stage(directory, track=(self.tracking if not self.tracked else None)) for mirror in self.submodules: - if mirror.path in self.submodule_checkout_overrides: - checkout = self.submodule_checkout_overrides[mirror.path] - else: - checkout = self.checkout_submodules - - if checkout: - mirror.stage(directory) + mirror.stage(directory) def get_source_fetchers(self): yield self.mirror @@ -694,6 +705,48 @@ class GitSource(Source): for submodule in self.submodules: yield submodule + def validate_cache(self): + discovered_submodules = {} + unlisted_submodules = [] + invalid_submodules = [] + + for path, url in self.mirror.submodule_list(): + discovered_submodules[path] = url + if self.ignore_submodule(path): + continue + + override_url = self.submodule_overrides.get(path) + if not override_url: + unlisted_submodules.append((path, url)) + + # Warn about submodules which are explicitly configured but do not exist + for path, url in self.submodule_overrides.items(): + if path not in discovered_submodules: + invalid_submodules.append((path, url)) + + if invalid_submodules: + detail = [] + for path, url in invalid_submodules: + detail.append(" Submodule URL '{}' at path '{}'".format(url, path)) + + self.warn("{}: Invalid submodules specified".format(self), + warning_token=WARN_INVALID_SUBMODULE, + detail="The following submodules are specified in the source " + "description but do not exist according to the repository\n\n" + + "\n".join(detail)) + + # Warn about submodules which exist but have not been explicitly configured + if unlisted_submodules: + detail = [] + for path, url in unlisted_submodules: + detail.append(" Submodule URL '{}' at path '{}'".format(url, path)) + + self.warn("{}: Unlisted submodules exist".format(self), + warning_token=WARN_UNLISTED_SUBMODULE, + detail="The following submodules exist but are not specified " + + "in the source description\n\n" + + "\n".join(detail)) + ########################################################### # Local Functions # ########################################################### @@ -718,12 +771,12 @@ class GitSource(Source): self.mirror.ensure() submodules = [] - # XXX Here we should issue a warning if either: - # A.) A submodule exists but is not defined in the element configuration - # B.) The element configuration configures submodules which dont exist at the current ref - # for path, url in self.mirror.submodule_list(): + # Completely ignore submodules which are disabled for checkout + if self.ignore_submodule(path): + continue + # Allow configuration to override the upstream # location of the submodules. override_url = self.submodule_overrides.get(path) @@ -747,6 +800,16 @@ class GitSource(Source): tags.append((tag, commit_ref, annotated)) return tags + # Checks whether the plugin configuration has explicitly + # configured this submodule to be ignored + def ignore_submodule(self, path): + try: + checkout = self.submodule_checkout_overrides[path] + except KeyError: + checkout = self.checkout_submodules + + return not checkout + # Plugin entry point def setup(): -- cgit v1.2.1 From 5fc9a1dae7cee661b5b30141afa15e8ad3c30434 Mon Sep 17 00:00:00 2001 From: Tristan Van Berkom Date: Thu, 6 Dec 2018 21:49:48 +0900 Subject: git source plugin: Emmit the ref-not-in-track warning from validate_cache() Now that we have Source.validate_cache(), this is a better place to emmit the ref-not-in-track warning, since it will be emmitted at the earliest opportunity and not only at Source.stage() or Source.init_workspace(). This also allows us to remove the `self.tracked` local state, and cleanup some convoluted calling paths, removing some unnecessary parameters from the usual codepaths and making the plugin overall more readable. --- buildstream/plugins/sources/git.py | 67 +++++++++++++++++--------------------- 1 file changed, 30 insertions(+), 37 deletions(-) (limited to 'buildstream') diff --git a/buildstream/plugins/sources/git.py b/buildstream/plugins/sources/git.py index 7c9675abf..ae8f36bfe 100644 --- a/buildstream/plugins/sources/git.py +++ b/buildstream/plugins/sources/git.py @@ -322,7 +322,7 @@ class GitMirror(SourceFetcher): return ref, list(tags) - def stage(self, directory, track=None): + def stage(self, directory): fullpath = os.path.join(directory, self.path) # Using --shared here avoids copying the objects into the checkout, in any @@ -341,11 +341,7 @@ class GitMirror(SourceFetcher): self._rebuild_git(fullpath) - # Check that the user specified ref exists in the track if provided & not already tracked - if track: - self.assert_ref_in_track(fullpath, track) - - def init_workspace(self, directory, track=None): + def init_workspace(self, directory): fullpath = os.path.join(directory, self.path) url = self.source.translate_url(self.url) @@ -361,10 +357,6 @@ class GitMirror(SourceFetcher): fail="Failed to checkout git ref {}".format(self.ref), cwd=fullpath) - # Check that the user specified ref exists in the track if provided & not already tracked - if track: - self.assert_ref_in_track(fullpath, track) - # List the submodules (path/url tuples) present at the given ref of this repo def submodule_list(self): modules = "{}:{}".format(self.ref, GIT_MODULES) @@ -430,28 +422,6 @@ class GitMirror(SourceFetcher): return None - # Assert that ref exists in track, if track has been specified. - def assert_ref_in_track(self, fullpath, track): - _, branch = self.source.check_output([self.source.host_git, 'branch', '--list', track, - '--contains', self.ref], - cwd=fullpath,) - if branch: - return - else: - _, tag = self.source.check_output([self.source.host_git, 'tag', '--list', track, - '--contains', self.ref], - cwd=fullpath,) - if tag: - return - - detail = "The ref provided for the element does not exist locally in the provided track branch / tag " + \ - "'{}'.\nYou may wish to track the element to update the ref from '{}' ".format(track, track) + \ - "with `bst track`,\nor examine the upstream at '{}' for the specific ref.".format(self.url) - - self.source.warn("{}: expected ref '{}' was not found in given track '{}' for staged repository: '{}'\n" - .format(self.source, self.ref, track, self.url), - detail=detail, warning_token=CoreWarnings.REF_NOT_IN_TRACK) - def _rebuild_git(self, fullpath): if not self.tags: return @@ -580,7 +550,6 @@ class GitSource(Source): self.submodule_checkout_overrides[path] = checkout self.mark_download_url(self.original_url) - self.tracked = False def preflight(self): # Check if git is installed, get the binary at the same time @@ -670,8 +639,6 @@ class GitSource(Source): # Update self.mirror.ref and node.ref from the self.tracking branch ret = self.mirror.latest_commit_with_tags(self.tracking, self.track_tags) - # Set tracked attribute, parameter for if self.mirror.assert_ref_in_track is needed - self.tracked = True return ret def init_workspace(self, directory): @@ -679,7 +646,7 @@ class GitSource(Source): self.refresh_submodules() with self.timed_activity('Setting up workspace "{}"'.format(directory), silent_nested=True): - self.mirror.init_workspace(directory, track=(self.tracking if not self.tracked else None)) + self.mirror.init_workspace(directory) for mirror in self.submodules: mirror.init_workspace(directory) @@ -695,7 +662,7 @@ class GitSource(Source): # Stage the main repo in the specified directory # with self.timed_activity("Staging {}".format(self.mirror.url), silent_nested=True): - self.mirror.stage(directory, track=(self.tracking if not self.tracked else None)) + self.mirror.stage(directory) for mirror in self.submodules: mirror.stage(directory) @@ -747,6 +714,32 @@ class GitSource(Source): "in the source description\n\n" + "\n".join(detail)) + # Assert that the ref exists in the track tag/branch, if track has been specified. + ref_in_track = False + if self.tracking: + _, branch = self.check_output([self.host_git, 'branch', '--list', self.tracking, + '--contains', self.mirror.ref], + cwd=self.mirror.mirror) + if branch: + ref_in_track = True + else: + _, tag = self.check_output([self.host_git, 'tag', '--list', self.tracking, + '--contains', self.mirror.ref], + cwd=self.mirror.mirror) + if tag: + ref_in_track = True + + if not ref_in_track: + detail = "The ref provided for the element does not exist locally " + \ + "in the provided track branch / tag '{}'.\n".format(self.tracking) + \ + "You may wish to track the element to update the ref from '{}' ".format(self.tracking) + \ + "with `bst track`,\n" + \ + "or examine the upstream at '{}' for the specific ref.".format(self.mirror.url) + + self.warn("{}: expected ref '{}' was not found in given track '{}' for staged repository: '{}'\n" + .format(self, self.mirror.ref, self.tracking, self.mirror.url), + detail=detail, warning_token=CoreWarnings.REF_NOT_IN_TRACK) + ########################################################### # Local Functions # ########################################################### -- cgit v1.2.1