Escape weird paths generated by %pyproject_save_files

pyproject-rpm-macros.spec

@@ -104,6 +104,10 @@ export HOSTNAME="rpmbuild"  # to speedup tox in network-less mock, see rhbz#1856
 %license LICENSE
 
 %changelog
+* Fri Jul 09 2021 Miro Hrončok <miro@hroncok.cz> - 0-44
+- Escape weird paths generated by %%pyproject_save_files
+- Fixes rhbz#1976363
+
 * Thu Jul 01 2021 Tomas Hrnciar <thrnciar@redhat.com> - 0-43
 - Generate BuildRequires from file
 - Fixes: rhbz#1936448

pyproject_save_files.py

@@ -7,6 +7,10 @@ from collections import defaultdict
 from pathlib import PosixPath, PurePosixPath
 
 
+# From RPM's build/files.c strtokWithQuotes delim argument
+RPM_FILES_DELIMETERS = ' \n\t'
+
+
 class BuildrootPath(PurePosixPath):
     """
     This path represents a path in a buildroot.
@@ -224,6 +228,53 @@ def classify_paths(
     return paths
 
 
+def escape_rpm_path(path):
+    """
+    Escape special characters in string-paths or BuildrootPaths
+
+    E.g. a space in path otherwise makes RPM think it's multiple paths,
+    unless we put it in "quotes".
+    Or a literal % symbol in path might be expanded as a macro if not escaped.
+
+    Due to limitations in RPM, paths with spaces and double quotes are not supported.
+
+    Examples:
+
+        >>> escape_rpm_path(BuildrootPath('/usr/lib/python3.9/site-packages/setuptools'))
+        '/usr/lib/python3.9/site-packages/setuptools'
+
+        >>> escape_rpm_path('/usr/lib/python3.9/site-packages/setuptools/script (dev).tmpl')
+        '"/usr/lib/python3.9/site-packages/setuptools/script (dev).tmpl"'
+
+        >>> escape_rpm_path('/usr/share/data/100%valid.path')
+        '/usr/share/data/100%%%%%%%%valid.path'
+
+        >>> escape_rpm_path('/usr/share/data/100 % valid.path')
+        '"/usr/share/data/100 %%%%%%%% valid.path"'
+
+        >>> escape_rpm_path('/usr/share/data/1000 %% valid.path')
+        '"/usr/share/data/1000 %%%%%%%%%%%%%%%% valid.path"'
+
+        >>> escape_rpm_path('/usr/share/data/spaces and "quotes"')
+        Traceback (most recent call last):
+          ...
+        NotImplementedError: ...
+    """
+    orig_path = path = str(path)
+    if "%" in path:
+        # Escaping by 8 %s has been verified in RPM 4.16 and 4.17, but probably not stable
+        # See this thread http://lists.rpm.org/pipermail/rpm-list/2021-June/002048.html
+        # On the CI, we build tests/escape_percentages.spec to verify this assumption
+        path = path.replace("%", "%" * 8)
+    if any(symbol in path for symbol in RPM_FILES_DELIMETERS):
+        if '"' in path:
+            # As far as we know, RPM cannot list such file individually
+            # See this thread http://lists.rpm.org/pipermail/rpm-list/2021-June/002048.html
+            raise NotImplementedError(f'" symbol in path with spaces is not supported by %pyproject_save_files: {orig_path!r}')
+        return f'"{path}"'
+    return path
+
+
 def generate_file_list(paths_dict, module_globs, include_others=False):
     """
     This function takes the classified paths_dict and turns it into lines
@@ -238,16 +289,16 @@ def generate_file_list(paths_dict, module_globs, include_others=False):
     files = set()
 
     if include_others:
-        files.update(f"{p}" for p in paths_dict["other"]["files"])
+        files.update(f"{escape_rpm_path(p)}" for p in paths_dict["other"]["files"])
         try:
             for lang_code in paths_dict["lang"][None]:
-                files.update(f"%lang({lang_code}) {path}" for path in paths_dict["lang"][None][lang_code])
+                files.update(f"%lang({lang_code}) {escape_rpm_path(p)}" for p in paths_dict["lang"][None][lang_code])
         except KeyError:
             pass
 
-    files.update(f"{p}" for p in paths_dict["metadata"]["files"])
+    files.update(f"{escape_rpm_path(p)}" for p in paths_dict["metadata"]["files"])
     for macro in "dir", "doc", "license":
-        files.update(f"%{macro} {p}" for p in paths_dict["metadata"][f"{macro}s"])
+        files.update(f"%{macro} {escape_rpm_path(p)}" for p in paths_dict["metadata"][f"{macro}s"])
 
     modules = paths_dict["modules"]
     done_modules = set()
@@ -259,12 +310,12 @@ def generate_file_list(paths_dict, module_globs, include_others=False):
                 if name not in done_modules:
                     try:
                         for lang_code in paths_dict["lang"][name]:
-                            files.update(f"%lang({lang_code}) {path}" for path in paths_dict["lang"][name][lang_code])
+                            files.update(f"%lang({lang_code}) {escape_rpm_path(p)}" for p in paths_dict["lang"][name][lang_code])
                     except KeyError:
                         pass
                     for module in modules[name]:
-                        files.update(f"%dir {p}" for p in module["dirs"])
+                        files.update(f"%dir {escape_rpm_path(p)}" for p in module["dirs"])
-                        files.update(f"{p}" for p in module["files"])
+                        files.update(f"{escape_rpm_path(p)}" for p in module["files"])
                     done_modules.add(name)
                 done_globs.add(glob)
 

tests/escape_percentages.spec

@@ -0,0 +1,25 @@
+Name:           escape_percentages
+Version:        0
+Release:        0
+Summary:        ...
+License:        MIT
+BuildArch:      noarch
+
+%description
+This spec file verifies that escaping percentage signs in paths is possible via
+exactly 8 percentage signs in a filelist and directly in the %%files section.
+It serves as a regression test for pyproject_save_files:escape_rpm_path().
+When this breaks, the function needs to be adapted.
+
+%install
+# the paths on disk will have 1 percentage sign if we type 2 in the spec
+# we use the word 'version' after the sign, as that is a known existing macro
+touch '%{buildroot}/one%%version'
+touch '%{buildroot}/two%%version'
+
+# the filelist will contain 8 percentage signs when we type 16 in spec
+echo '/one%%%%%%%%%%%%%%%%version' > filelist
+test $(wc -c filelist | cut -f1 -d' ') -eq 20  # 8 signs + /one (4) + version (7) + newline (1)
+
+%files -f filelist
+/two%%%%%%%%version

tests/python-setuptools.spec

@@ -59,11 +59,6 @@ sed -i pytest.ini -e 's/ --flake8//' \
 rm -rf %{buildroot}%{python3_sitelib}/pkg_resources/tests/
 sed -i '/tests/d' %{pyproject_files}
 
-# Paths with spaces are not properly protected by %%pyproject_save_files
-# https://bugzilla.redhat.com/show_bug.cgi?id=1976363
-# This workaround will most likely break once fixed
-sed -Ei 's|/(.+) (.+)|"/\1 \2"|' %{pyproject_files}
-
 
 %check
 # https://github.com/pypa/setuptools/discussions/2607

tests/tests.yml

@@ -79,6 +79,9 @@
     - fake_requirements:
         dir: .
         run: ./mocktest.sh fake-requirements
+    - escape_percentages:
+        dir: .
+        run: rpmbuild -ba escape_percentages.spec
     required_packages:
     - mock
     - rpmdevtools