go.library_to_source should only accept Targets for deps

[jayconrod]

go.library_to_source accepts an attr parameter, which is usually but not always ctx.attr. When retrieving the list of dependencies, we call get_archive on each element of attr["deps"], which is normally a list of Targets.

Callers that generate packages (go_test, nogo) call library_to_source with a struct instead of ctx.attr. These callers pass in a list of GoArchives instead of a list of Targets for attr["deps"]. This makes it hard to do anything with attr["deps"] not related to archives.

The documentation says attr is just a set of attributes. We should clarify what to put here for clients that are generating packages.

[hugelgupf]

I just happened upon this issue - another case of what if one rule generates more than one library/archive?

In particular, our rule generates several libraries that a binary then depends on. I saw this comment in the code base before https://github.com/bazelbuild/rules_go/blob/3b5f0c950e0deec63cd77cdcec7765b5f41fefbd/go/private/context.bzl#L199-L200
and it made me decide to have a macro that generates several rules, so that the binary can depend on the generated library targets. (Looks like this at the moment.)

Unfortunately, that means that users cannot specify target patterns in the cmds list and every cmd has to be listed individually (and with an absolute label, but that's my fault).

Because of this, I'd like it if library_to_source's attr["deps"] could officially accept a list of Targets and GoArchives (I didn't want to depend on that behavior today just to have to migrate away some day). Or do you have an alternative that would solve this issue for me?

[jayconrod]

@hugelgupf

Seems reasonable to change to drop that TODO and change the documentation to reflact that library_to_source takes Targets or GoArchives. I think I just wanted the API to have stricter types, but the reality is that GoArchives are needed in a couple cases, and that probably isn't going away soon.

In particular, our rule generates several libraries that a binary then depends on.

Just to double check, does the rule generate multiple packages that could be imported by the binary, or one importable package with others that need to be passed to the linker? If it's the first case, you would just need one GoArchive or Target. Sounds like the second case though.

Unfortunately, that means that users cannot specify target patterns in the cmds list and every cmd has to be listed individually (and with an absolute label, but that's my fault).

Hopefully you won't need this, but you can set tags = ["manual"] on these targets. That will prevent them from being matched by ... wildcards.

[hugelgupf]

Just to double check, does the rule generate multiple packages that could be imported by the binary, or one importable package with others that need to be passed to the linker?

It generates both multiple packages, and the binary depending on all of them. Hm. How can I put many different libraries into on GoArchive/Target? Could split the process into 2 steps (n packages rewritten in one rule + binary depending on all those packages), rather than n+1; that'd keep it a Target.

[jayconrod]

How can I put many different libraries into on GoArchive/Target?

You can't. I was just thinking that if you only had one importable package, no change would be needed.

I think this ought to be possible though. I'll add a note to #2578. If there's a substantial change to the API in the future, that ought to be included.

For the short term, documenting that in go.library_to_source, attr["deps"] could contain Targets or GoArchives seems like the best way to resolve this.

[hugelgupf]

Got it, SGTM. Thank you!