Removing GOPATH in source mode #44
Conversation
gomock.bzl
Outdated
| doc = "A map from packages to auxilliary Go source files to load for those packages.", | ||
| doc = "A map auxilliary Go source files to their packages.", |
There was a problem hiding this comment.
Go source files should be referred to as labels, not strings (see the implementation of go_library as an example). Labels allow us to refer source files from different packages. However, there is no way to map a string to label in Bazel rules. We can only map a label to a string. I know this will break existing usage, but what other choice do we have?
gomock.bzl
Outdated
| for pkg, files in ctx.attr.aux_files.items(): | ||
| for f in files: | ||
| mapped_f = gopath + "/src/" + ctx.attr.library[GoLibrary].importmap + "/"+ f | ||
| aux_files.append("{0}={1}".format(pkg, mapped_f)) | ||
| for target, pkg in ctx.attr.aux_files.items(): | ||
| f = target.files.to_list()[0] | ||
| aux_files.append("{0}={1}".format(pkg, f.path)) | ||
| needed_files.append(f) |
There was a problem hiding this comment.
This looks like an API break. Why are we doing it?
There was a problem hiding this comment.
Refer to my other comment
There was a problem hiding this comment.
Also, confirmed with @josmad, who contributed aux_files attribute, that he is not using the attribute anywhere right now
There was a problem hiding this comment.
@jmhodges Can you make a major version bump for this API break?
|
@jmhodges I updated the performance comparison data. Please review. |
|
@jmhodges Can you take another look at this PR? |
|
@jmhodges ping... |
|
Superseded by bazel-contrib/rules_go#3121 |
After golang/mock#420, GoMock source mode no longer requires full transitive dependency closure in GOPATH. As a result, the gomock rule can avoid calling the
gopathrule, it only need to make sure the source file and auxiliary files in the GOPATH.When building
//gomock:mocksfrom linzhp/bazel_examples@b7f4b75, bazel_gomock (4f2ee84) spent 2.287 seconds on gomock rule:After applying this patch, it became 1.498 seconds:
The difference would be more significant for packages with large dependency tree