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.

Sign up for GitHub

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

Jump to bottom

Aspects: Merge validation output group #19624

Closed
ismell opened this issue Sep 25, 2023 · 1 comment
Closed

Aspects: Merge validation output group #19624

ismell opened this issue Sep 25, 2023 · 1 comment
Labels
team-Core Skyframe, bazel query, BEP, options parsing, bazelrc type: feature request untriaged

Comments

@ismell
Copy link

ismell commented Sep 25, 2023

Description of the feature request:

If I have a rule that returns the special _validation output group, it's not possible for an aspect also return the _validation output group. This results in a Output group _validation provided twice error.

According to the aspect documentation:

It is an error if a target and an aspect that is applied to it each provide a provider with the same type, with the exceptions of OutputGroupInfo (which is merged, so long as the rule and aspect specify different output groups)

It would be nice to extend this allow merging the _validation output group.

Which category does this issue belong to?

Core

What underlying problem are you trying to solve with this feature?

Allow multiple aspects to declare their own validations.

Which operating system are you running Bazel on?

Linux

What is the output of bazel info release?

6.2.0

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

https://groups.google.com/g/bazel-discuss/c/TCo0msh1My4 was used as an inspiration to add validations via aspects.

Any other information, logs, or outputs that you want to share?

No response
@iancha1992 iancha1992 added the team-Core Skyframe, bazel query, BEP, options parsing, bazelrc label Sep 25, 2023
@fmeum fmeum mentioned this issue Sep 26, 2023
Closed
@ismell ismell mentioned this issue Sep 26, 2023
Open
nya3jp pushed a commit to nya3jp/cros-bazel that referenced this issue Sep 27, 2023
bazel: Add a hash tracer aspect that prints out artifact hashes
885108a 
This will help us debug cache busts by just looking at the CQ build
logs. This works by attaching the hash_tracer aspect to the top level
build target specified on the command line, and then applying the aspect
to each of its dependencies. The hash_tracer_validator is also
attached to the top level build target. This one will convert the
"hash" files created by the hash_tracer into an output validation group.
This causes the files to be built. The hash_tracer_validator wouldn't
be necessary if bazel actually ran the validation actions added by
aspects for transitive dependencies.
See bazelbuild/bazel#19636

I also had to move the ebuild validator into the hash_tracer due
to bazelbuild/bazel#19624.

e.g.,
* bazel-out/k8-fastbuild/bin/bazel/portage/sdk/extra_tarball.tar.gz -> e30ee3749e0d668a8e1e0186e43b009c2bb420218f13825b962f8fc785a52780
* bazel-out/k8-fastbuild/bin/bazel/portage/sdk/sdk_from_archive -> c9633a4aa68b56da04e31b7747c5811d3da84ac3d57a918cd43e9c53fa4fe323
* bazel-out/k8-fastbuild/bin/external/_main~portage~portage/internal/overlays/amd64-host/layer.tar.zst -> 1ee17dbb05d68c7cb94ff791fc935bea2fced6f9f43e1f7a08d1b5054b2472b7
* bazel-out/k8-fastbuild/bin/external/_main~portage~portage/internal/overlays/chromiumos/eclass/cros-credentials.tar.zst -> b460cb890cb952898ccb52f2b9edaeddae2afab3869500

BUG=b:302001628
TEST=BOARD=amd64-generic bazel build @portage//internal/sdk/stage2

Change-Id: I25583eb36b05cb0bc90d48d87924a7ea357c4be0
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/bazel/+/4894807
Commit-Queue: Raul Rangel <rrangel@chromium.org>
Tested-by: Raul Rangel <rrangel@chromium.org>
Reviewed-by: Tim Bain <tbain@google.com>
Auto-Submit: Raul Rangel <rrangel@chromium.org>
fmeum added a commit to fmeum/bazel that referenced this issue Oct 6, 2023
@fmeum
Merge rule and aspect validation output groups
e3c27b6 
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 fmeum mentioned this issue Oct 6, 2023
Merged
iancha1992 pushed a commit that referenced this issue Oct 6, 2023
@fmeum
Merge rule and aspect validation output groups (#19745)
ddc3e52 
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
@iancha1992
Copy link
Member

A fix for this issue has been included in Bazel 6.4.0 RC2. Please test out the release candidate and report any issues as soon as possible. Thanks!

nya3jp pushed a commit to nya3jp/cros-bazel that referenced this issue Oct 19, 2023
portage: Move ebuild validator back to ebuild rule
2b029c4 
bazelbuild/bazel#19624 has been fixed, so we
no longer need the hash_tracer to apply the ebuild validation action.
This also means we can disable it by default. If someone wants to enable
it, they can add `--config=hash_tracer` to their bazel build command
line.

BUG=b:304895109
TEST=BOARD=amd64-generic bazel build @portage//internal/packages/stage1/target/board/chromiumos/sys-libs/glibc:2.35-r25
TEST=BOARD=amd64-generic bazel build --config=hash_tracer @portage//internal/packages/stage1/target/board/chromiumos/sys-libs/glibc:2.35-r25

Change-Id: I335abf89aabdec83dc0279af42e718c0d9cd717f
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/bazel/+/4950206
Commit-Queue: Raul Rangel <rrangel@chromium.org>
Tested-by: Raul Rangel <rrangel@chromium.org>
Reviewed-by: Shuhei Takahashi <nya@chromium.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
team-Core Skyframe, bazel query, BEP, options parsing, bazelrc type: feature request untriaged
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Merge rule and aspect validation output groups fmeum/bazel
4 participants
@ismell @sgowroji @Pavank1992 @iancha1992