bazel: Add @envoy_repo target with VERSION AND PATH

[phlax]

Signed-off-by: Ryan Northey ryan@synca.io

Commit Message: bazel: Add versions.bzl
Additional Description:

this allows the version to be used in bazel rules in ways that the normal VERSION file cannot

Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]
[Optional API Considerations:]

[phlax]

cc @htuch

[github-actions]

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!

[phlax]

@lizan @htuch this is a second attempt at resolving this, and, as suggested, uses a repository_rule

it also adds a PATH variable, which is only useful for local tasks, but means we can set up jobs like code_formatting without needing to pass the (absolute) path to the repo. Its also useful for generating data from bazel queries, as genquery is pretty constrained (no recursion, limited syntax)

[htuch]

LGTM, will defer to @lizan for review and merge.

[lizan]

What do you plan to use this for? I'm afraid this is an anti-pattern because wherever you pull this in rules/targets it invalidates cache across different builds.

[phlax]

for tooling such as code_format where it needs to run across the whole repo, or where it needs to make bazel queries which imply access to the repo (ie not genqueriable)

i get that using the path in bazel is a bit of an anti-pattern, not least because its only relevant to the local machine, but we have quite a few rules that rely on having access to the repo, and currently that has to be passed around (as an absolute url), so i think from a pattern pov this would be preferable to what we do now

im not sure if iiuc the point about invalidating caches, but i dont think running dep check or code formatting tools is going to have any negative lateral effects, and afaict using this with them everything is cached correctly

[phlax]

as an example - here is some WIP code to run a bazel query from other tools:

https://github.com/envoyproxy/envoy/pull/17992/files#diff-7a7d58af3edfdbe7825cd20fb6a29494ae293d40fe25e2b9774a4aac8ee27dcfR22-R30

the tools updated in the linked PR dont currently use bazel - ie they are currently run as scripts

if i shift them to bazel then, like with several others, we need to add a path to be able to run the bazel queries

the tools that need a path are a pita to run as you always have to type ${PWD} in addtion to any args, and it increases their code/complexity having to handle the path arg. This would eliminate that for these cases

[phlax]

perhaps i could add something to the comment to clarify any situations where there are some adverse effect from using the PATH other than for rbe

[lizan]

My point is this should not be exported in a .bzl file as Starlark, for the tooling we only need the py_library?

[phlax]

trying to understand further what you might mean here.

im wondering if you mean we dont need the ".bzl" part for the PATH, but we could keep the python part and only expose it to python targets

as im working mostly with python that would wfm

ill update with this change and then hopefully (if that was your meaning) we can get this landed today

[htuch]

Have you considered the trick we do in

envoy/tools/config_validation/validate_fragment.py

Line 71 in 68f674e

descriptor_path = r.Rlocation(

?

[phlax]

that gives the path within the sandbox, not the repo, iiuc

[phlax]

ah, i see, you can get the file path from the symlink that way

tbh, i think this is much more explicit - retrieving the repo metadata will be possible with the @envoy_repo target - which can provide version as well as path

see https://github.com/envoyproxy/envoy/pull/18021/files#diff-615ebb7b91831a9c7da2a3835e8b7c1ef89c9d2e72777cc229e128f6c8229214R51 and https://github.com/envoyproxy/envoy/pull/18021/files#diff-7a7d58af3edfdbe7825cd20fb6a29494ae293d40fe25e2b9774a4aac8ee27dcfR22 for an example

possibly the envoy_repo namespace can also provide data mappings as in #17998

[htuch]

I like the idea of @envoy_repo for things like version, I just wonder if we can pair it back to that and we don't need to do path this way.

[repokitteh-read-only]

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).

🐱

Caused by: #17293 was synchronize by phlax.

see: more, trace.

[htuch]

/lgtm deps

[htuch]

LGTM will leave for @lizan to approve/merge.

[phlax]

@lizan any chance of a gtm on this, it unblocks several other prs