Thanks for the review! Answers inline:
On Sun, Jan 15, 2017 at 10:51 PM, Holger Levsen <hol...@layer-acht.org> wrote:
> Hi Michael,
>
> On Wed, Jan 11, 2017 at 11:34:52PM +0100, Michael Stapelberg wrote:
>> Attached you can find a first stab at implementing this feature. I
>> introduced a new protocol message to transfer base64-encoded arbitrary
>> binary data.
>
> the patch looks very nice. thanks for adding all the documentation
> already.
>
>> This can easily be used to transfer other files in the
>> same spirit, should that become necessary in the future (it also seems
>> like the clean thing to do, even if we’re just talking about a single
>> file).
>
> nice!
>
>> The files are then made available at
>> /<section>/aux/<package>_<version>/<filename>, e.g.
>> /sid/aux/libva1_1.7.3-2/alternatives.tar.gz.
>
> fine in principle, except this is part of the three concerns I have
> about the patch:
>
> - "aux" is maybe a bit too short (as a directory name), maybe not :)
I’m happy to change it if you have a specific suggestion. If you
don’t, I propose keeping it: auxiliary is hard to spell (especially
for non-native speakers), and aux is precise enough.
> - the option --auxdir is good, but in the current patches this is not
> seperated from --record-alternatives-in-auxdir or whatever that option
> to enable alternatives collection should be called. Not sure about the
> name for that option but you'll get the idea…
Added an --export-alternatives-aux option.
> - please also add a debian/changelog entry…
Done.
I also changed the slave to clean up after itself and implemented
exporting the before/after versions. Further, I changed to code to
only export tarballs once (instead of twice).
Please consider merging the attached updated patch.
--
Best regards,
Michael
From 38e445f219cd98a9468012af8a13610bd4c63243 Mon Sep 17 00:00:00 2001
From: Michael Stapelberg <stapelb...@debian.org>
Date: Wed, 11 Jan 2017 23:24:00 +0100
Subject: [PATCH] Export /var/lib/dpkg/alternatives tarball as aux file
---
README_server.txt | 11 +++++++++++
debian/changelog | 7 +++++++
piuparts-master-backend.py | 13 +++++++++++++
piuparts-slave.py | 21 +++++++++++++++++++++
piuparts.py | 29 +++++++++++++++++++++++++++++
5 files changed, 81 insertions(+)
diff --git a/README_server.txt b/README_server.txt
index 506d5149..f82450cb 100644
--- a/README_server.txt
+++ b/README_server.txt
@@ -242,6 +242,17 @@ Success: ok
Slave informs master it cannot test the desired version of a
package (perhaps it went away from the mirror?).
+----
+Command: aux <packagename> <packageversion> <filename>
+ base64-encoded file contents
+ .
+Success: ok
+----
+
+Slave transfers an auxiliary file (e.g. the /var/lib/dpkg/alternatives
+tarball) to the master.
+
+
----
Command: status
Success: ok <package-state>=<count> <package-state>=<count>...
diff --git a/debian/changelog b/debian/changelog
index 9cb15652..f4951416 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+piuparts (0.74) unstable; urgency=medium
+
+ * Export /var/lib/dpkg/alternatives before and after installation to aux
+ directory (Closes: #850917)
+
+ -- Michael Stapelberg <stapelb...@debian.org> Sun, 22 Jan 2017 14:38:39 +0100
+
piuparts (0.73) unstable; urgency=medium
* Add new option, --hard-link, and do *not* use it by default.
diff --git a/piuparts-master-backend.py b/piuparts-master-backend.py
index 3e87c412..69b86786 100644
--- a/piuparts-master-backend.py
+++ b/piuparts-master-backend.py
@@ -32,6 +32,7 @@ import os
import fcntl
import time
import random
+import base64
from urllib2 import URLError
import piupartslib
@@ -142,6 +143,7 @@ class Master(Protocol):
"pass": self._pass,
"fail": self._fail,
"untestable": self._untestable,
+ "aux": self._aux,
# debug commands, unstable and undocumented interface
"_state": self._state,
@@ -367,6 +369,17 @@ class Master(Protocol):
% ("untestable", args[0], args[1]))
self._short_response("ok")
+ def _aux(self, command, args):
+ self._check_args(3, command, args)
+ encoded = self._read_long_part()
+ decoded = base64.b64decode(encoded)
+ destdir = os.path.join(self._section, "aux", args[0] + "_" + args[1])
+ if not os.path.isdir(destdir):
+ os.makedirs(destdir)
+ with open(os.path.join(destdir, args[2]), "wb") as f:
+ f.write(decoded)
+ self._short_response("ok")
+
# debug command
def _state(self, command, args):
self._check_args(1, command, args)
diff --git a/piuparts-slave.py b/piuparts-slave.py
index 9b147529..23d732d8 100644
--- a/piuparts-slave.py
+++ b/piuparts-slave.py
@@ -36,6 +36,7 @@ import fcntl
import random
import ConfigParser
import apt_pkg
+import base64
import piupartslib.conf
import piupartslib.packagesdb
@@ -270,6 +271,20 @@ class Slave:
if line != "ok\n":
raise MasterNotOK()
+ def send_aux(self, section, filename):
+ logging.info("Sending aux data for %s/%s" % (section, filename))
+ basename = os.path.basename(filename)
+ package, rest = basename.split("_", 1)
+ version = rest[:-len(".log")]
+ for filename in os.listdir("aux"):
+ self._writeline("aux", package, version, filename)
+ with open(os.path.join("aux", filename), "r") as f:
+ self._writeline(" " + base64.b64encode(f.read()))
+ self._writeline(".")
+ line = self._readline()
+ if line != "ok\n":
+ raise MasterNotOK()
+
def get_status(self, section):
self._writeline("status")
line = self._readline()
@@ -576,6 +591,11 @@ class Section:
self._slave.send_log(self._config.section, logdir, fullname)
os.remove(fullname)
+ if os.path.exists("aux"):
+ self._slave.send_aux(self._config.section, fullname)
+ for filename in os.listdir("aux"):
+ os.remove(os.path.join("aux", filename))
+
if unreserve:
for logdir in ["new", "reserved"]:
for basename in os.listdir(logdir):
@@ -690,6 +710,7 @@ class Section:
if self._config["keep-sources-list"] in ["yes", "true"]:
command.append("--keep-sources-list")
command.extend(["--apt", "%s=%s" % (pname, pvers)])
+ command.extend(["--auxdir", "aux"])
subdir = "fail"
ret = 0
diff --git a/piuparts.py b/piuparts.py
index dbf2b18d..68bc2991 100644
--- a/piuparts.py
+++ b/piuparts.py
@@ -1151,6 +1151,20 @@ class Chroot:
installed = False
return installed
+ def extract_alternatives(self, basename):
+ target_name = os.path.join(settings.auxdir, basename+".tar.gz")
+ print 'Exporting /var/lib/dpkg/alternatives to %s for manpages.debian.org' % target_name
+ self.run(["tar", "czf", "/tmp/export.tar.gz", "-C", "/var/lib/dpkg/alternatives", "."])
+ source_name = self.relative("tmp/export.tar.gz")
+ try:
+ if not os.path.isdir(settings.auxdir):
+ os.makedirs(settings.auxdir)
+ shutil.copy(source_name, target_name)
+ except IOError as detail:
+ logging.error("Error copying %s to %s: %s" %
+ (source_name, target_name, detail))
+ panic()
+
def install_packages(self, package_files, packages, with_scripts=True, reinstall=False):
if package_files:
self.install_package_files(package_files, packages, with_scripts=with_scripts)
@@ -2357,6 +2371,9 @@ def install_purge_test(chroot, chroot_state, package_files, packages, extra_pack
chroot.run_scripts("pre_install")
+ if settings.export_alternatives_aux:
+ chroot.extract_alternatives('alternatives-before')
+
chroot.install_packages([], extra_packages, with_scripts=False)
if settings.warn_on_others or settings.install_purge_install:
@@ -2442,6 +2459,9 @@ def install_purge_test(chroot, chroot_state, package_files, packages, extra_pack
chroot.run_scripts("post_install")
+ if settings.export_alternatives_aux:
+ chroot.extract_alternatives('alternatives-after')
+
if settings.install_purge_install:
file_owners = chroot.get_files_owned_by_packages()
chroot.restore_selections(chroot_state_with_deps["selections"], packages)
@@ -2807,6 +2827,13 @@ def parse_command_line():
help="Write log file to FILENAME in addition to " +
"the standard output.")
+ parser.add_option("--auxdir", metavar="AUXDIR",
+ help="Write auxiliary output files (see also "
+ "--export-alternatives-aux) to AUXDIR.")
+
+ parser.add_option("--export-alternatives-aux", action="store_true", default=False,
+ help="Export /var/lib/dpkg/alternatives to --auxdir.")
+
parser.add_option("--list-installed-files",
action="store_true", default=False,
help="List files added to the chroot after the " +
@@ -2984,6 +3011,8 @@ def parse_command_line():
settings.defaults = opts.defaults
defaults = DefaultsFactory().new_defaults()
+ settings.auxdir = opts.auxdir
+ settings.export_alternatives_aux = opts.export_alternatives_aux
settings.tmpdir = opts.tmpdir
settings.keep_tmpdir = opts.keep_tmpdir
settings.single_changes_list = opts.single_changes_list
--
2.11.0
