-
-
Notifications
You must be signed in to change notification settings - Fork 682
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
go_binary output paths contain a spurious os_arch component #1239
Comments
cc @ianthehat We added this to avoid cache poisoning when building the same targets in different modes. For example, if you ran a bunch of tests in race mode, then build a binary that depends on the same libraries as the tests, all the libraries have to be rebuilt. We work around this by including mode information in the output paths, which allows Bazel to cache files in multiple modes.
Is there another set of Skylark rules that work this way? The C / C++ / Objective C rules may have more flexibility than we do since they're part of Bazel itself. |
I'm not sure what you mean by "modes" in this context, but it seems to me like the mode should land in the build configuration, thus changing (say) |
The way bazel works, that part of the path is constant for any given build, and not configurable in any way. It's affected by the existing options only (so we could not add race to it for instance). We also support building the same binary in multiple modes within a single build right now. If you hard code the path in a sh_test it's going to be wrong on windows (or any other platform where the binary name does not match the rule name). The same is true for all the languages, it's never safe to make this assumption. For the most common languages (c and, java), on linux and mac, the assumption holds, but it's not a good idea to rely on it. This is documented in the FAQ |
I just hit this too -- it's pretty frustrating to deal with, considering that it seems to be unique among per-language Since the cache is per-target, would it be possible to leave the arch in the library file names, but have binary names return to archless? Then most of the build would be cached, and the only actions that need to re-run on a compilation mode change are the final link steps. |
Another +1 to this issue. I just upgraded a project to 0.9.0 (from 0.7.0) and all my support tools and tests started failing because they could no longer find the binary artifacts declared as data dependencies. I find this very confusing because no other rule that I know of behaves this way: building The suggestion above to use |
Most I chose the latter even though it forced me to tweak the scripts a bit, because the former wound up being a lot more code and required a rule instead of just a macro. Clearly either approach could have worked, though. The other problem I ran into with plumbing
But that fails for It seems fairly clear that the design of |
I agree that it's not nearly as easy to use $location and friends as it should be, and that bazel could do with some improvements here. If we can come up with a good proposal I am happy to push the Bazel team to accept it, but it would probably have to be something much more general dealing with runfiles in all targets. Maybe the data attribute could accept some kind of mapping directive and bazel would automatically set an environment variable of that name to the location of the file. |
What about @jmillikin-stripe's proposal? |
What about supporting (This doesn't resolve the issue of not being able to deterministically invoke built binaries outside of Bazel... which is very annoying and inconsistent because all other binary rules support this.... but oh well.) |
If you're using bazel in scripts, what about the Build Event Protocol? It lets you programmatically find the output files generated by your builds, and it's probably more robust than hard-coding paths relative to bazel-bin. |
I don't follow how that would help in this case. How is a script supposed to get information from the protocol without explicitly rebuilding all its tools inline? |
@creachadair Whoops, I don't have a good answer for that. I was trying to address "This doesn't resolve the issue of not being able to deterministically invoke built binaries outside of Bazel" (I'm assuming for CI/CD). |
@rogerhub Yes... there can be ways to figure out the name of a binary, but using the BEP to find the location of an artifact? This is like using a cannon to kill flies. If you have a build tool and you want to build something, there better be a simple way to access the build results. Not everybody is going to use I personally find this whole thing a bit ridiculous. Per the first reply in this bug, this breaking change was to "avoid cache poisoning when building the same targets in different modes." but... all Bazel rules are affected by this problem, aren't they? Therefore, it's not the Go rules' job to fix this: it's Bazel who should fix it. And unless Bazel fixes it, being different here is just adding a different source of pain that no other rules expose. (Note that the whole point of the The reason I'm calling this ridiculous is because working around this change is extremely hard and brittle. See bazelbuild/sandboxfs#13 for the changes I had to apply to a project... and they are far from nice. I'm at the point of questioning whether going the Bazel route was a good idea at all. |
I'm not really sure why this is such a big deal, in the cases where you know for sure you are never going to have a colliding binary, the genrule workaround is just
and then you have a dependable binary filename ready to go. In the mean time I am adding an example of using this approach in #1286. Not all rules are affected, because very few of the other languages have modes that can be controlled the way the go rules do. If you are specifying it from the command line, and the execution root is changing, then you don't have this problem. |
Please look at the PR I referenced in the previous comment. There is more going on than the In particular: how do you tell users to run a binary that they have just built? Related to the previous point: how can a program locate its runfiles if it's executed outside of |
I don't understand why the genrule is not sufficient. Locating your runfiles from outside of bazel-run is generally an unsafe idea. There is no guarantee that the path inside the runfiles will be the same as the one the user might see through the symlinks. Especially as the user is allowed to rename or suppress the symlinks. You are making a binary that may be referring to the wrong things, or missing things, and it's going to hang around for ever, some user is going to copy it out to their bin directory so they can run it easily and it's going to cause obtuse bugs in the long run. Instead you add a packaging rule to put everything into a single file, possibly a self installing archive, and produce that for the user to run. This is clean and safe, and produces a single file you can give to other people, or hold on to and run again without depending on your build state. You can also do this by making a single go binary with all the things that would be in runfiles as embedded data, which is slightly more convoluted but is really clean and has no extra dependencies. |
Well, I can't apply the |
Yes, gazelle needs to be able to cope with non standard binary names. |
I'll try to figure out a matching policy that makes sense to fix bazel-contrib/bazel-gazelle#106 tomorrow. I want to be careful here, especially in situations where there are multiple |
I would recommend you also consider updating https://docs.bazel.build/versions/master/build-ref.html#data, which currently says
This is currently false when using the Go rules. |
That is talking about data files that are present in the source folder, generated files don't have a relative to workspace-relative path because they are not in the workspace... |
Nothing about that section specifies source files. Just above, it specifically calls out that this applies to rules, not just files:
Granted, a rule could just be a |
Coming back to this because I still haven't been able to workaround this problem. Turns out the The question is: if you have a |
This change adds a bunch of Bazel-specific functions to locate the runfiles trees of built binaries and to find binaries within them, and then changes the lint and install tools to use these new features. This extra logic is required to later deal with a change in the Go rules that causes the paths to built Go binaries to not be deterministic (bazel-contrib/rules_go#1239). Needless to mention that this has been a huge time sink.
As part of this change, rerun Gazelle to update all BUILD.bazel files with new changes. We also have to workaround a change in the paths of the built Go binaries (bazel-contrib/rules_go#1239) in our instructions and build scripts.
As part of this change, rerun Gazelle to update all BUILD.bazel files with new changes. We also have to workaround a change in the paths of the built Go binaries (bazel-contrib/rules_go#1239) in our instructions and build scripts.
This change adds a bunch of Bazel-specific functions to locate the runfiles trees of built binaries and to find binaries within them. This extra logic is required to support the recent change in the Go rules that causes the paths to built Go binaries to not be deterministic (issue bazel-contrib#1239).
This change adds a bunch of Bazel-specific functions to locate the runfiles trees of built binaries and to find binaries within them. This extra logic is required to support the recent change in the Go rules that causes the paths to built Go binaries to not be deterministic (issue #1239).
FTR #1331 has added a bunch of utility functions to the go/tools/bazel package to locate the runfiles tree of a binary and to find other binaries within it. |
Change the lint and install tools to use the APIs in the Go rules package to find their runfiles trees and built binaries within them. This extra logic is required to later deal with a change in the Go rules that causes the paths to built Go binaries to not be deterministic (bazel-contrib/rules_go#1239).
As part of this change, rerun Gazelle to update all BUILD.bazel files with new changes. We also have to workaround a change in the paths of the built Go binaries (bazel-contrib/rules_go#1239) in our instructions and build scripts.
go_binary now has an "out" attribute which allows users to set the output filename for the generated executable. When set, go_binary will write this file without mode-specific directory prefixes, without linkmode-specific prefixes, and without platform-specific extensions. Fixes bazel-contrib#1239
go_binary now has an "out" attribute which allows users to set the output filename for the generated executable. When set, go_binary will write this file without mode-specific directory prefixes, without linkmode-specific prefixes, and without platform-specific extensions. Fixes bazel-contrib#1239
go_binary now has an "out" attribute which allows users to set the output filename for the generated executable. When set, go_binary will write this file without mode-specific directory prefixes, without linkmode-specific prefixes, and without platform-specific extensions. Fixes #1239
I know this was closed, but in case anyone else continues to be annoyed by |
dropping this here in case others find it useful. For compat with bazel 3 and 4, we need Since the proto output of aquery has the bits spread around, the
|
@alexeagle The jq query there does not work for me in Bazel 5.1.0 I got to this
which could not be assigned to
Then you have to find the full path by keep recursively traverse upward via I think the
and doing Worth to note that the binary needs to be in |
With bazelbuild/bazel#8739 fixed in Bazel 5.3.0 and current Bazel HEAD, you can instead do (example uses a
If you need to get the path from within a Bazel build, I would recommend writing a small custom rule that takes the |
Building a
go_binary
results in an output path that differs from the package path by the addition of $GOHOSTENV, $GOOS, and $GOARCH, e.g.,leaves the binary at
(or whatever your os/arch/etc. works out to be)
This makes it difficult to use Go binaries as part of
sh_test
rules: Other Bazel*_binary
tools can be used by relative path without having to pre-bake these values into the script, e.g., if mysh_binary
declares//path/to/my:binary
as adata
dependency, it should be executable by the pathpath/to/my/binary
relative to the exec root when the test runs.It works as described for everything but Go. Given that the Bazel output already reflects host and architecture constraints, the additional path decoration gets in the way of common practice.
One can work around this by injecting rewrite rules, but that shouldn't be necessary.
The text was updated successfully, but these errors were encountered: