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

go_library to support gotags #3203

Closed
edieYoung opened this issue Jun 15, 2022 · 7 comments
Closed

go_library to support gotags #3203

edieYoung opened this issue Jun 15, 2022 · 7 comments

Comments

@edieYoung
Copy link

edieYoung commented Jun 15, 2022

Hi there, we have a library which requires custom build tags to build it (because there is a TinyGo specific target that cannot be complied by Bazel for now #3197, and we want to exclude it using go build tags).
Can we pass in the gotags to it? Why does go_library not support gotags? We found this previous issue similarly #1351, but it seems only added support to go_binary and go_test, not go_library.

@sluongng
Copy link
Contributor

sluongng commented Feb 7, 2023

@fmeum A friend of mine ran into this problem recently:

Package @com_github_containers_image_v5//signature would depends on gpgme which require some special C header to compile. To avoid that, @com_github_containers_image_v5 allow you to set a tag containers_image_openpgp to switch to use the openpgp implementation.

We applied the build_tags flag on Gazelle's go_repository to exclude the unwanted source files, and then added gotags to appropriate go_binary and go_test. Resulting in bazel build //:my_binary //:my_test working correctly.

However, bazel build //... would failed. This was caused by the GoCompilePkg action created by go_library does not include the needed gotags.


Overall, this seems to be caused by the go_transition rule being applied only to go_test and go_binary and not go_library. Wdyt if we enable go_transition on go_library rule to better control the default package compilation action?

@fmeum
Copy link
Collaborator

fmeum commented Feb 7, 2023

@sluongng If a go_library can only be built through a transition, then wouldn't adding tags= ["manual"] help with that? If we allow individual go_librarys to transition, I fear it would be difficult to ensure the output is consistent with what you get if you build though a binary.

@sluongng
Copy link
Contributor

sluongng commented Feb 7, 2023

Adding "manual" is a great idea!

We also tinkered with "--define gotags=" but that would have set it up globally for entire repo.

@fmeum
Copy link
Collaborator

fmeum commented Feb 7, 2023

Actually, with all the transitions around, maybe go_library should be manual by default to prevent build failure or duplication? @linzhp Have you thought about this before?

@achew22
Copy link
Member

achew22 commented Feb 8, 2023

One of the nice things about having go_libraries not be manual is that they will perform type/syntax checking in most CI systems that bazel build //... as the last step, or I'm misunderstanding something possibly?

@fmeum
Copy link
Collaborator

fmeum commented Feb 8, 2023

Yes, but they would still typecheck if they were dependencies of something else.

Having thought about this some more, I think that the current semantics of //... aren't ideal in the presence of transitions anymore. It would make far more sense if //... ended up being equivalent to specifying all targets that aren't dependencies of other targets matched by the wildcard (i.e. the nodes in the directed dependency graph with no incoming edges). I will see whether I can start this discussion in a more general venue.

Since adding gotags to go_library would have potentially very surprising semantics, I would prefer to close this issue with the recommendation to use tags = ["manual"] as a workaround until Bazel as a whole does a better of dealing with intermediate targets that rely on transitions to build correctly.

If anyone has an issue with this where tags = ["manual"] doesn't help, please post here and I can reopen.

@fmeum fmeum closed this as completed Feb 8, 2023
@sluongng
Copy link
Contributor

sluongng commented Feb 8, 2023

# .bazelrc
build --define gotags=containers_image_openpgp

A quick hack for use cases where you could apply a global go tag to all go_library.
gotags accept comma-separated values like gotags=tag_a,tag_b.

"manual" bazel tag seems like a decent workaround for now. I can see cases where a package was primarily made for external consumption (imported by repositories outside of Bazel workspace) to have a go_library and does not go_binary depending on it. However, it should be relatively straight forward to create a workaround:

  • Adjust go build tag to give it a sensible default
  • Add a dummy test (and go_test) that depends on the library, then mark the library with "manual"

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

No branches or pull requests

4 participants