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

@@ -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"],

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,15 +368,15 @@ 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),
       suggests=FLAGS.suggests,
       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)

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),

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.

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é[email protected]",
+    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",

pkg/tests/build_test.sh

@@ -177,22 +177,20 @@ 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
 ./usr/
 ./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é[email protected]"
@@ -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"