[Piuparts-commits] rev 94 - in trunk: . debian

John Wright jsw at alioth.debian.org
Mon Sep 17 04:13:09 UTC 2007


Author: jsw
Date: 2007-09-17 04:13:09 +0000 (Mon, 17 Sep 2007)
New Revision: 94

Modified:
   trunk/debian/changelog
   trunk/piuparts.docbook
   trunk/piuparts.py
Log:
* Fix the --warn-on-others option.  Now, dependencies are installed before
  the packages we wish to test, and an inventory is taken then, so that we
  can know which errors were caused by the packages explicitly specified on
  the command-line.  Currently, this requires --apt, and doesn't work quite
  as advertised if there are circular dependencies with the packages you wish
  to test (see the man page for more details). (Closes: #440310)

Modified: trunk/debian/changelog
===================================================================
--- trunk/debian/changelog	2007-09-11 00:00:19 UTC (rev 93)
+++ trunk/debian/changelog	2007-09-17 04:13:09 UTC (rev 94)
@@ -4,10 +4,16 @@
     piuparts-master.py and piuparts-slave.py.  Please see the README file,
     piuparts-master.conf.sample and piuparts-slave.conf.sample for more
     details. (Closes: #349365)
+  * Fix the --warn-on-others option.  Now, dependencies are installed before
+    the packages we wish to test, and an inventory is taken then, so that we
+    can know which errors were caused by the packages explicitly specified on
+    the command-line.  Currently, this requires --apt, and doesn't work quite
+    as advertised if there are circular dependencies with the packages you wish
+    to test (see the man page for more details). (Closes: #440310)
   * debian/control:
     - Update my email address in the Uploaders field
 
- -- John Wright <jsw at debian.org>  Tue, 04 Sep 2007 19:19:52 -0600
+ -- John Wright <jsw at debian.org>  Sun, 16 Sep 2007 21:49:01 -0600
 
 piuparts (0.26) unstable; urgency=low
 

Modified: trunk/piuparts.docbook
===================================================================
--- trunk/piuparts.docbook	2007-09-11 00:00:19 UTC (rev 93)
+++ trunk/piuparts.docbook	2007-09-17 04:13:09 UTC (rev 94)
@@ -265,8 +265,8 @@
                 <listitem>
                 
                     <para>Print a warning rather than failing if files are 
-                    left behind by a package that was not given on
-                    the command-line.</para>
+                    left behind, modified, or removed by a package that was
+                    not given on the command-line.</para>
 
                     <para>This way, you can basically isolate the purge test
                     to your own packages.  If a package that is brought in as
@@ -274,13 +274,10 @@
                     fail because of it (but a warning message will be
                     printed).</para>
 
-                    <para>This does have its shortcomings:  if a file is left
-                    behind that doesn't seem to belong to any package, using
-                    this option will make that a warning rather than a failure.
-                    In my opinion, this is an acceptable risk -- the offending
-                    file would still be output (as a warning), and the
-                    developer would hopefully know which files belong to their
-                    package.</para>
+                    <para>Currently, this option only works in conjunction
+                    with <option>--apt</option>, and errors in packages
+                    with circular dependencies with specified packages will
+                    be treated as errors rather than warnings.</para>
 
                 </listitem>
             

Modified: trunk/piuparts.py
===================================================================
--- trunk/piuparts.py	2007-09-11 00:00:19 UTC (rev 93)
+++ trunk/piuparts.py	2007-09-17 04:13:09 UTC (rev 94)
@@ -907,6 +907,18 @@
     return pkgset
 
 
+def prune_files_list(files, depsfiles):
+    """Remove elements from 'files' that are in 'depsfiles', and return the
+    list of removed elements.
+    """
+    warn = []
+    for file in depsfiles:
+        if file in files:
+            files.remove(file)
+            warn.append(file)
+    return warn
+
+
 def diff_selections(chroot, selections):
     """Compare original and current package selection.
        Return dict where dict[package_name] = original_status, that is,
@@ -934,25 +946,35 @@
     return list
 
 
-def check_results(chroot, root_info, file_owners, packages=[]):
-    """Check that current chroot state matches 'root_info'."""
-    if settings.warn_on_others:
-        pkgset = sets.Set(packages)
+def check_results(chroot, root_info, file_owners, deps_info=None):
+    """Check that current chroot state matches 'root_info'.
+    
+    If settings.warn_on_others is True and deps_info is not None, then only
+    print a warning rather than failing if the current chroot contains files
+    that are in deps_info but not in root_info.  (In this case, deps_info
+    should be the result of chroot.save_meta_data() right after the
+    dependencies are installed, but before the actual packages to test are
+    installed.)
+    """
+
+    current_info = chroot.save_meta_data()
+    if settings.warn_on_others and deps_info is not None:
+        (new, removed, modified) = diff_meta_data(root_info, current_info)
+        (depsnew, depsremoved, depsmodified) = diff_meta_data(root_info,
+                                                              deps_info)
+
+        warnnew = prune_files_list(new, depsnew)
+        warnremoved = prune_files_list(removed, depsremoved)
+        warnmodified = prune_files_list(modified, depsmodified)
+
     else:
-        pkgset = sets.Set()
-    current_info = chroot.save_meta_data()
-    (new, removed, modified) = diff_meta_data(root_info, current_info)
+        (new, removed, modified) = diff_meta_data(root_info, current_info)
+
     ok = True
     if new:
-        msg = "Package purging left files on system:\n" + \
-              file_list(new, file_owners)
-        offendingset = offending_packages(new, file_owners)
-        if settings.warn_on_others and not offendingset.intersection(pkgset):
-            # Just print a warning since none of our packages caused the error
-            logging.info("Warning: " + msg)
-        else:
-            logging.error(msg)
-            ok = False
+        logging.error("Package purging left files on system:\n" +
+                       file_list(new, file_owners))
+        ok = False
     if removed:
         logging.error("After purging files have disappeared:\n" +
                       file_list(removed, file_owners))
@@ -962,6 +984,26 @@
                       file_list(modified, file_owners))
         ok = False
 
+    if ok and settings.warn_on_others and deps_info is not None:
+        if warnnew:
+            msg = ("Warning: Package puring left files on system:\n" +
+                   file_list(warnnew, file_owners) + \
+                   "These files seem to have been left by dependencies rather "
+                   "than by packages\nbeing explicitly tested.\n")
+            logging.info(msg)
+        if warnremoved:
+            msg = ("After purging files have dissappeared:\n" +
+                   file_list(warnremoved, file_owners) +
+                   "This seems to have been caused by dependencies rather "
+                   "than by packages\nbbeing explicitly tested.\n")
+            logging.info(msg)
+        if warnmodified:
+            msg = ("After purging files have been modified:\n" +
+                   file_list(warnmodified, file_owners) +
+                   "This seems to have been caused by dependencies rather "
+                   "than by packages\nbbeing explicitly tested.\n")
+            logging.info(msg)
+
     return ok
 
 
@@ -975,6 +1017,20 @@
     if args:
         chroot.install_package_files(args)
     else:
+        if settings.warn_on_others:
+            # First install only the dependencies.  We do this by giving
+            # apt-get the list of packages to install, and following that list
+            # with the same packages with '-' appended to the end.  Then,
+            # apt-get ensures that the packages never actually get installed,
+            # but kindly installs their dependencies for us.  Once we've got
+            # the dependencies installed, save the file ownership information
+            # so we can tell which modifications were caused by the actual
+            # packages we are testing, rather than by their dependencies.
+            chroot.install_packages_by_name(packages +
+                                            ["%s-" % p for p in packages])
+            deps_info = chroot.save_meta_data()
+        else:
+            deps_info = None
         chroot.install_packages_by_name(packages)
         chroot.run(["apt-get", "clean"])
 
@@ -991,7 +1047,7 @@
     chroot.check_for_broken_symlinks()
     chroot.unmount_proc()
 
-    return check_results(chroot, root_info, file_owners, packages=packages)
+    return check_results(chroot, root_info, file_owners, deps_info=deps_info)
 
 
 def install_upgrade_test(chroot, root_info, selections, args, package_names):
@@ -1196,8 +1252,13 @@
     parser.add_option("--warn-on-others",
                       action="store_true", default=False,
                       help="Print a warning rather than failing if "
-                           "files are left behind by a package that "
-                           "was not given on the command-line.")
+                           "files are left behind, modified, or removed "
+                           "by a package that was not given on the "
+                           "command-line.  This currently only works in "
+                           "conjunction with --apt, and errors in "
+                           "packages with circular dependencies with "
+                           "specified packages will be treated as "
+                           "errors rather than warnings.")
 			   
     parser.add_option("--skip-minimize", 
                       action="store_true", default=False,
@@ -1317,6 +1378,11 @@
         logging.error("--keep-sources-list only makes sense with --basetgz "
                       "and only one distribution")
         exit = 1
+
+    if settings.warn_on_others and settings.args_are_package_files:
+        logging.error("--warn-on-others currently only works in conjunction "
+                      "with --apt")
+        exit = 1
     
     if not args:
         logging.error("Need command line arguments: " +




More information about the Piuparts-commits mailing list