Skip to content

Conversation

@patrickmscott
Copy link
Contributor

@patrickmscott patrickmscott commented Mar 24, 2025

This fixes github.com/cloudflare/circl relative imports by making those headers available at compile time.

What type of PR is this?

Feature

What does this PR do? Why is it needed?

Expose headers in GoArchive so that compilepkg can make them visible. This will make it possible for relative imports to find header files.

Which issues(s) does this PR fix?

Fixes #4154

Other notes for review

@patrickmscott patrickmscott marked this pull request as ready for review March 25, 2025 16:33
@albertocavalcante
Copy link
Contributor

I don't want to deviate the PR discussion, just wondering, this change alone should fix circl issues or it would also require a change in gazelle?

@patrickmscott
Copy link
Contributor Author

I don't want to deviate the PR discussion, just wondering, this change alone should fix circl issues or it would also require a change in gazelle?

In my testing, this change alone fixed circl.

@fmeum
Copy link
Member

fmeum commented May 6, 2025

@patrickmscott Friendly ping, do you have capacity to address Jay's comments?

@patrickmscott
Copy link
Contributor Author

@patrickmscott Friendly ping, do you have capacity to address Jay's comments?

Yes, it just slipped off my radar. Let me see if I can address them.

This fixes github.com/cloudflare/circl relative imports by making those
headers available at compile time.
The headers from dependencies _may_ be used to produce an artifact so
they should be direct dependencies.
- Collect headers and transitive headers into a depset.
- Provide the headers depset to emit_compilepkg
- Add a compilation target that includes headers with transitive,
  relative include paths.
Copy link
Collaborator

@jayconrod jayconrod left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for your patience. @fmeum any last thoughts on this one?

Copy link
Member

@fmeum fmeum left a comment

Choose a reason for hiding this comment

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

No, looks good to me too!

@fmeum fmeum merged commit 393faea into bazel-contrib:master May 22, 2025
1 check passed
jvandew added a commit to foursquare/scala-gazelle that referenced this pull request Jun 2, 2025
While this repo was previously consumable as a go dependency (which is
in fact how our internal monorepo consumes it), it is also useful to
allow installation via Bzlmod or WORKSPACE for repos which otherwise
don't require a go environment. Support for these is added here,
although Bzlmod currently requires an `--override_module` flag as we
aren't published to the Bazel Central Registry.

The two main bits of complexity here are:

1. Making sure the requisite go module dependencies get installed.
Bzlmod handles this for us, but WORKSPACE requires that we set up
Gazelle to generate a macro users can call to install them.

2. Solving the patching situation with go-tree-sitter. There is an
[unreleased fix](bazel-contrib/rules_go#4298)
merged to rules_go which this PR uses to remove the need for patching in
this repo. However, we still require the patch being available for
downstream users who might not have the rules_go fix available. In the
WORKSPACE setup this is handled for them by the `scala_gazelle_deps`
macro, but Bzlmod users will have to add the `go_deps.module_override`
manually as it is only callable from their repo's root module.
@armfazh
Copy link

armfazh commented Jun 6, 2025

I've created a minimal reproducible example showing (now solved) CIRCL Bazel-Compilation.

github-merge-queue bot pushed a commit to bazel-contrib/rules_python that referenced this pull request Aug 17, 2025
)

Update rules_go to include
bazel-contrib/rules_go#4298
Update gazelle to align with the version the rules_go bzlmod will bring
in, and ensure the go.mod version is the same as bzlmod version.
Update go to 1.21 to include the `slices` library that some of the
go.mod updates depend on.

Fixes #2956.
github-merge-queue bot pushed a commit to bazel-contrib/rules_python that referenced this pull request Aug 17, 2025
)

Update rules_go to include
bazel-contrib/rules_go#4298
Update gazelle to align with the version the rules_go bzlmod will bring
in, and ensure the go.mod version is the same as bzlmod version.
Update go to 1.21 to include the `slices` library that some of the
go.mod updates depend on.

Fixes #2956.

---------

Co-authored-by: Douglas Thor <[email protected]>
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.

Binary that depends on cloudflare/circl can't be built cross-platform

6 participants