Skip to content

Conversation

@thaJeztah
Copy link
Member

The v28.3.3 tag was created from master, but the v28.x branch
wasn't fast-forwarded, and PR's merged after that. This should
bring the v28.3.3 tag's changes into the v28.x branch.

- What I did

git merge --no-ff --signoff -S v28.3.3

- 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)

thaJeztah and others added 21 commits July 13, 2025 13:48
…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 <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
We were depending on pkg/stringid to truncate IDs for presentation. While
traditionally, we used a fixed length for "truncated" IDs, this is not
a strict requirement (any ID-prefix should work, but conflicts may
happen on shorter IDs).

This patch adds a local `TruncateID()` utility in the formatter package;
it's currently using the same implementation and length as the
`stringid.TruncateID` function, but may diverge in future.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
This utility was only used for testing, and to generate a random
suffix for Dockerfiles. As we don't need the same contract as
pkg/stringid.GenerateRandomID() (not allow all-numeric IDs as they
would not be usable for hostnames), we can use a local test-utility,
and local implementation for the random suffix instead.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
It was the only utility we consumed from the package, and it's trivial
to implement, so let's create local copies of it.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
…rWithFile

cli/command/container: don't set CopyToContainerOptions.AllowOverwriteDirWithFile
client.ContainerDiff already validates the given container name/ID, and
produces an error when empty, so we don't have to check for this;
https://github.com/moby/moby/blob/abba330bbfe10765822b59bb68af99db439736ba/client/container_diff.go#L13-L16

While updating, also;

- remove the diffOptions type, as there were no other options, and make
  the container name/ID a string argument.
- fix camelCase nameing of dockerCLI

Before this patch:

    docker diff ""
    Container name cannot be empty

With this patch:

    docker diff ""
    invalid container name or ID: value is empty

Signed-off-by: Sebastiaan van Stijn <[email protected]>
remove uses of github.com/docker/docker/pkg/stringid
    docker build --call outline .

    TARGET: binary

    BUILD ARG               VALUE    DESCRIPTION
    BASE_VARIANT            alpine
    ALPINE_VERSION          3.21     sets the version of the alpine base image to use, including for the golang image.
    GO_VERSION              1.24.5
    XX_VERSION              1.6.1
    GOVERSIONINFO_VERSION   v1.4.1
    GO_LINKMODE             static   defines if static or dynamic binary should be produced
    GO_BUILDTAGS                     defines additional build tags
    GO_STRIP                         strips debugging symbols if set
    CGO_ENABLED                      manually sets if cgo is used
    VERSION                          sets the version for the produced binary
    PACKAGER_NAME                    sets the company that produced the windows binary

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Dockerfile: document ALPINE_VERSION build-arg
Dockerfile: bump gotest.tools/gotestsum v1.12.3 (for go1.25)
Dockerfile: update Buildx v0.25.0, compose v2.38.2
cli/command: remove some redundant import-aliases
remove use of github.com/docker/docker/pkg/longpath
remove uses of github.com/docker/docker/pkg/ioutils ReadCloserWrapper
cli/command/container: diff: remove redundant validation and cleanup
The v28.3.3 tag was created from master, but the v28.x branch
wasn't fast-forwarded, and PR's merged after that. This should
bring the v28.3.3 tag's changes into the v28.x branch.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah added this to the 28.4.0 milestone Aug 14, 2025
@vvoland vvoland changed the base branch from master to 28.x August 14, 2025 15:51
@thaJeztah thaJeztah marked this pull request as ready for review August 14, 2025 15:55
Copy link
Collaborator

@vvoland vvoland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, good catch! 🤦🏻

@thaJeztah
Copy link
Member Author

@vvoland PTAL; I noticed some patches were in v28.3.3 and then I missed them in the 28.x branch. Not sure if there's a cleaner way; at least I was hesitant to rebase the 28.x branch (which would bring back a more linear history); suggestions welcome though!

@thaJeztah
Copy link
Member Author

Oh! Race condition in commenting 😂 ❤️

@thaJeztah
Copy link
Member Author

OK; let's move forward with this. The alternative is a rebase of the 28.x branch, which is also a bit risky as it'd be rewriting history in the upstream repo, and as we already started cherry-picking, I guess it's OK (merge commit or cherry-pick - potatoes/potatoes).

Go modules pseudo versions will start to get confused now (master being "older" than v28.x), but I guess there's no good way around that, other than tagging the commit after v28.3.3 as v29.0.0-alpha.0

@thaJeztah thaJeztah merged commit efee587 into docker:28.x Aug 14, 2025
4 checks passed
@thaJeztah thaJeztah deleted the 28.x_merge_28.3.3 branch August 14, 2025 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants