abderrahim commented on code in PR #2036:
URL: https://github.com/apache/buildstream/pull/2036#discussion_r2201062972


##########
src/buildstream/sandbox/_sandboxremote.py:
##########
@@ -263,9 +263,17 @@ def _execute_action(self, action, flags):
                 "Uploading input root", element_name=self._get_element_name()
             ):
                 # Determine blobs missing on remote
+                root_digests = [action.input_root_digest]
+
+                # Add virtual directories for subsandboxes
+                for subsandbox in self._get_subsandboxes():
+                    vdir = subsandbox.get_virtual_directory()
+                    root_digests.append(vdir._get_digest())

Review Comment:
   I wonder if we should have an option to only upload these conditionally. It 
think it's not worth the trouble, but wanted to have a second opinion.



##########
src/buildstream/buildelement.py:
##########
@@ -285,10 +305,20 @@ def configure_sandbox(self, sandbox):
             command_dir = build_root
         sandbox.set_work_directory(command_dir)
 
-        # Setup environment
-        sandbox.set_environment(self.get_environment())
-
     def stage(self, sandbox):
+        # Setup environment

Review Comment:
   I've moved this here because buildstream wouldn't let me stage outside of 
`stage()`.
   
   It might be better to try to work around that limitation rather than doing 
this virtual staging here.



##########
src/buildstream/buildelement.py:
##########
@@ -285,10 +305,20 @@ def configure_sandbox(self, sandbox):
             command_dir = build_root
         sandbox.set_work_directory(command_dir)
 
-        # Setup environment
-        sandbox.set_environment(self.get_environment())
-
     def stage(self, sandbox):
+        # Setup environment
+        env = self.get_environment()
+
+        # Add "CAS digest" environment variables
+        sorted_envs = sorted(self.__digest_environment)
+        for digest_variable in sorted_envs:
+            element_list = [element for element, _ in 
self.__digest_environment[digest_variable]]
+            subsandbox = sandbox.create_subsandbox()
+            self.stage_dependency_artifacts(subsandbox, element_list)

Review Comment:
   @gtristan was worried that the CAS digest might depend on the order the 
elements are declared in the .bst file
   
   See https://github.com/apache/buildstream/pull/1991#discussion_r2153808770



##########
src/buildstream/buildelement.py:
##########
@@ -285,10 +305,20 @@ def configure_sandbox(self, sandbox):
             command_dir = build_root
         sandbox.set_work_directory(command_dir)
 
-        # Setup environment
-        sandbox.set_environment(self.get_environment())
-
     def stage(self, sandbox):
+        # Setup environment
+        env = self.get_environment()
+
+        # Add "CAS digest" environment variables
+        sorted_envs = sorted(self.__digest_environment)
+        for digest_variable in sorted_envs:
+            element_list = [element for element, _ in 
self.__digest_environment[digest_variable]]
+            subsandbox = sandbox.create_subsandbox()
+            self.stage_dependency_artifacts(subsandbox, element_list)
+            digest = subsandbox.get_virtual_directory()._get_digest()
+            env[digest_variable] = "{}/{}".format(digest.hash, 
digest.size_bytes)

Review Comment:
   I also noticed that these environment variables don't end up in the build 
logs.



-- 
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]

Reply via email to