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

Fix go_busybox's cmds field #38

Closed
hugelgupf opened this issue Sep 17, 2020 · 0 comments
Closed

Fix go_busybox's cmds field #38

hugelgupf opened this issue Sep 17, 2020 · 0 comments

Comments

@hugelgupf
Copy link
Member

the go_busybox rule cmds field currently requires each target to be listed individually (no aggregators like :... or :all) and requires each target to be an absolute label (no :foobar labels).

This is due to the fact that go_busybox is currently a macro that expands to many rules:

def go_busybox(name, cmds = [], **kwargs):

That's like that for 2 reasons, today:

  1. every rule can only generate one go.archive at the moment because they will all be named "%s.a" % ctx.attr.name. That is fixed with archive: use library's name for archive name bazelbuild/rules_go#2652, but not released in rules_go yet.

  2. go_busybox is a rule that generates many GoLibrary rules (one for each cmd), and then one GoBinary that depends on all those GoLibrarys (the busybox command). The deps field of the GoSource provider is technically required to be a list of Target (which each provide GoArchive). If we consolidated all those macro-generated rules into one rule, GoSource.deps would have to depend on a list of Target or GoArchive, because there would be no targets to depend on for the individual command libraries. In practice, GoSource.deps does accept list of Target or GoArchive right now, but rules_go wants to drop that ability. We'll need to convince them not to, or ask what it's about.

See actions.archive's resolution of deps calling get_archive (the "type == struct" thing is for when a GoArchive is directly supplied).

In particular, see this comment saying

TODO(#1784): we allow deps to be a list of GoArchive since go_test and nogo work this way. We should force deps to be a list of Targets.

Refers to #1784, which I suppose I'll comment on.

@hugelgupf hugelgupf closed this as not planned Won't fix, can't repro, duplicate, stale Feb 9, 2024
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

1 participant