vndr github.com/docker/docker from components/engine #116

andrewhsu · 2017-07-13T05:10:56Z

Found differences between the engine and the cli vendor of engine in the docker-ce 17.06 branch. This PR will vendor github.com/docker/docker from components/engine dir.

Signed-off-by: Andrew Hsu <andrewhsu@docker.com>

thaJeztah · 2017-07-13T05:37:35Z

components/cli/vendor/github.com/docker/docker/opts/config.go

@@ -1,98 +0,0 @@
-package opts


this was removed in moby/moby#33296

i traced the sequence of events and it looks like this missing vendor change is a result of creating the docker-ce 17.06 branch before the cli was re-vendored. here is the sequence of events:

May 19: merge Remove more opts unused in docker/docker moby/moby#33296 to moby/moby which removed opts/config.go

May 19: create docker-ce 17.06 branch

Jul 7: re-vndr docker/docker to remove opts/config.go from vendor docker/cli@44ac808

thaJeztah · 2017-07-13T05:37:47Z

components/cli/vendor/github.com/docker/docker/opts/opts.go


 import (
 	"fmt"
-	"math/big"


moby/moby#33296

same reason as #116 (comment), vendor was not in sync when 17.06 branch was created

thaJeztah · 2017-07-13T05:38:51Z

components/cli/vendor/github.com/docker/docker/opts/throttledevice.go

@@ -1,111 +0,0 @@
-package opts


moby/moby#33296

same reason as #116 (comment), vendor was not in sync when 17.06 branch was created

thaJeztah · 2017-07-13T05:39:24Z

components/cli/vendor/github.com/docker/docker/pkg/pools/pools.go

 	"github.com/docker/docker/pkg/ioutils"
 )

+const buffer32K = 32 * 1024


this change to components/cli was a result of a missing vendor update when #76 was cherry-picked to components/engine. should have been a separate commit in PR #76 to update the vendor

thaJeztah · 2017-07-13T05:40:58Z

components/cli/vendor/github.com/docker/docker/pkg/gitutils/gitutils.go

 	"github.com/pkg/errors"
 )

+type gitRepo struct {


this change we definitely need to get in to fix stuff to do stuff

cause of this discrepancy is same reason as #116 (comment): PR #100 should have had another commit to update components/cli/vendor

thaJeztah · 2017-07-13T05:42:12Z

components/cli/vendor/github.com/docker/docker/pkg/term/tc_solaris_cgo.go

 import "C"

 // Termios is the Unix API for terminal I/O.
-// It is passthrough for syscall.Termios in order to make it portable with


moby/moby#33269

same reason as #116 (comment), vendor was not in sync when 17.06 branch was created

thaJeztah · 2017-07-13T05:43:55Z

components/cli/vendor/github.com/docker/docker/registry/config_unix.go

 // installCliPlatformFlags handles any platform specific flags for the service.
 func (options *ServiceOptions) installCliPlatformFlags(flags *pflag.FlagSet) {
-	flags.BoolVar(&options.V2Only, "disable-legacy-registry", false, "Disable contacting legacy registries")
+	flags.BoolVar(&options.V2Only, "disable-legacy-registry", true, "Disable contacting legacy registries")


moby/moby#33629 (cherry-pick: #71)

cause of this discrepancy is same reason as #116 (comment): PR #71 should have had another commit to update components/cli/vendor

thaJeztah · 2017-07-13T05:46:39Z

components/cli/vendor/github.com/docker/docker/volume/volume.go

-func (m *MountPoint) Setup(mountLabel string, rootUID, rootGID int) (path string, err error) {
+// The, optional, checkFun parameter allows doing additional checking
+// before creating the source directory on the host.
+func (m *MountPoint) Setup(mountLabel string, rootUID, rootGID int, checkFun func(m *MountPoint) error) (path string, err error) {


moby/moby#33330

Looks like PR #48 should have had another commit to update components/cli/vendor

thaJeztah · 2017-07-13T06:00:37Z

I linked the related PR's either upstream (moby/moby) and/or the cherry-picks in components/engine

thaJeztah

LGTM

ping @andrewhsu still "WIP"?

andrewhsu · 2017-07-24T22:48:35Z

I'm ok to have this PR in as a stop gap measure for now to get these guys in sync, but we have to think about a way to get the vendor dir updated on a more regular basis.

andrewhsu · 2017-07-24T22:48:51Z

@tiborvass fyi

thaJeztah · 2017-07-25T11:10:27Z

By now we probably should do the same again (various PR's were merged)?

tiborvass · 2017-07-27T00:28:49Z

@vieux @andrewhsu @thaJeztah wdyt of 17.06...tiborvass:fix-vendor ?

tiborvass · 2017-07-27T03:20:08Z

I opened my version at #148

andrewhsu · 2017-08-02T06:44:44Z

I've scrubbed all of the comments put in by @thaJeztah (thanks for digging into it) and found that the discrepancies between components/engine and components/cli/vendor/github.com/docker/docker are caused by one of two reasons:

creating branch 17.06 when the upstream components docker/cli had not vendored moby/moby to keep in sync
creating PRs to 17.06 that modify code in components/engine but with no corresponding sync to vendor in components/cli/vendor/github.com/docker/docker

For the first item, I suggest we setup a regular vendor sync in docker/cli repo to vendor changes from moby/moby /cc @dnephin

For the second item, I suggest we setup automatic vendor check to make sure code changes to components/engine are vendored into components/cli/vendor/github.com/docker/docker on PRs to docker-ce /cc @seemethere and I'd say #148 (thanks @tiborvass) is a step towards that direction.

So, I'm gong to close this PR in favor of #148 since it synced up the vendor directory a little later in time than this PR and caught 2 more vendor mismatches.

dnephin · 2017-08-02T14:35:21Z

Why is vendor mismatch a problem if the tests pass?

[18.06] Remove Debian Jessie for arm64 Upstream-commit: 68e62582600895376c743e0e6b7e3df3478cd0b9 Component: packaging

…al_templates [18.09 backport] apparmor: allow receiving of signals from 'docker kill' Upstream-commit: 12b8ec42b6ac60188ac48137677ee69ce98a3822 Component: engine

vndr github.com/docker/docker from components/engine

ed98a2f

Signed-off-by: Andrew Hsu <andrewhsu@docker.com>

andrewhsu changed the title ~~vndr github.com/docker/docker from components/engine~~ WIP vndr github.com/docker/docker from components/engine Jul 13, 2017

thaJeztah reviewed Jul 13, 2017

View reviewed changes

andrewhsu added this to the 17.06.1 milestone Jul 14, 2017

thaJeztah approved these changes Jul 18, 2017

View reviewed changes

andrewhsu changed the title ~~WIP vndr github.com/docker/docker from components/engine~~ vndr github.com/docker/docker from components/engine Jul 24, 2017

tiborvass mentioned this pull request Jul 27, 2017

Add top-level make vendor and run it. #148

Merged

andrewhsu closed this Aug 2, 2017

andrewhsu removed this from the 17.06.1 milestone Aug 2, 2017

andrewhsu deleted the sync-components branch August 2, 2017 06:49

docker-jenkins pushed a commit that referenced this pull request Jul 3, 2018

Merge pull request #116 from seemethere/remove

b55756b

[18.06] Remove Debian Jessie for arm64 Upstream-commit: 68e62582600895376c743e0e6b7e3df3478cd0b9 Component: packaging

vndr github.com/docker/docker from components/engine #116

vndr github.com/docker/docker from components/engine #116

Uh oh!

Conversation

andrewhsu commented Jul 13, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewhsu Aug 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewhsu Aug 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewhsu Aug 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Jul 13, 2017

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

andrewhsu commented Jul 24, 2017

Uh oh!

andrewhsu commented Jul 24, 2017

Uh oh!

thaJeztah commented Jul 25, 2017

Uh oh!

tiborvass commented Jul 27, 2017

Uh oh!

tiborvass commented Jul 27, 2017

Uh oh!

andrewhsu commented Aug 2, 2017

Uh oh!

dnephin commented Aug 2, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

andrewhsu Aug 2, 2017 •

edited

Loading

andrewhsu Aug 2, 2017 •

edited

Loading

andrewhsu Aug 2, 2017 •

edited

Loading