Have I written custom code (as opposed to using a stock example script
provided in TensorFlow Model Analysis): No
OS Platform and Distribution (e.g., Linux Ubuntu 16.04): Linux 6.6.57-1-lts x86_64
TensorFlow Model Analysis installed from (source or binary): Source
TensorFlow Model Analysis version (use command below): 7a068ec4e15224f5a1e3f23a7bcbd9bd4b030181
Python version: 3.11.9
Jupyter Notebook version: N/A
Exact command to reproduce: N/A
Describe the problem
This issue is a catch-all for a number of problems with the current build system. Starting from the beginning:
Hardcoded protobuf v3.21.9 dependency: Although the WORKSPACE file defines targets from com_google_protobuf for v3.21.9, it doesn't actually use _PROTOBUF_COMMIT except in stripping output. It should use _PROTOBUF_COMMIT in both the archive name and for stripping output.
Version inconsistencies: setup.pyrequiresprotobuf>=3.20.3 for python<3.11, which doesn't match the version grabbed by bazel. For python>=3.11, protobuf>=4.25.2 is required, which is a full major version different and even more likely to be incompatible.
Bazel doesn't build the protocol buffers: setup.py does an ad-hoc platform-dependent search for protoc, meaning that the version of protobuf downloaded by bazel never gets used. If the build environment already contains any version of protobuf, setup.py will happily use it, leading to generated files which are incompatible with the rest of the code.
com_google_protobuf gets clobbered by rules_rust transitive dependency: Bazel never downloads the version of protobuf that you request in the WORKSPACE file because rules_rust has a transitive dependency on com_google_protobuf that takes precedence. Bazel silently builds the protocol buffers using a much older version of protobuf as a result, again leading to library incompatibilities at runtime.
bazel is never invoked from setup.py: Although there is a BUILD file for generating python code from the protobufs, bazel is never called from setup.py.
tensorflow_model_analysis/proto/BUILD points to the wrong protobuf dependency: Needs to point to the explicit protobuf dependency in WORKSPACE.
Even if bazel is made to build the protocol buffers, setup.py will need to be modified to grab the sources from bazel-bin/ when the wheel is being built. IMO this could be much more easily done with meson-python, which has a first-class build backend for Python already, would allow for robust version control for external tooling with fallback options as well if the host doesn't have the right version of protoc; we'd also avoid problems with transitive dependencies clobbering our actual dependencies too. If this is something folks are interested in, I'm happy to write the meson.build. Otherwise we can stick with bazel and call it by hand in setup.py.
On my system I'm unable to run tests because of this, but because bazel provides partial build isolation, whether you are affected by this or not really depends on the build environment.
System information
Describe the problem
This issue is a catch-all for a number of problems with the current build system. Starting from the beginning:
WORKSPACE
file defines targets fromcom_google_protobuf
forv3.21.9
, it doesn't actually use_PROTOBUF_COMMIT
except in stripping output. It should use_PROTOBUF_COMMIT
in both the archive name and for stripping output.setup.py
requiresprotobuf>=3.20.3
forpython<3.11
, which doesn't match the version grabbed by bazel. Forpython>=3.11
,protobuf>=4.25.2
is required, which is a full major version different and even more likely to be incompatible.setup.py
does an ad-hoc platform-dependent search forprotoc
, meaning that the version ofprotobuf
downloaded by bazel never gets used. If the build environment already contains any version ofprotobuf
,setup.py
will happily use it, leading to generated files which are incompatible with the rest of the code.protobuf
that you request in theWORKSPACE
file becauserules_rust
has a transitive dependency oncom_google_protobuf
that takes precedence. Bazel silently builds the protocol buffers using a much older version ofprotobuf
as a result, again leading to library incompatibilities at runtime.setup.py
: Although there is a BUILD file for generating python code from the protobufs, bazel is never called fromsetup.py
.protobuf
dependency inWORKSPACE
.Even if
bazel
is made to build the protocol buffers,setup.py
will need to be modified to grab the sources frombazel-bin/
when the wheel is being built. IMO this could be much more easily done withmeson-python
, which has a first-class build backend for Python already, would allow for robust version control for external tooling with fallback options as well if the host doesn't have the right version ofprotoc
; we'd also avoid problems with transitive dependencies clobbering our actual dependencies too. If this is something folks are interested in, I'm happy to write themeson.build
. Otherwise we can stick withbazel
and call it by hand insetup.py
.On my system I'm unable to run tests because of this, but because bazel provides partial build isolation, whether you are affected by this or not really depends on the build environment.
cc @smokestacklightnin