[Notes] [Git][BuildStream/buildstream][valentindavid/crash_in_scheduler_857] 7 commits: _stream.py: Add use_artifact_config opt arg for load_selection()

[Jürg Billeter <gitlab mg gitlab com>]

Title: GitLab

Jürg Billeter pushed to branch valentindavid/crash_in_scheduler_857 at BuildStream / buildstream

Commits:

• a3e2cdd2
  by Tom Pollard at 2019-01-28T12:14:40Z

  _stream.py: Add use_artifact_config opt arg for load_selection()
  
  use_artifact_config added as an optional default arg, allowing
  for loading of given elements artifact remote config.
  

• 805baf7d
  by Tom Pollard at 2019-01-28T12:14:40Z

  Download buildtrees on demand for bst shell --use-buildtree
  
  Provide bst shell --use-buildtree the ability to attempt to
  acquire missing buildtrees, given respective option, user
  pull-buildtree context and remote availability.
  
  _frontend/cli.py: Refactor logic for determining --use-buildtree
  option with given opportunity to attempt pulling a non-local
  buildtree. Element loaded with artifact_config to allow remote
  querying.
  
  _stream.py: With given user option and element state, construct
  PullQueue to fetch remote buildtree. Continue or Error without
  buildtree if cannot be attained.
  
  tests/integration/build-tree.py: Update to support new usecases
  

• 38356932
  by Tom Pollard at 2019-01-28T13:44:37Z

  Merge branch 'tpollard/829' into 'master'
  
  Download buildtrees on demand for bst shell --use-buildtree
  
  Closes #829
  
  See merge request BuildStream/buildstream!1050

• 3590ca8c
  by Abderrahim Kitouni at 2019-01-28T14:59:19Z

  requirements/requirements.in: require protobuf >= 3.6
  
  This is needed since 0f2bc3754
  

• 39b952dc
  by Abderrahim Kitouni at 2019-01-28T15:19:04Z

  requirements/requirements.in: require Click >= 7.0
  
  This is needed since 629a6e524, and was lost in the conversion to requirements.in
  

• 80b36d0c
  by Javier Jardón at 2019-01-28T17:54:52Z

  Merge branch 'abderrahim/protobuf-version' into 'master'
  
  requirements/requirements.in: update minimum versions
  
  Closes #884
  
  See merge request BuildStream/buildstream!1114

• a1ab48da
  by Valentin David at 2019-01-28T21:30:26Z

  Fix crash when spawned job completes very fast
  
  Job can complete before we return from `Job.span()` to
  `Scheduler._spawn_job()`, so that `_active_jobs` would not yet contain
  the job.
  
  This would print a stack on the console and try to run a second time
  the job which can have unexpected effects.
  
  In order to reproduce the issue, in
  `buildstream/_scheduler/jobs/job.py`, in `Job.spawn`,
  add a call to `time.sleep()` right before call to
  `asyncio.get_child_watcher()`.
  
  This fixes issue #857.
  

5 changed files:

• buildstream/_frontend/cli.py
• buildstream/_scheduler/scheduler.py
• buildstream/_stream.py
• requirements/requirements.in
• tests/integration/build-tree.py

Changes:

buildstream/_frontend/cli.py

---

...	...	@@ -526,7 +526,7 @@ def shell(app, element, sysroot, mount, isolate, build_, cli_buildtree, command)
526	526	else:
527	527	scope = Scope.RUN
528	528
529		- use_buildtree = False
	529	+ use_buildtree = None
530	530
531	531	with app.initialized():
532	532	if not element:
...	...	@@ -534,7 +534,8 @@ def shell(app, element, sysroot, mount, isolate, build_, cli_buildtree, command)
534	534	if not element:
535	535	raise AppError('Missing argument "ELEMENT".')
536	536
537		- dependencies = app.stream.load_selection((element,), selection=PipelineSelection.NONE)
	537	+ dependencies = app.stream.load_selection((element,), selection=PipelineSelection.NONE,
	538	+ use_artifact_config=True)
538	539	element = dependencies[0]
539	540	prompt = app.shell_prompt(element)
540	541	mounts = [
...	...	@@ -543,20 +544,31 @@ def shell(app, element, sysroot, mount, isolate, build_, cli_buildtree, command)
543	544	]
544	545
545	546	cached = element._cached_buildtree()
546		- if cli_buildtree == "always":
547		- if cached:
548		- use_buildtree = True
549		- else:
550		- raise AppError("No buildtree is cached but the use buildtree option was specified")
551		- elif cli_buildtree == "never":
552		- pass
553		- elif cli_buildtree == "try":
554		- use_buildtree = cached
	547	+ if cli_buildtree in ("always", "try"):
	548	+ use_buildtree = cli_buildtree
	549	+ if not cached and use_buildtree == "always":
	550	+ click.echo("WARNING: buildtree is not cached locally, will attempt to pull from available remotes",
	551	+ err=True)
555	552	else:
556		- if app.interactive and cached:
557		- use_buildtree = bool(click.confirm('Do you want to use the cached buildtree?'))
	553	+ # If the value has defaulted to ask and in non interactive mode, don't consider the buildtree, this
	554	+ # being the default behaviour of the command
	555	+ if app.interactive and cli_buildtree == "ask":
	556	+ if cached and bool(click.confirm('Do you want to use the cached buildtree?')):
	557	+ use_buildtree = "always"
	558	+ elif not cached:
	559	+ try:
	560	+ choice = click.prompt("Do you want to pull & use a cached buildtree?",
	561	+ type=click.Choice(['try', 'always', 'never']),
	562	+ err=True, show_choices=True)
	563	+ except click.Abort:
	564	+ click.echo('Aborting', err=True)
	565	+ sys.exit(-1)
	566	+
	567	+ if choice != "never":
	568	+ use_buildtree = choice
	569	+
558	570	if use_buildtree and not element._cached_success():
559		- click.echo("Warning: using a buildtree from a failed build.")
	571	+ click.echo("WARNING: using a buildtree from a failed build.", err=True)
560	572
561	573	try:
562	574	exitcode = app.stream.shell(element, scope, prompt,

buildstream/_scheduler/scheduler.py

---

...	...	@@ -314,10 +314,10 @@ class Scheduler():
314	314	# job (Job): The job to spawn
315	315	#
316	316	def _spawn_job(self, job):
317		- job.spawn()
318	317	self._active_jobs.append(job)
319	318	if self._job_start_callback:
320	319	self._job_start_callback(job)
	320	+ job.spawn()
321	321
322	322	# Callback for the cache size job
323	323	def _cache_size_job_complete(self, status, cache_size):

buildstream/_stream.py

---

...	...	@@ -101,19 +101,22 @@ class Stream():
101	101	# targets (list of str): Targets to pull
102	102	# selection (PipelineSelection): The selection mode for the specified targets
103	103	# except_targets (list of str): Specified targets to except from fetching
	104	+ # use_artifact_config (bool): If artifact remote config should be loaded
104	105	#
105	106	# Returns:
106	107	# (list of Element): The selected elements
107	108	def load_selection(self, targets, *,
108	109	selection=PipelineSelection.NONE,
109		- except_targets=()):
	110	+ except_targets=(),
	111	+ use_artifact_config=False):
110	112
111	113	profile_start(Topics.LOAD_SELECTION, "_".join(t.replace(os.sep, '-') for t in targets))
112	114
113	115	elements, _ = self._load(targets, (),
114	116	selection=selection,
115	117	except_targets=except_targets,
116		- fetch_subprojects=False)
	118	+ fetch_subprojects=False,
	119	+ use_artifact_config=use_artifact_config)
117	120
118	121	profile_end(Topics.LOAD_SELECTION, "_".join(t.replace(os.sep, '-') for t in targets))
119	122
...	...	@@ -131,7 +134,7 @@ class Stream():
131	134	# mounts (list of HostMount): Additional directories to mount into the sandbox
132	135	# isolate (bool): Whether to isolate the environment like we do in builds
133	136	# command (list): An argv to launch in the sandbox, or None
134		- # usebuildtree (bool): Wheather to use a buildtree as the source.
	137	+ # usebuildtree (str): Whether to use a buildtree as the source, given cli option
135	138	#
136	139	# Returns:
137	140	# (int): The exit code of the launched shell
...	...	@@ -141,7 +144,7 @@ class Stream():
141	144	mounts=None,
142	145	isolate=False,
143	146	command=None,
144		- usebuildtree=False):
	147	+ usebuildtree=None):
145	148
146	149	# Assert we have everything we need built, unless the directory is specified
147	150	# in which case we just blindly trust the directory, using the element
...	...	@@ -156,8 +159,31 @@ class Stream():
156	159	raise StreamError("Elements need to be built or downloaded before staging a shell environment",
157	160	detail="\n".join(missing_deps))
158	161
	162	+ buildtree = False
	163	+ # Check if we require a pull queue attempt, with given artifact state and context
	164	+ if usebuildtree:
	165	+ if not element._cached_buildtree():
	166	+ require_buildtree = self._buildtree_pull_required([element])
	167	+ # Attempt a pull queue for the given element if remote and context allow it
	168	+ if require_buildtree:
	169	+ self._message(MessageType.INFO, "Attempting to fetch missing artifact buildtree")
	170	+ self._add_queue(PullQueue(self._scheduler))
	171	+ self._enqueue_plan(require_buildtree)
	172	+ self._run()
	173	+ # Now check if the buildtree was successfully fetched
	174	+ if element._cached_buildtree():
	175	+ buildtree = True
	176	+ if not buildtree:
	177	+ if usebuildtree == "always":
	178	+ raise StreamError("Buildtree is not cached locally or in available remotes")
	179	+ else:
	180	+ self._message(MessageType.INFO, """Buildtree is not cached locally or in available remotes,
	181	+ shell will be loaded without it""")
	182	+ else:
	183	+ buildtree = True
	184	+
159	185	return element._shell(scope, directory, mounts=mounts, isolate=isolate, prompt=prompt, command=command,
160		- usebuildtree=usebuildtree)
	186	+ usebuildtree=buildtree)
161	187
162	188	# build()
163	189	#

requirements/requirements.in

---

1		-Click
	1	+Click >= 7.0
2	2	grpcio >= 1.10
3	3	Jinja2 >= 2.10
4	4	pluginbase
5		-protobuf >= 3.5
	5	+protobuf >= 3.6
6	6	psutil
7	7	# According to ruamel.yaml's PyPI page, we are suppose to use
8	8	# "<=0.15" in production until 0.15 becomes API stable.

tests/integration/build-tree.py

---

...	...	@@ -101,7 +101,7 @@ def test_buildtree_from_failure(cli_integration, tmpdir, datafiles):
101	101	'shell', '--build', element_name, '--use-buildtree', 'always', '--', 'cat', 'test'
102	102	])
103	103	res.assert_success()
104		- assert "Warning: using a buildtree from a failed build" in res.output
	104	+ assert "WARNING: using a buildtree from a failed build" in res.stderr
105	105	assert 'Hi' in res.output
106	106
107	107
...	...	@@ -141,7 +141,7 @@ def test_buildtree_pulled(cli, tmpdir, datafiles):
141	141	res.assert_success()
142	142
143	143
144		-# This test checks for correct behaviour if a buildtree is not present.
	144	+# This test checks for correct behaviour if a buildtree is not present in the local cache.
145	145	@pytest.mark.datafiles(DATA_DIR)
146	146	@pytest.mark.skipif(IS_LINUX and not HAVE_BWRAP, reason='Only available with bubblewrap on Linux')
147	147	def test_buildtree_options(cli, tmpdir, datafiles):
...	...	@@ -156,6 +156,7 @@ def test_buildtree_options(cli, tmpdir, datafiles):
156	156	result = cli.run(project=project, args=['build', element_name])
157	157	result.assert_success()
158	158	assert cli.get_element_state(project, element_name) == 'cached'
	159	+ assert share.has_artifact('test', element_name, cli.get_element_key(project, element_name))
