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

py_wheel support in bzlmod #1369

Closed
rickeylev opened this issue Aug 7, 2023 · 4 comments · Fixed by #1572
Closed

py_wheel support in bzlmod #1369

rickeylev opened this issue Aug 7, 2023 · 4 comments · Fixed by #1572
Labels
type: bzlmod bzlmod work
Milestone

Comments

@rickeylev
Copy link
Contributor

We don't currently have any tests verifying py_wheel works under bzlmod. I don't think any of us have tried it out ourselves, either.

It should just work. I don't recall it doing anything particularly special, but you never know.

So, at the least, manually verify it works and post the result here. Ideally, add some tests to verify it actually works.

@rickeylev rickeylev added the type: bzlmod bzlmod work label Aug 7, 2023
@rickeylev rickeylev added this to the Bzlmod GA milestone Aug 7, 2023
@aignas
Copy link
Collaborator

aignas commented Aug 10, 2023

FYI we have it working at $dayjob with:

pip.parse(
    hub_name = "publish_deps",
    python_version = "3.10",
    requirements_darwin = "@rules_python//tools/publish:requirements_darwin.txt",
    requirements_lock = "@rules_python//tools/publish:requirements.txt",
    requirements_windows = "@rules_python//tools/publish:requirements_windows.txt",
)
use_repo(pip, "publish_deps_310_twine", #extras )

It would help if this entry was in @rules_python//:MODULE.bazel and it was used by default in bzlmod.

@chrislovecnm
Copy link
Collaborator

@aignas how can we add this as a test?

@rickeylev
Copy link
Contributor Author

That users have to manually define twine is annoying, but is consistent with workspace. From https://rules-python.readthedocs.io/en/latest/api/packaging.html#py-wheel

To publish the wheel to Pypi, the twine package is required. rules_python doesn’t provide twine itself, see #1016 However you can install it with pip_parse, just like we do in the WORKSPACE file in rules_python.

So it's OK to still require users add something to MODULE.bazel to make twine work. I'm just talking about bzlmod feature parity; a separate FR so that users don't have to do this themselves makes sense.

That said:

use_repo(pip, "publish_deps_310_twine")

I'm not a fan of that, since it's (1) version specific, and (2) I think the "310" part is an internal detail leaking out. The workspace equiv is @publish_deps_twince//:pkg (which would be @publish_deps//twine now that generate_aliases is enabled by default.

Is that old code perhaps? I would think @publish_deps//twine should work under bzlmod?

aignas added a commit to aignas/rules_python that referenced this issue Nov 20, 2023
aignas added a commit to aignas/rules_python that referenced this issue Nov 20, 2023
@aignas
Copy link
Collaborator

aignas commented Nov 20, 2023

The code that accepts the twine label expects to find a Python file in the same third-party repository, which is not present there when bzlmod like hub aliases are used. I created a draft PR that works around that.

aignas added a commit to aignas/rules_python that referenced this issue Mar 25, 2024
github-merge-queue bot pushed a commit that referenced this issue Mar 27, 2024
Implements a test that starts a [`pypiserver`] and checks
that the publishing with the new machinery still works.

Fixes #1369

[pypiserver]: https://github.com/pypiserver/pypiserver
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bzlmod bzlmod work
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants