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

[7.4.0] Fix NPE in execlog when runfiles middleman are not top-level #23888

Merged
merged 1 commit into from
Oct 7, 2024

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Oct 7, 2024

The cherry-pick of the compact execlog runfiles changes assumed that runfiles only appear in the top-level tool nested set, but this isn't true for genrules. Instead, as in Bazel 8, it can be assumed that all middleman are also contained in inputs, which makes it possible to remove the extra middleman parameter and pass all runfiles tree information down to transitive log calls.

Fixes #23884

The cherry-pick of the compact execlog runfiles changes assumed that runfiles only appear in the top-level tool nested set, but this isn't true for genrules. Instead, as in Bazel 8, it can be assumed that all middleman are also contained in inputs, which makes it possible to remove the extra middleman parameter and pass all runfiles tree information down to transitive log calls.
@fmeum fmeum requested a review from a team as a code owner October 7, 2024 09:05
@fmeum fmeum changed the title Fix NPE in execlog when runfiles middleman are not top-level [7.4.0] Fix NPE in execlog when runfiles middleman are not top-level Oct 7, 2024
@fmeum fmeum requested a review from meisterT October 7, 2024 09:05
@fmeum
Copy link
Collaborator Author

fmeum commented Oct 7, 2024

@meisterT It would be great to get this fix into 7.4.0rc1. Bazel@HEAD is not affected, but I will submit the new test case as a separate PR.

@github-actions github-actions bot added the awaiting-review PR is awaiting review from an assigned reviewer label Oct 7, 2024
@meisterT meisterT added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Oct 7, 2024
@iancha1992 iancha1992 added this pull request to the merge queue Oct 7, 2024
@iancha1992 iancha1992 removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Oct 7, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 7, 2024
@iancha1992 iancha1992 added this pull request to the merge queue Oct 7, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 7, 2024
@iancha1992 iancha1992 added this pull request to the merge queue Oct 7, 2024
Merged via the queue into bazelbuild:release-7.4.0 with commit a6ad182 Oct 7, 2024
51 checks passed
@fmeum fmeum deleted the 23884-crash-execlog branch October 8, 2024 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants