Bump up runtime-spec/image-spec to the latest version #69

hustcat · 2016-11-02T09:37:43Z

v1.MediaTypeImageLayer in latest image-spec has changed, need to be bumped up to the latest version.

This commit is only focused on bumping the vendored image-spec and runtime-spec to v1.0.0-rc2. cobra and pflag doesn't tag releases, so I'm pinning this at our currently-vendored commit. I'm not aware of any reason we can't advance this to a later pflag commit, but haven't done enough research to say for sure.

Signed-off-by: Ye Yin eyniy@qq.com

wking · 2016-11-02T15:06:03Z

glide.yaml

@@ -12,3 +12,6 @@ import:
 - package: github.com/pkg/errors
  version: ~0.7.1
 - package: github.com/spf13/cobra
+  version: 9c28e4bbd74e5c3ed7aacbc552b2cab7cfdfe744
+- package: github.com/spf13/pflag
+  version: 7b17cc4658ef5ca157b986ea5c0b43af7938532b


It doesn't look like pflag tags releases. Is there a particular reason you're pinning here instead of at the current tip (spf13/pflag@5ccb023)? Just because that's the currently-vendored commit? I'm fine with that, but would like a note to that effect in the commit message. Something like:

pflag doesn't tag releases, so I'm pinning this at our currently-vendored commit. I'm not aware of any reason we can't advance this to a later pflag commit, but haven't done enough research to say for sure. This commit is only focused on bumping the vendored image-spec.

OK,It's fine to add some notes.

Did you add a note like this somewhere?

I add it in comment:

Do you mean add this note in git commit message?

Yeah, in Git somewhere, in case we leave GitHub or something ;)

It's a good idea.Updated it:)

wking · 2016-11-02T15:10:24Z

Bumping image-spec to a tagged release sounds great to me.

On Wed, Nov 02, 2016 at 02:37:44AM -0700, Ye Yin wrote:

M vendor/github.com/opencontainers/runtime-spec/specs-go/config.go (44)
M vendor/github.com/opencontainers/runtime-spec/specs-go/state.go (2)
M vendor/github.com/opencontainers/runtime-spec/specs-go/version.go (2)

I still think we should be pinning runtime-spec 1.

hustcat · 2016-11-03T02:21:12Z

@wking Updated, PTAL.

coolljt0725 · 2016-11-03T03:08:37Z

vendor/github.com/opencontainers/image-spec/specs-go/descriptor.go

@@ -1,27 +0,0 @@
-// Copyright 2016 The Linux Foundation


why this file has been removed? This make the build failed

2.03s$ make test go test -race -cover github.com/opencontainers/image-tools/cmd/oci-create-runtime-bundle github.com/opencontainers/image-tools/cmd/oci-image-validate github.com/opencontainers/image-tools/cmd/oci-unpack github.com/opencontainers/image-tools/image # github.com/opencontainers/image-tools/vendor/github.com/opencontainers/image-spec/specs-go/v1 vendor/github.com/opencontainers/image-spec/specs-go/v1/manifest.go:25: undefined: Descriptor vendor/github.com/opencontainers/image-spec/specs-go/v1/manifest.go:28: undefined: Descriptor make: *** [test] Error 2 The command "make test" exited with 2. 1.61s$ make tools go build -ldflags "-X main.gitCommit=c8f837309b85f9ef9d039beb3fcb6e09a9ce88f1" ./cmd/oci-create-runtime-bundle # github.com/opencontainers/image-tools/vendor/github.com/opencontainers/image-spec/specs-go/v1 vendor/github.com/opencontainers/image-spec/specs-go/v1/manifest.go:25: undefined: Descriptor vendor/github.com/opencontainers/image-spec/specs-go/v1/manifest.go:28: undefined: Descriptor

Has moved to opencontainers/image-spec/blob/master/specs-go/v1/descriptor.go

# make test go test -race -cover github.com/opencontainers/image-tools/cmd/oci-create-runtime-bundle github.com/opencontainers/image-tools/cmd/oci-image-validate github.com/opencontainers/image-tools/cmd/oci-unpack github.com/opencontainers/image-tools/image ? github.com/opencontainers/image-tools/cmd/oci-create-runtime-bundle [no test files] ? github.com/opencontainers/image-tools/cmd/oci-image-validate [no test files] ? github.com/opencontainers/image-tools/cmd/oci-unpack [no test files] ok github.com/opencontainers/image-tools/image 1.074s coverage: 32.9% of statements

