From 96e34abe84353387fd378fba48be88836f6c3f2d Mon Sep 17 00:00:00 2001 From: aiuto Date: Wed, 3 Jul 2019 17:17:40 -0400 Subject: [PATCH] Update make_deb.py to work for Python2 or 3 (#47) * Convert make_deb.py to Python2 + Python 3 * Pass environment to make_deb and other helpers so that Python expect UTF-8 IO * Handle argv correctly w.r.t PEP-383 * Test for python2 and python3. --- pkg/BUILD | 13 ++++++++++ pkg/make_deb.py | 53 +++++++++++++++++++++++++++++++---------- pkg/pkg.bzl | 18 +++++++++++--- pkg/rpm.bzl | 8 ++++++- pkg/tests/BUILD | 24 +++++++++++++++++++ pkg/tests/build_test.sh | 43 +++++++++++++++++++-------------- 6 files changed, 125 insertions(+), 34 deletions(-) diff --git a/pkg/BUILD b/pkg/BUILD index 5f20f523..17f7fe1c 100644 --- a/pkg/BUILD +++ b/pkg/BUILD @@ -41,9 +41,22 @@ py_binary( ], ) + py_binary( name = "make_deb", srcs = ["make_deb.py"], + python_version = "PY3", + visibility = ["//visibility:public"], + deps = [ + ":archive", + "@abseil_py//absl/flags", + ], +) + +py_binary( + name = "make_deb_py2", + srcs = ["make_deb.py"], + main = "make_deb.py", python_version = "PY2", srcs_version = "PY2AND3", visibility = ["//visibility:public"], diff --git a/pkg/make_deb.py b/pkg/make_deb.py index 9dc12831..3854ea23 100644 --- a/pkg/make_deb.py +++ b/pkg/make_deb.py @@ -139,7 +139,7 @@ def AddArFileEntry(fileobj, filename, def MakeDebianControlField(name, value, wrap=False): """Add a field to a debian control file.""" result = name + ': ' - if isinstance(value, str): + if isinstance(value, bytes): value = value.decode('utf-8') if isinstance(value, list): value = u', '.join(value) @@ -156,7 +156,7 @@ def MakeDebianControlField(name, value, wrap=False): def CreateDebControl(extrafiles=None, **kwargs): """Create the control.tar.gz file.""" # create the control file - controlfile = '' + controlfile = u'' for values in DEBIAN_FIELDS: fieldname = values[0] key = fieldname[0].lower() + fieldname[1:].replace('-', '') @@ -167,9 +167,9 @@ def CreateDebControl(extrafiles=None, **kwargs): with gzip.GzipFile('control.tar.gz', mode='w', fileobj=tar, mtime=0) as gz: with tarfile.open('control.tar.gz', mode='w', fileobj=gz) as f: tarinfo = tarfile.TarInfo('control') - # Don't discard unicode characters when computing the size - tarinfo.size = len(controlfile.encode('utf-8')) - f.addfile(tarinfo, fileobj=BytesIO(controlfile.encode('utf-8'))) + control_file_data = controlfile.encode('utf-8') + tarinfo.size = len(control_file_data) + f.addfile(tarinfo, fileobj=BytesIO(control_file_data)) if extrafiles: for name, (data, mode) in extrafiles.items(): tarinfo = tarfile.TarInfo(name) @@ -276,7 +276,7 @@ def CreateChanges(output, debsize = str(os.path.getsize(deb_file)) deb_basename = os.path.basename(deb_file) - changesdata = ''.join([ + changesdata = u''.join([ MakeDebianControlField('Format', '1.8'), MakeDebianControlField('Date', time.ctime(timestamp)), MakeDebianControlField('Source', package), @@ -303,16 +303,45 @@ def CreateChanges(output, 'Checksums-Sha256', '\n' + ' '.join([checksums['sha256'], debsize, deb_basename])) ]) - with open(output, 'w') as changes_fh: + with open(output, 'wb') as changes_fh: changes_fh.write(changesdata.encode('utf-8')) def GetFlagValue(flagvalue, strip=True): + """Converts a raw flag string to a useable value. + + 1. Expand @filename style flags to the content of filename. + 2. Cope with Python3 strangness of sys.argv. + sys.argv is not actually proper str types on Unix with Python3 + The bytes of the arg are each directly transcribed to the characters of + the str. It is actually more complex than that, as described in the docs. + 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/ + + Args: + flagvalue: (str) raw flag value + strip: (bool) Strip white space. + + Returns: + Python2: unicode + Python3: str + """ if flagvalue: - flagvalue = flagvalue.decode('utf-8') + if sys.version_info[0] < 3: + # python2 gives us raw bytes in argv. + flagvalue = flagvalue.decode('utf-8') + # assertion: py2: flagvalue is unicode + # assertion: py3: flagvalue is str, but in weird format if flagvalue[0] == '@': - with open(flagvalue[1:], 'r') as f: + # Subtle: We do not want to re-encode the value here, because it + # is encoded in the right format for file open operations. + with open(flagvalue[1:], 'rb') as f: flagvalue = f.read().decode('utf-8') + else: + # convert fs specific encoding back to proper unicode. + flagvalue = os.fsencode(flagvalue).decode('utf-8') + if strip: return flagvalue.strip() return flagvalue @@ -339,7 +368,7 @@ def main(unused_argv): package=FLAGS.package, version=GetFlagValue(FLAGS.version), description=GetFlagValue(FLAGS.description), - maintainer=FLAGS.maintainer, + maintainer=GetFlagValue(FLAGS.maintainer), section=FLAGS.section, architecture=FLAGS.architecture, depends=GetFlagValues(FLAGS.depends), @@ -347,7 +376,7 @@ def main(unused_argv): enhances=FLAGS.enhances, preDepends=FLAGS.pre_depends, recommends=FLAGS.recommends, - homepage=FLAGS.homepage, + homepage=GetFlagValue(FLAGS.homepage), builtUsing=GetFlagValue(FLAGS.built_using), priority=FLAGS.priority, conflicts=FLAGS.conflicts, @@ -357,7 +386,7 @@ def main(unused_argv): deb_file=FLAGS.output, architecture=FLAGS.architecture, short_description=GetFlagValue(FLAGS.description).split('\n')[0], - maintainer=FLAGS.maintainer, package=FLAGS.package, + maintainer=GetFlagValue(FLAGS.maintainer), package=FLAGS.package, version=GetFlagValue(FLAGS.version), section=FLAGS.section, priority=FLAGS.priority, distribution=FLAGS.distribution, urgency=FLAGS.urgency) diff --git a/pkg/pkg.bzl b/pkg/pkg.bzl index fd6d46c4..b4421f1d 100644 --- a/pkg/pkg.bzl +++ b/pkg/pkg.bzl @@ -15,7 +15,7 @@ load(":path.bzl", "compute_data_path", "dest_path") -version = '0.2.0' +version = "0.2.0" # Filetype to restrict inputs tar_filetype = [".tar", ".tar.gz", ".tgz", ".tar.xz", ".tar.bz2"] @@ -115,11 +115,17 @@ def _pkg_tar_impl(ctx): ctx.actions.write(arg_file, "\n".join(args)) ctx.actions.run( + mnemonic = "PackageTar", inputs = file_inputs + ctx.files.deps + [arg_file], executable = ctx.executable.build_tar, arguments = ["--flagfile", arg_file.path], outputs = [ctx.outputs.out], - mnemonic = "PackageTar", + env = { + "LANG": "en_US.UTF-8", + "LC_CTYPE": "UTF-8", + "PYTHONIOENCODING": "UTF-8", + "PYTHONUTF8": "1", + }, use_default_shell_env = True, ) @@ -216,11 +222,17 @@ def _pkg_deb_impl(ctx): args += ["--recommends=" + d for d in ctx.attr.recommends] ctx.actions.run( + mnemonic = "MakeDeb", executable = ctx.executable.make_deb, arguments = args, inputs = files, outputs = [ctx.outputs.deb, ctx.outputs.changes], - mnemonic = "MakeDeb", + env = { + "LANG": "en_US.UTF-8", + "LC_CTYPE": "UTF-8", + "PYTHONIOENCODING": "UTF-8", + "PYTHONUTF8": "1", + }, ) ctx.actions.run_shell( command = "ln -s %s %s" % (ctx.outputs.deb.basename, ctx.outputs.out.path), diff --git a/pkg/rpm.bzl b/pkg/rpm.bzl index 16b3d89f..3b9e6705 100644 --- a/pkg/rpm.bzl +++ b/pkg/rpm.bzl @@ -82,12 +82,18 @@ def _pkg_rpm_impl(ctx): # Call the generator script. # TODO(katre): Generate a source RPM. ctx.actions.run( + mnemonic = "MakeRpm", executable = ctx.executable._make_rpm, use_default_shell_env = True, arguments = args, inputs = files, outputs = [ctx.outputs.rpm], - mnemonic = "MakeRpm", + env = { + "LANG": "en_US.UTF-8", + "LC_CTYPE": "UTF-8", + "PYTHONIOENCODING": "UTF-8", + "PYTHONUTF8": "1", + }, ) # Link the RPM to the expected output name. diff --git a/pkg/tests/BUILD b/pkg/tests/BUILD index da8afdb0..21c07b26 100644 --- a/pkg/tests/BUILD +++ b/pkg/tests/BUILD @@ -82,6 +82,29 @@ pkg_deb( version = "test", ) +pkg_deb( + name = "test-deb-py2", + built_using = "some_test_data (0.1.2)", + conffiles = [ + "/etc/nsswitch.conf", + "/etc/other", + ], + config = ":testdata/config", + data = ":test-tar-gz.tar.gz", + depends = [ + "dep1", + "dep2", + ], + description = "toto ®, Й, ק ,م, ๗, あ, 叶, 葉, 말, ü and é", + distribution = "trusty", + maintainer = "soméone@somewhere.com", + make_deb = "@rules_pkg//:make_deb_py2", + package = "titi", + templates = ":testdata/templates", + urgency = "low", + version = "test_py2", +) + [pkg_tar( name = "test-tar-%s" % ext[1:], srcs = [ @@ -191,6 +214,7 @@ sh_test( data = [ "testenv.sh", ":test-deb.deb", + ":test-deb-py2.deb", ":test-tar-.tar", ":test-tar-bz2.tar.bz2", ":test-tar-empty_dirs.tar", diff --git a/pkg/tests/build_test.sh b/pkg/tests/build_test.sh index 30e5ef0e..562a674f 100755 --- a/pkg/tests/build_test.sh +++ b/pkg/tests/build_test.sh @@ -177,11 +177,9 @@ drwxrwxrwx 0/0 0 2000-01-01 00:00 ./pmt/" \ "$(get_tar_verbose_listing test-tar-mtime.tar)" } -function test_deb() { - if ! (which dpkg-deb); then - echo "Unable to run test for debian, no dpkg-deb!" >&2 - return 0 - fi +function check_deb() { + package="$1" + local listing="./ ./etc/ ./etc/nsswitch.conf @@ -189,10 +187,10 @@ function test_deb() { ./usr/titi ./usr/bin/ ./usr/bin/java -> /path/to/bin/java" - check_eq "$listing" "$(get_deb_listing test-deb.deb)" - check_eq "-rwxr-xr-x" "$(get_deb_permission test-deb.deb ./usr/titi)" - check_eq "-rw-r--r--" "$(get_deb_permission test-deb.deb ./etc/nsswitch.conf)" - get_deb_description test-deb.deb >$TEST_log + check_eq "$listing" "$(get_deb_listing ${package})" + check_eq "-rwxr-xr-x" "$(get_deb_permission ${package} ./usr/titi)" + check_eq "-rw-r--r--" "$(get_deb_permission ${package} ./etc/nsswitch.conf)" + get_deb_description ${package} >$TEST_log expect_log "Description: toto ®, Й, ק ,م, ๗, あ, 叶, 葉, 말, ü and é" expect_log "Package: titi" expect_log "soméone@somewhere.com" @@ -203,14 +201,14 @@ function test_deb() { expect_log "Urgency: low" expect_log "Distribution: trusty" - get_deb_ctl_file test-deb.deb templates >$TEST_log + get_deb_ctl_file ${package} templates >$TEST_log expect_log "Template: titi/test" expect_log "Type: string" - get_deb_ctl_file test-deb.deb config >$TEST_log + get_deb_ctl_file ${package} config >$TEST_log expect_log "# test config file" - if ! dpkg_deb_supports_ctrl_tarfile test-deb.deb ; then + if ! dpkg_deb_supports_ctrl_tarfile ${package} ; then echo "Unable to test deb control files listing, too old dpkg-deb!" >&2 return 0 fi @@ -221,14 +219,23 @@ templates" # TODO: The config and templates come out with a+x permissions. Because I am # currently seeing the same behavior in the Bazel sources, I am going to look # at root causes later. I am not sure if this is WAI or not. - check_eq "$ctrl_listing" "$(get_deb_ctl_listing test-deb.deb)" - check_eq "-rw-r--r--" "$(get_deb_ctl_permission test-deb.deb conffiles)" - check_eq "-rwxr-xr-x" "$(get_deb_ctl_permission test-deb.deb config)" - check_eq "-rw-r--r--" "$(get_deb_ctl_permission test-deb.deb control)" - check_eq "-rwxr-xr-x" "$(get_deb_ctl_permission test-deb.deb templates)" + check_eq "$ctrl_listing" "$(get_deb_ctl_listing ${package})" + check_eq "-rw-r--r--" "$(get_deb_ctl_permission ${package} conffiles)" + check_eq "-rwxr-xr-x" "$(get_deb_ctl_permission ${package} config)" + check_eq "-rw-r--r--" "$(get_deb_ctl_permission ${package} control)" + check_eq "-rwxr-xr-x" "$(get_deb_ctl_permission ${package} templates)" local conffiles="/etc/nsswitch.conf /etc/other" - check_eq "$conffiles" "$(get_deb_ctl_file test-deb.deb conffiles)" + check_eq "$conffiles" "$(get_deb_ctl_file ${package} conffiles)" +} + +function test_deb() { + if ! (which dpkg-deb); then + echo "Unable to run test for debian, no dpkg-deb!" >&2 + return 0 + fi + check_deb "test-deb.deb" + check_deb "test-deb-py2.deb" } run_suite "build_test"