launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad-buildd:py3-tempfiles-text into launchpad-buildd:master.

Commit message:
Open temporary files in text mode in more cases

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/+git/launchpad-buildd/+merge/383224

Python 3 defaults these to binary mode, which is reasonable in general but doesn't make so much sense here.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad-buildd:py3-tempfiles-text into launchpad-buildd:master.

diff --git a/debian/changelog b/debian/changelog
index 8943f0a..287e9da 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -3,6 +3,7 @@ launchpad-buildd (190) UNRELEASED; urgency=medium
   * Switch to git; add Vcs-* fields.
   * Always call Resource.putChild with path as bytes.
   * Switch RotatableFileLogObserver from class advice to a class decorator.
+  * Open temporary files in text mode in more cases.
 
  -- Colin Watson <cjwatson@xxxxxxxxxx>  Tue, 28 Apr 2020 10:19:27 +0100
 
diff --git a/lpbuildd/target/apt.py b/lpbuildd/target/apt.py
index 31c2bfa..fc1827a 100644
--- a/lpbuildd/target/apt.py
+++ b/lpbuildd/target/apt.py
@@ -33,14 +33,14 @@ class OverrideSourcesList(Operation):
 
     def run(self):
         logger.info("Overriding sources.list in build-%s", self.args.build_id)
-        with tempfile.NamedTemporaryFile() as sources_list:
+        with tempfile.NamedTemporaryFile(mode="w+") as sources_list:
             for archive in self.args.archives:
                 print(archive, file=sources_list)
             sources_list.flush()
             os.fchmod(sources_list.fileno(), 0o644)
             self.backend.copy_in(sources_list.name, "/etc/apt/sources.list")
         if self.args.apt_proxy_url is not None:
-            with tempfile.NamedTemporaryFile() as apt_proxy_conf:
+            with tempfile.NamedTemporaryFile(mode="w+") as apt_proxy_conf:
                 print(
                     'Acquire::http::Proxy "{}";'.format(
                         self.args.apt_proxy_url),
diff --git a/lpbuildd/target/lxd.py b/lpbuildd/target/lxd.py
index 2da7804..4fdfd33 100644
--- a/lpbuildd/target/lxd.py
+++ b/lpbuildd/target/lxd.py
@@ -379,12 +379,12 @@ class LXD(Backend):
             ["hostname"], universal_newlines=True).rstrip("\n")
         fqdn = subprocess.check_output(
             ["hostname", "--fqdn"], universal_newlines=True).rstrip("\n")
-        with tempfile.NamedTemporaryFile(mode="w+b") as hosts_file:
+        with tempfile.NamedTemporaryFile(mode="w+") as hosts_file:
             try:
                 self.copy_out("/etc/hosts", hosts_file.name)
             except LXDException:
                 hosts_file.seek(0, os.SEEK_SET)
-                hosts_file.write(fallback_hosts.encode("UTF-8"))
+                hosts_file.write(fallback_hosts)
             hosts_file.seek(0, os.SEEK_END)
             print("\n127.0.1.1\t%s %s" % (fqdn, hostname), file=hosts_file)
             hosts_file.flush()
diff --git a/lpbuildd/target/tests/test_lxd.py b/lpbuildd/target/tests/test_lxd.py
index d7a8b6d..7377de1 100644
--- a/lpbuildd/target/tests/test_lxd.py
+++ b/lpbuildd/target/tests/test_lxd.py
@@ -91,7 +91,7 @@ class FakeHostname:
         parser.add_argument("--fqdn", action="store_true", default=False)
         args = parser.parse_args(proc_args["args"][1:])
         output = self.fqdn if args.fqdn else self.hostname
-        return {"stdout": io.BytesIO((output + "\n").encode("UTF-8"))}
+        return {"stdout": io.StringIO(output + u"\n")}
 
 
 class FakeFilesystem(_FakeFilesystem):