-
Notifications
You must be signed in to change notification settings - Fork 640
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
Fix broken oci-image-tool #177
Changes from 4 commits
0e8f74b
116950e
6541392
44210d0
08bed93
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ import ( | |
"io" | ||
"os" | ||
"path/filepath" | ||
"strings" | ||
|
||
"github.com/pkg/errors" | ||
) | ||
|
@@ -32,6 +33,10 @@ type descriptor struct { | |
Size int64 `json:"size"` | ||
} | ||
|
||
func (d *descriptor) normalizeDigest() string { | ||
return strings.Replace(d.Digest, ":", "-", -1) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd rethink the method name, maybe There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this actually doesn't need to be attached as a receiver method on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like this way since it's something on the descriptor itself... |
||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On Fri, Jul 22, 2016 at 07:18:32AM -0700, Antonio Murdaca wrote:
I'm not sure what's holding #159 up, it's been out there for a while There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I don't know either - just that you PR is 1000+ loc changed - while this one is just pulling in 10 additions (0e8f74b) to fix a fatal bug. I believe this should go in asap. Potentially #159 can take a lot more time to be reviewed (not maintainers reviewed it yet) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On Fri, Jul 22, 2016 at 07:52:32AM -0700, Antonio Murdaca wrote:
I think it would be more efficient to put better abstractions into There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the point now is that, unfortunately this tool is broken and it's broken since a lot now probably (#165 is 28 days old why was that closed) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On Fri, Jul 22, 2016 at 08:09:47AM -0700, Antonio Murdaca wrote:
It's been broken (on this semicolon point) since it was born, because |
||
|
||
func findDescriptor(w walker, name string) (*descriptor, error) { | ||
var d descriptor | ||
dpath := filepath.Join("refs", name) | ||
|
@@ -71,7 +76,7 @@ func (d *descriptor) validate(w walker) error { | |
} | ||
|
||
digest, err := filepath.Rel("blobs", filepath.Clean(path)) | ||
if err != nil || d.Digest != digest { | ||
if err != nil || d.normalizeDigest() != digest { | ||
return nil // ignore | ||
} | ||
|
||
|
@@ -84,11 +89,11 @@ func (d *descriptor) validate(w walker) error { | |
|
||
switch err := w.walk(f); err { | ||
case nil: | ||
return fmt.Errorf("%s: not found", d.Digest) | ||
return fmt.Errorf("%s: not found", d.normalizeDigest()) | ||
case errEOW: | ||
// found, continue below | ||
default: | ||
return errors.Wrapf(err, "%s: validation failed", d.Digest) | ||
return errors.Wrapf(err, "%s: validation failed", d.normalizeDigest()) | ||
} | ||
|
||
return 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.
is
sh
spec'ed as a default?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.
yes
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.
ocitools is also merging a library which will ease all of this and painlessly merge the image's config and the default one.