159	160
160	161	# Discard the cache
161	162	cli.configure({
...	...	@@ -168,8 +169,6 @@ def test_buildtree_options(cli, tmpdir, datafiles):
168	169	result = cli.run(project=project, args=['artifact', 'pull', '--deps', 'all', element_name])
169	170	result.assert_success()
170	171
171		- # The above is the simplest way I know to create a local cache without any buildtrees.
172		-
173	172	# Check it's not using the cached build tree
174	173	res = cli.run(project=project, args=[
175	174	'shell', '--build', element_name, '--use-buildtree', 'never', '--', 'cat', 'test'
...	...	@@ -177,24 +176,51 @@ def test_buildtree_options(cli, tmpdir, datafiles):
177	176	res.assert_shell_error()
178	177	assert 'Hi' not in res.output
179	178
180		- # Check it's not correctly handling the lack of buildtree
	179	+ # Check it's not using the cached build tree, default is to ask, and fall back to not
	180	+ # for non interactive behavior
181	181	res = cli.run(project=project, args=[
182		- 'shell', '--build', element_name, '--use-buildtree', 'try', '--', 'cat', 'test'
	182	+ 'shell', '--build', element_name, '--', 'cat', 'test'
183	183	])
184	184	res.assert_shell_error()
185	185	assert 'Hi' not in res.output
186	186
187		- # Check it's not using the cached build tree, default is to ask, and fall back to not
188		- # for non interactive behavior
	187	+ # Check correctly handling the lack of buildtree, with 'try' not attempting to
	188	+ # pull the buildtree as the user context is by default set to not pull them
189	189	res = cli.run(project=project, args=[
190		- 'shell', '--build', element_name, '--', 'cat', 'test'
	190	+ 'shell', '--build', element_name, '--use-buildtree', 'try', '--', 'cat', 'test'
191	191	])
192		- res.assert_shell_error()
193	192	assert 'Hi' not in res.output
	193	+ assert 'Attempting to fetch missing artifact buildtrees' not in res.stderr
	194	+ assert """Buildtree is not cached locally or in available remotes,
	195	+ shell will be loaded without it"""
194	196
195		- # Check it's using the cached build tree
	197	+ # Check correctly handling the lack of buildtree, with 'try' attempting and succeeding
	198	+ # to pull the buildtree as the user context allow the pulling of buildtrees and it is
	199	+ # available in the remote
	200	+ res = cli.run(project=project, args=[
	201	+ '--pull-buildtrees', 'shell', '--build', element_name, '--use-buildtree', 'try', '--', 'cat', 'test'
	202	+ ])
	203	+ assert 'Attempting to fetch missing artifact buildtree' in res.stderr
	204	+ assert 'Hi' in res.output
	205	+ shutil.rmtree(os.path.join(os.path.join(cli.directory, 'artifacts2')))
	206	+ assert cli.get_element_state(project, element_name) != 'cached'
	207	+
	208	+ # Check it's not loading the shell at all with always set for the buildtree, when the
	209	+ # user context does not allow for buildtree pulling
	210	+ result = cli.run(project=project, args=['artifact', 'pull', '--deps', 'all', element_name])
	211	+ result.assert_success()
196	212	res = cli.run(project=project, args=[
197	213	'shell', '--build', element_name, '--use-buildtree', 'always', '--', 'cat', 'test'
198	214	])
199	215	res.assert_main_error(ErrorDomain.PROG_NOT_FOUND, None)
	216	+ assert 'Buildtree is not cached locally or in available remotes' in res.stderr
200	217	assert 'Hi' not in res.output
	218	+ assert 'Attempting to fetch missing artifact buildtree' not in res.stderr
	219	+
	220	+ # Check that when user context is set to pull buildtrees and a remote has the buildtree,
	221	+ # 'always' will attempt and succeed at pulling the missing buildtree.
	222	+ res = cli.run(project=project, args=[
	223	+ '--pull-buildtrees', 'shell', '--build', element_name, '--use-buildtree', 'always', '--', 'cat', 'test'
	224	+ ])
	225	+ assert 'Hi' in res.output
	226	+ assert 'Attempting to fetch missing artifact buildtree' in res.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.