[reprotest] 01/02: Make --no-clean-on-error a bit more reliable

[Ximin Luo]

This is an automated email from the git hooks/post-receive script.

infinity0 pushed a commit to branch master
in repository reprotest.

commit 131e91869561aaee3fae21715216781879e041de
Author: Ximin Luo <infinity0 at debian.org>
Date:   Mon Nov 27 13:51:44 2017 +0100

    Make --no-clean-on-error a bit more reliable
---
 debian/TODO                  |  4 ----
 debian/changelog             |  1 +
 reprotest/__init__.py        | 12 ++++++++----
 reprotest/lib/VirtSubproc.py |  9 ++++++---
 4 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/debian/TODO b/debian/TODO
index c226e60..e004833 100644
--- a/debian/TODO
+++ b/debian/TODO
@@ -1,7 +1,3 @@
---no-clean-on-error leaves detached autopkgtest-virt-* processes around,
-amongst other things. either fix, or warn about it.
-
-
 Make the CLI nicer
 ==================
 
diff --git a/debian/changelog b/debian/changelog
index 408603e..5e92116 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -6,6 +6,7 @@ reprotest (0.7.4) UNRELEASED; urgency=medium
   * Properly drop privs when running the build. (Closes: #877813)
   * Fix the time variation to actually make the time constant. This drops the
     time.auto_faketimes variation-spec option.
+  * Make --no-clean-on-error a bit more reliable.
 
  -- Ximin Luo <infinity0 at debian.org>  Fri, 20 Oct 2017 12:33:21 +0200
 
diff --git a/reprotest/__init__.py b/reprotest/__init__.py
index e81f948..d3c3fc9 100644
--- a/reprotest/__init__.py
+++ b/reprotest/__init__.py
@@ -77,6 +77,8 @@ def start_testbed(args, temp_dir, no_clean_on_error=False, host_distro=None):
     # path for the correct virt-server script.
     server_path = get_server_path(args[0])
     logger.info('STARTING VIRTUAL SERVER %r', [server_path] + args[1:])
+    if no_clean_on_error:
+        os.environ["REPROTEST_NO_CLEAN_ON_ERROR"] = "1"
     # TODO: make the user configurable, like autopkgtest
     testbed = Testbed([server_path] + args[1:], temp_dir,
                       getpass.getuser(), host_distro=host_distro)
@@ -94,9 +96,11 @@ def start_testbed(args, temp_dir, no_clean_on_error=False, host_distro=None):
         raise
     finally:
         if should_clean:
-            # TODO: we could probably do *some* level of cleanup even if
-            # should_clean is False; investigate this further...
             testbed.stop()
+        else:
+            # give VirtSubproc a command it doesn't recognise, to force a non-zero Quit
+            # a bit hacky, but it works...
+            testbed.send('quit_with_error')
 
 
 # put build artifacts in ${dist}/source-root, to support tools that put artifacts in ..
@@ -615,8 +619,8 @@ def cli_parser():
         'expression. Default: %(default)s')
     group3.add_argument('--no-clean-on-error', action='store_true', default=False,
         help='Don\'t clean the virtual_server if there was an error. '
-        'Useful for debugging, but WARNING: this is currently not '
-        'implemented very well and may leave cruft on your system.')
+        'Useful for debugging but will leave cruft on your system depending on '
+        'the virtual_server used; we hint about some but there may be others.')
     group3.add_argument('--dry-run', action='store_true', default=False,
         help='Don\'t run the builds, just print what would happen.')
     group3.add_argument('--print-sudoers', action='store_true', default=False,
diff --git a/reprotest/lib/VirtSubproc.py b/reprotest/lib/VirtSubproc.py
index d1c6465..5605433 100644
--- a/reprotest/lib/VirtSubproc.py
+++ b/reprotest/lib/VirtSubproc.py
@@ -674,11 +674,14 @@ def cleanup():
         downtmp = None
 
 
-def error_cleanup():
+def error_cleanup(q=None):
     try:
         ok = False
         try:
-            cleanup()
+            if (q and not q.ec) or os.getenv("REPROTEST_NO_CLEAN_ON_ERROR", "") != "1":
+                cleanup()
+            else:
+                sys.stderr.write('Not cleaning up because REPROTEST_NO_CLEAN_ON_ERROR=1\n')
             ok = True
         except Quit as q:
             sys.stderr.write(q.m)
@@ -716,7 +719,7 @@ def mainloop():
         while True:
             command()
     except Quit as q:
-        error_cleanup()
+        error_cleanup(q)
         if q.m:
             sys.stderr.write(q.m)
             sys.stderr.write('\n')

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/reproducible/reprotest.git