abderrahim commented on code in PR #1991:
URL: https://github.com/apache/buildstream/pull/1991#discussion_r2154209469
##########
src/buildstream/buildelement.py:
##########
@@ -285,10 +297,19 @@ def configure_sandbox(self, sandbox):
command_dir = build_root
sandbox.set_work_directory(command_dir)
+ def stage(self, sandbox):
# Setup environment
- sandbox.set_environment(self.get_environment())
+ env = self.get_environment()
- def stage(self, sandbox):
+ for digest_variable, element_list in self.__digest_environment.items():
+ dummy_sandbox = SandboxDummy(
+ self._get_context(), self._get_project(), plugin=self,
stdout=None, stderr=None, config={}
+ )
+ self.stage_dependency_artifacts(dummy_sandbox, element_list)
Review Comment:
> In the case that we are staging elements in element_list which don't
depend on eachother, I don't believe that Element.stage_dependency_artifacts()
will do any sorting.
I find that statement weird given that `stage_dependency_artifact()` is
advertised to do just that.
I would also argue that if this is true, then it is orthogonal to the
changes in this pull request. There is no difference between
```
filename:
- a.bst
- b.bst
- c.bst
config:
location: /sysroot
```
and
```
filename:
- a.bst
- b.bst
- c.bst
config:
digest-environment: DEPENDENCIES_CAS_DIGEST
```
in the current code, except for the fact that the former stages in a
subdirectory of the build sandbox and the latter stages in a dummy sandbox.
> I don't believe it should be algorithmically necessary to do the actual
staging in order to calculate what the resulting CAS digest would be, probably
there should be a way to support this better in the CasBasedDirectory code
without doing an actual staging process.
Well, it depends. Currently an element plugin has no way to create a
`CasBasedDirectory` using public API. The only way to get a `CasBasedDirectory`
is throgh `Sandbox.get_virtual_directory()` (and even that advertises it
returns a `Directory` base class). It also can't get the artifact contents of
an element as a `Directory`: it only has access to `Element.stage_artifact()`.
While this `BuildElement` is in core and can technically use private APIs, I
tried to avoid going that route.
I think the difference we need to consider between the two approaches is
whether or not to consider the dependencies? i.e. if I depend on gcc, is your
expectation that this only gets the digest of the gcc artifact or should it be
gcc and its runtime dependencies?
--
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]
