It would be extremely nice to have a way to specify the minimum version of bazel that is required to build a given project.
If the required version is not met, it would produce an informative error message after a simple check, instead of exiting with obscure errors.
That would help up as lot to mitigate the flood of issues we get every time we update our bazel dependency.
:) We have been talking for this feature for a long time. Yes we should do it right now.
Thanks! :)
I'm trying to implement a function using this now, and I'm running into the problem that I don't know how to check for the existence of bazel_version in native. I want to do something like this:
def check_version(required):
if 'bazel_version' in native.__dict__:
found = native.bazel_version
if _parse_version(required) > _parse_version(found):
fail("Required version {} of bazel, found {}".format(required, found))
else:
fail("Required version {} of bazel, found pre-0.2.0".format(required))
But that fails. Without this, there's no way to tell if we're on an older version of Bazel and fail gracefully, which reduces its usefulness considerably. @damienmg do you have any suggestions about how to tackle this, since it would be incredibly helpful to lots of TensorFlow users!
Nevermind, Vijay beat me to it! The answer for reference is to do a if 'bazel_version' in dir(native)
instead.
Thanks for adding this! To confirm, the test will only fail if you have check_version(A) and the user's version is < A and >= 0.2.2 since older versions don't have native.check_version defined and will gracefully pass. The command line command to run bazel version
works for older versions so it'd be nice if we can catch older versions as well, but I guess with time this will just sort itself out.
Another point is that in the current latest release of 0.2.2b, 2b fails to parse and the check fails. In order to support the bazel_version check, please make sure to make all future versions purely numerical.
Bazel version number are not supposed to be purely numerical. Patch release are likely to happens again (hopefully the less). @meteorcloudy have been upstreaming a modification to TensorFlow's implementation of the method to strip out the patch letter.
This test indeed will fails only if the bazel version is > 0.2.1 because earlier does not have native.bazel_version
. The correct solution is to actually error outs if "bazel_version" not in dir(native)
, once you drop support for bazel < 0.2.1.
For reference https://github.com/tensorflow/tensorflow/pull/2094 is @meteorcloudy PR. I agree that ideally we should not use letter but there is not good recommendation in the semver to label those kind of release. Technically 0.2.2 = 0.2.2b + 1 cherry-pick because homebrew sandbox cannot build 0.2.2. Depending on how the discussion goes with homebrew, we might even need to do another cherry-pick (in which case it might make sense to just forfeit having 0.2.2 in homebrew and target the next release for landing in homebrew).
The check_bazel_version
function seems pretty useful, and I'd rather not copy it into our own repo (as has been done elsewhere).
Would it be reasonable to add it to the @bazel_tools
workspace so it can be reused more easily?
Jeff: I am adding it to the integration testing repo for general
consumption :)
On Wed, Aug 30, 2017 at 11:54 PM Jeff Grafton notifications@github.com
wrote:
The check_bazel_version function seems pretty useful, and I'd rather not
copy it into our own repo (as has been done elsewhere
https://github.com/bazelbuild/eclipse/blob/master/bazel_version.bzl).Would it be reasonable to add it to the @bazel_tools workspace so it can
be reused more easily?—
You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub
https://github.com/bazelbuild/bazel/issues/1014#issuecomment-326130263,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADjHf9i2a1xvCi3_PtU5ga80KFO_DzU4ks5sddoQgaJpZM4HpuFf
.
I'm confused -- this is closed, but I'm not sure what the recommended solution is?
Edit: I didn't unassign anyone @damienmg -- not sure why that happened...
@joshclimacell it was added to bazelbuild/bazel-skylib.
In your WORKSPACE
, you can do something like
http_archive(
name = "bazel_skylib",
sha256 = "b5f6abe419da897b7901f90cbab08af958b97a8f3575b0d3dd062ac7ce78541f",
strip_prefix = "bazel-skylib-0.5.0",
urls = ["https://github.com/bazelbuild/bazel-skylib/archive/0.5.0.tar.gz"],
)
load("@bazel_skylib//:lib.bzl", "versions")
versions.check(minimum_bazel_version = "0.17.2")
Note that this solution works great whenever the old version of bazel is capable of interpreting the file which contains this directive. It would still be nice to have a "safe" version check which works even if the new bazel language would result in an error in the old bazel version (this has happened before, though arguably it is becoming less likely)
Most helpful comment
@joshclimacell it was added to bazelbuild/bazel-skylib.
In your
WORKSPACE
, you can do something like