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

refactor: add a version label function for consistent labels #1328

Merged
merged 2 commits into from
Jul 23, 2023

Conversation

aignas
Copy link
Collaborator

@aignas aignas commented Jul 19, 2023

Before this PR there would be at least a few places where we would be
converting a X.Y.Z version string to a shortened X_Y or XY string segment
to be used in repository rule labels. This PR adds a small utility function
that helps making things consistent.

Work towards #1262, split from #1294.

@aignas aignas requested a review from rickeylev as a code owner July 19, 2023 08:55
@aignas aignas force-pushed the refactor/version-label-func branch from 2db1440 to c3d7224 Compare July 22, 2023 01:41
Copy link
Collaborator

@chrislovecnm chrislovecnm left a comment

Choose a reason for hiding this comment

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

LGTM - are we doing this anywhere else? Looking to see if we can replace more code with the function call.

@chrislovecnm chrislovecnm added this pull request to the merge queue Jul 23, 2023
Merged via the queue into bazelbuild:main with commit bb7004b Jul 23, 2023
@aignas aignas deleted the refactor/version-label-func branch July 24, 2023 02:50
aignas added a commit to aignas/rules_python that referenced this pull request Jul 24, 2023
Before this PR the `bzlmod` example would only work if the Python
interpreter version is specified in the `X.Y` format. However, users may
desire to use the full version (`X.Y.Z`) for reproducibility reasons and
expect everything to continue to work.

This fixes the versions used everywhere by leveraging functionality
introduced in bazelbuild#1328 and fixes the `python_versions` to always contain
`@python_versions//X.Y:defs.bzl` imports. NOTE, bazelbuild#1328 also fixed the
case where the Python-version specific `pip` hub repos would be named
`<hub_name>_XYZ` as opposed to the `<hub_name>_XY`, which is expected
across the codebase.

Breaking for `bzlmod` users who previously used
`@python_versions//X.Y.Z:defs.bzl` in their BUILD.bazel files and/or
depended on the `@pip_XYZ//:requirements.bzl`.

This essentially limits the users of the `@python_versions` to a single
`X.Y` toolchain version, whilst the full functionality should still be
available by using the `python_version` parameter in the `py_binary` and
`py_library` rules.

Fixes bazelbuild#1339.
aignas added a commit to aignas/rules_python that referenced this pull request Jul 29, 2023
Before this PR the `bzlmod` example would only work if the Python
interpreter version is specified in the `X.Y` format. However, users may
desire to use the full version (`X.Y.Z`) for reproducibility reasons and
expect everything to continue to work.

This fixes the versions used everywhere by leveraging functionality
introduced in bazelbuild#1328 and fixes the `python_versions` to always contain
`@python_versions//X.Y:defs.bzl` imports. NOTE, bazelbuild#1328 also fixed the
case where the Python-version specific `pip` hub repos would be named
`<hub_name>_XYZ` as opposed to the `<hub_name>_XY`, which is expected
across the codebase.

Breaking for `bzlmod` users who previously used
`@python_versions//X.Y.Z:defs.bzl` in their BUILD.bazel files and/or
depended on the `@pip_XYZ//:requirements.bzl`.

This essentially limits the users of the `@python_versions` to a single
`X.Y` toolchain version, whilst the full functionality should still be
available by using the `python_version` parameter in the `py_binary` and
`py_library` rules.

Fixes bazelbuild#1339.
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.

2 participants