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

image/cache: fix isValidParent logic #31189

Merged
merged 1 commit into from Feb 21, 2017

Conversation

Projects
None yet
7 participants
@runcom
Member

runcom commented Feb 20, 2017

Fix #31186

Signed-off-by: Antonio Murdaca runcom@redhat.com

@runcom

This comment has been minimized.

Show comment
Hide comment
@runcom

runcom Feb 20, 2017

Member

I'll add a test asap, in the middle of something else now.

Member

runcom commented Feb 20, 2017

I'll add a test asap, in the middle of something else now.

@vdemeester

This comment has been minimized.

Show comment
Hide comment
@vdemeester
Member

vdemeester commented Feb 20, 2017

@tonistiigi

This comment has been minimized.

Show comment
Hide comment
@tonistiigi

tonistiigi Feb 21, 2017

Member

LGTM, but needs a test

Member

tonistiigi commented Feb 21, 2017

LGTM, but needs a test

@runcom

This comment has been minimized.

Show comment
Hide comment
@runcom

runcom Feb 21, 2017

Member

Yes, as noted above, I'll need to add one.

Member

runcom commented Feb 21, 2017

Yes, as noted above, I'll need to add one.

@aaronlehmann

This comment has been minimized.

Show comment
Hide comment
@aaronlehmann

aaronlehmann Feb 21, 2017

Contributor

I think isValidParent should be renamed, since it's no longer checking for a parent/child relationship.

Contributor

aaronlehmann commented Feb 21, 2017

I think isValidParent should be renamed, since it's no longer checking for a parent/child relationship.

image/cache: fix isValidParent logic
Signed-off-by: Antonio Murdaca <runcom@redhat.com>
@runcom

This comment has been minimized.

Show comment
Hide comment
@runcom

runcom Feb 21, 2017

Member

I think isValidParent should be renamed, since it's no longer checking for a parent/child relationship.

I'll add another commit to change that or follow up (do you have any suggestion on the new name?)
@tonistiigi added a test

Member

runcom commented Feb 21, 2017

I think isValidParent should be renamed, since it's no longer checking for a parent/child relationship.

I'll add another commit to change that or follow up (do you have any suggestion on the new name?)
@tonistiigi added a test

@aaronlehmann

This comment has been minimized.

Show comment
Hide comment
@aaronlehmann

aaronlehmann Feb 21, 2017

Contributor

I'll add another commit to change that or follow up (do you have any suggestion on the new name?)

Actually, I might have misunderstood the fix.

I see that isValidParent still checks if len(parent.History) >= len(img.History) { (not >).

So it seems like it still is checking for a parent, but you fixed a problem that happens if the parent has the same number of filesystem layers. If this is the case, the name doesn't need to change.

LGTM

Contributor

aaronlehmann commented Feb 21, 2017

I'll add another commit to change that or follow up (do you have any suggestion on the new name?)

Actually, I might have misunderstood the fix.

I see that isValidParent still checks if len(parent.History) >= len(img.History) { (not >).

So it seems like it still is checking for a parent, but you fixed a problem that happens if the parent has the same number of filesystem layers. If this is the case, the name doesn't need to change.

LGTM

@LK4D4

LK4D4 approved these changes Feb 21, 2017

LGTM

@LK4D4 LK4D4 merged commit 093867b into moby:master Feb 21, 2017

4 checks passed

dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 30997 has succeeded
Details
janky Jenkins build Docker-PRs 39609 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 10673 has succeeded
Details

@GordonTheTurtle GordonTheTurtle added this to the 17.04.0 milestone Feb 21, 2017

@runcom runcom deleted the runcom:fix-build-cache-from branch Feb 22, 2017

@vieux vieux referenced this pull request Feb 22, 2017

Merged

17.03 cherry-picks #31266

@vieux vieux modified the milestones: 17.03.0, 17.04.0 Feb 22, 2017

@tonistiigi tonistiigi referenced this pull request Mar 2, 2017

Closed

Pulling build cache #20316

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment