tooling: Use envoy_entry_point
#19536
Conversation
phlax
force-pushed
the
tooling-entry-points
branch
2 times, most recently
from
b1411fd to
857f43f
Compare
phlax
changed the title
envoy_entry_pointenvoy_entry_point
phlax
force-pushed
the
tooling-entry-points
branch
5 times, most recently
from
3962070 to
1050fbf
Compare
Signed-off-by: Ryan Northey <ryan@synca.io>
phlax
force-pushed
the
tooling-entry-points
branch
from
1050fbf to
d6beb75
Compare
phlax
changed the title
envoy_entry_pointenvoy_entry_point
| def find_tool_path(): | ||
| entry_point_alias = ENTRY_POINT_ALIAS.split("/external/")[1] | ||
|
|
||
| for x in pathlib.Path(".").glob(f"**/{entry_point_alias}"): |
There was a problem hiding this comment.
This seems a bit of a wide net to cast. PR LGTM otherwise.
There was a problem hiding this comment.
the entry point alias that its matching is a pretty long string - if it matches we can be fairly sure we found the binary
i couldnt find any other way to do it - not sure why but the location im getting back from the alias wasnt matching exactly, even tho the tool is actually there (on a slightly different filepath)
There was a problem hiding this comment.
Can you provide some concrete path examples? I think it's probably OK as a workaround, but it just has a code smell to it, where we might end up with some hard to diagnose bug later due to this use of globbing.
There was a problem hiding this comment.
it just has a code smell to it
yep, agreed, i spent quite a few hours on this, and didnt find an alternative - so reluctantly did it this way - ill dump what is being mangled now and add as a comment...
There was a problem hiding this comment.
hmm, not sure about adding a comment as the lines are very long, ill post here for now and we can figure out
essentially there are 2 situations that it needs to work in - when its called directly with bazel run //path/to:entry_point and where are a build rule calls it - ie bazel build //rule/that/calls:entry_point
This is the before/after mangling for the first - ie bazel run
ENTRY_POINT_ALIAS: bazel-out/k8-opt-exec-2B5CBBC6/bin/external/base_pip3_envoy_docs_sphinx_runner/rules_python_wheel_entry_point_envoy.docs.sphinx_runner
FOUND PATH: external/base_pip3_envoy_docs_sphinx_runner/rules_python_wheel_entry_point_envoy.docs.sphinx_runner
And this is for the second:
ENTRY_POINT_ALIAS: bazel-out/host/bin/external/base_pip3_envoy_docs_sphinx_runner/rules_python_wheel_entry_point_envoy.docs.sphinx_runner
FOUND PATH: bazel-out/host/bin/tools/docs/sphinx_runner.runfiles/base_pip3_envoy_docs_sphinx_runner/rules_python_wheel_entry_point_envoy.docs.sphinx_runner
There was a problem hiding this comment.
ive cleaned the macro up a little, but whatever way i look at it the path that the genrule sees is different from the path that the py_binary sees
im pretty confident that this workaround works, not sure how else to achieve the desired outcome
There was a problem hiding this comment.
OK. The only other suggestions I would have here are to think of (1) passing the path in via command-line from Bazel which should know what it is or (2) using Bazel query. @lizan might also have some insight. In any case, it's probably not worth spending a ton of time optimizing, so maybe just document why this is the way it is and we can land.
There was a problem hiding this comment.
passing via the path wont work - its the reason for shifting to using a template - the arg gets lost when the rule is called from another rule, ill add a comment now...
phlax
force-pushed
the
tooling-entry-points
branch
from
56a76a6 to
8ec1c68
Compare
phlax
force-pushed
the
tooling-entry-points
branch
from
8ec1c68 to
e1ed46b
Compare
Signed-off-by: Ryan Northey <ryan@synca.io>
This is a follow up to use the macro added in envoyproxy#19450 for existing entry_points Also contains a fix to use a template to create the py_binary main to ensure the py_binary retains the path to the entry point Signed-off-by: Ryan Northey <ryan@synca.io> Signed-off-by: Josh Perry <josh.perry@mx.com>
Signed-off-by: Ryan Northey ryan@synca.io
Commit Message:
This is a follow up to use the macro added in #19450 for existing entry_points
Also contains a fix to use a template to create the py_binary main to ensure the py_binary retains the path to the entry point
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:]