juergbi commented on code in PR #2147:
URL: https://github.com/apache/buildstream/pull/2147#discussion_r3880135503
##########
src/buildstream/element.py:
##########
@@ -2076,6 +2091,17 @@ def _shell(self, scope=None, *, mounts=None,
isolate=False, prompt=None, command
if prompt is not None:
environment["PS1"] = prompt
+ with self.timed_activity("Staging other_targets",
silent_nested=True), self.__collect_overlaps(sandbox):
+ self.stage_dependency_artifacts(sandbox, other_elements)
+
+ if other_elements:
+ # Stage artifacts from other_elements into the sandbox.
+ for element in other_elements:
+ # Stage deps in the sandbox root
+ with element.timed_activity("Integrating sandbox"),
sandbox.batch():
+ for dep in element._dependencies(_Scope.RUN):
+ dep.integrate(sandbox)
Review Comment:
While injecting deps in the in-memory representation should theoretically be
possible, it would probably be too invasive just for this feature.
A possible tweak to the temp file approach could be to add some kind of
substitution / path override dict to the `Loader`. This could be set in
`Stream.shell_with()`. `_load_file_no_deps()` would then use the path override,
instead of constructing the regular `fullpath`. However, it would still use the
regular `filename` as `shortname`.
I haven't prototyped this, but the possible advantages are:
* No cache key difference for runtime shells, being able to use a previously
cached build
* Logging wouldn't expose temp filenames
* Temp file could be placed in a temporary directory instead of polluting
the elements directory with a temporary file
Any thoughts?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
