diff --git a/buildstream/_versions.py b/buildstream/_versions.py
index 42dcb1a316a2ede290cd0f0a0a61eda7a197f8b6..8472a5b33eb10d0eef4bbf5ff7091bcc539f6e72 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 7496f1f13561670ae2ef21f14a5c0b466c9bc177..ae8f36bfe96e30fde265f8f335c7184d61f4dc7f 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 5dc5abb63a999f210ec95c240b06d3ed7eb925e3..bb54110ca27c72be3dce494520c4d22eeb07fe25 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):
diff --git a/tests/sources/git.py b/tests/sources/git.py
index 124f036cf61359331a4cb70efe267e1753621be2..b9251e36a88a9f871d3a4d7989e7e6627e16c543 100644
--- a/tests/sources/git.py
+++ b/tests/sources/git.py
@@ -455,6 +455,274 @@ def test_ref_not_in_track(cli, tmpdir, datafiles, fail):
assert "ref-not-in-track" in result.stderr
+@pytest.mark.skipif(HAVE_GIT is False, reason="git is not available")
+@pytest.mark.datafiles(os.path.join(DATA_DIR, 'template'))
+@pytest.mark.parametrize("fail", ['warn', 'error'])
+def test_unlisted_submodule(cli, tmpdir, datafiles, fail):
+ project = os.path.join(datafiles.dirname, datafiles.basename)
+
+ # Make the warning an error if we're testing errors
+ if fail == 'error':
+ project_template = {
+ "name": "foo",
+ "fatal-warnings": ['git:unlisted-submodule']
+ }
+ _yaml.dump(project_template, os.path.join(project, 'project.conf'))
+
+ # Create the submodule first from the 'subrepofiles' subdir
+ subrepo = create_repo('git', str(tmpdir), 'subrepo')
+ subrepo.create(os.path.join(project, 'subrepofiles'))
+
+ # Create the repo from 'repofiles' subdir
+ repo = create_repo('git', str(tmpdir))
+ ref = repo.create(os.path.join(project, 'repofiles'))
+
+ # Add a submodule pointing to the one we created
+ ref = repo.add_submodule('subdir', 'file://' + subrepo.repo)
+
+ # Create the source, and delete the explicit configuration
+ # of the submodules.
+ #
+ # We expect this to cause an unlisted submodule warning
+ # after the source has been fetched.
+ #
+ gitsource = repo.source_config(ref=ref)
+ del gitsource['submodules']
+
+ # Write out our test target
+ element = {
+ 'kind': 'import',
+ 'sources': [
+ gitsource
+ ]
+ }
+ _yaml.dump(element, os.path.join(project, 'target.bst'))
+
+ # We will not see the warning or error before the first fetch, because
+ # we don't have the repository yet and so we have no knowledge of
+ # the unlisted submodule.
+ result = cli.run(project=project, args=['show', 'target.bst'])
+ result.assert_success()
+ assert "git:unlisted-submodule" not in result.stderr
+
+ # We will notice this directly in fetch, as it will try to fetch
+ # the submodules it discovers as a result of fetching the primary repo.
+ result = cli.run(project=project, args=['fetch', 'target.bst'])
+
+ # Assert a warning or an error depending on what we're checking
+ if fail == 'error':
+ result.assert_main_error(ErrorDomain.STREAM, None)
+ result.assert_task_error(ErrorDomain.PLUGIN, 'git:unlisted-submodule')
+ else:
+ result.assert_success()
+ assert "git:unlisted-submodule" in result.stderr
+
+ # Now that we've fetched it, `bst show` will discover the unlisted submodule too
+ result = cli.run(project=project, args=['show', 'target.bst'])
+
+ # Assert a warning or an error depending on what we're checking
+ if fail == 'error':
+ result.assert_main_error(ErrorDomain.PLUGIN, 'git:unlisted-submodule')
+ else:
+ result.assert_success()
+ assert "git:unlisted-submodule" in result.stderr
+
+
+@pytest.mark.skipif(HAVE_GIT is False, reason="git is not available")
+@pytest.mark.datafiles(os.path.join(DATA_DIR, 'template'))
+@pytest.mark.parametrize("fail", ['warn', 'error'])
+def test_track_unlisted_submodule(cli, tmpdir, datafiles, fail):
+ project = os.path.join(datafiles.dirname, datafiles.basename)
+
+ # Make the warning an error if we're testing errors
+ if fail == 'error':
+ project_template = {
+ "name": "foo",
+ "fatal-warnings": ['git:unlisted-submodule']
+ }
+ _yaml.dump(project_template, os.path.join(project, 'project.conf'))
+
+ # Create the submodule first from the 'subrepofiles' subdir
+ subrepo = create_repo('git', str(tmpdir), 'subrepo')
+ subrepo.create(os.path.join(project, 'subrepofiles'))
+
+ # Create the repo from 'repofiles' subdir
+ repo = create_repo('git', str(tmpdir))
+ ref = repo.create(os.path.join(project, 'repofiles'))
+
+ # Add a submodule pointing to the one we created, but use
+ # the original ref, let the submodules appear after tracking
+ repo.add_submodule('subdir', 'file://' + subrepo.repo)
+
+ # Create the source, and delete the explicit configuration
+ # of the submodules.
+ gitsource = repo.source_config(ref=ref)
+ del gitsource['submodules']
+
+ # Write out our test target
+ element = {
+ 'kind': 'import',
+ 'sources': [
+ gitsource
+ ]
+ }
+ _yaml.dump(element, os.path.join(project, 'target.bst'))
+
+ # Fetch the repo, we will not see the warning because we
+ # are still pointing to a ref which predates the submodules
+ result = cli.run(project=project, args=['fetch', 'target.bst'])
+ result.assert_success()
+ assert "git:unlisted-submodule" not in result.stderr
+
+ # We won't get a warning/error when tracking either, the source
+ # has not become Consistency.CACHED so the opportunity to check
+ # for the warning has not yet arisen.
+ result = cli.run(project=project, args=['track', 'target.bst'])
+ result.assert_success()
+ assert "git:unlisted-submodule" not in result.stderr
+
+ # Fetching the repo at the new ref will finally reveal the warning
+ result = cli.run(project=project, args=['fetch', 'target.bst'])
+ if fail == 'error':
+ result.assert_main_error(ErrorDomain.STREAM, None)
+ result.assert_task_error(ErrorDomain.PLUGIN, 'git:unlisted-submodule')
+ else:
+ result.assert_success()
+ assert "git:unlisted-submodule" in result.stderr
+
+
+@pytest.mark.skipif(HAVE_GIT is False, reason="git is not available")
+@pytest.mark.datafiles(os.path.join(DATA_DIR, 'template'))
+@pytest.mark.parametrize("fail", ['warn', 'error'])
+def test_invalid_submodule(cli, tmpdir, datafiles, fail):
+ project = os.path.join(datafiles.dirname, datafiles.basename)
+
+ # Make the warning an error if we're testing errors
+ if fail == 'error':
+ project_template = {
+ "name": "foo",
+ "fatal-warnings": ['git:invalid-submodule']
+ }
+ _yaml.dump(project_template, os.path.join(project, 'project.conf'))
+
+ # Create the repo from 'repofiles' subdir
+ repo = create_repo('git', str(tmpdir))
+ ref = repo.create(os.path.join(project, 'repofiles'))
+
+ # Create the source without any submodules, and add
+ # an invalid submodule configuration to it.
+ #
+ # We expect this to cause an invalid submodule warning
+ # after the source has been fetched and we know what
+ # the real submodules actually are.
+ #
+ gitsource = repo.source_config(ref=ref)
+ gitsource['submodules'] = {
+ 'subdir': {
+ 'url': 'https://pony.org/repo.git'
+ }
+ }
+
+ # Write out our test target
+ element = {
+ 'kind': 'import',
+ 'sources': [
+ gitsource
+ ]
+ }
+ _yaml.dump(element, os.path.join(project, 'target.bst'))
+
+ # We will not see the warning or error before the first fetch, because
+ # we don't have the repository yet and so we have no knowledge of
+ # the unlisted submodule.
+ result = cli.run(project=project, args=['show', 'target.bst'])
+ result.assert_success()
+ assert "git:invalid-submodule" not in result.stderr
+
+ # We will notice this directly in fetch, as it will try to fetch
+ # the submodules it discovers as a result of fetching the primary repo.
+ result = cli.run(project=project, args=['fetch', 'target.bst'])
+
+ # Assert a warning or an error depending on what we're checking
+ if fail == 'error':
+ result.assert_main_error(ErrorDomain.STREAM, None)
+ result.assert_task_error(ErrorDomain.PLUGIN, 'git:invalid-submodule')
+ else:
+ result.assert_success()
+ assert "git:invalid-submodule" in result.stderr
+
+ # Now that we've fetched it, `bst show` will discover the unlisted submodule too
+ result = cli.run(project=project, args=['show', 'target.bst'])
+
+ # Assert a warning or an error depending on what we're checking
+ if fail == 'error':
+ result.assert_main_error(ErrorDomain.PLUGIN, 'git:invalid-submodule')
+ else:
+ result.assert_success()
+ assert "git:invalid-submodule" in result.stderr
+
+
+@pytest.mark.skipif(HAVE_GIT is False, reason="git is not available")
+@pytest.mark.datafiles(os.path.join(DATA_DIR, 'template'))
+@pytest.mark.parametrize("fail", ['warn', 'error'])
+def test_track_invalid_submodule(cli, tmpdir, datafiles, fail):
+ project = os.path.join(datafiles.dirname, datafiles.basename)
+
+ # Make the warning an error if we're testing errors
+ if fail == 'error':
+ project_template = {
+ "name": "foo",
+ "fatal-warnings": ['git:invalid-submodule']
+ }
+ _yaml.dump(project_template, os.path.join(project, 'project.conf'))
+
+ # Create the submodule first from the 'subrepofiles' subdir
+ subrepo = create_repo('git', str(tmpdir), 'subrepo')
+ subrepo.create(os.path.join(project, 'subrepofiles'))
+
+ # Create the repo from 'repofiles' subdir
+ repo = create_repo('git', str(tmpdir))
+ ref = repo.create(os.path.join(project, 'repofiles'))
+
+ # Add a submodule pointing to the one we created
+ ref = repo.add_submodule('subdir', 'file://' + subrepo.repo)
+
+ # Add a commit beyond the ref which *removes* the submodule we've added
+ repo.remove_path('subdir')
+
+ # Create the source, this will keep the submodules so initially
+ # the configuration is valid for the ref we're using
+ gitsource = repo.source_config(ref=ref)
+
+ # Write out our test target
+ element = {
+ 'kind': 'import',
+ 'sources': [
+ gitsource
+ ]
+ }
+ _yaml.dump(element, os.path.join(project, 'target.bst'))
+
+ # Fetch the repo, we will not see the warning because we
+ # are still pointing to a ref which predates the submodules
+ result = cli.run(project=project, args=['fetch', 'target.bst'])
+ result.assert_success()
+ assert "git:invalid-submodule" not in result.stderr
+
+ # In this case, we will get the error directly after tracking,
+ # since the new HEAD does not require any submodules which are
+ # not locally cached, the Source will be CACHED directly after
+ # tracking and the validations will occur as a result.
+ #
+ result = cli.run(project=project, args=['track', 'target.bst'])
+ if fail == 'error':
+ result.assert_main_error(ErrorDomain.STREAM, None)
+ result.assert_task_error(ErrorDomain.PLUGIN, 'git:invalid-submodule')
+ else:
+ result.assert_success()
+ assert "git:invalid-submodule" in result.stderr
+
+
@pytest.mark.skipif(HAVE_GIT is False, reason="git is not available")
@pytest.mark.datafiles(os.path.join(DATA_DIR, 'template'))
@pytest.mark.parametrize("ref_format", ['sha1', 'git-describe'])
diff --git a/tests/testutils/repo/git.py b/tests/testutils/repo/git.py
index fe3ebd547aebe11d2cb1d38b22c9b47847aa3d27..b2d480975d5eb7375ebc0c057cccc4f2db5341b8 100644
--- a/tests/testutils/repo/git.py
+++ b/tests/testutils/repo/git.py
@@ -76,6 +76,12 @@ class Git(Repo):
self._run_git('commit', '-m', 'Added the submodule')
return self.latest_commit()
+ # This can also be used to a file or a submodule
+ def remove_path(self, path):
+ self._run_git('rm', path)
+ self._run_git('commit', '-m', 'Removing {}'.format(path))
+ return self.latest_commit()
+
def source_config(self, ref=None, checkout_submodules=None):
config = {
'kind': 'git',