Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create output directories for remote execution #15366

Closed
wants to merge 1 commit into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Apr 28, 2022

By explicitly declaring output directories as inputs to a remote action,
this commit ensures that these directories are created by the remote
executor prior to the execution of the action. This brings the behavior
of remote execution regarding tree artifacts in line with that of local
or sandboxed execution.

Getting the tests to pass requires modifying a check in Bazel's own
remote worker implementation: Previously, the worker explicitly verified
that output directories don't exist after the inputs have been staged.
This behavior is not backed by the spec and has thus been modified: Now,
it is only checked that the output directories either don't exist or are
directories.

Fixes #6393

@fmeum fmeum force-pushed the create-tree-artifact-outputs branch 5 times, most recently from ae79855 to a006fc1 Compare May 1, 2022 13:17
@sgowroji sgowroji added team-Remote-Exec Issues and PRs for the Execution (Remote) team awaiting-user-response Awaiting a response from the author labels May 2, 2022
@fmeum fmeum force-pushed the create-tree-artifact-outputs branch 3 times, most recently from ecb600d to 6e4cb45 Compare May 2, 2022 09:06
@fmeum fmeum changed the title WIP: Create tree artifact outputs Create output directories for remote execution May 2, 2022
@fmeum fmeum marked this pull request as ready for review May 2, 2022 09:26
@fmeum fmeum requested a review from a team as a code owner May 2, 2022 09:26
@fmeum
Copy link
Collaborator Author

fmeum commented May 2, 2022

@coeuvre This PR is the follow-up to #15276 that deals with tree artifact outputs in remote execution. It is stacked on the commits from #15276, only the last commit is new.

Copy link
Member

@coeuvre coeuvre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Please rebase after #15276 is merged.

@fmeum fmeum force-pushed the create-tree-artifact-outputs branch 3 times, most recently from 5e6283a to 3696825 Compare May 2, 2022 19:44
By explicitly declaring output directories as inputs to a remote action,
this commit ensures that these directories are created by the remote
executor prior to the execution of the action. This brings the behavior
of remote execution regarding tree artifacts in line with that of local
or sandboxed execution.

Getting the tests to pass requires modifying a check in Bazel's own
remote worker implementation: Previously, the worker explicitly verified
that output directories don't exist after the inputs have been staged.
This behavior is not backed by the spec and has thus been modified: Now,
it is only checked that the output directories either don't exist or are
directories.
@fmeum fmeum force-pushed the create-tree-artifact-outputs branch from 3696825 to 9d7bc34 Compare May 4, 2022 14:53
@fmeum
Copy link
Collaborator Author

fmeum commented May 4, 2022

@coeuvre I rebased onto master.

@bazel-io bazel-io closed this in 4310aeb May 9, 2022
@fmeum fmeum deleted the create-tree-artifact-outputs branch May 9, 2022 13:25
@fmeum
Copy link
Collaborator Author

fmeum commented May 9, 2022

@bazel-io flag

@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label May 9, 2022
@ckolli5
Copy link

ckolli5 commented May 11, 2022

@bazel-io fork 5.2.0

@bazel-io bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label May 11, 2022
fmeum added a commit to fmeum/bazel that referenced this pull request Jul 6, 2022
By explicitly declaring output directories as inputs to a remote action,
this commit ensures that these directories are created by the remote
executor prior to the execution of the action. This brings the behavior
of remote execution regarding tree artifacts in line with that of local
or sandboxed execution.

Getting the tests to pass requires modifying a check in Bazel's own
remote worker implementation: Previously, the worker explicitly verified
that output directories don't exist after the inputs have been staged.
This behavior is not backed by the spec and has thus been modified: Now,
it is only checked that the output directories either don't exist or are
directories.

Fixes bazelbuild#6393

Closes bazelbuild#15366.

PiperOrigin-RevId: 447451303
ckolli5 added a commit that referenced this pull request Jul 7, 2022
By explicitly declaring output directories as inputs to a remote action,
this commit ensures that these directories are created by the remote
executor prior to the execution of the action. This brings the behavior
of remote execution regarding tree artifacts in line with that of local
or sandboxed execution.

Getting the tests to pass requires modifying a check in Bazel's own
remote worker implementation: Previously, the worker explicitly verified
that output directories don't exist after the inputs have been staged.
This behavior is not backed by the spec and has thus been modified: Now,
it is only checked that the output directories either don't exist or are
directories.

Fixes #6393

Closes #15366.

PiperOrigin-RevId: 447451303

Co-authored-by: Chenchu K <ckolli@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-user-response Awaiting a response from the author team-Remote-Exec Issues and PRs for the Execution (Remote) team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TreeArtifacts output directories aren't created with remote execution enabled
5 participants