From 6c493053281f2fd9adc19e01da591aa9920c4675 Mon Sep 17 00:00:00 2001 From: Steeve Morin Date: Fri, 12 Jul 2019 12:29:30 +0200 Subject: [PATCH 1/2] Ensure go_binary.embed libraries' importpath is main when compiling Go 1.13 is stricter on the package path passed to go tool compile -p. This means that go_library passed in go_binary.embed fails to declare a main.main function, as they now retain their original import path. Fix that by adding a is_main boolean to the GoLibrary provider, which will tell wether a library is supposed to be a main package; that is, compiled from within a go_binary. Fixes #2133 Signed-off-by: Steeve Morin --- go/private/actions/archive.bzl | 9 +++++---- go/private/context.bzl | 3 ++- go/private/providers.bzl | 2 ++ go/private/rules/binary.bzl | 2 +- go/providers.rst | 5 +++++ 5 files changed, 15 insertions(+), 6 deletions(-) diff --git a/go/private/actions/archive.bzl b/go/private/actions/archive.bzl index 3962b2c4de..895762644f 100644 --- a/go/private/actions/archive.bzl +++ b/go/private/actions/archive.bzl @@ -85,6 +85,7 @@ def emit_archive(go, source = None): cxxopts = [f for fs in source.cxxopts for f in fs.split(" ")] clinkopts = [f for fs in source.clinkopts for f in fs.split(" ")] + importpath, importmap = effective_importpath_pkgpath(source.library) if source.cgo and not go.mode.pure: cgo = cgo_configure( go, @@ -103,8 +104,8 @@ def emit_archive(go, source = None): go, sources = split.go + split.c + split.asm + split.cxx + split.objc + split.headers, cover = source.cover, - importpath = effective_importpath_pkgpath(source.library)[0], - importmap = source.library.importmap, + importpath = importpath, + importmap = importmap, archives = direct, out_lib = out_lib, out_export = out_export, @@ -127,8 +128,8 @@ def emit_archive(go, source = None): go, sources = split.go + split.c + split.asm + split.cxx + split.objc + split.headers, cover = source.cover, - importpath = effective_importpath_pkgpath(source.library)[0], - importmap = source.library.importmap, + importpath = importpath, + importmap = importmap, archives = direct, out_lib = out_lib, out_export = out_export, diff --git a/go/private/context.bzl b/go/private/context.bzl index c5f85e5991..ff0f9c8e51 100644 --- a/go/private/context.bzl +++ b/go/private/context.bzl @@ -145,7 +145,7 @@ def _tool_args(go): args.set_param_file_format("multiline") return args -def _new_library(go, name = None, importpath = None, resolver = None, importable = True, testfilter = None, **kwargs): +def _new_library(go, name = None, importpath = None, resolver = None, importable = True, testfilter = None, is_main = False, **kwargs): if not importpath: importpath = go.importpath importmap = go.importmap @@ -164,6 +164,7 @@ def _new_library(go, name = None, importpath = None, resolver = None, importable pathtype = pathtype, resolve = resolver, testfilter = testfilter, + is_main = is_main, **kwargs ) diff --git a/go/private/providers.bzl b/go/private/providers.bzl index 6f89f537a3..82108ae6b6 100644 --- a/go/private/providers.bzl +++ b/go/private/providers.bzl @@ -108,6 +108,8 @@ def effective_importpath_pkgpath(lib): A tuple of effective import path and effective package path. Both are "" for synthetic archives (e.g., generated testmain). """ + if lib.is_main: + return "main", "main" if lib.pathtype not in (EXPLICIT_PATH, EXPORT_PATH): return "", "" importpath = lib.importpath diff --git a/go/private/rules/binary.bzl b/go/private/rules/binary.bzl index fbee2c59c1..455fa9428a 100644 --- a/go/private/rules/binary.bzl +++ b/go/private/rules/binary.bzl @@ -71,7 +71,7 @@ def _go_binary_impl(ctx): """go_binary_impl emits actions for compiling and linking a go executable.""" go = go_context(ctx) - library = go.new_library(go, importable = False) + library = go.new_library(go, importable = False, is_main = True) source = go.library_to_source(go, ctx.attr, library, ctx.coverage_instrumented()) name = ctx.attr.basename if not name: diff --git a/go/providers.rst b/go/providers.rst index f62945e180..3ca44f9645 100644 --- a/go/providers.rst +++ b/go/providers.rst @@ -100,6 +100,11 @@ an input to the `library_to_source`_ helper method, which produces GoSource_. | A function called by `library_to_source`_ that can be used to resolve this | | library to a mode-specific GoSource_. | +--------------------------------+-----------------------------------------------------------------+ +| :param:`is_main` | :type:`bool` | ++--------------------------------+-----------------------------------------------------------------+ +| Defines wether the library is supposed to be used as `main` package, despite not having `main` | +| as its `importpath`. | ++--------------------------------+-----------------------------------------------------------------+ GoSource ~~~~~~~~ From 43b2136cee9d5611541997655e933bd7d75c5804 Mon Sep 17 00:00:00 2001 From: Jay Conrod Date: Sat, 13 Jul 2019 09:50:52 -0400 Subject: [PATCH 2/2] Several fixes * Revert change in effective_importpath_importmap. It looks like go_path needs these to stay exactly the same. Instead, adjust importmap in emit_archive. * Also set is_main in go_test and nogo, which create GoLibrary providers without calling new_library. * Minor doc rephrasing. --- go/private/actions/archive.bzl | 7 ++++--- go/private/providers.bzl | 2 -- go/private/rules/nogo.bzl | 1 + go/private/rules/test.bzl | 1 + go/providers.rst | 5 +++-- 5 files changed, 9 insertions(+), 7 deletions(-) diff --git a/go/private/actions/archive.bzl b/go/private/actions/archive.bzl index 895762644f..fcf9897473 100644 --- a/go/private/actions/archive.bzl +++ b/go/private/actions/archive.bzl @@ -85,7 +85,8 @@ def emit_archive(go, source = None): cxxopts = [f for fs in source.cxxopts for f in fs.split(" ")] clinkopts = [f for fs in source.clinkopts for f in fs.split(" ")] - importpath, importmap = effective_importpath_pkgpath(source.library) + importpath, _ = effective_importpath_pkgpath(source.library) + importmap = "main" if source.library.is_main else source.library.importmap if source.cgo and not go.mode.pure: cgo = cgo_configure( go, @@ -153,7 +154,7 @@ def emit_archive(go, source = None): go.compile( go, sources = split.go, - importpath = source.library.importmap, + importpath = importmap, archives = direct, out_lib = out_lib, out_export = out_export, @@ -170,7 +171,7 @@ def emit_archive(go, source = None): go.compile( go, sources = split.go + split.asm + split.headers, - importpath = source.library.importmap, + importpath = importmap, archives = direct, out_lib = partial_lib, out_export = out_export, diff --git a/go/private/providers.bzl b/go/private/providers.bzl index 82108ae6b6..6f89f537a3 100644 --- a/go/private/providers.bzl +++ b/go/private/providers.bzl @@ -108,8 +108,6 @@ def effective_importpath_pkgpath(lib): A tuple of effective import path and effective package path. Both are "" for synthetic archives (e.g., generated testmain). """ - if lib.is_main: - return "main", "main" if lib.pathtype not in (EXPLICIT_PATH, EXPORT_PATH): return "", "" importpath = lib.importpath diff --git a/go/private/rules/nogo.bzl b/go/private/rules/nogo.bzl index 4fa062cb3a..4dd15d5b31 100644 --- a/go/private/rules/nogo.bzl +++ b/go/private/rules/nogo.bzl @@ -66,6 +66,7 @@ def _nogo_impl(ctx): importmap = "nogomain", importpath_aliases = (), pathtype = EXPORT_PATH, + is_main = True, resolve = None, ) diff --git a/go/private/rules/test.bzl b/go/private/rules/test.bzl index baf9410d1a..0aeedf2edb 100644 --- a/go/private/rules/test.bzl +++ b/go/private/rules/test.bzl @@ -133,6 +133,7 @@ def _go_test_impl(ctx): importmap = "testmain", importpath_aliases = (), pathtype = INFERRED_PATH, + is_main = True, resolve = None, ) test_deps = external_archive.direct + [external_archive] diff --git a/go/providers.rst b/go/providers.rst index 3ca44f9645..b447c3278c 100644 --- a/go/providers.rst +++ b/go/providers.rst @@ -102,8 +102,9 @@ an input to the `library_to_source`_ helper method, which produces GoSource_. +--------------------------------+-----------------------------------------------------------------+ | :param:`is_main` | :type:`bool` | +--------------------------------+-----------------------------------------------------------------+ -| Defines wether the library is supposed to be used as `main` package, despite not having `main` | -| as its `importpath`. | +| Indicates whether the library should be compiled as a `main` package. | +| `main` packages may have arbitrary `importpath` and `importmap` values, | +| but the compiler and linker must see them as `main`. | +--------------------------------+-----------------------------------------------------------------+ GoSource