It's OK for me.

I think it's because your run make update-deps and forget to git add vendor/github.com/opencontainers/image-spec/specs-go/v1/descriptor.go,
descriptor.go was in vendor/github.com/opencontainers/image-spec/specs-go before bumping.

Thanks, I have fixed it:)

wking · 2016-11-03T03:48:03Z

image/manifest_test.go

@@ -108,7 +108,7 @@ func TestUnpackLayer(t *testing.T) {

 	testManifest := manifest{
 		Layers: []descriptor{descriptor{
-			MediaType: "application/vnd.oci.image.layer.tar+gzip",
+			MediaType: "application/vnd.oci.image.layer.v1.tar+gzip",


I think you're missing another one of these on line 172:

$ git grep -n vnd.oci.image.layer.tar origin/pr/69 origin/pr/69:image/manifest_test.go:172: MediaType: "application/vnd.oci.image.layer.tar+gzip",

Thanks, Updated:)

wking · 2016-11-03T04:02:56Z

a4857bc looks good to me :).

coolljt0725 · 2016-11-03T05:47:00Z

Look good to me

runcom · 2016-11-03T08:42:38Z

lgtm :)

cyphar · 2016-11-04T10:30:23Z

Can we merge it then? Everything is broken at the moment with this PR not merged (namely umoci and all of the OCI tooling in this repo).

coolljt0725 · 2016-11-09T01:23:08Z

LGTM we could merge this.

xiekeyang · 2016-11-09T01:35:29Z

LGTM

cyphar · 2016-11-09T05:38:24Z

Aren't @coolljt0725 and @xiekeyang maintainers?

@caniszczyk PullApprove is broken (again).

Sn0rt · 2016-11-10T08:41:53Z

@xiekeyang @coolljt0725 hi, can you merge this pr ?

wking · 2016-11-10T08:47:30Z

On Thu, Nov 10, 2016 at 12:41:54AM -0800, Sn0rt wrote:

@xiekeyang @coolljt0725 hi, can you merge this pr ?

Neither were maintainers when this PR was last bumped [1,2], so I
suspect someone needs to kick PullApprove or @hustcat needs to
force-push with a new hash (e.g. after rebasing on master).

coolljt0725 · 2016-11-10T10:15:10Z

@hustcat try with @wking 's suggesntion

needs to
force-push with a new hash (e.g. after rebasing on master).

`v1.MediaTypeImageLayer` in latest image-spec has changed, need to be bumped up to the latest version. This commit is only focused on bumping the vendored image-spec and runtime-spec to v1.0.0-rc2. `cobra` and `pflag` doesn't tag releases, so I'm pinning this at our currently-vendored commit. I'm not aware of any reason we can't advance this to a later pflag commit, but haven't done enough research to say for sure. Signed-off-by: Ye Yin <eyniy@qq.com>

hustcat · 2016-11-11T01:57:46Z

@coolljt0725 rebased and force-pushed, PTAL.

wking · 2016-11-11T02:13:58Z

Doesn't seem to have helped. Maybe PullApprove freezes the approver set at PR creation? @caniszczyk?

cyphar · 2016-11-11T03:10:11Z

Screw this.

@coolljt0725 @xiekeyang Can one of you please just manually merge it and push it? Just do:

% git fetch origin pull/69/head:pr-69
% git merge --no-ff pr-69
% git push origin master

coolljt0725 · 2016-11-11T03:30:22Z

@cyphar manually merge failed

Delta compression using up to 8 threads.
Compressing objects: 100% (24/24), done.
Writing objects: 100% (27/27), 4.83 KiB | 0 bytes/s, done.
Total 27 (delta 18), reused 5 (delta 3)
remote: Resolving deltas: 100% (18/18), completed with 16 local objects.
remote: error: GH006: Protected branch update failed for refs/heads/master.
remote: error: Required status check "code-review/pullapprove" is pending.
To https://github.com/opencontainers/image-tools
 ! [remote rejected] master -> master (protected branch hook declined)
error: failed to push some refs to 'https://github.com/opencontainers/image-tools'

it still need pullapprove

cyphar · 2016-11-11T03:39:16Z

Dammit. Yeah, it looks like GitHub only lets you do manual merges if the PR has all protection checks passing. This is just getting silly now...

wking · 2016-11-11T04:28:31Z

Maybe an admin can kick PullApprove as described in dropseed/pullapprove-support#33.

coolljt0725 · 2016-11-17T06:13:49Z

LGTM

coolljt0725 · 2016-11-17T06:14:35Z

PullApprove works :)

