Rules_go: Proposal: Gazelle build tag handling

[jayconrod]

Background

In #396, we stopped applying build constraints by default in Gazelle. The goal of this change was to produce platform-independent BUILD files. Unfortunately, this has caused several problems.

1. We now generate rules for files with // +build ignore or other tags indicating they will never be built. These rules frequently end up empty when build constraints are applied at compile-time. If there are files declaring multiple packages in a directory, Gazelle may report a MultiplePackageError even if go/build would not have reported that error because of ignored files. This affects golang.org/x/sys/unix/linux. See #399.
2. We are generating rules for files with // +build appengine. We don't support AppEngine yet. This pulls in AppEngine dependencies from external repositories, which probably aren't defined. This affects several godoc-related packages in golang.org/x/tools. In general, we should avoid adding dependencies on packages imported by files that are never built.
3. In #402, generated rules include dependencies and flags that aren't available on the target system. Specifically, Windows libraries are included in a Linux build. This proposal won't fix that issue, but I mention it for completeness.

There must be a compromise between not applying build constraints at all and applying build constraints that only match the platform Gazelle is run on.

Proposal

Gazelle should determine whether a file may be built, using tags it knows are maybe true. When Gazelle is invoked with -build_tags, it will interpret those tags as maybe true. Other tags will be interpreted as never true. By default, Gazelle will treat all OS, architecture, and Go version tags as maybe true.

Gazelle will parse and evaluate // +build comments in source files to determine whether they may be built. The logic for evaluating conditions with Maybe and Never is different than True and False:

• NOT Maybe = Maybe. NOT Never = Maybe.
• Maybe AND Maybe = Maybe. Never AND X = Never.
• Maybe OR X = Maybe. Never OR Never = Never.

This will let us evaluate conditions like // +build windows,!linux. If windows and linux were both True, this would evaluate to False, and we would not build a file with this condition. This approach is necessarily conservative since SAT is NP-complete: we may still include more files than necessary. For example, a contradictory condition like // +build ignore,!ignore would still be built.

With this proposal, rules generated by Bazel will include files that may be built from any platform. The normal build constraint logic will still be applied by the rules at compile-time to determine which files should actually be built.

We will still pull in dependencies and flags for files that may be filtered out at compile-time (the third problem above, #402). In this situation, users will need to set -build_tags for their target platform (this can be done through new_go_repository, too) or write BUILD files manually.

[jayconrod]

cc @ianthehat @pmbethe09 WDYT?

[swsnider]

I like this proposal, but I'm slightly confused by the exact way that the described maybe/never logic maps onto the build tags in the sources. With your example build tag // +build windows,!linux, and no -build_tags on the command line, you're saying it would evaluate as:

1. maybe AND (NOT maybe)
2. maybe AND maybe
3. maybe

Then, if I specified -build_tags=linux,darwin,amd64, let's say:

1. never AND (NOT maybe)
2. never AND maybe
3. never

This is exactly what I'd want, I really like this proposal. What other corner cases would be built than just simple contradictions? Is there anyway we could, say, scan through the golang.org/x/... packages and ensure we don't trip up there?

[jayconrod]

I'll make a list of the tags used in golang.org/x/... and some popular projects like Kubernetes to see if we can handle that.

I'm also looking into whether we could do something with config_setting and select. We might be able to handle some common cases there at least, and then we wouldn't have the problem described in #402.

[swsnider]

@jayconrod I was actually thinking about that too, maybe you could handled config_setting like you do for external repos -- use a well-defined naming of them (one per build tag?), and let the user actually write the config_setting defs? Maybe that'd be too much of a pain though.

[swsnider]

Hmm, or actually, maybe the user can tell gazelle via tags on config_settings, which build_tags they correspond to?

[jayconrod]

config_setting is pretty limited. I think the only thing we could do is check what --cpu or --host_cpu is in use (as appropriate). We can also look for explicit --defines. There isn't a clear mapping from cpu to GOOS and GOARCH, especially since users can define their own crosstools named whatever they want. But we could handle the default platforms at least.

[pmbethe09]

If the list of configs was well understood, we could define some commonly used config_setting in rules_go and then construct srcs as well as other flags using those configs and then require the use of --define for non-standard configs -- I need to think more on this.

[jayconrod]

Alternate proposal

We can solve the #402 problem by putting sources and dependencies with build constraints inside a select expression.

We'll define some config_settings in rules_go that map well-known --cpu names onto GOOS, GOARCH pairs.

• k8 → linux,amd64
• darwin → darwin,amd64
• x64_windows_msvc → windows,amd64
• x64_windows_msys → windows,amd64

Rules generated by Gazelle will have a main list of sources, followed by a select with a case for each of the above config_settings. For each source file in a directory, we'll check whether there are any build constraints (filename suffixes or // +build comments). If there are no constraints, we'll add the file to the main list of sources. If there are constraints, we'll call go/build.Context.MatchFile with each of the above tag lists. For each tag list that matches, we'll add the file to a select case for the corresponding config_setting. Files may match zero or more cases.

Example: suppose we have three files in a library:

• foo_common.go - no build tags
• foo_windows.go - filename suffix
• foo_posix.go - // +build linux darwin

Gazelle would generate this rule:

go_library(
    name = "go_default_library",
    srcs = ["foo_common.go"] +
        select({
            "@io_bazel_rules_go//k8_setting": ["foo_posix.go"],
            "@io_bazel_rules_go//darwin_setting": ["foo_posix.go"],
            "@io_bazel_rules_go//x64_windows_msvc_setting": ["foo_windows.go"],
            "@io_bazel_rules_go//x64_windows_msys_setting": ["foo_windows.go"],
            "//conditions:default": [],
        }),
)

Dependencies will be handled similarly to sources. If a source is added to the main srcs list, its dependencies will be added to the main deps list. If a source is added to a select case, its dependencies will be added to the corresponding select case in deps. If a dependency is added to the main deps list by a different source, it will not appear in any select case though.

If -build_tags is specified, Gazelle will skip this process and just apply the given tags to all source files. This is the current behavior.

Gazelle will continue handling # keep comments on sources. We'll need to handle # keep comments on sources inside select cases, too.

Generalization

We can define config_settings in rules_go for well-known cpu values, but we can't do this for custom tags or custom toolchains. Gazelle should allow additional config_setting labels and tag lists to be specified on the command line and via new_go_repository. This would allow developers to add support for their own toolchains and custom tags in the future.

This probably won't be part of any initial implementation, but it's a way we can handle more cases.

Comparison with above proposal

This handles all three problems mentioned in the earlier proposal. It's a less general approach, but it does a better job of handling common cases. In particular, this helps us avoid dependencies which are conditionally included, which is a big problem.

[pmbethe09]

This also could be used for cflags and such on cgo_library that may be conditional to the configuration

[swsnider]

I think the alternate proposal is probably the better of the two for now -- I liked the tag logic from the initial one, but it probably would get pretty confusing (and it would be annoying to have to always remember to the pass the right set to gazelle, without being able to specify them in like the top-level BUILD file or something for the vendored use case).

[twpayne]

I think the select approach shows a lot of promise. It also solves the problem that the same BUILD file can be used on a multiple platforms.

The platform on which the BUILD files are generated by Gazelle can be different from the platform on which the final build occurs. This happens, for example, if developers develop locally on their macOS machines, then check their code and BUILD files into version control, from where a continuous integration server running Linux builds the final binaries. In this case the BUILD files stored in version control have to work on both Linux and macOS.