feat: allow slashes in partition names (#1002)

Signed-off-by: Paul Mars <[email protected]>

craft_parts/utils/partition_utils.py

@@ -15,21 +15,23 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 """Unit tests for partition utilities."""
 
+import itertools
 import re
 from collections.abc import Iterable, Sequence
 from pathlib import Path
 
 from craft_parts import errors, features
 
-VALID_PARTITION_REGEX = re.compile(r"(?!-)[a-z0-9-]+(?<!-)", re.ASCII)
-VALID_NAMESPACE_REGEX = re.compile(r"[a-z0-9]+", re.ASCII)
+# Allow alphanumeric characters, hyphens and slashes, not starting or ending
+# with a hyphen or a slash
+VALID_PARTITION_REGEX = re.compile(r"(?!-|/)[a-z0-9-/]+(?<!-|/)", re.ASCII)
 VALID_NAMESPACED_PARTITION_REGEX = re.compile(
-    VALID_NAMESPACE_REGEX.pattern + r"/" + VALID_PARTITION_REGEX.pattern, re.ASCII
+    r"[a-z0-9]+/" + VALID_PARTITION_REGEX.pattern, re.ASCII
 )
 
 PARTITION_INVALID_MSG = (
     "Partitions must only contain lowercase letters, numbers,"
-    "and hyphens, and may not begin or end with a hyphen."
+    "hyphens and slashes, and may not begin or end with a hyphen or a slash."
 )
 
 
@@ -39,7 +41,7 @@ def validate_partition_names(partitions: Sequence[str] | None) -> None:
     If the partition feature is enabled, then:
       - the first partition must be "default"
       - each partition name must contain only lowercase alphanumeric characters
-        and hyphens, but not begin or end with a hyphen
+        hyphens and slashes, but not begin or end with a hyphen or a slash
       - partitions are unique
 
     Namespaced partitions can also be validated in addition to regular (or
@@ -71,8 +73,7 @@ def validate_partition_names(partitions: Sequence[str] | None) -> None:
         raise errors.FeatureError("Partitions must be unique.")
 
     _validate_partition_naming_convention(partitions)
-
-    _validate_namespace_conflicts(partitions)
+    _validate_partitions_conflicts(partitions)
 
 
 def _is_valid_partition_name(partition: str) -> bool:
@@ -103,9 +104,7 @@ def _validate_partition_naming_convention(partitions: Sequence[str]) -> None:
     :raises FeatureError: if a partition name is not valid
     """
     for partition in partitions:
-        if _is_valid_partition_name(partition) or _is_valid_namespaced_partition_name(
-            partition
-        ):
+        if _is_valid_namespaced_partition_name(partition):
             continue
 
         if "/" in partition:
@@ -118,58 +117,98 @@ def _validate_partition_naming_convention(partitions: Sequence[str]) -> None:
                 ),
             )
 
+        if _is_valid_partition_name(partition):
+            continue
         raise errors.FeatureError(
             message=f"Partition {partition!r} is invalid.",
             details=PARTITION_INVALID_MSG,
         )
 
 
-def _validate_namespace_conflicts(partitions: Sequence[str]) -> None:
-    """Validate conflicts between regular partitions and namespaces.
+def _validate_partitions_conflicts(partitions: Sequence[str]) -> None:
+    """Validate conflicts between partitions.
 
-    For example, `foo` conflicts in ['default', 'foo', 'foo/bar'].
-    Assumes partition names are valid.
+    Assumes partition names are valid and unique.
 
-    :raises FeatureError: for namespace conflicts
+    :raises FeatureError: for conflicts
     """
-    namespaced_partitions: set[str] = set()
-    regular_partitions: set[str] = set()
+    conflicting_partitions = _detect_conflicts(partitions)
 
-    # sort partitions
-    for partition in partitions:
-        if _is_valid_partition_name(partition):
-            regular_partitions.add(partition)
-        else:
-            namespaced_partitions.add(partition)
-
-    for regular_partition in regular_partitions:
-        for namespaced_partition in namespaced_partitions:
-            if namespaced_partition.startswith(regular_partition + "/"):
-                raise errors.FeatureError(
-                    f"Partition {regular_partition!r} conflicts with the namespace of "
-                    f"partition {namespaced_partition!r}"
-                )
-
-    # At this point we know that any remaining conflicts will be overlaps
-    # caused by hyphens and namespaces.  For example, "foo-bar" and "foo/bar"
-    # would both result in environment variable FOO_BAR.
-    underscored_partitions = {}
-    for partition in partitions:
-        underscored = partition.replace("-", "_").replace("/", "_")
-        if underscored not in underscored_partitions:
-            underscored_partitions[underscored] = partition
-            continue
+    if not conflicting_partitions:
+        return
 
-        # Collision.  Figure out which is which so we can raise a good error message.
-        namespaced_partition = underscored_partitions[underscored]
-        hyphenated_partition = partition
-        if "/" in partition:
-            namespaced_partition = partition
-            hyphenated_partition = underscored_partitions[underscored]
-        raise errors.FeatureError(
-            f"Namespaced partition {namespaced_partition!r} conflicts with hyphenated "
-            f"partition {hyphenated_partition!r}."
-        )
+    # Raise the full list of conflicts
+    lines: list[str] = []
+    for conflicts in conflicting_partitions:
+        conflict_list = list(conflicts)
+        conflict_list.sort()
+        lines.append(f"- {str(conflict_list)[1:-1]}")
+
+    msg = "\n".join(lines)
+
+    raise errors.FeatureError(
+        message=f"Partition name conflicts:\n{msg}",
+        details="Hyphens and slashes are converted to underscores to associate partitions names with environment variables. 'foo-bar' and 'foo/bar' would result in environment variable FOO_BAR.",
+    )
+
+
+def _detect_conflicts(partitions: Sequence[str]) -> list[set[str]]:
+    """Detect and return every conflicts between partitions.
+
+    Rules:
+      1: partition name must not conflict with namespace (or sub-namespace) names
+        - `foo` and 'foo/bar' conflict
+        - `foo/bar` and 'foo/bar/baz' conflict
+      2: environment variables derived from partition names must not conflicts
+        - `foo/bar-baz` and `foo/bar/baz` conflict (would be converted to FOO_BAR_BAZ)
+        - `foo/bar/baz-qux` and `foo/bar-baz/qux` conflict (would be converted to FOO_BAR_BAZ_QUX)
+
+    """
+    conflict_sets: list[set[str]] = []
+    env_var_translation = {ord("-"): "_", ord("/"): "_"}
+
+    for candidate_partition, partition in itertools.combinations(partitions, 2):
+        candidate_underscored = candidate_partition.translate(env_var_translation)
+        underscored = partition.translate(env_var_translation)
+
+        if (
+            _namespace_conflicts(candidate_partition, partition)
+            or candidate_underscored == underscored
+        ):
+            _register_conflict(conflict_sets, partition, candidate_partition)
+
+    return conflict_sets
+
+
+def _register_conflict(
+    conflict_sets: list[set[str]], partition: str, candidate_partition: str
+) -> None:
+    """Register the conflict, avoiding duplicates.
+
+    If the partition or the one it conflicts with is already in a known set of
+    conflicts, expand this set.
+    Otherwise, add a new set in the conflicts list.
+    """
+    new_conflict_set = {partition, candidate_partition}
+
+    for conflict_set in conflict_sets:
+        if partition in conflict_set or candidate_partition in conflict_set:
+            conflict_set.update(new_conflict_set)
+            break
+    else:
+        conflict_sets.append(new_conflict_set)
+
+
+def _namespace_conflicts(a: str, b: str) -> bool:
+    """Split candidates with slashes and check if the shorter one is a subset of the other."""
+    separator = "/"
+    a_list = a.split(separator)
+    b_list = b.split(separator)
+
+    if len(a_list) > len(b_list):
+        return a_list[: len(b_list)] == b_list
+
+    return b_list[: len(a_list)] == a_list
 
 
 def get_partition_dir_map(

craft_parts/utils/path_utils.py

@@ -15,6 +15,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 """Utility functions for paths."""
+
 import re
 from pathlib import PurePath
 from typing import NamedTuple, TypeVar
@@ -27,13 +28,7 @@
 
 # regex for a path beginning with a (partition), like "(boot)/bin/sh"
 HAS_PARTITION_REGEX = re.compile(
-    r"^\(" + partition_utils.VALID_PARTITION_REGEX.pattern + r"\)(/.*)?$"
-)
-
-# regex for a path beginning with a namespaced partition, like "(a/boot)/bin/sh"
-# Note that unlike HAS_PARTITION_REGEX, this one captures both namespaced partition and path.
-HAS_NAMESPACED_PARTITION_REGEX = re.compile(
-    r"^(\(" + partition_utils.VALID_NAMESPACED_PARTITION_REGEX.pattern + r"\))(/.*)?$"
+    r"^(\(" + partition_utils.VALID_PARTITION_REGEX.pattern + r"\))(/.*)?$"
 )
 
 
@@ -46,10 +41,7 @@ class PartitionPathPair(NamedTuple):
 
 def _has_partition(path: PurePath | str) -> bool:
     """Check whether a path has an explicit partition."""
-    return bool(
-        HAS_PARTITION_REGEX.match(str(path))
-        or HAS_NAMESPACED_PARTITION_REGEX.match(str(path))
-    )
+    return bool(HAS_PARTITION_REGEX.match(str(path)))
 
 
 def get_partition_and_path(path: FlexiblePath) -> PartitionPathPair:
@@ -85,17 +77,7 @@ def _split_partition_and_inner_path(str_path: str) -> tuple[str, str]:
 
     :raises FeatureError: If `str_path` does not begin with a partition.
     """
-    # split regular partitions
-    if re.match(HAS_PARTITION_REGEX, str_path):
-        if "/" in str_path:
-            # split into partition and inner_path
-            partition, inner_path = str_path.split("/", maxsplit=1)
-            # remove extra forward slashes between the partition and inner path
-            return partition, inner_path.lstrip("/")
-        return str_path, ""
-
-    # split namespaced partitions
-    match = re.match(HAS_NAMESPACED_PARTITION_REGEX, str_path)
+    match = re.match(HAS_PARTITION_REGEX, str_path)
 
     if not match:
         raise FeatureError(f"Filepath {str_path!r} does not begin with a partition.")

docs/common/craft-parts/craft-parts.wordlist.txt

@@ -1,4 +1,8 @@
 ARGS
+FOO
+BAR
+BAZ
+QUX
 ActionProperties
 ActionType
 AfterValidator
@@ -307,6 +311,8 @@ artifacts
 autogen
 autotools
 backend
+bar
+baz
 behavior
 bool
 boolean
@@ -432,6 +438,7 @@ pypi
 pyproject
 qmake
 qualname
+qux
 readthedocs
 recognized
 reorganize

docs/reference/changelog.rst

@@ -2,9 +2,13 @@
 Changelog
 *********
 
-X.X.X (2025-MM-DD)
+2.6.0 (2025-MM-DD)
 ------------------
 
+New features:
+
+- Partition names can include slashes.
+
 Bug fixes:
 
 - Allow for a non-specific system Python interpreter when using the

tests/unit/features/partitions/test_lifecycle_manager.py

@@ -14,6 +14,7 @@
 # You should have received a copy of the GNU Lesser General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 """Unit tests for the lifecycle manager with the partitions feature."""
+
 import sys
 import textwrap
 from string import ascii_lowercase, digits
@@ -295,8 +296,7 @@ def test_partitions_conflicts_with_namespace(self, new_dir, parts_data, partitio
         with pytest.raises(
             errors.FeatureError,
             match=(
-                r"Partition '[\w-]*' conflicts with the "
-                r"namespace of partition '[\w/-]*'.*"
+                r"Partition name conflicts:\n[\w\s/\-,']*\nHyphens and slashes are converted to underscores to associate partitions names with environment variables. 'foo-bar' and 'foo/bar' would result in environment variable FOO_BAR.\nThis operation cannot be executed"
             ),
         ):
             lifecycle_manager.LifecycleManager(

tests/unit/features/partitions/test_parts.py

@@ -448,7 +448,9 @@ def test_part_invalid_partition_usage_complex(
             no path specified after partition in '(foo/bar-baz)/'
             unknown partition 'foo/bar-baz' in '(foo/bar-baz)/test'
             unknown partition 'foo-bar/baz' in '(foo-bar/baz)'
+            no path specified after partition in '(foo-bar/baz)'
             unknown partition 'foo-bar/baz' in '(foo-bar/baz)/'
+            no path specified after partition in '(foo-bar/baz)/'
             unknown partition 'foo-bar/baz' in '(foo-bar/baz)/test'
             unknown partition '123' in '(123)/test'
             unknown partition '123' in '(123)/test/(456)'

tests/unit/utils/test_partition_utils.py

@@ -50,10 +50,17 @@ def test_validate_partitions_failure_feature_disabled(partitions, message):
         ["default", "mypart"],
         ["default", "mypart1"],
         ["default", "my-part"],
+        ["default", "mypart", "mypart2"],
+        ["default", "mypart2", "mypart"],
         ["default", "mypart", "test/foo"],
+        ["default", "mypart", "test/mypart"],
         ["default", "mypart", "test1/foo2"],
         ["default", "mypart", "test/foo-bar"],
         ["default", "mypart", "test1/foo-bar2"],
+        ["default", "mypart", "test1/foo/bar2"],
+        ["default", "test1/mypart", "test1/mypart2"],
+        ["default", "test/foo-bar", "test/foo-baz"],
+        ["default", "test/foo-bar-baz", "test/foo-bar"],
     ],
 )
 def test_validate_partitions_success_feature_enabled(partitions):
@@ -72,25 +79,45 @@ def test_validate_partitions_success_feature_enabled(partitions):
         (["default", "!!!"], "Partition '!!!' is invalid."),
         (["default", "-"], "Partition '-' is invalid."),
         (["default", "woop-"], "Partition 'woop-' is invalid."),
+        (["default", "woop."], "Partition 'woop.' is invalid."),
+        (["default", "/"], "Namespaced partition '/' is invalid."),
         (["default", "test/!!!"], "Namespaced partition 'test/!!!' is invalid."),
         (["default", "test/-"], "Namespaced partition 'test/-' is invalid."),
         (["default", "te-st/foo"], "Namespaced partition 'te-st/foo' is invalid."),
         (
             ["default", "test", "test/foo"],
-            "Partition 'test' conflicts with the namespace of partition 'test/foo'",
+            "Partition name conflicts:\n- 'test', 'test/foo'",
+        ),
+        (
+            ["default", "test/foo/bar", "test/foo"],
+            ("Partition name conflicts:\n- 'test/foo', 'test/foo/bar'"),
+        ),
+        (
+            ["default", "test/foo", "test/foo/bar"],
+            ("Partition name conflicts:\n- 'test/foo', 'test/foo/bar'"),
         ),
         (
             ["default", "test-foo", "test/foo"],
-            (
-                "Namespaced partition 'test/foo' conflicts with hyphenated partition "
-                "'test-foo'."
-            ),
+            ("Partition name conflicts:\n- 'test-foo', 'test/foo'"),
         ),
         (
             ["default", "test/foo", "test-foo"],
+            ("Partition name conflicts:\n- 'test-foo', 'test/foo'"),
+        ),
+        (
+            [
+                "default",
+                "test",
+                "test/foo/bar/baz",
+                "test/foo-bar/baz",
+                "test/foo/bar-baz",
+                "qux/baz",
+                "qux-baz",
+            ],
             (
-                "Namespaced partition 'test/foo' conflicts with hyphenated partition "
-                "'test-foo'."
+                "Partition name conflicts:\n"
+                "- 'test', 'test/foo-bar/baz', 'test/foo/bar-baz', 'test/foo/bar/baz'\n"
+                "- 'qux-baz', 'qux/baz'"
             ),
         ),
     ],