Hi Martin,

I'm looking into de-duplicating reprotest's embedded copies of autopkgtest code.

In #833407 you said "Using the Python API (adt_testbed.py) is a lot easier and 
more robust than talking to the backend servers directly", but adt_testbed.py 
is currently not available as a public Python module. How do you expect users 
to use it?

Might I suggest you make "/usr/share/autopkgtest/lib" available as a python 
package? autopkgtest is a fine name for it. I understand you might have some 
reservations about exposing some "internal" files like this, but the Pythonic 
way is to prefix those with an underscore (as the stdlib does) and/or document 
this fact clearly.

By contrast, putting a whole python package into a private path and then 
hacking sys.path to be able to find this, is not really the appropriate 
"Pythonic" way to do this. I can certainly sympathise, if you have 
philosophical problems with Python's approach to module-hiding, but I think 
this approach is not the best way to fight that fight. Rather, one would try to 
make a PEP that constructs something clean, that everyone can use.

I've also attached the additional changes that Ceridwen made to the code, since 
she imported it just before 4.0 was released. I have yet to study it in detail, 
but feel free to comment in the meantime - if you see any major 
incompatibilities that would be hard to fix in reprotest, it would be good for 
me to know quickly and early.

X

On Sat, 3 Sep 2016 13:47:26 -0300 Antonio Terceiro <terce...@debian.org> wrote:
> Package: reprotest
> Version: 0.2
> Severity: normal
> 
> The virtualization backend binaries provided by autopkgtest are supposed
> to be consumable by external tools¹. Maybe we are not in a point where
> actually doing that is easy/convenient, but having a full copy of the
> autopkgtest code base in reprotest does not seem to be right.
> 
> I even understand how this helped bootstrap the project and made it
> easier at the beginning, but I'm sure you will agree with me that reuse
> by copy-and-paste and embedded code copies are not good ideas.
> 
> ¹ see e.g. https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=833407
> 
> -- System Information:
> Debian Release: stretch/sid
>   APT prefers unstable-debug
>   APT policy: (500, 'unstable-debug'), (500, 'testing-debug'), (500, 
> 'unstable'), (500, 'testing'), (1, 'experimental-debug'), (1, 'experimental')
> Architecture: amd64 (x86_64)
> 
> Kernel: Linux 4.6.0-1-amd64 (SMP w/4 CPU cores)
> Locale: LANG=pt_BR.UTF-8, LC_CTYPE=pt_BR.UTF-8 (charmap=UTF-8)
> Shell: /bin/sh linked to /bin/dash
> Init: systemd (via /run/systemd/system)
> 
> Versions of packages reprotest depends on:
> ii  apt-utils       1.3~rc4
> ii  diffoscope      59
> ii  libdpkg-perl    1.18.10
> ii  procps          2:3.3.12-2
> ii  python3-debian  0.1.29
> pn  python3:any     <none>
> 
> Versions of packages reprotest recommends:
> ii  autodep8     0.8
> ii  disorderfs   0.4.3-1
> ii  locales-all  2.24-1
> ii  qemu-system  1:2.6+dfsg-3
> ii  qemu-utils   1:2.6+dfsg-3
> ii  schroot      1.6.10-2+b1
> 
> reprotest suggests no packages.
> 
> -- no debconf information

-- 
GPG: ed25519/56034877E1F87C35
GPG: rsa4096/1318EFAC5FBBDBCE
https://github.com/infinity0/pubkeys.git
diff --git a/reprotest/lib/VirtSubproc.py b/reprotest/lib/VirtSubproc.py
index 19b4e0f..c37051d 100644
--- a/reprotest/lib/VirtSubproc.py
+++ b/reprotest/lib/VirtSubproc.py
@@ -35,7 +35,7 @@ import pipes
 import socket
 import shutil
 
-import adtlog
+from reprotest.lib import adtlog
 
 progname = "<VirtSubproc>"
 devnull_read = open('/dev/null', 'r')
@@ -183,8 +183,8 @@ def check_exec(argv, downp=False, outp=False, timeout=0):
                                          stdout=stdout, stderr=subprocess.PIPE)
 
     if status:
-        bomb("%s%s failed (exit status %d)" %
-             ((downp and "(down) " or ""), argv, status))
+        bomb("%s%s failed (exit status %d)\n%s" %
+             ((downp and "(down) " or ""), argv, status, err))
     if err:
         bomb("%s unexpectedly produced stderr output `%s'" %
              (argv, err))
diff --git a/reprotest/lib/__init__.py b/reprotest/lib/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/reprotest/lib/adt_testbed.py b/reprotest/lib/adt_testbed.py
index 7904bf6..7a55be3 100644
--- a/reprotest/lib/adt_testbed.py
+++ b/reprotest/lib/adt_testbed.py
@@ -33,10 +33,15 @@ import tempfile
 import shutil
 import urllib.parse
 
-from debian import debian_support
 
-import adtlog
-import VirtSubproc
+# TODO: removing this import disables install_tmp, may want to restore
+# it at some point if I'm improving support for building Debian packages in
+# particular.
+
+# from debian import debian_support
+
+from reprotest.lib import adtlog
+from reprotest.lib import VirtSubproc
 
 
 timeouts = {'short': 100, 'copy': 300, 'install': 3000, 'test': 10000,
@@ -326,7 +331,7 @@ class Testbed:
 
     def bomb(self, m, _type=adtlog.TestbedFailure):
         adtlog.debug('%s %s' % (_type.__name__, m))
-        self.stop()
+        # self.stop()
         raise _type(m)
 
     def badpkg(self, m):
@@ -384,6 +389,9 @@ class Testbed:
             ll = list(map(urllib.parse.unquote, ll))
         return ll
 
+    # TODO: with stdout and stderr defaulting to None, this function
+    # eats all errors/output from its call, which is not the right
+    # thing.
     def execute(self, argv, xenv=[], stdout=None, stderr=None, kind='short'):
         '''Run command in testbed.
 
@@ -408,6 +416,7 @@ class Testbed:
         if env:
             argv = ['env'] + env + argv
 
+        # import pdb; pdb.set_trace()
         VirtSubproc.timeout_start(timeouts[kind])
         try:
             proc = subprocess.Popen(self.exec_cmd + argv,
@@ -437,11 +446,16 @@ class Testbed:
         adtlog.debug('testbed command exited with code %i' % proc.returncode)
 
         if proc.returncode in (254, 255):
-            self.bomb('testbed auxverb failed with exit code %i' % proc.returncode)
+            msg = 'testbed auxverb failed with exit code %i' % proc.returncode
+            if out:
+                msg += '\n---- stdout ----\n%s----------------\n' % out
+            if err:
+                msg += '\n---- stderr ----\n%s----------------\n' % err
+            self.bomb(msg)
 
         return (proc.returncode, out, err)
 
-    def check_exec(self, argv, stdout=False, kind='short'):
+    def check_exec(self, argv, stdout=False, kind='short', xenv=[]):
         '''Run argv in testbed.
 
         If stdout is True, capture stdout and return it. Otherwise, don't
@@ -450,6 +464,7 @@ class Testbed:
         argv must succeed and not print any stderr.
         '''
         (code, out, err) = self.execute(argv,
+                                        xenv=xenv,
                                         stdout=(stdout and subprocess.PIPE or None),
                                         stderr=subprocess.PIPE, kind=kind)
         if err:
_______________________________________________
Reproducible-builds mailing list
Reproducible-builds@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/reproducible-builds

Reply via email to