Skip to content

Build tools with a toolchain#908

Merged
ianthehat merged 1 commit intobazel-contrib:masterfrom
ianthehat:tool
Oct 13, 2017
Merged

Build tools with a toolchain#908
ianthehat merged 1 commit intobazel-contrib:masterfrom
ianthehat:tool

Conversation

@ianthehat
Copy link
Copy Markdown
Contributor

This merges the implementations of go_tool_binary and go_binary
We then substitute the compile and link actions for ones that work without the
intermediate tools in the boostrap toolchain.
This means the tools are now built in the normal way, rather than with go build

This is more work towards #814, as it will enable me to change the way we handle
the environment.

This merges the implementations of go_tool_binary and go_binary
We then substitute the compile and link actions for ones that work without the
intermediate tools in the boostrap toolchain.
This means the tools are now built in the normal way, rather than with `go
build`

This is more work towards bazel-contrib#814, as it will enable me to change the way we handle
the environment.

if sources == None: fail("sources is a required parameter")
if out_lib == None: fail("out_lib is a required parameter")
if golibs: fail("compile does not accept deps in bootstrap mode")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Check for non-default values in all unused parameters. Let's be careful not to silently ignore things that shouldn't be specified.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The only thing not used is mode, which we need to accept but ignore for it to work.

go_tool_binary = rule(
_go_binary_impl,
attrs = {
"data": attr.label_list(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

go_tool_binary has a lot of attributes that can never be used. Maybe it's fine since this is an internal rule, but until this is merged with go_binary, maybe it makes more sense to delete these, and use getattr in the implementation or have separate implementations?

No need to change this if it doesn't make sense in the context of the rest of your changes. It just seems a little odd. Should be fine when optional toolchains lands and this can be cleaned up.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I want to leave it identical to the attribute list on go_binary to make it easy to merge them one day.
I think that every parameter that should not be set will end up with an error at the compile or link action anyway.

@ianthehat ianthehat merged commit d19c1cb into bazel-contrib:master Oct 13, 2017
@ianthehat ianthehat deleted the tool branch October 13, 2017 18:50
yushan26 pushed a commit to yushan26/rules_go that referenced this pull request Jun 16, 2025
…g packages (bazel-contrib#2514) (bazel-contrib#2584)

This reverts commit fbf8bc1 (bazel-contrib#2514)

Also, update the CHANGELOG about the reverting.

Fixes bazel-contrib#908, which is about the `pip-compile` not using the right files
for performing the locking. It seems that the `pip` upgrade regressed
this error.
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