Skip to content

Commit f5ccae9

Browse files
committed
fix: correct three claims the review found the branch could not support
1 parent 6337b02 commit f5ccae9

5 files changed

Lines changed: 23 additions & 18 deletions

File tree

‎compose2pod/parsing.py‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -161,12 +161,14 @@ def _classify_volume(volume: str) -> tuple[str, str | None]:
161161
# even when that letter is declared top-level, a declaration Docker ignores.
162162
# Two letters (`CC:\data:/var`) is an ordinary named-volume reference instead.
163163
#
164-
# podman refuses every mount either reading makes (measured, podman 4.9.3): a
165-
# colon inside a source has nowhere to go in a `-v` spec, which splits into at
166-
# most source:target:options (`invalid option type "/var"`), and a container
167-
# path that is not absolute is refused outright (`invalid container path`).
168-
# So the whole family is a rule-two refusal, and a one-character volume name
169-
# is reachable only through the long form, where Docker honours `source: v`.
164+
# podman refuses the anonymous readings outright (measured, podman 4.9.3): a
165+
# container path that is not absolute is an `invalid container path`, and no
166+
# spelling names one. The bind readings it does mount, through `--mount
167+
# type=bind`; only the short `-v` spec emitted here cannot, since that splits
168+
# into at most source:target:options (`invalid option type "/var"`). So the
169+
# bind half is a limitation rather than rule two, tracked in
170+
# docs/adr/0006-docker-rejection-parity.md. Either way, a one-character volume
171+
# name is reachable through the long form, where Docker honours `source: v`.
170172
_DRIVE_SHAPED_SOURCE = re.compile(r"^[a-zA-Z]:")
171173

172174

‎docs/adr/0006-docker-rejection-parity.md‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@ Docker's verdict binds only on the document, not the host: `env_file` existence,
1414
negative on a top-level numeric key are facts about the machine that runs the script and are
1515
deferred to it. `tests/conformance/` runs both oracles for real over a probe matrix generated
1616
from `SERVICE_KEYS | STRUCTURAL_KEYS | IGNORED_SERVICE_KEYS`, so a new key is probed the moment
17-
it is added, and `tests/integration/refusals.py` measures the other side, pairing each refusal above
18-
with the `--mount` that expresses what Docker says the document means -- so a claim that podman
19-
cannot express something cannot go stale unnoticed either. Verdicts are per version: the rulings
17+
it is added, and `tests/integration/refusals.py` measures the other side, pairing the volume-family
18+
refusals above with the `--mount` that expresses what Docker says the document means, so a claim
19+
that podman cannot express something cannot go stale unnoticed either. The rest of the refusals get
20+
rows, and a gate that every site has one, under
21+
[#109](https://github.com/modern-python/compose2pod/issues/109). Verdicts are per version: the rulings
2022
here are measured against `docker compose config` v5.1.2 and podman 4.9.3. Two residuals are open by
2123
design: `depends_on` errors among services outside the target's closure are accepted here and
2224
rejected by Docker ([#87](https://github.com/modern-python/compose2pod/issues/87)), and the

‎tests/integration/conftest.py‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,17 @@
3030
_PODMAN_VERDICTS: pytest.StashKey[list[str]] = pytest.StashKey()
3131

3232
# A container that starts and exits immediately, so a probe's verdict is its mount's,
33-
# not its workload's. Shared with the scenario tests, which pull the same tag.
33+
# not its workload's.
3434
_PROBE_IMAGE = "busybox:1.36"
3535

3636

3737
def _podman_version() -> str:
3838
if _PODMAN is None:
3939
return "podman not installed"
40-
proc = subprocess.run([_PODMAN, "--version"], capture_output=True, text=True, check=False) # noqa: S603
40+
41+
proc = subprocess.run( # noqa: S603 - _PODMAN is an absolute path from shutil.which
42+
[_PODMAN, "--version"], capture_output=True, text=True, check=False
43+
)
4144
return proc.stdout.strip() or "podman version unknown"
4245

4346

‎tests/integration/refusals.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@
1212
would produce: a short `-v` spec re-splits on colons and so can fail for its own
1313
grammar rather than for podman's inability to mount, which would make a row pass
1414
for the wrong reason and hide an emit bug behind a parity claim.
15+
16+
Not every refusal has a row yet. The drive-qualified *bind* readings
17+
(`C:\data:/var`, `C:data:/var`) are refused by the same site as the rows below
18+
but belong to no row, because podman does mount them through `--mount`
19+
(issue #111); the refusals outside the volume family are issue #109 phase 2.
1520
"""
1621

1722
from dataclasses import dataclass

‎tests/integration/test_podman_refusals.py‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,3 @@ def test_a_documented_refusal_names_a_mount_podman_will_not_make(
2828
assert probe_podman(refusal.id, refusal.podman_argv) != 0, (
2929
"podman made the mount this document asks for, so refusing it is a limitation, not rule two"
3030
)
31-
32-
33-
def test_every_row_has_a_distinct_id() -> None:
34-
"""Ids label the summary lines and the parametrize cases; a duplicate hides one row behind another."""
35-
ids = [refusal.id for refusal in REFUSALS]
36-
37-
assert sorted(ids) == sorted(set(ids))

0 commit comments

Comments
 (0)