[Notes] [Git][BuildStream/buildstream][tristan/submodule-warnings] 6 commits: git source plugin: Emmit the ref-not-in-track warning from validate_cache()

[Tristan Van Berkom <gitlab mg gitlab com>]

Title: GitLab

Tristan Van Berkom pushed to branch tristan/submodule-warnings at BuildStream / buildstream

Commits:

• 7f73488e
  by Tristan Van Berkom at 2018-12-06T14:04:44Z

  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.
  

• 75782116
  by Tristan Van Berkom at 2018-12-06T14:04:44Z

  tests/testutils/repo/git.py: Adding remove_path() helper
  
  A function for removing paths from a git repo, can also
  be used to remove submodules.
  

• 39bf5836
  by Tristan Van Berkom at 2018-12-06T14:04:44Z

  tests/sources/git.py: Testing the git:unlisted-submodule warning
  
   o Test that it is not triggered in show before fetch, because we
     don't know about the unlisted submodules yet
  
   o Test that it is triggered by a fetch command
  
   o Test that it is triggered by `show` after having completed a
     fetch command, since now we have the repository and know about
     the unlisted submodule
  
   o Test all of this under warning or error conditions (parameterized
     for fatal-warnings)
  

• 7950e831
  by Tristan Van Berkom at 2018-12-06T14:04:44Z

  tests/sources/git.py: Testing the git:invalid-submodule warning
  
     o Test that it is not triggered in show before fetch, because we
       don't know the submodules yet so we cannot know if they are
       valid or not.
  
     o Test that it is triggered by a fetch command
  
     o Test that it is triggered by `show` after having completed a
       fetch command, since now we have the repository and know which
       specified submodules are invalid
  
     o Test all of this under warning or error conditions (parameterized
       for fatal-warnings)
  

• 092a4476
  by Tristan Van Berkom at 2018-12-06T14:04:44Z

  tests/sources/git.py: Test unlisted submodules warning appearing after track
  

• 27203cd9
  by Tristan Van Berkom at 2018-12-06T14:04:44Z

  tests/sources/git.py: Test invalid submodules warning appearing after track
  

3 changed files:

• buildstream/plugins/sources/git.py
• tests/sources/git.py
• tests/testutils/repo/git.py

Changes:

buildstream/plugins/sources/git.py

---

...	...	@@ -322,7 +322,7 @@ class GitMirror(SourceFetcher):
322	322
323	323	return ref, list(tags)
324	324
325		- def stage(self, directory, track=None):
	325	+ def stage(self, directory):
326	326	fullpath = os.path.join(directory, self.path)
327	327
328	328	# Using --shared here avoids copying the objects into the checkout, in any
...	...	@@ -341,11 +341,7 @@ class GitMirror(SourceFetcher):
341	341
342	342	self._rebuild_git(fullpath)
343	343
344		- # Check that the user specified ref exists in the track if provided & not already tracked
345		- if track:
346		- self.assert_ref_in_track(fullpath, track)
347		-
348		- def init_workspace(self, directory, track=None):
	344	+ def init_workspace(self, directory):
349	345	fullpath = os.path.join(directory, self.path)
350	346	url = self.source.translate_url(self.url)
351	347
...	...	@@ -361,10 +357,6 @@ class GitMirror(SourceFetcher):
361	357	fail="Failed to checkout git ref {}".format(self.ref),
362	358	cwd=fullpath)
363	359
364		- # Check that the user specified ref exists in the track if provided & not already tracked
365		- if track:
366		- self.assert_ref_in_track(fullpath, track)
367		-
368	360	# List the submodules (path/url tuples) present at the given ref of this repo
369	361	def submodule_list(self):
370	362	modules = "{}:{}".format(self.ref, GIT_MODULES)
...	...	@@ -430,28 +422,6 @@ class GitMirror(SourceFetcher):
430	422
431	423	return None
432	424
433		- # Assert that ref exists in track, if track has been specified.
434		- def assert_ref_in_track(self, fullpath, track):
435		- _, branch = self.source.check_output([self.source.host_git, 'branch', '--list', track,
436		- '--contains', self.ref],
437		- cwd=fullpath,)
438		- if branch:
439		- return
440		- else:
441		- _, tag = self.source.check_output([self.source.host_git, 'tag', '--list', track,
442		- '--contains', self.ref],
443		- cwd=fullpath,)
444		- if tag:
445		- return
446		-
447		- detail = "The ref provided for the element does not exist locally in the provided track branch / tag " + \
448		- "'{}'.\nYou may wish to track the element to update the ref from '{}' ".format(track, track) + \
449		- "with `bst track`,\nor examine the upstream at '{}' for the specific ref.".format(self.url)
450		-
451		- self.source.warn("{}: expected ref '{}' was not found in given track '{}' for staged repository: '{}'\n"
452		- .format(self.source, self.ref, track, self.url),
453		- detail=detail, warning_token=CoreWarnings.REF_NOT_IN_TRACK)
454		-
455	425	def _rebuild_git(self, fullpath):
456	426	if not self.tags:
457	427	return
...	...	@@ -580,7 +550,6 @@ class GitSource(Source):
580	550	self.submodule_checkout_overrides[path] = checkout
581	551
582	552	self.mark_download_url(self.original_url)
583		- self.tracked = False
584	553
585	554	def preflight(self):
586	555	# Check if git is installed, get the binary at the same time
...	...	@@ -670,8 +639,6 @@ class GitSource(Source):
670	639	# Update self.mirror.ref and node.ref from the self.tracking branch
671	640	ret = self.mirror.latest_commit_with_tags(self.tracking, self.track_tags)
672	641
673		- # Set tracked attribute, parameter for if self.mirror.assert_ref_in_track is needed
674		- self.tracked = True
675	642	return ret
676	643
677	644	def init_workspace(self, directory):
...	...	@@ -679,7 +646,7 @@ class GitSource(Source):
679	646	self.refresh_submodules()
680	647
681	648	with self.timed_activity('Setting up workspace "{}"'.format(directory), silent_nested=True):
682		- self.mirror.init_workspace(directory, track=(self.tracking if not self.tracked else None))
	649	+ self.mirror.init_workspace(directory)
683	650	for mirror in self.submodules:
684	651	mirror.init_workspace(directory)
685	652
...	...	@@ -695,7 +662,7 @@ class GitSource(Source):
695	662	# Stage the main repo in the specified directory
696	663	#
697	664	with self.timed_activity("Staging {}".format(self.mirror.url), silent_nested=True):
698		- self.mirror.stage(directory, track=(self.tracking if not self.tracked else None))
	665	+ self.mirror.stage(directory)
699	666	for mirror in self.submodules:
700	667	mirror.stage(directory)
701	668
...	...	@@ -747,6 +714,32 @@ class GitSource(Source):
747	714	"in the source description\n\n" +
748	715	"\n".join(detail))
749	716
	717	+ # Assert that the ref exists in the track tag/branch, if track has been specified.
	718	+ ref_in_track = False
	719	+ if self.tracking:
	720	+ _, branch = self.check_output([self.host_git, 'branch', '--list', self.tracking,
	721	+ '--contains', self.mirror.ref],
	722	+ cwd=self.mirror.mirror)
	723	+ if branch:
	724	+ ref_in_track = True
	725	+ else:
	726	+ _, tag = self.check_output([self.host_git, 'tag', '--list', self.tracking,
	727	+ '--contains', self.mirror.ref],
	728	+ cwd=self.mirror.mirror)
	729	+ if tag:
	730	+ ref_in_track = True
	731	+
	732	+ if not ref_in_track:
	733	+ detail = "The ref provided for the element does not exist locally " + \
	734	+ "in the provided track branch / tag '{}'.\n".format(self.tracking) + \
	735	+ "You may wish to track the element to update the ref from '{}' ".format(self.tracking) + \
	736	+ "with `bst track`,\n" + \
	737	+ "or examine the upstream at '{}' for the specific ref.".format(self.mirror.url)
	738	+
	739	+ self.warn("{}: expected ref '{}' was not found in given track '{}' for staged repository: '{}'\n"
	740	+ .format(self, self.mirror.ref, self.tracking, self.mirror.url),
	741	+ detail=detail, warning_token=CoreWarnings.REF_NOT_IN_TRACK)
	742	+
750	743	###########################################################
751	744	# Local Functions #
752	745	###########################################################

tests/sources/git.py

---

...	...	@@ -455,6 +455,274 @@ def test_ref_not_in_track(cli, tmpdir, datafiles, fail):
455	455	assert "ref-not-in-track" in result.stderr
456	456
457	457
	458	+@pytest.mark.skipif(HAVE_GIT is False, reason="git is not available")
	459	+@pytest.mark.datafiles(os.path.join(DATA_DIR, 'template'))
	460	+@pytest.mark.parametrize("fail", ['warn', 'error'])
	461	+def test_unlisted_submodule(cli, tmpdir, datafiles, fail):
	462	+ project = os.path.join(datafiles.dirname, datafiles.basename)
	463	+
	464	+ # Make the warning an error if we're testing errors
	465	+ if fail == 'error':
	466	+ project_template = {
	467	+ "name": "foo",
	468	+ "fatal-warnings": ['git:unlisted-submodule']
	469	+ }
	470	+ _yaml.dump(project_template, os.path.join(project, 'project.conf'))
	471	+
	472	+ # Create the submodule first from the 'subrepofiles' subdir
	473	+ subrepo = create_repo('git', str(tmpdir), 'subrepo')
	474	+ subrepo.create(os.path.join(project, 'subrepofiles'))
	475	+
	476	+ # Create the repo from 'repofiles' subdir
	477	+ repo = create_repo('git', str(tmpdir))
	478	+ ref = repo.create(os.path.join(project, 'repofiles'))
	479	+
	480	+ # Add a submodule pointing to the one we created
	481	+ ref = repo.add_submodule('subdir', 'file://' + subrepo.repo)
	482	+
	483	+ # Create the source, and delete the explicit configuration
	484	+ # of the submodules.
	485	+ #
	486	+ # We expect this to cause an unlisted submodule warning
	487	+ # after the source has been fetched.
	488	+ #
	489	+ gitsource = repo.source_config(ref=ref)
	490	+ del gitsource['submodules']
	491	+
	492	+ # Write out our test target
	493	+ element = {
	494	+ 'kind': 'import',
	495	+ 'sources': [
	496	+ gitsource
	497	+ ]
	498	+ }
	499	+ _yaml.dump(element, os.path.join(project, 'target.bst'))
	500	+
	501	+ # We will not see the warning or error before the first fetch, because
	502	+ # we don't have the repository yet and so we have no knowledge of
	503	+ # the unlisted submodule.
	504	+ result = cli.run(project=project, args=['show', 'target.bst'])
	505	+ result.assert_success()
	506	+ assert "git:unlisted-submodule" not in result.stderr
	507	+
	508	+ # We will notice this directly in fetch, as it will try to fetch
	509	+ # the submodules it discovers as a result of fetching the primary repo.
	510	+ result = cli.run(project=project, args=['fetch', 'target.bst'])
	511	+
	512	+ # Assert a warning or an error depending on what we're checking
	513	+ if fail == 'error':
	514	+ result.assert_main_error(ErrorDomain.STREAM, None)
	515	+ result.assert_task_error(ErrorDomain.PLUGIN, 'git:unlisted-submodule')
	516	+ else:
	517	+ result.assert_success()
	518	+ assert "git:unlisted-submodule" in result.stderr
	519	+
	520	+ # Now that we've fetched it, `bst show` will discover the unlisted submodule too
	521	+ result = cli.run(project=project, args=['show', 'target.bst'])
	522	+
	523	+ # Assert a warning or an error depending on what we're checking
	524	+ if fail == 'error':
	525	+ result.assert_main_error(ErrorDomain.PLUGIN, 'git:unlisted-submodule')
	526	+ else:
	527	+ result.assert_success()
	528	+ assert "git:unlisted-submodule" in result.stderr
	529	+
	530	+
	531	+@pytest.mark.skipif(HAVE_GIT is False, reason="git is not available")
	532	+@pytest.mark.datafiles(os.path.join(DATA_DIR, 'template'))
	533	+@pytest.mark.parametrize("fail", ['warn', 'error'])
	534	+def test_track_unlisted_submodule(cli, tmpdir, datafiles, fail):
	535	+ project = os.path.join(datafiles.dirname, datafiles.basename)
	536	+
	537	+ # Make the warning an error if we're testing errors
	538	+ if fail == 'error':
	539	+ project_template = {
	540	+ "name": "foo",
	541	+ "fatal-warnings": ['git:unlisted-submodule']
	542	+ }
	543	+ _yaml.dump(project_template, os.path.join(project, 'project.conf'))
	544	+
	545	+ # Create the submodule first from the 'subrepofiles' subdir
	546	+ subrepo = create_repo('git', str(tmpdir), 'subrepo')
	547	+ subrepo.create(os.path.join(project, 'subrepofiles'))
	548	+
	549	+ # Create the repo from 'repofiles' subdir
	550	+ repo = create_repo('git', str(tmpdir))
	551	+ ref = repo.create(os.path.join(project, 'repofiles'))
	552	+
	553	+ # Add a submodule pointing to the one we created, but use
	554	+ # the original ref, let the submodules appear after tracking
	555	+ repo.add_submodule('subdir', 'file://' + subrepo.repo)
	556	+
	557	+ # Create the source, and delete the explicit configuration
	558	+ # of the submodules.
	559	+ gitsource = repo.source_config(ref=ref)
	560	+ del gitsource['submodules']
	561	+
	562	+ # Write out our test target
	563	+ element = {
	564	+ 'kind': 'import',
	565	+ 'sources': [
	566	+ gitsource
	567	+ ]
	568	+ }
	569	+ _yaml.dump(element, os.path.join(project, 'target.bst'))
	570	+
	571	+ # Fetch the repo, we will not see the warning because we
	572	+ # are still pointing to a ref which predates the submodules
	573	+ result = cli.run(project=project, args=['fetch', 'target.bst'])
	574	+ result.assert_success()
	575	+ assert "git:unlisted-submodule" not in result.stderr
	576	+
	577	+ # We won't get a warning/error when tracking either, the source
	578	+ # has not become Consistency.CACHED so the opportunity to check
	579	+ # for the warning has not yet arisen.
	580	+ result = cli.run(project=project, args=['track', 'target.bst'])
	581	+ result.assert_success()
	582	+ assert "git:unlisted-submodule" not in result.stderr
	583	+
	584	+ # Fetching the repo at the new ref will finally reveal the warning
	585	+ result = cli.run(project=project, args=['fetch', 'target.bst'])
	586	+ if fail == 'error':
	587	+ result.assert_main_error(ErrorDomain.STREAM, None)
	588	+ result.assert_task_error(ErrorDomain.PLUGIN, 'git:unlisted-submodule')
	589	+ else:
	590	+ result.assert_success()
	591	+ assert "git:unlisted-submodule" in result.stderr
	592	+
	593	+
	594	+@pytest.mark.skipif(HAVE_GIT is False, reason="git is not available")
	595	+@pytest.mark.datafiles(os.path.join(DATA_DIR, 'template'))
	596	+@pytest.mark.parametrize("fail", ['warn', 'error'])
	597	+def test_invalid_submodule(cli, tmpdir, datafiles, fail):
	598	+ project = os.path.join(datafiles.dirname, datafiles.basename)
	599	+
	600	+ # Make the warning an error if we're testing errors
	601	+ if fail == 'error':
	602	+ project_template = {
	603	+ "name": "foo",
	604	+ "fatal-warnings": ['git:invalid-submodule']
	605	+ }
	606	+ _yaml.dump(project_template, os.path.join(project, 'project.conf'))
	607	+
	608	+ # Create the repo from 'repofiles' subdir
	609	+ repo = create_repo('git', str(tmpdir))
	610	+ ref = repo.create(os.path.join(project, 'repofiles'))
	611	+
	612	+ # Create the source without any submodules, and add
	613	+ # an invalid submodule configuration to it.
	614	+ #
	615	+ # We expect this to cause an invalid submodule warning
	616	+ # after the source has been fetched and we know what
	617	+ # the real submodules actually are.
	618	+ #
	619	+ gitsource = repo.source_config(ref=ref)
	620	+ gitsource['submodules'] = {
	621	+ 'subdir': {
	622	+ 'url': 'https://pony.org/repo.git'
	623	+ }
	624	+ }
	625	+
	626	+ # Write out our test target
	627	+ element = {
	628	+ 'kind': 'import',
	629	+ 'sources': [
	630	+ gitsource
	631	+ ]
	632	+ }
	633	+ _yaml.dump(element, os.path.join(project, 'target.bst'))
	634	+
	635	+ # We will not see the warning or error before the first fetch, because
	636	+ # we don't have the repository yet and so we have no knowledge of
	637	+ # the unlisted submodule.
	638	+ result = cli.run(project=project, args=['show', 'target.bst'])
	639	+ result.assert_success()
	640	+ assert "git:invalid-submodule" not in result.stderr
	641	+
	642	+ # We will notice this directly in fetch, as it will try to fetch
	643	+ # the submodules it discovers as a result of fetching the primary repo.
	644	+ result = cli.run(project=project, args=['fetch', 'target.bst'])
	645	+
	646	+ # Assert a warning or an error depending on what we're checking
	647	+ if fail == 'error':
	648	+ result.assert_main_error(ErrorDomain.STREAM, None)
	649	+ result.assert_task_error(ErrorDomain.PLUGIN, 'git:invalid-submodule')
	650	+ else:
	651	+ result.assert_success()
	652	+ assert "git:invalid-submodule" in result.stderr
	653	+
	654	+ # Now that we've fetched it, `bst show` will discover the unlisted submodule too
	655	+ result = cli.run(project=project, args=['show', 'target.bst'])
	656	+
	657	+ # Assert a warning or an error depending on what we're checking
	658	+ if fail == 'error':
	659	+ result.assert_main_error(ErrorDomain.PLUGIN, 'git:invalid-submodule')
	660	+ else:
	661	+ result.assert_success()
	662	+ assert "git:invalid-submodule" in result.stderr
	663	+
	664	+
	665	+@pytest.mark.skipif(HAVE_GIT is False, reason="git is not available")
	666	+@pytest.mark.datafiles(os.path.join(DATA_DIR, 'template'))
	667	+@pytest.mark.parametrize("fail", ['warn', 'error'])
	668	+def test_track_invalid_submodule(cli, tmpdir, datafiles, fail):
	669	+ project = os.path.join(datafiles.dirname, datafiles.basename)
	670	+
	671	+ # Make the warning an error if we're testing errors
	672	+ if fail == 'error':
	673	+ project_template = {
	674	+ "name": "foo",
	675	+ "fatal-warnings": ['git:invalid-submodule']
	676	+ }
	677	+ _yaml.dump(project_template, os.path.join(project, 'project.conf'))
	678	+
	679	+ # Create the submodule first from the 'subrepofiles' subdir
	680	+ subrepo = create_repo('git', str(tmpdir), 'subrepo')
	681	+ subrepo.create(os.path.join(project, 'subrepofiles'))
	682	+
	683	+ # Create the repo from 'repofiles' subdir
	684	+ repo = create_repo('git', str(tmpdir))
	685	+ ref = repo.create(os.path.join(project, 'repofiles'))
	686	+
	687	+ # Add a submodule pointing to the one we created
	688	+ ref = repo.add_submodule('subdir', 'file://' + subrepo.repo)
	689	+
	690	+ # Add a commit beyond the ref which *removes* the submodule we've added
	691	+ repo.remove_path('subdir')
	692	+
	693	+ # Create the source, this will keep the submodules so initially
	694	+ # the configuration is valid for the ref we're using
	695	+ gitsource = repo.source_config(ref=ref)
	696	+
	697	+ # Write out our test target
	698	+ element = {
	699	+ 'kind': 'import',
	700	+ 'sources': [
	701	+ gitsource
	702	+ ]
	703	+ }
	704	+ _yaml.dump(element, os.path.join(project, 'target.bst'))
	705	+
	706	+ # Fetch the repo, we will not see the warning because we
	707	+ # are still pointing to a ref which predates the submodules
	708	+ result = cli.run(project=project, args=['fetch', 'target.bst'])
	709	+ result.assert_success()
	710	+ assert "git:invalid-submodule" not in result.stderr
	711	+
	712	+ # In this case, we will get the error directly after tracking,
	713	+ # since the new HEAD does not require any submodules which are
	714	+ # not locally cached, the Source will be CACHED directly after
	715	+ # tracking and the validations will occur as a result.
	716	+ #
	717	+ result = cli.run(project=project, args=['track', 'target.bst'])
	718	+ if fail == 'error':
	719	+ result.assert_main_error(ErrorDomain.STREAM, None)
	720	+ result.assert_task_error(ErrorDomain.PLUGIN, 'git:invalid-submodule')
	721	+ else:
	722	+ result.assert_success()
	723	+ assert "git:invalid-submodule" in result.stderr
	724	+
	725	+
458	726	@pytest.mark.skipif(HAVE_GIT is False, reason="git is not available")
459	727	@pytest.mark.datafiles(os.path.join(DATA_DIR, 'template'))
460	728	@pytest.mark.parametrize("ref_format", ['sha1', 'git-describe'])

tests/testutils/repo/git.py

---

...	...	@@ -76,6 +76,12 @@ class Git(Repo):
76	76	self._run_git('commit', '-m', 'Added the submodule')
77	77	return self.latest_commit()
78	78
	79	+ # This can also be used to a file or a submodule
	80	+ def remove_path(self, path):
	81	+ self._run_git('rm', path)
	82	+ self._run_git('commit', '-m', 'Removing {}'.format(path))
	83	+ return self.latest_commit()
	84	+
79	85	def source_config(self, ref=None, checkout_submodules=None):
80	86	config = {
81	87	'kind': 'git',

—
View it on GitLab.
You're receiving this email because of your account on gitlab.com. If you'd like to receive fewer emails, you can adjust your notification settings.