-
Notifications
You must be signed in to change notification settings - Fork 588
history: add history import command #3039
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
eaa9bd5 to
fea7459
Compare
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
| cmd := &cobra.Command{ | ||
| Use: "import [OPTIONS] < bundle.dockerbuild", | ||
| Short: "Import a build into Docker Desktop", | ||
| Args: cobra.NoArgs, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should use args as file input instead of a flag and require at least one arg. Multiple files should be supported like Docker Desktop does atm imo:
docker buildx history import FILE...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not usually the convention I think when the file is optional and defaults to stdin. If file is arg then for stdin, the user should write import -
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Importing multiple files together, I'm ok with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah indeed I missed it was reading from stdin. Sounds good to support multiple --file then
commands/history/import.go
Outdated
| if i == 0 { | ||
| err = browser.OpenURL(url) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If multiple files are provided I think we should skip this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because who knows what user wants to see? Is it the last one or the first one? But I guess that's fine to open the very first imported record in Docker Desktop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although I wonder if there are more than one record imported we could instead open the Builds view and filter by imported builds 🤔
bc7bb88 to
6f08838
Compare
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
6f08838 to
cadf4a5
Compare
| if os.Getenv("WSL_DISTRO_NAME") != "" { | ||
| return "unix://" + filepath.Join(wslSocketPath, socketName), nil | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushed this change for testing WSL support with dev build of Docker Desktop but doesn't work yet. I'm taking a look if this can be solved for next Docker Desktop release but in the meantime we could disable support for import command on WSL and explain that it should be invoked on Windows host instead while this is being investigated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to check if socket path exists and returns error if not:
$ docker buildx history import < rec-20250119-docker~build-push-action~JBMEH5.dockerbuild.zip
ERROR: Docker Desktop Build backend is not supported on WSL. Please run this command on Windows host instead.
c3d59b4 to
a8cef6f
Compare
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
a8cef6f to
812b42b
Compare

Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.

part of #2711