Skip to content

Conversation

@fmeum
Copy link
Collaborator

@fmeum fmeum commented Sep 26, 2023

By merging the special _validation output groups across a rule and its attached aspects, aspects and rules can simultaneously use validation actions.

Fixes #19624

@fmeum fmeum requested a review from a team as a code owner September 26, 2023 07:43
@fmeum fmeum requested review from a team, ahumesky and katre and removed request for a team and katre September 26, 2023 07:43
@github-actions github-actions bot added awaiting-review PR is awaiting review from an assigned reviewer team-Configurability platforms, toolchains, cquery, select(), config transitions labels Sep 26, 2023
By merging the special `_validation` output groups across a rule and its
attached aspects, aspects and rules can simultaneously use validation
actions.
@fmeum fmeum force-pushed the 19624-merge-validation-outputs branch from e7d115e to caef2f8 Compare September 26, 2023 10:01
@ismell
Copy link

ismell commented Sep 26, 2023

Awesome, thanks for jumping on this so quickly!

@meisterT
Copy link
Member

cc @comius

@ismell
Copy link

ismell commented Oct 4, 2023

cc @lberki

@lberki
Copy link
Contributor

lberki commented Oct 5, 2023

Since both @ahumesky and @comius were tagged into this code review and they didn't respond, I hereby declare it to be Good Enough To Be Merged.

@lberki lberki 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 5, 2023
@copybara-service copybara-service bot closed this in cd72583 Oct 5, 2023
@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Oct 5, 2023
@fmeum fmeum deleted the 19624-merge-validation-outputs branch October 5, 2023 21:55
@fmeum
Copy link
Collaborator Author

fmeum commented Oct 5, 2023

@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 Oct 5, 2023
@iancha1992
Copy link
Member

@bazel-io fork 6.4.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 Oct 5, 2023
@iancha1992
Copy link
Member

@fmeum @ahumesky @lberki @ismell There were some conflicts when cherry-picking this to release-6.4.0.

  1. src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java
for (OutputGroupInfo provider : providers) {
  for (String outputGroup : provider.outputGroups.keySet()) {
    if (!seenGroups.add(outputGroup)) {
      throw new DuplicateException(
          "Output group " + outputGroup + " provided twice");
    }

    resultBuilder.put(outputGroup, provider.getOutputGroup(outputGroup));
  }
}

is in release-6.4.0 branch. But expected below before the cherry-pick

for (OutputGroupInfo provider : providers) {
  for (String group : provider) {
    if (outputGroups.put(group, provider.getOutputGroup(group)) != null) {
      throw new DuplicateException("Output group " + group + " provided twice");
    }
  }
}

Also, return new OutputGroupInfo(resultBuilder.buildOrThrow()); is in release-6.4.0 branch currently, which was not expected before the cherry-pick.

cc: @bazelbuild/triage

fmeum added a commit to fmeum/bazel that referenced this pull request Oct 6, 2023
By merging the special `_validation` output groups across a rule and its attached aspects, aspects and rules can simultaneously use validation actions.

Fixes bazelbuild#19624

Closes bazelbuild#19630.

PiperOrigin-RevId: 571084964
Change-Id: I3135ce1f9e61124e96a3b7d8e0ea0c3a3cdf526d

Fixes bazelbuild#19742
@fmeum
Copy link
Collaborator Author

fmeum commented Oct 6, 2023

@iancha1992 I performed a manual cherry-pick in #19745.

iancha1992 pushed a commit that referenced this pull request Oct 6, 2023
By merging the special `_validation` output groups across a rule and its
attached aspects, aspects and rules can simultaneously use validation
actions.

Fixes #19624

Closes #19630.

PiperOrigin-RevId: 571084964
Change-Id: I3135ce1f9e61124e96a3b7d8e0ea0c3a3cdf526d

Fixes #19742
@ismell
Copy link

ismell commented Oct 9, 2023

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team-Configurability platforms, toolchains, cquery, select(), config transitions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Aspects: Merge validation output group

6 participants