diff options
author | Tristan Van Berkom <tristan.van.berkom@gmail.com> | 2018-12-06 15:14:37 +0000 |
---|---|---|
committer | Tristan Van Berkom <tristan.van.berkom@gmail.com> | 2018-12-06 15:14:37 +0000 |
commit | 60e9678117183b3a7762dd592fc3be16f1ea57ce (patch) | |
tree | cce5e2b0ec4d406140a716b571d9031725d30a25 /buildstream | |
parent | bea4d4f541ce6de41def7bb87c32613931877f2a (diff) | |
parent | a029762537e6d3744edbe2bf30b3532490bfd065 (diff) | |
download | buildstream-60e9678117183b3a7762dd592fc3be16f1ea57ce.tar.gz |
Merge branch 'tristan/submodule-warnings' into 'master'
Implement submodule warnings
See merge request BuildStream/buildstream!996
Diffstat (limited to 'buildstream')
-rw-r--r-- | buildstream/_versions.py | 2 | ||||
-rw-r--r-- | buildstream/plugins/sources/git.py | 154 | ||||
-rw-r--r-- | buildstream/source.py | 25 |
3 files changed, 130 insertions, 51 deletions
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..ae8f36bfe 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 <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 <project_format_version>` + +- ``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 <project_format_version>` This plugin also utilises the following configurable :class:`core warnings <buildstream.types.CoreWarnings>`: @@ -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 @@ -305,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 @@ -324,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) @@ -344,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) @@ -413,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 @@ -563,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 @@ -653,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): @@ -662,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) @@ -678,15 +662,9 @@ 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: - 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 +672,74 @@ 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)) + + # 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 # ########################################################### @@ -718,12 +764,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 +793,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(): 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 <core_source_fetcher>`. +* :func:`Source.validate_cache() <buildstream.source.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 <buildstream.types.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): |