xiekeyang · 2016-11-17T10:59:20Z

LGTM

caniszczyk · 2016-11-17T11:02:54Z

@coolljt0725 @xiekeyang sorry for the delay on looking at this, was busy with KubeCon :)

coolljt0725 · 2016-11-17T11:11:27Z

@caniszczyk thank you :)

glestaris · 2016-11-22T19:04:32Z

Yay!

wking reviewed Nov 2, 2016

View reviewed changes

hustcat force-pushed the bump-up branch from 88c56d4 to aab938e Compare November 3, 2016 02:16

coolljt0725 reviewed Nov 3, 2016

View reviewed changes

hustcat force-pushed the bump-up branch 2 times, most recently from bb6e16d to 0bb57af Compare November 3, 2016 03:41

wking reviewed Nov 3, 2016

View reviewed changes

hustcat force-pushed the bump-up branch from 0bb57af to a4857bc Compare November 3, 2016 03:53

runcom mentioned this pull request Nov 3, 2016

Update dependencies to work with new layer media type #56

Closed

wking mentioned this pull request Nov 10, 2016

Update opencontainers/image-spec and fix gotest #81

Closed

hustcat force-pushed the bump-up branch from a4857bc to dc8cc40 Compare November 11, 2016 01:57

xiekeyang merged commit 66832e7 into opencontainers:master Nov 17, 2016

coolljt0725 mentioned this pull request Jan 17, 2017

image_test.go: Improve the test content #100

Merged

wking mentioned this pull request Jan 23, 2017

image: Port to go-digest #104

Merged

xiekeyang mentioned this pull request Feb 16, 2017

Bump version for v0.1.0 #116

Merged

Bump up runtime-spec/image-spec to the latest version #69

Bump up runtime-spec/image-spec to the latest version #69

Conversation

hustcat commented Nov 2, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wking commented Nov 2, 2016

hustcat commented Nov 3, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coolljt0725 Nov 3, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wking commented Nov 3, 2016 via email

coolljt0725 commented Nov 3, 2016

runcom commented Nov 3, 2016

cyphar commented Nov 4, 2016 • edited Loading

coolljt0725 commented Nov 9, 2016

xiekeyang commented Nov 9, 2016

cyphar commented Nov 9, 2016

Sn0rt commented Nov 10, 2016

wking commented Nov 10, 2016

coolljt0725 commented Nov 10, 2016

hustcat commented Nov 11, 2016

wking commented Nov 11, 2016

cyphar commented Nov 11, 2016 • edited Loading

coolljt0725 commented Nov 11, 2016

cyphar commented Nov 11, 2016

wking commented Nov 11, 2016

coolljt0725 commented Nov 17, 2016 • edited by caniszczyk Loading

coolljt0725 commented Nov 17, 2016

xiekeyang commented Nov 17, 2016 • edited by caniszczyk Loading

caniszczyk commented Nov 17, 2016

coolljt0725 commented Nov 17, 2016

glestaris commented Nov 22, 2016 • edited Loading

hustcat commented Nov 2, 2016 •

edited

Loading

coolljt0725 Nov 3, 2016 •

edited

Loading

cyphar commented Nov 4, 2016 •

edited

Loading

cyphar commented Nov 11, 2016 •

edited

Loading

coolljt0725 commented Nov 17, 2016 •

edited by caniszczyk

Loading

xiekeyang commented Nov 17, 2016 •

edited by caniszczyk

Loading

glestaris commented Nov 22, 2016 •

edited

Loading