Skip to content
This repository has been archived by the owner on Oct 13, 2023. It is now read-only.

builder: fix copy —from conflict with force pull #86

Merged
merged 1 commit into from
Jul 25, 2017

Conversation

tiborvass
Copy link
Contributor

Signed-off-by: Tonis Tiigi tonistiigi@gmail.com
(cherry picked from commit e430b58811813084df2b9f8b1a9e929114b2187a)
Signed-off-by: Tibor Vass tibor@docker.com

@tiborvass tiborvass changed the title builder: fix copy —from conflict with force pull [WIP] builder: fix copy —from conflict with force pull Jun 20, 2017
@andrewhsu andrewhsu mentioned this pull request Jun 20, 2017
6 tasks
@tiborvass
Copy link
Contributor Author

It's WIP until moby/moby#33735 is merged

if !opts.ForcePull {
image, _ := daemon.GetImage(refOrID)
id, _ := daemon.GetImageID(refOrID)
refIsID := id.String() == refOrID // detect if ref is an ID to skip pulling
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's not merge this just yet. This really doesn't seem like the right fix to me.

Either we should use the official "is ref or ID" logic from GetImageID() or we should have the caller pass in opts.ForcePull = false for this case (when it knows it has an ID).

@tiborvass
Copy link
Contributor Author

Closing for now

@tiborvass tiborvass closed this Jun 20, 2017
@tonistiigi
Copy link
Contributor

@tiborvass Can you reopen with updated fix

@tiborvass tiborvass reopened this Jun 27, 2017
@thaJeztah
Copy link
Member

ping @tiborvass @tonistiigi what's the status on this one?

@thaJeztah thaJeztah mentioned this pull request Jul 21, 2017
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
(cherry picked from commit e430b58811813084df2b9f8b1a9e929114b2187a)
Signed-off-by: Tibor Vass <tibor@docker.com>
@tiborvass tiborvass changed the title [WIP] builder: fix copy —from conflict with force pull builder: fix copy —from conflict with force pull Jul 22, 2017
@tiborvass
Copy link
Contributor Author

Ping @tonistiigi sorry for the delay.

@thaJeztah
Copy link
Member

Compile is failing on Windows;

05:41:10 # github.com/docker/cli/vendor/github.com/docker/docker/pkg/system
05:41:10 ..\..\vendor\github.com\docker\docker\pkg\system\init.go:10: maxTime redeclared in this block
05:41:10 	previous declaration at ..\..\vendor\github.com\docker\docker\pkg\system\chtimes.go:11
05:41:10 ..\..\vendor\github.com\docker\docker\pkg\system\path_windows.go:13: DefaultPathEnv redeclared in this block
05:41:10 	previous declaration at ..\..\vendor\github.com\docker\docker\pkg\system\path.go:10

@vieux
Copy link
Contributor

vieux commented Jul 25, 2017

LGTM

1 similar comment
@tonistiigi
Copy link
Contributor

LGTM

@vieux vieux merged commit b379923 into docker-archive:17.06 Jul 25, 2017
@vieux vieux mentioned this pull request Jul 25, 2017
@andrewhsu andrewhsu added this to the 17.06.1 milestone Jul 27, 2017
docker-jenkins pushed a commit that referenced this pull request Feb 15, 2018
Update readme to supported packages
Upstream-commit: a54da87
Component: packaging
docker-jenkins pushed a commit that referenced this pull request Oct 13, 2018
[18.09] backport Fix mount propagation for btrfs
Upstream-commit: 4d0b8cc2d701e2a09b8e1f889b98c08d225d8145
Component: engine
silvin-lubecki pushed a commit to silvin-lubecki/docker-ce that referenced this pull request Jan 30, 2020
Update readme to supported packages
silvin-lubecki pushed a commit to silvin-lubecki/docker-ce that referenced this pull request Feb 3, 2020
builder: fix copy —from conflict with force pull
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants