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

Ensure go_binary.embed libraries' importpath is main #2135

Merged
merged 2 commits into from Jul 13, 2019
Merged

Ensure go_binary.embed libraries' importpath is main #2135

merged 2 commits into from Jul 13, 2019

Conversation

steeve
Copy link
Contributor

@steeve steeve commented Jul 12, 2019

Go 1.13 is now 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, as they now retain their import path. Fix
this by forcing a importmain=main on go_binary.

Fixes #2133

@steeve
Copy link
Contributor Author

steeve commented Jul 12, 2019

While this fixes my test case, gazelle is failing to build still for the same reason. Digging.

@steeve
Copy link
Contributor Author

steeve commented Jul 12, 2019

Gotcha.
I'll try that, thanks!

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 <[email protected]>
@steeve
Copy link
Contributor Author

steeve commented Jul 13, 2019

Updated the PR to add the is_main flag, works well !

* 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.
@jayconrod jayconrod merged commit a386ca7 into bazel-contrib:master Jul 13, 2019
@jayconrod
Copy link
Contributor

Thanks for working on this. Hope you don't mind that I fixed a couple things and updated the PR directly. My earlier advice on updating effective_importpath_pkgpath was not correct. This is the kind of change that needs some tinkering. It's hard to say how it will turn out without actually writing it.

@steeve
Copy link
Contributor Author

steeve commented Jul 13, 2019

No worries! Thank you for amending the PR on a Saturday!

@steeve
Copy link
Contributor Author

steeve commented Jul 13, 2019

I had limited connectivity but I wanted to at least update the PR

@steeve steeve deleted the steeve/go1.13 branch July 13, 2019 22:35
jayconrod pushed a commit to jayconrod/bazel-gazelle that referenced this pull request Aug 21, 2019
Necessary to make the go1.13 linker happy.

Corresponding change in go_binary: bazel-contrib/rules_go#2135

Also: upgrade to rules_go 0.19.3
jayconrod pushed a commit to bazel-contrib/bazel-gazelle that referenced this pull request Aug 21, 2019
Necessary to make the go1.13 linker happy.

Corresponding change in go_binary: bazel-contrib/rules_go#2135

Also: upgrade to rules_go 0.19.3
jayconrod pushed a commit to jayconrod/bazel-gazelle that referenced this pull request Aug 23, 2019
…b#625)

Necessary to make the go1.13 linker happy.

Corresponding change in go_binary: bazel-contrib/rules_go#2135

Also: upgrade to rules_go 0.19.3
jayconrod pushed a commit to jayconrod/rules_go that referenced this pull request Aug 23, 2019
)

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 bazel-contrib#2133
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

go 1.13 compat
3 participants