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

chore: publish a runfiles library as a wheel #995

Merged
merged 3 commits into from
Jan 21, 2023
Merged

chore: publish a runfiles library as a wheel #995

merged 3 commits into from
Jan 21, 2023

Conversation

alexeagle
Copy link
Collaborator

@alexeagle alexeagle commented Jan 19, 2023

Wire it up to GH actions so it is published for each release.

Tested locally with:

$ bazel build python/runfiles:wheel --embed_label=1.0.2 --stamp
$ PYTHONPATH=bazel-bin/python/runfiles/bazel_runfiles-_BUILD_EMBED_LABEL_-py3-none-any.whl python
>>> import runfiles
>>> runfiles.Create()

Note, I would have liked to call the package bazel-runfiles, but this isn't possible without either refactoring the paths in this repo, or doing some fancy starlark to copy files around to create a folder that we turn into the wheel. There is no project https://pypi.org/project/runfiles though there is a https://pypi.org/project/runfile

We could try harder to get the name we prefer.

@alexeagle
Copy link
Collaborator Author

Looks like our buildifier presubmit check is broken, it complains about MODULE.bazel which I didn't edit in this PR.

##### :bazel: buildifier: found 1 format issue in your WORKSPACE, BUILD and *.bzl files
--
  | Please download <a href="https://github.com/bazelbuild/buildtools/releases/tag/6.0.0">buildifier 6.0.0</a> and run the following command in your workspace:<br/><pre><code>buildifier MODULE.bazel</code></pre>

Wire it up to GH actions so it is published for each release.

Tested locally with:
bazel build python/runfiles:wheel --embed_label=1.0.2 --stamp
PYTHONPATH=bazel-bin/python/runfiles/bazel_runfiles-_BUILD_EMBED_LABEL_-py3-none-any.whl python
>>> import runfiles
>>> runfiles.Create()

Note, I would have liked to call the package bazel-runfiles, but this isn't possible without either refactoring the paths in this repo, or doing some fancy starlark to copy files around to create a folder that we turn into the wheel.
There is no project https://pypi.org/project/runfiles though there is a https://pypi.org/project/runfile

We could try harder to get the name we prefer.
@fmeum
Copy link
Contributor

fmeum commented Jan 19, 2023

It's broken because the new buildifier release formats MODULE.bazel files differently. You either need to pin to the old release or apply the fix.

@alexeagle
Copy link
Collaborator Author

Yeah, I rebased over a commit I added on main that updated that formatting, and yeah I knew about your buildifier format fix. It feels like a bug to me that our presubmit.yml uses "latest" of buildifier but not a fight I want to pick today :)

@alexeagle alexeagle requested a review from groodt January 20, 2023 01:32
@fmeum
Copy link
Contributor

fmeum commented Jan 20, 2023

The logic in CurrentRepository makes some assumptions about the runfiles path of the file that contains it. We probably have to add tests for it when loaded from the wheel.

@alexeagle
Copy link
Collaborator Author

Yeah I'll try adding the integration test before merging this.

Copy link
Contributor

@rickeylev rickeylev left a comment

Choose a reason for hiding this comment

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

overall LGTM.
+1 to having some sort of test; I'm ok with that being a separate change though, too

python/runfiles/README.md Outdated Show resolved Hide resolved
python/runfiles/README.md Outdated Show resolved Hide resolved
python/runfiles/runfiles.py Show resolved Hide resolved
alexeagle and others added 2 commits January 20, 2023 17:24
Co-authored-by: Richard Levasseur <richardlev@gmail.com>
@alexeagle
Copy link
Collaborator Author

@rickeylev looks like your personal email isn't properly connected with your GitHub identity in a way that Google's CLA bot understands - this is the result of applying your suggestions. You might need to add that email address in http://go/github ?

@rickeylev
Copy link
Contributor

CLA issue fixed

@alexeagle
Copy link
Collaborator Author

Cool, thanks!

@alexeagle alexeagle merged commit 1722988 into main Jan 21, 2023
@alexeagle alexeagle deleted the runfiles branch January 21, 2023 19:16
alexeagle added a commit that referenced this pull request Jan 27, 2023
our 0.17 release is stuck on this step.
Partially reverts #995
alexeagle added a commit that referenced this pull request Jan 27, 2023
our 0.17 release is stuck on this step.
Partially reverts #995
ianpegg-bc pushed a commit to ianpegg-bc/rules_python that referenced this pull request May 12, 2023
* chore: publish a runfiles library as a wheel

Wire it up to GH actions so it is published for each release.

Tested locally with:
bazel build python/runfiles:wheel --embed_label=1.0.2 --stamp
PYTHONPATH=bazel-bin/python/runfiles/bazel_runfiles-_BUILD_EMBED_LABEL_-py3-none-any.whl python
>>> import runfiles
>>> runfiles.Create()

Note, I would have liked to call the package bazel-runfiles, but this isn't possible without either refactoring the paths in this repo, or doing some fancy starlark to copy files around to create a folder that we turn into the wheel.
There is no project https://pypi.org/project/runfiles though there is a https://pypi.org/project/runfile

We could try harder to get the name we prefer.

* Apply suggestions from code review

Co-authored-by: Richard Levasseur <richardlev@gmail.com>

* more code review cleanup

Co-authored-by: Richard Levasseur <richardlev@gmail.com>
ianpegg-bc pushed a commit to ianpegg-bc/rules_python that referenced this pull request May 12, 2023
our 0.17 release is stuck on this step.
Partially reverts bazelbuild#995
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants