Skip to content
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: error out on duplicate tar entry #240

Merged
merged 1 commit into from
Sep 1, 2016

Conversation

runcom
Copy link
Member

@runcom runcom commented Sep 1, 2016

Should address the code change requested in #196

/cc @philips @stevvooe @vbatts @s-urbaniak

Signed-off-by: Antonio Murdaca [email protected]

@philips
Copy link
Contributor

philips commented Sep 1, 2016

LGTM

Approved with PullApprove

@runcom
Copy link
Member Author

runcom commented Sep 1, 2016

unrelated network error in Travis, any way to re-start it?

@philips
Copy link
Contributor

philips commented Sep 1, 2016

@runcom add whitespace to your commit message with git commit --amend and repush...

@jonboulle
Copy link
Contributor

@caniszczyk could we get maintainers access to Travis to be able to restart builds?

@@ -154,6 +155,10 @@ loop:
}
}
path := filepath.Join(dest, hdr.Name)
if entries[path] {
return fmt.Errorf("%s has been already processed", path)
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO "duplicate entry for %s" would be better

Copy link
Member Author

Choose a reason for hiding this comment

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

fixing

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@runcom
Copy link
Member Author

runcom commented Sep 1, 2016

tests now green though

@jonboulle
Copy link
Contributor

jonboulle commented Sep 1, 2016

lgtm

Approved with PullApprove

@vbatts
Copy link
Member

vbatts commented Sep 1, 2016

LGTM

Approved with PullApprove

@vbatts vbatts merged commit 08ef0b1 into opencontainers:master Sep 1, 2016
@runcom runcom deleted the fix-196 branch September 1, 2016 15:10
@wking
Copy link
Contributor

wking commented Sep 1, 2016 via email

@runcom
Copy link
Member Author

runcom commented Sep 1, 2016

@wking I have a branch which will take care of what you're saying but I'm buried with other stuff/PRs that are higher priority to me right now.

@runcom
Copy link
Member Author

runcom commented Sep 1, 2016

Can you open an issue to track this refactor of the unpack code?

@wking
Copy link
Contributor

wking commented Sep 1, 2016

On Thu, Sep 01, 2016 at 09:51:16AM -0700, Antonio Murdaca wrote:

Can you open an issue to track this refactor of the unpack code?

I've opened #242 to track cleanup on unpack errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants