Update make_deb.py to work for Python2 or 3

pkg/BUILD

@@ -42,11 +42,22 @@ py_binary(
 )
 
 
-# NOTE: This works for PY2 and PY3.
-# TODO: Figure out how to use "whatever version" we have on the machine.
 py_binary(
     name = "make_deb",
     srcs = ["make_deb.py"],
+    python_version = "PY3",
+    srcs_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

@@ -308,26 +308,40 @@ def CreateChanges(output,
 
 
 def GetFlagValue(flagvalue, strip=True):
-  """Normalize a flag value.
+  """Converts a raw flag string to a useable value.
 
-  Handles @filename expansions.
+  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: (bytes|str|unicode) A value
-    strip: Strip white space?
+    flagvalue: (str) raw flag value
+    strip: (bool) Strip white space.
 
   Returns:
     Python2: unicode
     Python3: str
   """
   if flagvalue:
     if sys.version_info[0] < 3:
+      # python2 gives us raw bytes in argv.
       flagvalue = flagvalue.decode('utf-8')
-    else:
-      flagvalue = str(flagvalue)
+    # assertion: py2: flagvalue is unicode
+    # assertion: py3: flagvalue is str, but in weird format
     if flagvalue[0] == '@':
+      # 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
@@ -372,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)
@@ -386,4 +400,5 @@ def main(unused_argv):
   # 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/
-  main(FLAGS([os.fsencode(arg).decode('utf-8') for arg in sys.argv]))
+  # main(FLAGS([os.fsencode(arg).decode('utf-8') for arg in sys.argv]))
+  main(FLAGS(sys.argv))

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"