-
Notifications
You must be signed in to change notification settings - Fork 162
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.
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
build: do not run pip install outside the bazel build framework #4647
Conversation
along with the dependdencies. This should also allow us to do away with pip_install as a pre-build step (and get rid of the associated shenanigans).
…The only reason to do this was "make mocks" which bypassed bazel. Bazel is bypassed no-more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @JordiSubira)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @JordiSubira)
scion.sh was still using togen.py, topodot.py and set_ipv6_addr.py directly; with the consequence that any significant deviation between the python environment in the build tree and that of the global environment could cause these tools to fail. Instead, we now produce py_binaries for these three tools, install them in bin/, and use them from there. To make them work when not invoked from bazel, we have to package them as self contained executables, which is achieved by adding the --build_python_zip command line flag. Suprisingly, this is the only method available. There isn't even an equivalent parameter to the py_binary rule. All other workarounds turn out to do crazy things (e.g. pkg_tar with the "include_run_files" option produces non-sense). The tools are requested as part of make all. Another case of calling python scripts outside bazel was router_benchmark/benchmark.py. For that one, I arranged so it can be invoked through bazel (like its twin brother test.py) instead of only from the command line.
For some very mysterious reasons, flake8 gets broken by build_python_zip. I have never been able to find how exactly flake8 is made as a py binary because its creations is mediated by 6 layers of bazel code generation. But it is indeed made during the build. If I turn build_python_zip ON for the "build" config only, then flake8 is still broken, but if I turn it ON everywhere except for the "test:lint" config, then flake8 works. That makes no sense to me, but I'll be content with that. Life is short and I am sick of spending mine trying to deconstruct bazel and python frivolous and broken magic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 7 of 7 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @JordiSubira)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @JordiSubira)
Deal with more bazel/python non-sense. py_binary cannot be made with an imported entry point. There has to be a redundant local wrapper.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 5 of 5 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @jiceatscion and @JordiSubira)
tools/supervisorctl.py
line 5 at r5 (raw file):
# This is exact replica of supervisord's entry point because I haven't # found an intelligible way of creating a py_binary target that doesn't
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @JordiSubira and @lukedirtwalker)
tools/supervisorctl.py
line 5 at r5 (raw file):
Previously, lukedirtwalker (Lukas Vogel) wrote…
Had seen the page but not recognized this section as relevant to my case. (I was also seriously running out of patience with the whole thing). I'll give that a try. See if I can make it work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 10 of 13 files reviewed, 1 unresolved discussion (waiting on @JordiSubira and @lukedirtwalker)
tools/supervisorctl.py
line 5 at r5 (raw file):
Previously, jiceatscion wrote…
Had seen the page but not recognized this section as relevant to my case. (I was also seriously running out of patience with the whole thing). I'll give that a try. See if I can make it work.
seems to work. so, done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 3 of 3 files at r6, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @JordiSubira)
I was sick and tired of stepping on that Lego every time I built on a new machine...
make mocks, was running mocks.py directly instead of calling bazel run. That used whatever ambient python and libraries to match instead of the python env specified by WORKSPACE, which meant that we had to include pip install in the tools/install_deps process, which often enough would fail or require no-end of messy work-arounds (e.g. create a python venv and remember to activate it before building - but just the mocks).