Skip to content

cli/command/container: don't set CopyToContainerOptions.AllowOverwriteDirWithFile#6176

Merged
thaJeztah merged 1 commit intodocker:masterfrom
thaJeztah:rm_use_AllowOverwriteDirWithFile
Jul 14, 2025
Merged

cli/command/container: don't set CopyToContainerOptions.AllowOverwriteDirWithFile#6176
thaJeztah merged 1 commit intodocker:masterfrom
thaJeztah:rm_use_AllowOverwriteDirWithFile

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

relates to:

cli/command/container: don't set CopyToContainerOptions.AllowOverwriteDirWithFile

The AllowOverwriteDirWithFile option was added when reimplementing the CLI using the API Client lib in moby@1b2b91b. Before that refactor, the noOverwriteDirNonDir query argument would be set unconditionally by the CLI, with no options to control the behavior.

It's unclear why the noOverwriteDirNonDir was implemented as opt-in (not opt-out), as overwriting a file with a directory (or vice-versa) would generally be unexpected behavior.

We're considering making noOverwriteDirNonDir unconditional on the daemon side, and to deprecate the AllowOverwriteDirWithFile option. This patch removes its use, as it was set to the default either way, and there's no options to configure it from the CLI.

- What I did

- How I did it

- How to verify it

- Human readable description for the release notes

- A picture of a cute animal (not mandatory but encouraged)

…eDirWithFile

The `AllowOverwriteDirWithFile` option was added when reimplementing the
CLI using the API Client lib in [moby@1b2b91b]. Before that refactor, the
`noOverwriteDirNonDir` query argument [would be set unconditionally][1]
by the CLI, with no options to control the behavior.

It's unclear why the `noOverwriteDirNonDir` was implemented as opt-in (not
opt-out), as overwriting a file with a directory (or vice-versa) would
generally be unexpected behavior.

We're considering making `noOverwriteDirNonDir` unconditional on the daemon
side, and to deprecate the `AllowOverwriteDirWithFile` option. This patch
removes its use, as it was set to the default either way, and there's no
options to configure it from the CLI.

[1]: https://github.com/moby/moby/blob/8c9ad7b818c0a7b1e39f8df1fabba243a0961c2d/api/client/cp.go#L345-L346
[moby@1b2b91b]: moby/moby@1b2b91b

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
cli/command/container/cp.go 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@thaJeztah thaJeztah merged commit 7668b68 into docker:master Jul 14, 2025
117 of 118 checks passed
@thaJeztah thaJeztah deleted the rm_use_AllowOverwriteDirWithFile branch July 14, 2025 21:17
@thaJeztah thaJeztah modified the milestones: 29.0.0, 28.3.3 Aug 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants