Skip to content
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

[Bootstrap Template] Fixed bootstrap template not being passed to py_binary from py_runtime on bazel latest #18103

Conversation

ankit-agarwal-ai
Copy link
Contributor

This PR fixes a small bug where the bootstrap template runtime file was not being passed to Py Binary. This broke the bootstrap template feature on bazel latest authored in #16806

@google-cla
Copy link

google-cla bot commented Apr 14, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@github-actions github-actions bot added the awaiting-review PR is awaiting review from an assigned reviewer label Apr 14, 2023
@ankit-agarwal-ai
Copy link
Contributor Author

cc @rickeylev should be a relatively simple fix.

@rickeylev rickeylev added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally team-Rules-Python Native rules for Python type: bug and removed awaiting-review PR is awaiting review from an assigned reviewer labels Apr 14, 2023
@rickeylev
Copy link
Contributor

Ah that must have gotten lost when rewriting the java to starlark and a following refactor. Yeah, simple fix.

@Pavank1992
Copy link
Contributor

@ankit-agarwal1999, as PR is awaiting to merge, could you please fix the above buildkite errors? Thanks!

@Pavank1992 Pavank1992 added awaiting-user-response Awaiting a response from the author and removed awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally labels Apr 17, 2023
@@ -290,11 +290,13 @@ def _expand_bootstrap_template(

if runtime:
shebang = runtime.stub_shebang
template = runtime.bootstrap_template
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like just one test is failing because bootstrap template is None, which makes sense -- the test is specifically testing a manually created PyRuntimeInfo. It's expected that PyRuntimeInfo.bootstrap_template is set; Normally this is handled by the py_runtime rule, but the test is specifically checking for a custom implementation.

This is around the line you'll need to modify:
https://cs.opensource.google/bazel/bazel/+/master:src/test/java/com/google/devtools/build/lib/rules/python/PythonStarlarkApiTest.java;l=120;drc=17827eee79808fceb6558d632c5b5594e8e9a47a

The net changes should be something like:

  • In the call to PyRuntimeInfo(), add bootstrap_template=ctx.file.bootstrap_template
  • A couple lines below, to the userruntime rule definition, add 'bootstrap_template': attr.label_list(allow_files=True)
  • A couple lines below, in the call to userruntime() add bootstrap_template='bootstrap.txt'

And bazel test //src/test/java/com/google/devtools/build/lib/rules/python:PythonStarlarkApiTest

@ankit-agarwal-ai
Copy link
Contributor Author

Thanks for the help @rickeylev I think I was able to get it to work. The test passes when I run it locally.

@rickeylev
Copy link
Contributor

@Pavank1992 tests are fixed

@rickeylev rickeylev added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-user-response Awaiting a response from the author labels Apr 17, 2023
@sgowroji sgowroji removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Apr 18, 2023
fweikert pushed a commit to fweikert/bazel that referenced this pull request May 25, 2023
…y_runtime on bazel latest

This PR fixes a small bug where the bootstrap template runtime file was not being passed to Py Binary. This broke the bootstrap template feature on bazel latest authored in bazelbuild#16806

Closes bazelbuild#18103.

PiperOrigin-RevId: 525042380
Change-Id: I8b8a200634eb98e156b4d8ba6b2e5baef5d06c73
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Rules-Python Native rules for Python type: bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants