[Notes] [Git][BuildStream/buildstream][537-mirror-fallback-does-not-work-for-git] 8 commits: Add warning to git track if track and ref are not present

[Jonathan Maw <gitlab mg gitlab com>]

Title: GitLab

Jonathan Maw pushed to branch 537-mirror-fallback-does-not-work-for-git at BuildStream / buildstream

Commits:

• 42aa3999
  by William Salmon at 2018-08-07T13:41:02Z

  Add warning to git track if track and ref are not present
  
  This is to address https://gitlab.com/BuildStream/buildstream/issues/471 that
  documented unhelpfull behavour when tracking git sources.
  

• 2ceb5dec
  by Will Salmon at 2018-08-07T14:46:36Z

  Merge branch 'willsalmon/trackWarning' into 'master'
  
  Add warning to git track if track and ref are not present
  
  Closes #471
  
  See merge request BuildStream/buildstream!580

• eee4b674
  by Jürg Billeter at 2018-08-07T15:36:35Z

  Revert "Restrict version of pylint"
  
  This reverts commit 4f168b9b6a02216e2fae24d758ae6b778e545869.
  
  The latest version of pytest_pylint works fine with pylint 2, which
  means there is no longer a reason to restrict the pylint version.
  
  pylint 2 is required for Python 3.7.
  

• ea27e389
  by Jürg Billeter at 2018-08-07T15:36:35Z

  _artifactcache/cascache.py: Fix for PEP 479 / Python 3.7
  
  Do not rely on `StopIteration` bubbling up.
  
  https://www.python.org/dev/peps/pep-0479/
  

• fa5a59f0
  by Jürg Billeter at 2018-08-07T16:48:21Z

  Merge branch 'juerg/python3.7' into 'master'
  
  Python 3.7 support
  
  See merge request BuildStream/buildstream!615

• 7befbcad
  by Jonathan Maw at 2018-08-08T09:33:09Z

  tests: Test that fetching passes when upstream is absent
  

• b0e564e9
  by Jonathan Maw at 2018-08-08T09:33:09Z

  source.py: Fix documentation on get_source_fetchers
  

• a1fb15b8
  by Jonathan Maw at 2018-08-08T09:33:09Z

  git.py: Use the configured list of submodules when fetching with aliases
  
  When the source's URL uses an alias (i.e. Source.translate_url()
  actually changes the URL), we use the list of submodules defined in the
  config as the list of submodules used.
  This is because the source would otherwise have to fetch the main repo
  to know which submodules it actually has.
  This is inconvenient when using mirroring, because when the upstream
  repo is unreachable it'll fail when it doesn't have to.
  
  GitMirrors can now be instantiated without having their path and ref
  defined, provided the 'parent' mirror is defined. It will retrieve the
  path and ref as-needed instead.
  
  If the source's URL does not use an alias, it will behave as normally.
  

7 changed files:

• buildstream/_artifactcache/cascache.py
• buildstream/_pipeline.py
• buildstream/plugins/sources/git.py
• buildstream/source.py
• setup.py
• tests/frontend/mirror.py
• tests/sources/git.py

Changes:

buildstream/_artifactcache/cascache.py

---

...	...	@@ -846,6 +846,9 @@ class _CASRemote():
846	846
847	847
848	848	def _grouper(iterable, n):
849		- # pylint: disable=stop-iteration-return
850	849	while True:
851		- yield itertools.chain([next(iterable)], itertools.islice(iterable, n - 1))
	850	+ try:
	851	+ current = next(iterable)
	852	+ except StopIteration:
	853	+ return
	854	+ yield itertools.chain([current], itertools.islice(iterable, n - 1))

buildstream/_pipeline.py

---

...	...	@@ -358,10 +358,24 @@ class Pipeline():
358	358	inconsistent.append(element)
359	359
360	360	if inconsistent:
361		- detail = "Exact versions are missing for the following elements\n" + \
362		- "Try tracking these elements first with `bst track`\n\n"
	361	+ detail = "Exact versions are missing for the following elements:\n\n"
	362	+
	363	+ missingTrack = 0
363	364	for element in inconsistent:
364		- detail += " " + element._get_full_name() + "\n"
	365	+ detail += " " + element._get_full_name()
	366	+ for source in element.sources():
	367	+ if not source._get_consistency() and not source.get_ref():
	368	+ if hasattr(source, 'tracking') and source.tracking is None:
	369	+ detail += ": Source {} is missing ref and track. ".format(source._get_full_name()) + \
	370	+ "Please specify a ref or branch/tag to track."
	371	+ missingTrack = 1
	372	+
	373	+ detail += "\n"
	374	+
	375	+ if missingTrack:
	376	+ detail += "\nThen track these elements with `bst track`\n"
	377	+ else:
	378	+ detail += "\nTry tracking these elements first with `bst track`\n"
365	379	raise PipelineError("Inconsistent pipeline", detail=detail, reason="inconsistent-pipeline")
366	380
367	381	#############################################################

buildstream/plugins/sources/git.py

---

...	...	@@ -91,16 +91,18 @@ GIT_MODULES = '.gitmodules'
91	91	#
92	92	class GitMirror(SourceFetcher):
93	93
94		- def __init__(self, source, path, url, ref):
	94	+ def __init__(self, source, path, url, ref, *, parent=None):
95	95
96	96	super().__init__()
97	97	self.source = source
98		- self.path = path
	98	+ self.parent = parent
99	99	self.url = url
100		- self.ref = ref
101	100	self.mirror = os.path.join(source.get_mirror_directory(), utils.url_directory_name(url))
102	101	self.mark_download_url(url)
103	102
	103	+ self._path = path
	104	+ self._ref = ref
	105	+
104	106	# Ensures that the mirror exists
105	107	def ensure(self, alias_override=None):
106	108
...	...	@@ -223,8 +225,7 @@ class GitMirror(SourceFetcher):
223	225	fail="Failed to checkout git ref {}".format(self.ref),
224	226	cwd=fullpath)
225	227
226		- # List the submodules (path/url tuples) present at the given ref of this repo
227		- def submodule_list(self):
	228	+ def _read_gitmodules(self):
228	229	modules = "{}:{}".format(self.ref, GIT_MODULES)
229	230	exit_code, output = self.source.check_output(
230	231	[self.source.host_git, 'show', modules], cwd=self.mirror)
...	...	@@ -247,10 +248,15 @@ class GitMirror(SourceFetcher):
247	248	for section in parser.sections():
248	249	# validate section name against the 'submodule "foo"' pattern
249	250	if re.match(r'submodule "(.*)"', section):
250		- path = parser.get(section, 'path')
251		- url = parser.get(section, 'url')
	251	+ yield (parser, section)
252	252
253		- yield (path, url)
	253	+ # List the submodules (path/url tuples) present at the given ref of this repo
	254	+ def submodule_list(self):
	255	+ for parser, section in self._read_gitmodules():
	256	+ path = parser.get(section, 'path')
	257	+ url = parser.get(section, 'url')
	258	+
	259	+ yield (path, url)
254	260
255	261	# Fetch the ref which this mirror requires its submodule to have,
256	262	# at the given ref of this mirror.
...	...	@@ -287,6 +293,33 @@ class GitMirror(SourceFetcher):
287	293
288	294	return None
289	295
	296	+ def get_submodule_path(self, url):
	297	+ for parser, section in self._read_gitmodules():
	298	+ parsed_url = parser.get(section, 'url')
	299	+ if parsed_url == url:
	300	+ return parser.get(section, 'path')
	301	+
	302	+ raise SourceError("{}: No submodule found with url '{}'".format(self.source, url))
	303	+
	304	+ @property
	305	+ def path(self):
	306	+ if self._path is None:
	307	+ self._path = self.parent.get_submodule_path()
	308	+
	309	+ return self._path
	310	+
	311	+ @property
	312	+ def ref(self):
	313	+ # The top-level GitMirror may have ref as None, submodules don't.
	314	+ if self._ref is None and self.parent:
	315	+ self._ref = self.parent.submodule_ref(self.path)
	316	+
	317	+ return self._ref
	318	+
	319	+ @ref.setter
	320	+ def ref(self, ref):
	321	+ self._ref = ref
	322	+
290	323
291	324	class GitSource(Source):
292	325	# pylint: disable=attribute-defined-outside-init
...	...	@@ -303,6 +336,8 @@ class GitSource(Source):
303	336	self.checkout_submodules = self.node_get_member(node, bool, 'checkout-submodules', True)
304	337	self.submodules = []
305	338
	339	+ self.using_source_fetchers = (self.original_url != self.translate_url(self.original_url))
	340	+
306	341	# Parse a dict of submodule overrides, stored in the submodule_overrides
307	342	# and submodule_checkout_overrides dictionaries.
308	343	self.submodule_overrides = {}
...	...	@@ -311,6 +346,11 @@ class GitSource(Source):
311	346	for path, _ in self.node_items(modules):
312	347	submodule = self.node_get_member(modules, Mapping, path)
313	348	url = self.node_get_member(submodule, str, 'url', None)
	349	+
	350	+ if self.using_source_fetchers:
	351	+ submodule_mirror = GitMirror(self, None, url, None, parent=self.mirror)
	352	+ self.submodules.append(submodule_mirror)
	353	+
314	354	self.submodule_overrides[path] = url
315	355	if 'checkout' in submodule:
316	356	checkout = self.node_get_member(submodule, bool, 'checkout')
...	...	@@ -363,6 +403,12 @@ class GitSource(Source):
363	403
364	404	# If self.tracking is not specified it's not an error, just silently return
365	405	if not self.tracking:
	406	+ # Is there a better way to check if a ref is given.
	407	+ if self.mirror.ref is None:
	408	+ detail = 'Without a tracking branch ref can not be updated. Please ' + \
	409	+ 'provide a ref or a track.'
	410	+ raise SourceError("{}: No track or ref".format(self),
	411	+ detail=detail, reason="track-attempt-no-track")
366	412	return None
367	413
368	414	with self.timed_activity("Tracking {} from {}"
...	...	@@ -376,6 +422,24 @@ class GitSource(Source):
376	422
377	423	return ret
378	424
	425	+ def fetch(self):
	426	+
	427	+ with self.timed_activity("Fetching {}".format(self.mirror.url), silent_nested=True):
	428	+
	429	+ # Here we are only interested in ensuring that our mirror contains
	430	+ # the self.mirror.ref commit.
	431	+ self.mirror.ensure()
	432	+ if not self.mirror.has_ref():
	433	+ self.mirror.fetch()
	434	+
	435	+ self.mirror.assert_ref()
	436	+
	437	+ # Here after performing any fetches, we need to also ensure that
	438	+ # we've cached the desired refs in our mirrors of submodules.
	439	+ #
	440	+ self.refresh_submodules()
	441	+ self.fetch_submodules()
	442	+
379	443	def init_workspace(self, directory):
380	444	# XXX: may wish to refactor this as some code dupe with stage()
381	445	self.refresh_submodules()
...	...	@@ -408,8 +472,11 @@ class GitSource(Source):
408	472	mirror.stage(directory)
409	473
410	474	def get_source_fetchers(self):
411		- self.refresh_submodules()
412		- return [self.mirror] + self.submodules
	475	+ # If the url does not contain an alias, then it does not need SourceFetchers
	476	+ if self.mirror.url == self.translate_url(self.mirror.url):
	477	+ return []
	478	+ else:
	479	+ return [self.mirror] + self.submodules
413	480
414	481	###########################################################
415	482	# Local Functions #
...	...	@@ -432,6 +499,11 @@ class GitSource(Source):
432	499	# Assumes that we have our mirror and we have the ref which we point to
433	500	#
434	501	def refresh_submodules(self):
	502	+
	503	+ # When using source fetchers, the submodule list is defined by the 'submodules' config field
	504	+ if self.using_source_fetchers:
	505	+ return
	506	+
435	507	self.mirror.ensure()
436	508	submodules = []
437	509
...	...	@@ -454,6 +526,19 @@ class GitSource(Source):
454	526
455	527	self.submodules = submodules
456	528
	529	+ # Ensures that we have mirrored git repositories for all
	530	+ # the submodules existing at the given commit of the main git source.
	531	+ #
	532	+ # Also ensure that these mirrors have the required commits
	533	+ # referred to at the given commit of the main git source.
	534	+ #
	535	+ def fetch_submodules(self):
	536	+ for mirror in self.submodules:
	537	+ mirror.ensure()
	538	+ if not mirror.has_ref():
	539	+ mirror.fetch()
	540	+ mirror.assert_ref()
	541	+
457	542
458	543	# Plugin entry point
459	544	def setup():

buildstream/source.py

---

...	...	@@ -393,8 +393,8 @@ class Source(Plugin):
393	393	"""Get the objects that are used for fetching
394	394
395	395	If this source doesn't download from multiple URLs,
396		- returning None and falling back on the default behaviour
397		- is recommended.
	396	+ returning an empty list and falling back on the default
	397	+ behaviour is recommended.
398	398
399	399	Returns:
400	400	list: A list of SourceFetchers. If SourceFetchers are not supported,

setup.py

---

...	...	@@ -272,6 +272,5 @@ setup(name='BuildStream',
272	272	'pytest-cov >= 2.5.0',
273	273	# Provide option to run tests in parallel, less reliable
274	274	'pytest-xdist',
275		- 'pytest >= 3.1.0',
276		- 'pylint >= 1.8 , < 2'],
	275	+ 'pytest >= 3.1.0'],
277	276	zip_safe=False)

tests/frontend/mirror.py

---

...	...	@@ -139,6 +139,63 @@ def test_mirror_fetch(cli, tmpdir, datafiles, kind):
139	139	result.assert_success()
140	140
141	141
	142	+@pytest.mark.datafiles(DATA_DIR)
	143	+@pytest.mark.parametrize("kind", [(kind) for kind in ALL_REPO_KINDS])
	144	+def test_mirror_fetch_upstream_absent(cli, tmpdir, datafiles, kind):
	145	+ bin_files_path = os.path.join(str(datafiles), 'files', 'bin-files', 'usr')
	146	+ dev_files_path = os.path.join(str(datafiles), 'files', 'dev-files', 'usr')
	147	+ upstream_repodir = os.path.join(str(tmpdir), 'upstream')
	148	+ mirror_repodir = os.path.join(str(tmpdir), 'mirror')
	149	+ project_dir = os.path.join(str(tmpdir), 'project')
	150	+ os.makedirs(project_dir)
	151	+ element_dir = os.path.join(project_dir, 'elements')
	152	+
	153	+ # Create repo objects of the upstream and mirror
	154	+ upstream_repo = create_repo(kind, upstream_repodir)
	155	+ ref = upstream_repo.create(dev_files_path)
	156	+ mirror_repo = upstream_repo.copy(mirror_repodir)
	157	+
	158	+ element = {
	159	+ 'kind': 'import',
	160	+ 'sources': [
	161	+ upstream_repo.source_config(ref=ref)
	162	+ ]
	163	+ }
	164	+
	165	+ element_name = 'test.bst'
	166	+ element_path = os.path.join(element_dir, element_name)
	167	+ full_repo = element['sources'][0]['url']
	168	+ upstream_map, repo_name = os.path.split(full_repo)
	169	+ alias = 'foo-' + kind
	170	+ aliased_repo = alias + ':' + repo_name
	171	+ element['sources'][0]['url'] = aliased_repo
	172	+ full_mirror = mirror_repo.source_config()['url']
	173	+ mirror_map, _ = os.path.split(full_mirror)
	174	+ os.makedirs(element_dir)
	175	+ _yaml.dump(element, element_path)
	176	+
	177	+ project = {
	178	+ 'name': 'test',
	179	+ 'element-path': 'elements',
	180	+ 'aliases': {
	181	+ alias: 'http://www.example.com/'
	182	+ },
	183	+ 'mirrors': [
	184	+ {
	185	+ 'name': 'middle-earth',
	186	+ 'aliases': {
	187	+ alias: [mirror_map + "/"],
	188	+ },
	189	+ },
	190	+ ]
	191	+ }
	192	+ project_file = os.path.join(project_dir, 'project.conf')
	193	+ _yaml.dump(project, project_file)
	194	+
	195	+ result = cli.run(project=project_dir, args=['fetch', element_name])
	196	+ result.assert_success()
	197	+
	198	+
142	199	@pytest.mark.datafiles(DATA_DIR)
143	200	def test_mirror_fetch_multi(cli, tmpdir, datafiles):
144	201	output_file = os.path.join(str(tmpdir), "output.txt")

tests/sources/git.py

---

...	...	@@ -359,3 +359,45 @@ def test_submodule_track_ignore_inconsistent(cli, tmpdir, datafiles):
359	359
360	360	# Assert that we are just fine without it, and emit a warning to the user.
361	361	assert "Ignoring inconsistent submodule" in result.stderr
	362	+
	363	+
	364	+@pytest.mark.skipif(HAVE_GIT is False, reason="git is not available")
	365	+@pytest.mark.datafiles(os.path.join(DATA_DIR, 'template'))
	366	+def test_submodule_track_no_ref_or_track(cli, tmpdir, datafiles):
	367	+ project = os.path.join(datafiles.dirname, datafiles.basename)
	368	+
	369	+ # Create the repo from 'repofiles' subdir
	370	+ repo = create_repo('git', str(tmpdir))
	371	+ ref = repo.create(os.path.join(project, 'repofiles'))
	372	+
	373	+ # Write out our test target
	374	+ gitsource = repo.source_config(ref=None)
	375	+ gitsource.pop('track')
	376	+ element = {
	377	+ 'kind': 'import',
	378	+ 'sources': [
	379	+ gitsource
	380	+ ]
	381	+ }
	382	+
	383	+ _yaml.dump(element, os.path.join(project, 'target.bst'))
	384	+
	385	+ # Track will encounter an inconsistent submodule without any ref
	386	+ result = cli.run(project=project, args=['track', 'target.bst'])
	387	+ result.assert_main_error(ErrorDomain.STREAM, None)
	388	+ result.assert_task_error(ErrorDomain.SOURCE, 'track-attempt-no-track')
	389	+
	390	+ # Assert that we are just fine without it, and emit a warning to the user.
	391	+ assert "FAILURE git source at" in result.stderr
	392	+ assert "Without a tracking branch ref can not be updated. Please " + \
	393	+ "provide a ref or a track." in result.stderr
	394	+
	395	+ # Track will encounter an inconsistent submodule without any ref
	396	+ result = cli.run(project=project, args=['build', 'target.bst'])
	397	+ result.assert_main_error(ErrorDomain.PIPELINE, 'inconsistent-pipeline')
	398	+ result.assert_task_error(None, None)
	399	+
	400	+ # Assert that we are just fine without it, and emit a warning to the user.
	401	+ assert "Exact versions are missing for the following elements" in result.stderr
	402	+ assert "is missing ref and track." in result.stderr
	403	+ assert "Then track these elements with `bst track`" in result.stderr

—
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.