deps: apply necessary changes for envoy_api MODULE without MODULE.bazel file #41835
Conversation
repokitteh-read-only
bot
added
the
deps
|
CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to |
|
/cc @mering, @veblush as you published previous versions of envoy_api in the BCR |
mmorel-35
force-pushed
the
module/envoy_api
branch
3 times, most recently
from
c76664a to
bc2076e
Compare
| if mapped == None: | ||
| prefix = "@" + Label(dep).workspace_name if not dep.startswith("//") else "" | ||
| prefix = "@@" if _IS_BZLMOD else "@" | ||
| prefix = prefix + Label(dep).repo_name if not dep.startswith("//") else "" |
There was a problem hiding this comment.
What do you mean ?
I took exactly what was provided here
Is something missing there ?
|
@phlax , Can you retry the failing job ? |
|
sure - you can too /retest |
I'm not so sure or I was sending the message too fast in the process but lately I wasn't seeing the rockets on my comment asking for retest |
|
it triggers this workflow https://github.com/envoyproxy/envoy/actions/workflows/command.yml so you can check there if its not doing the expected |
|
I'm not sure I'm in the right group for that. |
| license = "Apache-2.0", | ||
| license_url = "https://github.com/google/cel-spec/blob/v{version}/LICENSE", | ||
| ), | ||
| envoy_toolshed = dict( |
There was a problem hiding this comment.
wondering if we should remove this from the main repository_locations.bzl - dependency relations between api <> envoy have always been a bit vague
There was a problem hiding this comment.
Can we keep in both places until it is clearly decided ?
Once envoy_toolshed is published in BCR, it will directly be used in MODULE.bazel and not in repository_locations.
I can also do that in the same time
There was a problem hiding this comment.
the downside of keeping two are that they need to be kept in sync and its confusing as to what is authoritative
There was a problem hiding this comment.
oh - i see re modules - yeah was wondering how shared deps work in that context
There was a problem hiding this comment.
Can it stay this way as first step toward bzlmod for envoy_api or shall I publish envoy_toolshed to be, then validate its working in data-plane-api PR then remove it here or do you have another strategy in mind ?
There was a problem hiding this comment.
i hadnt thought that far ahead - im happy to go with whatever strategy you think is best - if that means duplicating this dep for now - fine
|
@mmorel-35 I also made some fixes for Bazel 9 in envoyproxy/data-plane-api#633, can you please integrate those changes as well? |
…el file Signed-off-by: Matthieu MOREL <[email protected]>
mmorel-35
force-pushed
the
module/envoy_api
branch
from
bc2076e to
abfa65d
Compare
|
I believe that this can be merged if it passes tests. That will allow to validate a first version of MODULE.bazel in data-plane-api. I want to publish a new version of xds in BCR but I want to first integrate #41746 Then validate cncf/xds#127 within envoy and publish XDS and envoy_toolshed to BCR to have them as dependencies in MODULE.bazel directly |
|
@mmorel-35 Thanks, looks good. I removed |
repokitteh-read-only
bot
removed
the
deps
5 participants
Description
This PR apply necessary changes for envoy_api MODULE without MODULE.bazel file.
cf. envoyproxy/data-plane-api#632
Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]