Skip to content

Commit

Permalink
Merge GoLibrary and GoSource providers (#4030)
Browse files Browse the repository at this point in the history
**What type of PR is this?**
Code simplification

**What does this PR do? Why is it needed?**
These providers are 1:1, and we always create a library followed by
`source_from_library`. Merging them allows to remove the `library` field
from `GoSource` as well as the the `resolve` field, since we don't need
to smuggle the closure between the providers anymore.

If we do this, we will also need to fix up Gazelle. We can either have a
patch in this repo so CI passes and then followup with the Gazelle fix,
or somehow shim the API until we release new versions in lockstep. Open
to ideas how to best do this.

**Which issues(s) does this PR fix?**
One step toward #2578

Fixes #

**Other notes for review**
  • Loading branch information
dzbarsky authored Nov 18, 2024
1 parent 2dc6308 commit 564f820
Show file tree
Hide file tree
Showing 27 changed files with 395 additions and 368 deletions.
6 changes: 2 additions & 4 deletions docs/go/core/rules.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@
[Bourne shell tokenization]: https://docs.bazel.build/versions/master/be/common-definitions.html#sh-tokenization
[Gazelle]: https://github.com/bazelbuild/bazel-gazelle
[GoArchive]: /go/providers.rst#GoArchive
[GoLibrary]: /go/providers.rst#GoLibrary
[GoPath]: /go/providers.rst#GoPath
[GoSource]: /go/providers.rst#GoSource
[GoInfo]: /go/providers.rst#GoInfo
[build constraints]: https://golang.org/pkg/go/build/#hdr-Build_Constraints
[cc_library deps]: https://docs.bazel.build/versions/master/be/c-cpp.html#cc_library.deps
[cgo]: http://golang.org/cmd/cgo/
Expand Down Expand Up @@ -51,9 +50,8 @@ sufficient to match the capabilities of the normal go tools.
- [Bourne shell tokenization]
- [Gazelle]
- [GoArchive]
- [GoLibrary]
- [GoPath]
- [GoSource]
- [GoInfo]
- [build constraints]:
- [cc_library deps]
- [cgo]
Expand Down
32 changes: 12 additions & 20 deletions docs/go/core/rules.md

Large diffs are not rendered by default.

16 changes: 8 additions & 8 deletions extras/gomock.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
load("@bazel_skylib//lib:paths.bzl", "paths")
load("//go/private:common.bzl", "GO_TOOLCHAIN", "GO_TOOLCHAIN_LABEL")
load("//go/private:context.bzl", "go_context")
load("//go/private:providers.bzl", "GoLibrary")
load("//go/private:providers.bzl", "GoInfo")
load("//go/private/rules:wrappers.bzl", go_binary = "go_binary_macro")

_MOCKGEN_TOOL = Label("//extras/gomock:mockgen")
Expand All @@ -36,10 +36,10 @@ def _gomock_source_impl(ctx):

# In Source mode, it's not necessary to pass through a library, as the only thing we use it for is setting up
# the relative file locations. Forcing users to pass a library makes it difficult in the case where a mock should
# be included as part of that same library, as it results in a dependency loop (GoMock -> GoLibrary -> GoMock).
# be included as part of that same library, as it results in a dependency loop (GoMock -> GoInfo -> GoMock).
# Allowing users to pass an importpath directly bypasses this issue.
# See the test case in //tests/extras/gomock/source_with_importpath for an example.
importpath = ctx.attr.source_importpath if ctx.attr.source_importpath else ctx.attr.library[GoLibrary].importmap
importpath = ctx.attr.source_importpath if ctx.attr.source_importpath else ctx.attr.library[GoInfo].importmap

# create GOPATH and copy source into GOPATH
go_path_prefix = "gopath"
Expand Down Expand Up @@ -113,7 +113,7 @@ _gomock_source = rule(
attrs = {
"library": attr.label(
doc = "The target the Go library where this source file belongs",
providers = [GoLibrary],
providers = [GoInfo],
mandatory = False,
),
"source_importpath": attr.string(
Expand Down Expand Up @@ -257,7 +257,7 @@ def _gomock_reflect(name, library, out, mockgen_tool, **kwargs):

def _gomock_prog_gen_impl(ctx):
args = ["-prog_only"]
args.append(ctx.attr.library[GoLibrary].importpath)
args.append(ctx.attr.library[GoInfo].importpath)
args.append(",".join(ctx.attr.interfaces))

cmd = ctx.file.mockgen_tool
Expand All @@ -280,7 +280,7 @@ _gomock_prog_gen = rule(
attrs = {
"library": attr.label(
doc = "The target the Go library is at to look for the interfaces in. When this is set and source is not set, mockgen will use its reflect code to generate the mocks. If source is set, its dependencies will be included in the GOPATH that mockgen will be run in.",
providers = [GoLibrary],
providers = [GoInfo],
mandatory = True,
),
"out": attr.output(
Expand Down Expand Up @@ -313,7 +313,7 @@ def _gomock_prog_exec_impl(ctx):

# annoyingly, the interfaces join has to go after the importpath so we can't
# share those.
args.append(ctx.attr.library[GoLibrary].importpath)
args.append(ctx.attr.library[GoInfo].importpath)
args.append(",".join(ctx.attr.interfaces))

ctx.actions.run_shell(
Expand All @@ -337,7 +337,7 @@ _gomock_prog_exec = rule(
attrs = {
"library": attr.label(
doc = "The target the Go library is at to look for the interfaces in. When this is set and source is not set, mockgen will use its reflect code to generate the mocks. If source is set, its dependencies will be included in the GOPATH that mockgen will be run in.",
providers = [GoLibrary],
providers = [GoInfo],
mandatory = True,
),
"out": attr.output(
Expand Down
3 changes: 1 addition & 2 deletions go/core.rst
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@ Core Go rules
.. _Bourne shell tokenization: https://docs.bazel.build/versions/master/be/common-definitions.html#sh-tokenization
.. _Gazelle: https://github.com/bazelbuild/bazel-gazelle
.. _GoArchive: providers.rst#GoArchive
.. _GoLibrary: providers.rst#GoLibrary
.. _GoPath: providers.rst#GoPath
.. _GoSource: providers.rst#GoSource
.. _GoInfo: providers.rst#GoInfo
.. _build constraints: https://golang.org/pkg/go/build/#hdr-Build_Constraints
.. _cc_library deps: https://docs.bazel.build/versions/master/be/c-cpp.html#cc_library.deps
.. _cgo: http://golang.org/cmd/cgo/
Expand Down
20 changes: 14 additions & 6 deletions go/def.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ load(
load(
"//go/private:context.bzl",
_go_context = "go_context",
_new_go_info = "new_go_info",
)
load(
"//go/private:go_toolchain.bzl",
Expand All @@ -38,10 +39,9 @@ load(
"//go/private:providers.bzl",
_GoArchive = "GoArchive",
_GoArchiveData = "GoArchiveData",
_GoLibrary = "GoLibrary",
_GoInfo = "GoInfo",
_GoPath = "GoPath",
_GoSDK = "GoSDK",
_GoSource = "GoSource",
)
load(
"//go/private/rules:cross.bzl",
Expand Down Expand Up @@ -134,11 +134,19 @@ go_tool_library = _go_tool_library
go_toolchain = _go_toolchain
nogo = _nogo

# See go/providers.rst#GoLibrary for full documentation.
GoLibrary = _GoLibrary
# This provider is deprecated and will be removed in a future release.
# Use GoInfo instead.
GoLibrary = _GoInfo

# See go/providers.rst#GoSource for full documentation.
GoSource = _GoSource
# This provider is deprecated and will be removed in a future release.
# Use GoInfo instead.
GoSource = _GoInfo

# See go/providers.rst#GoInfo for full documentation.
GoInfo = _GoInfo

# See go/toolchains.rst#new_go_info for full documentation.
new_go_info = _new_go_info

# See go/providers.rst#GoPath for full documentation.
GoPath = _GoPath
Expand Down
3 changes: 1 addition & 2 deletions go/nogo.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@
.. _go_library: /docs/go/core/rules.md#go_library
.. _analysis: https://godoc.org/golang.org/x/tools/go/analysis
.. _Analyzer: https://godoc.org/golang.org/x/tools/go/analysis#Analyzer
.. _GoLibrary: providers.rst#GoLibrary
.. _GoSource: providers.rst#GoSource
.. _GoInfo: providers.rst#GoInfo
.. _GoArchive: providers.rst#GoArchive
.. _vet: https://golang.org/cmd/vet/
.. _golangci-lint: https://github.com/golangci/golangci-lint
Expand Down
32 changes: 15 additions & 17 deletions go/private/actions/archive.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def emit_archive(go, source = None, _recompile_suffix = "", recompile_internal_d
if source == None:
fail("source is a required parameter")

testfilter = getattr(source.library, "testfilter", None)
testfilter = getattr(source, "testfilter", None)
pre_ext = ""
if go.mode.linkmode == LINKMODE_C_ARCHIVE:
pre_ext = "_" # avoid collision with go_binary output file with .a extension
Expand All @@ -53,17 +53,17 @@ def emit_archive(go, source = None, _recompile_suffix = "", recompile_internal_d
pre_ext = ".external"
if _recompile_suffix:
pre_ext += _recompile_suffix
out_lib = go.declare_file(go, name = source.library.name, ext = pre_ext + ".a")
out_lib = go.declare_file(go, name = source.name, ext = pre_ext + ".a")

# store export information for compiling dependent packages separately
out_export = go.declare_file(go, name = source.library.name, ext = pre_ext + ".x")
out_export = go.declare_file(go, name = source.name, ext = pre_ext + ".x")
out_cgo_export_h = None # set if cgo used in c-shared or c-archive mode

nogo = get_nogo(go)
if nogo:
out_facts = go.declare_file(go, name = source.library.name, ext = pre_ext + ".facts")
out_nogo_log = go.declare_file(go, name = source.library.name, ext = pre_ext + ".nogo.log")
out_nogo_validation = go.declare_file(go, name = source.library.name, ext = pre_ext + ".nogo")
out_facts = go.declare_file(go, name = source.name, ext = pre_ext + ".facts")
out_nogo_log = go.declare_file(go, name = source.name, ext = pre_ext + ".nogo.log")
out_nogo_validation = go.declare_file(go, name = source.name, ext = pre_ext + ".nogo")
else:
out_facts = None
out_nogo_log = None
Expand All @@ -78,8 +78,8 @@ def emit_archive(go, source = None, _recompile_suffix = "", recompile_internal_d
fail("Archive mode does not match {} is {} expected {}".format(a.data.label, mode_string(a.source.mode), mode_string(go.mode)))
runfiles = source.runfiles.merge_all(files)

importmap = "main" if source.library.is_main else source.library.importmap
importpath, _ = effective_importpath_pkgpath(source.library)
importmap = "main" if source.is_main else source.importmap
importpath, _ = effective_importpath_pkgpath(source)

if source.cgo and not go.mode.pure:
# TODO(jayconrod): do we need to do full Bourne tokenization here?
Expand Down Expand Up @@ -157,15 +157,13 @@ def emit_archive(go, source = None, _recompile_suffix = "", recompile_internal_d
# may be mutable. For now, new copied fields are private (named with
# a leading underscore) since they may change in the future.

# GoLibrary fields
name = source.library.name,
label = source.library.label,
importpath = source.library.importpath,
importmap = source.library.importmap,
importpath_aliases = source.library.importpath_aliases,
pathtype = source.library.pathtype,

# GoSource fields
# GoInfo fields
name = source.name,
label = source.label,
importpath = source.importpath,
importmap = source.importmap,
importpath_aliases = source.importpath_aliases,
pathtype = source.pathtype,
srcs = tuple(source.srcs),
_cover = source.cover,
_embedsrcs = tuple(source.embedsrcs),
Expand Down
11 changes: 7 additions & 4 deletions go/private/actions/stdlib.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ load(
"GO_TOOLCHAIN_LABEL",
"SUPPORTS_PATH_MAPPING_REQUIREMENT",
)
load(
"//go/private:context.bzl",
"new_go_info",
)
load(
"//go/private:mode.bzl",
"LINKMODE_NORMAL",
Expand All @@ -38,12 +42,11 @@ def emit_stdlib(go):
Otherwise, the standard library will be compiled for the target.
Returns:
A list of providers containing GoLibrary, GoSource and GoStdLib.
A list of providers containing GoInfo and GoStdLib.
"""
library = go.new_library(go)
source = go.library_to_source(go, {}, library, False)
go_info = new_go_info(go, {}, coverage_instrumented = False)
stdlib = _sdk_stdlib(go) if _should_use_sdk_stdlib(go) else _build_stdlib(go)
return [source, library, stdlib]
return [go_info, stdlib]

def _should_use_sdk_stdlib(go):
version = parse_version(go.sdk.version)
Expand Down
Loading

0 comments on commit 564f820

Please sign in to comment.