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

Support gc_goopts and gc_linkopts attributes #291

Merged
merged 1 commit into from
Mar 8, 2017

Conversation

jayconrod
Copy link
Contributor

go_library, cgo_library, go_binary, and go_test now support the
gc_goopts attribute. This is a list of flags to pass to the Go
compiler.

go_binary and go_test also support the gc_linkopts attribute. This is
a list of flags to be passed to the Go linker.

Note that _emit_go_compile_action and _emit_go_link_action have had
small interface changes and are now private. I didn't see any uses of
these functions on GitHub, and I'd like to avoid having people depend
on them in the future.

Fixes #217

@jayconrod
Copy link
Contributor Author

cc @alandonovan @mikedanese

@pmbethe09 pmbethe09 self-assigned this Mar 7, 2017
Copy link
Contributor

@pmbethe09 pmbethe09 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

go/def.bzl Outdated
@@ -206,7 +206,8 @@ def _is_external(p):
"""Checks if the string starts with ../"""
return p[0:3] == '../'

def emit_go_compile_action(ctx, sources, deps, out_lib, extra_objects=[]):
def _emit_go_compile_action(ctx, sources, deps, out_lib,
extra_objects=[], gc_goopts=[]):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think skylark is immune to the python bug where a default argument of [] is bad, but I still see common skylark practice to follow the convention of 'None' as the default value and then a
if not gc_goopts:
gc_goopts = []

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if Skylark is affected by this. I figured it's better to just make the arguments explicit though.

Will submit when CI is done again (Mac build is taking forever to run these days).

go_library, cgo_library, go_binary, and go_test now support the
gc_goopts attribute. This is a list of flags to pass to the Go
compiler.

go_binary and go_test also support the gc_linkopts attribute. This is
a list of flags to be passed to the Go linker.

Note that _emit_go_compile_action and _emit_go_link_action have had
small interface changes and are now private. I didn't see any uses of
these functions on GitHub, and I'd like to avoid having people depend
on them in the future.

Fixes bazel-contrib#217
@jayconrod jayconrod merged commit ce7fc8b into bazel-contrib:master Mar 8, 2017
@jayconrod jayconrod deleted the feature/gc-opts branch March 8, 2017 19:27
@spxtr spxtr mentioned this pull request Mar 15, 2017
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.

3 participants