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

Get module version from module_ctx and try to lookup integrities #103

Merged
merged 3 commits into from
Sep 27, 2024

Conversation

lalten
Copy link
Contributor

@lalten lalten commented Aug 30, 2024

Fixes #102

@jiawen
Copy link
Collaborator

jiawen commented Sep 12, 2024

I'll find some cycles to look at this in the morning.

@lalten
Copy link
Contributor Author

lalten commented Sep 23, 2024

Friendly bump

@@ -21,7 +20,8 @@ def _internal_configure_extension_impl(module_ctx):
name = "pybind11",
build_file = "//:pybind11-BUILD.bazel",
strip_prefix = "pybind11-%s" % version,
urls = ["https://github.com/pybind/pybind11/archive/v%s.zip" % version],
url = "https://github.com/pybind/pybind11/archive/refs/tags/v%s.tar.gz" % version,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this Github's preferred linking scheme? The releases page shows URLs like https://github.com/pybind/pybind11_bazel/releases/download/v2.12.0/pybind11_bazel-2.12.0.tar.gz

Is there any documentation for my own education?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bazel-contrib/SIG-rules-authors#11 (comment)

However your releases/download URL version is even better because those artifacts are not generated by GitHub on demand but actual files attached to the release

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, it looks like there aren't actually any artifacts of that form for pybind11, only the generated ones. So will keep as is.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The one you pointed at is this repo itself, which is doing a better job :)
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The "Source code (...)" ones are generated by GitHub, anything else is manually uploaded as part of the release.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry my day job kept me very busy for a few days. This looks good, I'm going to merge it.

internal_configure.bzl Show resolved Hide resolved
@lalten lalten requested a review from jiawen September 25, 2024 07:27

def _internal_configure_extension_impl(module_ctx):
version = _parse_my_own_version_from_module_dot_bazel(module_ctx)
(pybind11_bazel,) = [module for module in module_ctx.modules if module.name == "pybind11_bazel"]
version = pybind11_bazel.version
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice!

@@ -21,7 +20,8 @@ def _internal_configure_extension_impl(module_ctx):
name = "pybind11",
build_file = "//:pybind11-BUILD.bazel",
strip_prefix = "pybind11-%s" % version,
urls = ["https://github.com/pybind/pybind11/archive/v%s.zip" % version],
url = "https://github.com/pybind/pybind11/archive/refs/tags/v%s.tar.gz" % version,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry my day job kept me very busy for a few days. This looks good, I'm going to merge it.

@jiawen jiawen merged commit 2b6082a into pybind:master Sep 27, 2024
@lalten lalten deleted the integrities branch September 27, 2024 15:10
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.

Missing integrity attr on pybind11 http_archive
2 participants