pkg_deb flag decoding breaks for python3.

[aiuto]

From bazelbuild/bazel#7370
@jwnimmer-tri

If I'm understanding my build errors correctly, this change re-broke the py2/3 compatibility on make_deb.py that #6818 had previously fixed.

  File ".../bazel-out/k8-opt/bin/external/bazel_tools/tools/build_defs/pkg/make_deb.runfiles/bazel_tools/tools/build_defs/pkg/make_deb.py", line 301, in GetFlagValue
    flagvalue = flagvalue.decode('utf-8')
AttributeError: 'str' object has no attribute 'decode'

It's possible that the toolchain changes in 0.25.0 are also at fault and/or the only problem, but with the binary tagged PY2AND3, the code doesn't look correct to me on first read.

[qzmfranklin]

I'd believe that this is identical:
bazelbuild/bazel#8465

Would like to see this fixed.

[aiuto]

Yes. We are going to fix it in this repository, not in bazelbuild/bazel.
I am very happy to accept a patch if you want to send one.

[qzmfranklin]

Let me do that. We have been seeing this issue multiple times. Motivated enough to do something.

[qzmfranklin]

Though, the BUILD file and the soruce file stipulates that the make_deb target be run under python3.

py_binary(
    name = "make_deb",
    srcs = ["make_deb.py"],
    python_version = "PY2",
    srcs_version = "PY2AND3",
    visibility = ["//visibility:public"],
    deps = [
        ":archive",
        "@abseil_py//absl/flags",
    ],
)

Commenting out the python_version = "PY2" line, run the tests got:

13:58 $ bazel test ...
INFO: Analyzed 34 targets (15 packages loaded, 375 targets configured).
INFO: Found 29 targets and 5 test targets...
FAIL: //distro:packaging_test (see /home/zhongming/.cache/bazel/_bazel_zhongming/dcb9e5cca53c9c59e32893c1321b9420/execroot/rules_pkg/bazel-out/k8-fastbuild/testlogs/distro/packaging_test/test.log)
ERROR: /mnt/data/git/LogiOcean/third_party/bazel/pkg/pkg/tests/BUILD:62:1: MakeDeb tests/titi_test_all.deb failed (Exit 1) make_deb failed: error executing command bazel-out/host/bin/external/rules_pkg/make_deb '--output=bazel-out/k8-fastbuild/bin/tests/titi_test_all.deb' '--changes=bazel-out/k8-fastbuild/bin/tests/titi_test_all.changes' ... (remaining 15 argument(s) skipped)

Use --sandbox_debug to see verbose messages from the sandbox
Traceback (most recent call last):
  File "/home/zhongming/.cache/bazel/_bazel_zhongming/dcb9e5cca53c9c59e32893c1321b9420/sandbox/linux-sandbox/25/execroot/rules_pkg/bazel-out/host/bin/external/rules_pkg/make_deb.runfiles/rules_pkg/make_deb.py", line 365, in <module>
    main(FLAGS(sys.argv))
  File "/home/zhongming/.cache/bazel/_bazel_zhongming/dcb9e5cca53c9c59e32893c1321b9420/sandbox/linux-sandbox/25/execroot/rules_pkg/bazel-out/host/bin/external/rules_pkg/make_deb.runfiles/rules_pkg/make_deb.py", line 351, in main
    installedSize=GetFlagValue(FLAGS.installed_size))
  File "/home/zhongming/.cache/bazel/_bazel_zhongming/dcb9e5cca53c9c59e32893c1321b9420/sandbox/linux-sandbox/25/execroot/rules_pkg/bazel-out/host/bin/external/rules_pkg/make_deb.runfiles/rules_pkg/make_deb.py", line 208, in CreateDeb
    control = CreateDebControl(extrafiles=extrafiles, **kwargs)
  File "/home/zhongming/.cache/bazel/_bazel_zhongming/dcb9e5cca53c9c59e32893c1321b9420/sandbox/linux-sandbox/25/execroot/rules_pkg/bazel-out/host/bin/external/rules_pkg/make_deb.runfiles/rules_pkg/make_deb.py", line 169, in CreateDebControl
    tarinfo.size = len(controlfile.encode('utf-8'))
UnicodeEncodeError: 'utf-8' codec can't encode characters in position 123-124: surrogates not allowed
----------------
Note: The failure of target @rules_pkg//:make_deb (with exit code 1) may have been caused by the fact that it is running under Python 3 instead of Python 2. Examine the error to determine if that appears to be the problem. Since this target is built in the host configuration, the only way to change its version is to set --host_force_python=PY2, which affects the entire build.

If this error started occurring in Bazel 0.27 and later, it may be because the Python toolchain now enforces that targets analyzed as PY2 and PY3 run under a Python 2 and Python 3 interpreter, respectively. See https://github.com/bazelbuild/bazel/issues/7899 for more information.

Is the python2-only thing intentional?

I know that Google internally still uses Python2 predominantly. But in my team we only use Python3. Would like to understand the direction of this repo better.

[qzmfranklin]

Another complicating factor is the introduction of the python toolchain (bazelbuild/bazel#7899) as of Bazel 0.27.0. Whatever my change would be, it is likely going to break for some people.

[aiuto]

I was looking at this a bit today. The code is just not Python3 ready. My hunch is that I should make it PY2 strictly now, but work towards fixing things so it easily switches to python3.

…

On Mon, Jul 1, 2019 at 2:06 AM Zhongming Qu ***@***.***> wrote: Another complicating factor is the introduction of the python toolchain ( bazelbuild/bazel#7899 <bazelbuild/bazel#7899>) as of Bazel 0.27.0. Whatever my change would be, it is likely going to break for some people. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#35?email_source=notifications&email_token=AAXHHHDRK4VHMJZKJXHH5QLP5GNGBA5CNFSM4HWUUFE2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY5CSTY#issuecomment-507128143>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAXHHHDBR5ZNZENXOGM65DLP5GNGBANCNFSM4HWUUFEQ> .

[aiuto]

Ughh...... work in progress here. It is not pretty. AFAICT, the flags which were utf-8 encoded are manifesting as str types in Python 3, but were really the underlying byte stream.
https://github.com/bazelbuild/rules_pkg/compare/master...aiuto:py3?expand=1

[qzmfranklin]

I agree with your opinion that the code is not python3 ready.

Our team uses python3 exclusively. So we will just go ahead and patch it to work under python3 (the code breaks for python2 though).

We will wait for the code to become ready.

[qzmfranklin]

Also, I believe that a source of inconsistency is with the abseil.flags python library, which produces byte[] in python2 and str in python3.

What's a good reason for not using the argparse library? I do not think abseil.flags python library is a better alternative for this task specifically. Indeed, I'd argue it is worse than argparse.

[aiuto]

I have a PR out which updates to Python3.

On Wed, Jul 3, 2019 at 1:43 AM Zhongming Qu ***@***.***> wrote: Also, I believe that a source of inconsistency is with the abseil.flags python library, which produces byte[] in python2 and str in python3.

The problem was a little below that. The Python3 runtime does something funny with the args - they are utf-8 encoded strings - but type str rather than bytes. So they are not really strict unicode. https://docs.python.org/3/library/sys.html#sys.argv https://docs.python.org/3/library/os.html#os.fsencode https://www.python.org/dev/peps/pep-0383/ abseil.flags is not really the problem. What's a good reason for not using the argparse library? I do not think

abseil.flags python library is a better alternative for this task specifically. Indeed, I'd argue it is worse than argparse.

argparse is probably fine. The code was written by Googlers, who are most familiar with the abseil libraries, so whomever wrote it used that. I will not have time to convert to argparse myself, but I will happily review PRs which do so. —

…

You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#35?email_source=notifications&email_token=AAXHHHAHS2XPEMIDZT5Y6WLP5Q36LA5CNFSM4HWUUFE2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZDKOGQ#issuecomment-507946778>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAXHHHBYMIPCCNNDLMVOXF3P5Q36LANCNFSM4HWUUFEQ> .

[aiuto]

Fixed in #47