Skip to content

bazel: Add bzlmod support for API/#34355

Closed
keith wants to merge 6 commits intomainfrom
ks/bazel-add-bzlmod-support-for-api
Closed

bazel: Add bzlmod support for API/#34355
keith wants to merge 6 commits intomainfrom
ks/bazel-add-bzlmod-support-for-api

Conversation

@keith
Copy link
Member

@keith keith commented May 24, 2024

The enovy_api package must support bzlmod in order for other transitive
deps of envoy to support it as well.

Signed-off-by: Keith Smiley keithbsmiley@gmail.com

@repokitteh-read-only
Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #34355 was opened by keith.

see: more, trace.

@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label May 24, 2024
@repokitteh-read-only
Copy link

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @mattklein123

🐱

Caused by: #34355 was opened by keith.

see: more, trace.

@keith keith force-pushed the ks/bazel-add-bzlmod-support-for-api branch from e4e02eb to 10ff37c Compare May 24, 2024 20:27
@keith
Copy link
Member Author

keith commented May 24, 2024

this requires some upstream bazel changes before it will be green

@keith
Copy link
Member Author

keith commented May 24, 2024

cc @mmorel-35

@keith keith force-pushed the ks/bazel-add-bzlmod-support-for-api branch from 10ff37c to a1de1d7 Compare May 24, 2024 21:05
@keith keith force-pushed the ks/bazel-add-bzlmod-support-for-api branch from a4f9fa7 to 33fa329 Compare June 25, 2024 00:25
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Jul 25, 2024
@github-actions
Copy link

github-actions bot commented Aug 1, 2024

This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot closed this Aug 1, 2024
Comment on lines +21 to +24
switched_rules.use_languages(
cc = True,
go = True,
)
Copy link
Contributor

Choose a reason for hiding this comment

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

You might also need to add grpc = True.

@mering
Copy link
Contributor

mering commented Oct 25, 2024

We would like to have this.

@fredyw could you re-open and @keith do you plan to continue your work on this?

@keith keith reopened this Nov 26, 2024
The enovy_api package must support bzlmod in order for other transitive
deps of envoy to support it as well.

Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@keith keith force-pushed the ks/bazel-add-bzlmod-support-for-api branch from 33fa329 to f2d3c26 Compare November 26, 2024 01:48
Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@keith
Copy link
Member Author

keith commented Nov 26, 2024

@mering pushed some updates

Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@github-actions github-actions bot removed the stale stalebot believes this issue/PR has not been touched recently label Nov 26, 2024
@mering
Copy link
Contributor

mering commented Nov 26, 2024

@keith Thanks! Looks like you need to format your code https://github.com/envoyproxy/envoy/actions/runs/12022333130.

Apart from this, is there anything else missing?

@keith
Copy link
Member Author

keith commented Nov 28, 2024

i can't actually tell what failed there, but im attempting to run it locally to see. there might be more to fix once we get past the precheck CI

@keith
Copy link
Member Author

keith commented Dec 2, 2024

can't actually tell why that CI job is failing, running that command locally didn't result in any changes

@phlax
Copy link
Member

phlax commented Dec 2, 2024

this

2024/11/26 02:00:17 Downloading https://releases.bazel.build/6.5.0/release/bazel-6.5.0-linux-x86_64...

and this ...

2024/11/26 02:00:47 Downloading https://releases.bazel.build/7.4.1/release/bazel-7.4.1-linux-x86_64...

not sure why its using different bazel versions - but its a bazel fight

@keith
Copy link
Member Author

keith commented Dec 2, 2024

Looks like that is from this script:

cd api/ || exit 1
bazel "${BAZEL_STARTUP_OPTIONS[@]}" run "${BAZEL_BUILD_OPTIONS[@]}" @com_github_bufbuild_buf//:bin/buf lint

since it does a cd before running, and the version file isn't in there. I can symlink it there, but interestingly that part also isn't new

Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@phlax
Copy link
Member

phlax commented Dec 2, 2024

checking other ci (eg here https://github.com/envoyproxy/envoy/actions/runs/12128588097/job/33815284187#step:16:564) its using the same (already installed perhaps) bazel version

i guess with bzlmod, if you dont specify, bazelisk assumes 7+ for the better support and install different version

@keith
Copy link
Member Author

keith commented Dec 2, 2024

ah actually I think it's because I created api/WORKSPACE, so bazelisk no longer looks higher up the tree to find the version file (or .bazelrc), might mean this is broken in new ways

/bazel-*
/mobile/bazel-*
bazel-*
MODULE.bazel.lock
Copy link
Member

Choose a reason for hiding this comment

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

im still unconvinced about the wisdom of not checking in lock files

Copy link
Member Author

Choose a reason for hiding this comment

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

yea i think once we're on 7.4+ we can check them in, just have to add a CI job validating they're up to date. the format isn't very stable in 6.x tho

@mering
Copy link
Contributor

mering commented Dec 3, 2024

How is https://github.com/envoyproxy/data-plane-api created from this repo? Will the symlink be materialized?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is now included in main via #37659. Please sync.

"@com_google_absl//absl/strings:str_format",
"@com_google_protobuf//:protobuf",
"@com_google_protobuf//src/google/protobuf/compiler:code_generator",
"@com_google_protobuf//src/google/protobuf/compiler:retention",
Copy link
Contributor

Choose a reason for hiding this comment

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

A proper fix is being discussed in #37669.

bazel_dep(name = "googletest", version = "1.14.0.bcr.1", repo_name = "com_google_googletest")
bazel_dep(name = "grpc", version = "1.62.1", repo_name = "com_github_grpc_grpc")
bazel_dep(name = "opencensus-proto", version = "0.4.1.bcr.1", repo_name = "opencensus_proto")
bazel_dep(name = "opentelemetry-proto", version = "1.4.0", repo_name = "opentelemetry_proto")
Copy link
Contributor

Choose a reason for hiding this comment

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

Please update to include bazelbuild/bazel-central-registry#3415

Suggested change
bazel_dep(name = "opentelemetry-proto", version = "1.4.0", repo_name = "opentelemetry_proto")
bazel_dep(name = "opentelemetry-proto", version = "1.4.0.bcr.1", repo_name = "opentelemetry_proto")

The necessary modifications have been merged to main in #37662. Please sync past this.

meteorcloudy pushed a commit to bazelbuild/bazel-central-registry that referenced this pull request Dec 19, 2024
- Based on envoyproxy/envoy#34355
- Requires
#3420 and
#3423
- Upstreaming target name alignment in
#3415 and
envoyproxy/envoy#37662
- Issue for patched Protobuf:
envoyproxy/envoy#37669
Copy link
Contributor

Choose a reason for hiding this comment

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

You might want to introduce this in a separate PR to fix the CI issues unrelated to Bzlmod independently.

@mering
Copy link
Contributor

mering commented Dec 20, 2024

FYI: I uploaded a first version to BCR in bazelbuild/bazel-central-registry#3424.

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Jan 19, 2025
@github-actions
Copy link

This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot closed this Jan 26, 2025
@phlax phlax deleted the ks/bazel-add-bzlmod-support-for-api branch July 24, 2025 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deps Approval required for changes to Envoy's external dependencies stale stalebot believes this issue/PR has not been touched recently

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants