[Piuparts-devel] [Git][debian/piuparts][develop] 4 commits: piuparts: detect files moving between / and /usr on upgrade

Nicolas Dandrimont (@olasd) gitlab at salsa.debian.org
Wed Jul 13 19:10:03 BST 2022



Nicolas Dandrimont pushed to branch develop at Debian / piuparts


Commits:
e7538053 by Luca Boccassi at 2022-07-13T10:36:42+01:00
piuparts: detect files moving between / and /usr on upgrade

The TC issued a recommendation to avoid moving files between
bin|sbin|lib* and /usr/bin|sbin|lib* for the Bookworm cycle.
Detect such moves and raise an error.

For more details see:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=994388#80

- - - - -
4489752a by Luca Boccassi at 2022-07-13T10:36:42+01:00
piuparts: detect files moving between / and /usr on dist-upgrade

Same as the check on package install/upgrade, but across distribution
dist-upgrades.

- - - - -
59c00b8b by Luca Boccassi at 2022-07-13T10:36:42+01:00
p.conf: enable --warn-on-usr-move for bullseye/bookworm

- - - - -
92c1faff by Nicolas Dandrimont at 2022-07-13T20:06:49+02:00
Merge branch 'usrmerge' into develop

piuparts: detect files moving between / and /usr on upgrade

See merge request https://salsa.debian.org/debian/piuparts/-/merge_requests/37

- - - - -


6 changed files:

- docs/piuparts/piuparts.1.txt
- instances/piuparts.conf-template.pejacevic
- + known_problems/file_moved_usr_error.conf
- + known_problems/file_moved_usr_issue.conf
- piuparts-report.py
- piuparts.py


Changes:

=====================================
docs/piuparts/piuparts.1.txt
=====================================
@@ -330,6 +330,10 @@ Options must come before the other command line arguments.
 
   Behavior with multiple packages given on the command-line could be problematic, particularly if the dependency tree of one package in the list includes another in the list. Therefore, it is recommended to use this option with one package at a time.
 
+*-*-warn-on-usr-move*='disabled|warn|fail'::
+  Whether to enable the test (with a warning or a failure) that checks if files are moved between /{bin|sbin|lib*} and /usr/{bin|sbin|lib*}.
+  Accepted values: 'disabled' (default), 'warn', 'fail'.
+
 
 
 EXAMPLES


=====================================
instances/piuparts.conf-template.pejacevic
=====================================
@@ -40,15 +40,18 @@ flags-end-oldstable = %(flags-end-buster)s
 
 # common flags for tests starting in bookworm
 flags-start-bookworm =
-# no flags needed
+# https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=994388#80
+	--warn-on-usr-move fail
 
 # common flags for tests ending in bookworm
 flags-end-bookworm =
-# no flags needed
+# https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=994388#80
+	--warn-on-usr-move fail
 
 # common flags for tests starting in bullseye
 flags-start-bullseye =
-# no flags needed
+# https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=994388#80
+	--warn-on-usr-move fail
 
 # common flags for tests ending in bullseye
 flags-end-bullseye =


=====================================
known_problems/file_moved_usr_error.conf
=====================================
@@ -0,0 +1,13 @@
+# detect a file moving between bin/sbin/lib* and usr/bin|sbin|lib*
+#
+PATTERN='(WARN|FAIL): File\(s\) moved between /.* and /usr/.*:'
+WHERE='fail bugged affected'
+ISSUE=0
+HEADER='File(s) moved between /{bin|sbin|lib*} and /usr/{bin|sbin|lib*}'
+HELPTEXT='
+<p>
+The Technical Committee recommended against moving files between between /{bin|sbin|lib*}
+and /usr/{bin|sbin|lib*} during the Bookworm development cycle. See
+<a href="https://bugs.debian.org/994388#80">Debian bug #994388.</a>
+</p>
+'


=====================================
known_problems/file_moved_usr_issue.conf
=====================================
@@ -0,0 +1,13 @@
+# detect a file moving between bin/sbin/lib* and usr/bin|sbin|lib*
+#
+PATTERN='(WARN|FAIL): File\(s\) moved between /.* and /usr/.*:'
+WHERE='pass'
+ISSUE=1
+HEADER='File(s) moved between /{bin|sbin|lib*} and /usr/{bin|sbin|lib*}'
+HELPTEXT='
+<p>
+The Technical Committee recommended against moving files between between /{bin|sbin|lib*}
+and /usr/{bin|sbin|lib*} during the Bookworm development cycle. See
+<a href="https://bugs.debian.org/994388#80">Debian bug #994388.</a>
+</p>
+'


