Repository navigation
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2859 +/- ##
==========================================
- Coverage 76.75% 76.74% -0.01%
==========================================
Files 72 72
Lines 40805 40805
==========================================
- Hits 31319 31317 -2
- Misses 9486 9488 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
sylvestre
reviewed
Sep 28, 2026
| @# depends_on: service_completed_successfully -- podman-compose ignores that | ||
| @# condition, and the test would race ahead of provisioning. | ||
| @for s in $(filter %-setup,$(SERVICES_$*)); do \ | ||
| docker compose $(call profiles,$*) run --rm $$s || exit 1; \ |
Collaborator
There was a problem hiding this comment.
silo-setup and azurite-setup always exit 0, so || exit 1 never triggers, no?
Contributor
Author
There was a problem hiding this comment.
Can't the docker compose run --rm itself fail?
Better let the user know something is wrong.
sylvestre
reviewed
Sep 28, 2026
| @echo "Testing $*..." | ||
| @docker compose $(call profiles,$*) up --quiet-pull -d $(SERVICES_$*) | ||
| @docker compose $(call profiles,$*) up -d $(filter-out %-setup,$(SERVICES_$*)) | ||
| @# The one-shot *-setup services provision buckets/containers. Start them with |
Collaborator
There was a problem hiding this comment.
please make this comment shorter, one or two lines is enough
Contributor
Author
There was a problem hiding this comment.
Shortened to 1.6 lines.
Podman resolves unqualified image names against the registry list in
registries.conf. Under the common short-name-mode = "enforcing" setting it
cannot pick a registry non-interactively and every pull fails with:
Error: short-name resolution enforced but cannot prompt without a TTY
Prefix the Docker Hub images with docker.io (and docker.io/library for the
official ones) so resolution is unambiguous. This is what Docker already
resolves these names to, so its behaviour is unchanged.
The mcr.microsoft.com images were already fully qualified.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
podman-compose does not accept the flag and aborts argument parsing:
podman-compose: error: unrecognized arguments: --quiet-pull
It only suppresses image pull progress, so dropping it costs nothing beyond
slightly noisier output on the first run under Docker.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Podman rejects these options outright:
Error: unknown mount option "uid=1000": invalid mount option
They were redundant anyway. mode=1777 already makes /build sticky and
world-writable, like /tmp, so the test runner can create files there whatever
uid it runs as. The only difference is that the mount point itself is now owned
by root rather than by the test user, which nothing depends on.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rootless podman runs containers in a user namespace where the container's UID
is mapped to a subuid on the host. The compose services run as $(UID), so
everything they write into the bind-mounted target/ directory came back owned
by uid 100999 rather than by us, and the next run failed as soon as it tried
to touch its own output:
mkdir: cannot create directory 'target/integration-cargo-cache': Permission denied
rm: cannot remove 'target/integration-coverage/sccache.log': Permission denied
Pass userns_mode: keep-id for those services, which makes the mapping an
identity for our own UID.
The two build containers need the opposite treatment. They run as root in
order to apt-get install, and keep-id would drop them to a non-root user:
E: Could not open lock file /var/lib/apt/lists/lock - open (13: Permission denied)
Rootless podman already maps container root to the invoking user, so they need
no mapping at all -- and must skip the closing chown back to $(UID):$(GID),
which under that mapping would push the files out to a subuid instead of
bringing them back.
Both behaviours are selected off a podman check, so Docker keeps the original
path unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Docker silently creates a missing bind-mount source directory; podman does not:
Error: statfs .../target/integration-cargo-cache: no such file or directory
Both cache directories already have rules, but they were only prerequisites of
the build target, so nothing ordered them before the recipe that mounts them.
Make them order-only prerequisites of the binary itself.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The *-setup services provision the bucket or container the cache backend needs.
The test services order themselves after them with
depends_on:
azurite-setup:
condition: service_completed_successfully
but podman-compose ignores depends_on conditions, so `up -d` returned as soon
as the setup container had started and the test ran against an unprovisioned
backend:
storage write check failed: NotFound (permanent) at write =>
AzblobError { code: "ContainerNotFound", ... }
FAIL: azblob - No cache hits detected
azblob failed reliably because its setup image, azure-cli, takes several
seconds to start. s3, basedirs and multilevel carried the same race and only
passed because silo-setup is quick.
Start these services with `run` instead, which is synchronous and so completes
the provisioning before the test begins. Docker gets the ordering it already
had from depends_on; this just stops depending on a condition podman does not
implement.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
avikivity
force-pushed
the
qualify-dockerhub
branch
from
October 1, 2026 15:28
5e8b456 to
1e91715
Compare
Contributor
Author
|
v2:
|
Merging this PR will degrade performance by 8.29%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | time_macro_finder_chunked |
862.8 µs | 1,026.4 µs | -15.94% |
| ❌ | Simulation | time_macro_finder_no_macros |
2.9 ms | 3 ms | -5.39% |
| ❌ | Simulation | strip_basedirs_multiple |
95.3 µs | 98.2 µs | -3.02% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing avikivity:qualify-dockerhub (1e91715) with main (54b6f72)
Footnotes
-
6 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
Contributor
Author
|
@sylvestre please re-review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
podman is more or less compatible with docker, but there are a few rough edges.
This series smoothes them over.