bazelbuild / rules_pkg

Bazel rules for creating packages of many types (zip, tar, deb, rpm, ...)
Apache License 2.0
216 stars 171 forks source link

pkg_tar implicitly requires python 3.5+ to run #736

Closed ashi009 closed 1 year ago

ashi009 commented 1 year ago

The transitive dependency on rules_python 0.1 doesn't declare a hermetic python toolchain, so it falls back to the one installed on the host system. And https://github.com/bazelbuild/rules_pkg/commit/f98a82173ee0abb2c7d925ace346a73a7628ec4f introduces type hints to the source, thus the build_tar may or may not be able to run depends on the host python version.

Traceback (most recent call last):
  File "<redacted>/bin/external/rules_pkg/pkg/private/tar/build_tar.runfiles/rules_pkg/pkg/private/tar/build_tar.py", line 24, in <module>
    from pkg.private import manifest
  File "<redacted>/bin/external/rules_pkg/pkg/private/tar/build_tar.runfiles/rules_pkg/pkg/private/manifest.py", line 30
    type: str
        ^
SyntaxError: invalid syntax
aiuto commented 1 year ago

Yes. It requires python 3.5 or better to work. Python 3.5 is already EOL. 3.6 is what we target. TBH, I am considering requiring 3.7 or 3.8 to capture some Unicode handling fixes.

ashi009 commented 1 year ago

I should probably rephrase this, shall we use a newer version of rules_python and declare the minimal required toolchain?

I think people expect rules_pkg to function hermetically.

aiuto commented 1 year ago

It's not the toolchain that determines what python you have on your system. We could add a note that you must have a recent python at the top of the docs.

I think people expect rules_pkg to function hermetically.

Nothing functions hermetically. Every rule set in the bazel ecosystem depends on tools of various sorts. Things which depend on C++ don't include a C++ compiler, they depend on what you have installed on your dev machines. For example, I could rewrite all the python helpers in C++, but I would be implicitly depending on Cxx14 or 17 (depending on my mood that day). We would have the same concern, just shifted to a new language.

ashi009 commented 1 year ago

Sounds fair. But I do think recent versions of rules_python supports using the workspace defined toolchain instead of the one from the OS. But document works for me.

Thanks!