=====================================
piuparts-report.py
=====================================
@@ -501,6 +501,8 @@ linktarget_by_template = [
     ("missing_md5sums_error.tpl", "...and logfile reports missing md5sums"),
     ("unowned_lib_symlink_error.tpl", "...and logfile reports unowned lib symlinks"),
     ("piuparts-depends-dummy_error.tpl", "...and logfile reports piuparts-depends-dummy.deb could not be installed"),
+    ("file_moved_usr_error,tpl", "...and logfile reports a file moved between /{bin|sbin|lib*} and /usr/{bin|sbin|lib*}"),
+    ("file_moved_usr_issue,tpl", "but logfile reports a file moved between /{bin|sbin|lib*} and /usr/{bin|sbin|lib*}"),
     ("unclassified_failures.tpl", "due to unclassified failures"),
 ]
 


=====================================
piuparts.py
=====================================
@@ -53,6 +53,7 @@ import traceback
 import uuid
 import apt_pkg
 import pipes
+import pathlib
 from collections import namedtuple
 from signal import alarm, signal, SIGALRM, SIGTERM, SIGKILL
 
@@ -220,6 +221,7 @@ class Settings:
         self.warn_on_debsums_errors = False
         self.warn_on_install_over_symlink = False
         self.warn_if_inadequate = True
+        self.warn_on_usr_move = "disabled"
         self.pedantic_purge_test = False
         self.ignored_files = [
             # /root/.rnd should *not* be listed here, see #750099
@@ -1833,6 +1835,45 @@ class Chroot:
                 return True
         return False
 
+    def check_files_moved_usr(self, packages=[], files_before={}, files_after={}, warn_only=None):
+        """Check that no files were moved from /{bin|sbin|lib*} and /usr/{bin|sbin|lib*}"""
+
+        if settings.warn_on_usr_move == "disabled":
+            return
+
+        # For each path that is a file in each package, check that it did not move between
+        # /bin, /sbin or /lib* to the corresponding location under /usr, and viceversa.
+        # See: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=994388#80
+        # If a file moved in a package that we are inspecting, print an error by default.
+        # Otherwise, print a warning.
+        broken = []
+        for old_path in files_before:
+            # '/' is a separate element in the parts list
+            old_path_parts = pathlib.Path(old_path).parts
+            if len(old_path_parts) < 3:
+                continue
+
+            if old_path_parts[1] == 'usr' and (old_path_parts[2] in ['bin', 'sbin'] or old_path_parts[2].startswith('lib')):
+                new_path = os.path.join("/", *old_path_parts[2:])
+            elif old_path_parts[1] in ['bin', 'sbin'] or old_path_parts[1].startswith('lib'):
+                new_path = "/usr" + old_path
+            else:
+                continue
+
+            # Skip over directories, multiple packages can ship files in the same directories
+            if new_path in files_after and os.path.isfile(self.relative(new_path)):
+                broken.append("%s %s => %s %s" % (old_path, files_before[old_path], new_path, files_after[new_path]))
+
+        if broken:
+            if settings.warn_on_usr_move == "warn" or warn_only:
+                logging.warning("WARN: File(s) moved between /{bin|sbin|lib*} and /usr/{bin|sbin|lib*}: %s" % indent_string("\n".join(broken)))
+            else:
+                logging.error("FAIL: File(s) moved between /{bin|sbin|lib*} and /usr/{bin|sbin|lib*}: %s" % indent_string("\n".join(broken)))
+                panic()
+        else:
+            logging.debug("No file moved between /{bin|sbin|lib*} and /usr/{bin|sbin|lib*}.")
+
+
     def check_for_broken_symlinks(self, warn_only=None, file_owners={}):
         """Check that all symlinks in chroot are non-broken."""
         if not settings.check_broken_symlinks:
@@ -2456,6 +2497,9 @@ def install_upgrade_test(chroot, chroot_state, package_files, packages, old_pack
     chroot.check_for_no_processes()
     chroot.check_for_broken_symlinks()
 
+    if settings.warn_on_usr_move != "disabled":
+        file_owners_before = chroot.get_files_owned_by_packages()
+
     if settings.install_remove_install:
         chroot.remove_packages(packages, ignore_errors=True)
 
@@ -2468,10 +2512,12 @@ def install_upgrade_test(chroot, chroot_state, package_files, packages, old_pack
 
     chroot.disable_testdebs_repo()
 
-    file_owners = chroot.get_files_owned_by_packages()
+    file_owners_after = chroot.get_files_owned_by_packages()
 
     chroot.check_for_no_processes()
-    chroot.check_for_broken_symlinks(file_owners=file_owners)
+    chroot.check_for_broken_symlinks(file_owners=file_owners_after)
+    if settings.warn_on_usr_move != "disabled":
+        chroot.check_files_moved_usr(packages, file_owners_before, file_owners_after)
 
     # Remove all packages from the chroot that weren't there initially.
     chroot.restore_selections(chroot_state, packages)
@@ -2479,9 +2525,9 @@ def install_upgrade_test(chroot, chroot_state, package_files, packages, old_pack
     chroot.run_scripts("post_test")
 
     chroot.check_for_no_processes(fail=True)
-    chroot.check_for_broken_symlinks(file_owners=file_owners)
+    chroot.check_for_broken_symlinks(file_owners=file_owners_after)
 
-    return check_results(chroot, chroot_state, file_owners)
+    return check_results(chroot, chroot_state, file_owners_after)
 
 
 def save_meta_data(filename, chroot_state):
@@ -2610,6 +2656,9 @@ def install_and_upgrade_between_distros(package_files, packages_qualified):
 
     chroot.check_for_no_processes()
 
+    if settings.warn_on_usr_move != "disabled":
+        file_owners_before = chroot.get_files_owned_by_packages()
+
     os.environ["PIUPARTS_PHASE"] = "distupgrade"
 
     chroot.upgrade_to_distros(settings.debian_distros[1:-1], distupgrade_packages, settings.upgrade_before_dist_upgrade)
@@ -2634,6 +2683,10 @@ def install_and_upgrade_between_distros(package_files, packages_qualified):
 
     chroot.check_for_no_processes()
 
+    if settings.warn_on_usr_move != "disabled":
+        file_owners_after = chroot.get_files_owned_by_packages()
+        chroot.check_files_moved_usr(packages, files_before=file_owners_before, files_after=file_owners_after)
+
     # Remove all packages from the chroot that weren't in the reference chroot.
     chroot.restore_selections(chroot_state, packages_qualified)
 
@@ -3008,6 +3061,10 @@ def parse_command_line():
                       default=False,
                       help="Fail if broken symlinks are detected.")
 
+    parser.add_option("--warn-on-usr-move", action="store", default="disabled",
+                      help="Whether to enable the test (with a warning or a failure) that checks if files are moved "
+                           "between /{bin|sbin|lib*} and /usr/{bin|sbin|lib*}. Accepted values: 'disabled' (default), 'warn', 'fail'.")
+
     parser.add_option("--log-level", action="store", metavar='LEVEL',
                       default="dump",
                       help="Displays messages from LEVEL level, possible values are: error, info, dump, debug. The default is dump.")
@@ -3103,6 +3160,7 @@ def parse_command_line():
     settings.warn_on_debsums_errors = opts.warn_on_debsums_errors
     settings.warn_on_install_over_symlink = opts.warn_on_install_over_symlink
     settings.warn_if_inadequate = not opts.fail_if_inadequate
+    settings.warn_on_usr_move = opts.warn_on_usr_move
     settings.pedantic_purge_test = opts.pedantic_purge_test
     settings.ignored_files += opts.ignore
     settings.ignored_patterns += opts.ignore_regex
@@ -3138,6 +3196,10 @@ def parse_command_line():
             logging.error("Scripts directory is not a directory: %s" % sdir)
             panic()
 
+    if settings.warn_on_usr_move not in ["disabled", "warn", "fail"]:
+        logging.error("--warn-on-usr-move must be one of 'disabled', 'warn', 'fail'")
+        panic()
+
     if not settings.debian_distros:
         settings.debian_distros = defaults.get_distribution()
 



View it on GitLab: https://salsa.debian.org/debian/piuparts/-/compare/190399be209337dec2c67215368b0cb0343ed09d...92c1faff5d44c9ca06cb9dbd5a299378f0d4b9a7

-- 
View it on GitLab: https://salsa.debian.org/debian/piuparts/-/compare/190399be209337dec2c67215368b0cb0343ed09d...92c1faff5d44c9ca06cb9dbd5a299378f0d4b9a7
You're receiving this email because of your account on salsa.debian.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://alioth-lists.debian.net/pipermail/piuparts-devel/attachments/20220713/a6dcfeb1/attachment-0001.htm>


More information about the Piuparts-devel mailing list