-
Notifications
You must be signed in to change notification settings - Fork 84
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
Fix bazel 0.24 incompatibilities #111
Fix bazel 0.24 incompatibilities #111
Conversation
@@ -16,34 +16,34 @@ load("@io_bazel_rules_go//go:def.bzl", "GoLibrary", "GoPath", "go_context", "go_ | |||
|
|||
def _compute_genrule_variables(resolved_srcs, resolved_outs, dep_import_paths): | |||
variables = { | |||
"SRCS": cmd_helper.join_paths(" ", resolved_srcs), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cmd_helper
is deprecated
dep_import_paths, | ||
), | ||
tools = ctx.attr.tools, | ||
label_dict = label_dict, | ||
) | ||
|
||
paths = ["/bin", "/usr/bin"] | ||
ctx.action( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ctx.action
is deprecated
if len(resolved_outs) == 1: | ||
variables["@"] = list(resolved_outs)[0].path | ||
variables["@"] = resolved_outs[0].path | ||
return variables | ||
|
||
def _go_genrule_impl(ctx): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested this on k/k; most notably, the zz_generated.openapi.go
file remains unchanged.
strip_files = ":empty", | ||
supports_param_files = 1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the default
@@ -22,14 +22,10 @@ exports_files(["CROSSTOOL"]) | |||
name = "cc-gcc-" + cpu, | |||
all_files = ":empty", | |||
compiler_files = ":empty", | |||
cpu = cpu, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deprecated (I think it went optional in bazel 0.23)
/hold I like @fejta's fixes to build_tar better than mine. |
dwp_files = ":empty", | ||
dynamic_runtime_libs = [":empty"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the _runtime_libs
attributes changed names, but they're also optional now, so I just dropped them.
label_dict = {} | ||
go_paths = [] | ||
|
||
for dep in ctx.attr.srcs: | ||
all_srcs += dep.files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this one is due to --incompatible_depset_union
@@ -70,21 +75,23 @@ def _go_genrule_impl(ctx): | |||
attribute = "cmd", | |||
expand_locations = True, | |||
make_variables = _compute_genrule_variables( | |||
all_srcs, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--incompatible_depset_is_not_iterable
outputs = ctx.outputs.outs, | ||
env = ctx.configuration.default_shell_env + go.env + { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--incompatible_disallow_dict_plus
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Separate these into 3 PRs please
LGTM otherwise |
I put these all into one PR because otherwise CI wouldn't pass. |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fejta, ixdy The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Not technically true but close enough |
/hold cancel |
why did you |
Because things don't build, and we need to make forward progress |
This unblocks merging the other PRs |
I was going to rebase to pull in your #113, but whatever. |
I'm way more interested in finally getting off travis and migrating to go modules than merging my PR, when we already have one that works and has a chance of merging. |
Several
--incompatible
flags were flipped in bazel 0.24, causing a number of things to break as noted in #108.I made some additional changes to
go_genrule
and thecc_toolchain
target anticipating breaking changes coming in bazel 0.25+. At least a few of these preemptive changes require bazel 0.23+. (Thankfully, I think nothing was broken before bazel 0.24, so we can safely update CI to bazel 0.23 first.)Fixes #108.
x-ref #100.
/assign @fejta @mikedanese