fix(cp): clarify --archive flag description#6937
fix(cp): clarify --archive flag description#6937mohithshuka wants to merge 4 commits intodocker:masterfrom
Conversation
Fixes #24643 --kernel-memory was deprecated in v20.10 and removed in v23.0. Add deprecation callouts to both the options table and the Kernel memory constraints section of the run reference page.
The --filter flag for 'docker volume prune' previously only showed 'label=<label>' as an example, not mentioning the negation form 'label!=<label>', even though it is valid and supported by the daemon. Fixes docker#6918 Signed-off-by: mohithshuka <153504854+mohithshuka@users.noreply.github.com>
When a port is bound on both 0.0.0.0 and ::, hide the IPv6 entry and show only the IPv4 entry to reduce noise in docker ps output. Before: 0.0.0.0:8080->80/tcp, :::8080->80/tcp After: 0.0.0.0:8080->80/tcp Fixes docker#6869
The --archive (-a) flag description was misleading. Updated to clarify that it preserves uid/gid from the source, and that it only takes effect when copying files TO a container (not from). Fixes docker#6870
There was a problem hiding this comment.
Pull request overview
Updates user-facing CLI/docs text and output formatting to reduce confusion: clarifies docker cp --archive behavior, documents --kernel-memory deprecation/removal, expands volume prune filter examples, and tweaks docker ps port display to avoid duplicate wildcard bindings.
Changes:
- Clarify
docker cp -a/--archivehelp text to specify it preserves uid/gid from the source when copying to a container. - Document
--kernel-memoryas deprecated/removed and add a deprecation callout indocker runreference docs. - Suppress duplicate IPv6 wildcard (
::) port mappings indocker psoutput when an IPv4 wildcard mapping already exists.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| docs/reference/run.md | Marks --kernel-memory as deprecated/removed and adds a deprecation notice in the kernel memory section. |
| cli/command/volume/prune.go | Expands the --filter help example to include label!=.... |
| cli/command/formatter/container.go | Adds logic to suppress duplicate :: wildcard mappings when 0.0.0.0 is present. |
| cli/command/container/cp.go | Updates --archive flag description for docker cp. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if port.PublicPort != current { | ||
| // Suppress the IPv6 wildcard entry when an IPv4 wildcard | ||
| // entry already covers the same (hostPort, privatePort, proto) | ||
| // tuple. This merges: | ||
| // 0.0.0.0:8080->80/tcp, :::8080->80/tcp | ||
| // into the cleaner: | ||
| // 0.0.0.0:8080->80/tcp | ||
| if port.IP.Is6() && !port.IP.Is4In6() && port.IP.IsUnspecified() { | ||
| key := mappingKey{port.PublicPort, port.PrivatePort, port.Type} | ||
| if ipv4Bindings[key] { | ||
| continue | ||
| } | ||
| } |
There was a problem hiding this comment.
The IPv6-wildcard suppression only runs in the port.PublicPort != port.PrivatePort branch. If the host and container ports are the same (e.g. -p 80:80), docker ps output can still include both 0.0.0.0:80->80/tcp and [::]:80->80/tcp because those entries follow the grouping path. Consider applying the same suppression when building grouped entries (i.e., before using portKey = port.IP.String() + "/" + port.Type).
| if port.PublicPort != current { | |
| // Suppress the IPv6 wildcard entry when an IPv4 wildcard | |
| // entry already covers the same (hostPort, privatePort, proto) | |
| // tuple. This merges: | |
| // 0.0.0.0:8080->80/tcp, :::8080->80/tcp | |
| // into the cleaner: | |
| // 0.0.0.0:8080->80/tcp | |
| if port.IP.Is6() && !port.IP.Is4In6() && port.IP.IsUnspecified() { | |
| key := mappingKey{port.PublicPort, port.PrivatePort, port.Type} | |
| if ipv4Bindings[key] { | |
| continue | |
| } | |
| } | |
| // Suppress the IPv6 wildcard entry when an IPv4 wildcard | |
| // entry already covers the same (hostPort, privatePort, proto) | |
| // tuple. This merges: | |
| // 0.0.0.0:8080->80/tcp, :::8080->80/tcp | |
| // into the cleaner: | |
| // 0.0.0.0:8080->80/tcp | |
| // | |
| // Apply this before both formatting branches so grouped entries | |
| // for equal host/container ports (for example -p 80:80) also | |
| // suppress the redundant IPv6 wildcard binding. | |
| if port.IP.Is6() && !port.IP.Is4In6() && port.IP.IsUnspecified() { | |
| key := mappingKey{port.PublicPort, port.PrivatePort, port.Type} | |
| if ipv4Bindings[key] { | |
| continue | |
| } | |
| } | |
| if port.PublicPort != current { |
| // Pre-pass: record which (hostPort, privatePort, proto) tuples have an | ||
| // IPv4 wildcard (0.0.0.0) binding. Used below to suppress the matching | ||
| // IPv6 wildcard (::) entry, avoiding duplicate output such as: | ||
| // 0.0.0.0:8080->80/tcp, :::8080->80/tcp | ||
| // See: https://github.com/docker/cli/issues/6869 |
There was a problem hiding this comment.
DisplayablePorts formatting behavior changed to suppress certain IPv6 wildcard entries, but there’s no corresponding unit test coverage for the mixed 0.0.0.0 + :: cases (including both hostPort!=containerPort and hostPort==containerPort). Adding test cases in cli/command/formatter/container_test.go would prevent regressions in this output formatting.
| | `--memory-swap=""` | Total memory limit (memory + swap, format: `<number>[<unit>]`). Number is a positive integer. Unit can be one of `b`, `k`, `m`, or `g`. | | ||
| | `--memory-reservation=""` | Memory soft limit (format: `<number>[<unit>]`). Number is a positive integer. Unit can be one of `b`, `k`, `m`, or `g`. | | ||
| | `--kernel-memory=""` | Kernel memory limit (format: `<number>[<unit>]`). Number is a positive integer. Unit can be one of `b`, `k`, `m`, or `g`. Minimum is 4M. | | ||
| | `--kernel-memory=""` | **Deprecated**: Kernel memory limit. Deprecated in Docker v20.10, and removed in Docker v23.0. This option is ignored when set. | |
There was a problem hiding this comment.
The table entry says --kernel-memory was removed in Docker v23.0 but also says the option is ignored when set. Since the CLI still defines the flag (deprecated stub) and the Engine ignores it, consider rewording to clarify it was removed from / no longer supported by the Engine (and ignored), rather than implying the flag itself no longer exists.
| | `--kernel-memory=""` | **Deprecated**: Kernel memory limit. Deprecated in Docker v20.10, and removed in Docker v23.0. This option is ignored when set. | | |
| | `--kernel-memory=""` | **Deprecated**: Kernel memory limit. Deprecated in Docker v20.10, no longer supported by the Engine since Docker v23.0, and ignored when set. | |
| > **Deprecated** | ||
| > | ||
| > The `--kernel-memory` option was deprecated in Docker v20.10 and removed in | ||
| > Docker v23.0. The Linux kernel deprecated `kmem.limit_in_bytes` in kernel | ||
| > v5.4, and OCI runtimes such as runc no longer support this option. Docker API |
There was a problem hiding this comment.
After adding the deprecation notice, the rest of this section still describes --kernel-memory constraints and includes usage examples as if the limit is functional. Given the Engine ignores this option, consider either removing/updating the subsequent explanation/examples or clearly marking them as historical/unsupported to avoid misleading readers.
| flags := cmd.Flags() | ||
| flags.BoolVarP(&opts.followLink, "follow-link", "L", false, "Always follow symbol link in SRC_PATH") | ||
| flags.BoolVarP(&opts.copyUIDGID, "archive", "a", false, "Archive mode (copy all uid/gid information)") | ||
| flags.BoolVarP(&opts.copyUIDGID, "archive", "a", false, "Archive mode (preserve uid/gid from source when copying to container)") |
There was a problem hiding this comment.
The CLI help-string for --archive was updated, but the generated command reference docs (docs/reference/commandline/cp.md and docs/reference/commandline/container_cp.md) still contain the old description. If those docs are expected to match the CLI help output, they should be regenerated/updated as part of this change so the documentation fix is complete.
| flags.SetAnnotation("all", "version", []string{"1.42"}) | ||
| flags.BoolVarP(&options.force, "force", "f", false, "Do not prompt for confirmation") | ||
| flags.Var(&options.filter, "filter", `Provide filter values (e.g. "label=<label>")`) | ||
| flags.Var(&options.filter, "filter", `Provide filter values (e.g. "label=<label>" or "label!=<label>")`) |
There was a problem hiding this comment.
PR title/description focus on docker cp --archive help text, but this PR also changes docker run kernel-memory documentation, volume prune filter help, and docker ps port-formatting logic. Please update the PR description/title to reflect the additional scope, or split these unrelated changes into separate PRs to make review/revert safer.
Description
Fixes the misleading description of the
--archive/-aflag indocker cp.Fixes #6870
What was wrong
The previous description:
This was vague and inaccurate — it didn't clarify:
What changed
Updated the flag description in
cli/command/container/cp.go:Before:
After:
Testing
docker cp --helpcontainer_cp.md