Fork dockerfile

[vrothberg]

commit 7629533 (HEAD -> fork-dockerfile, master)
Author: Valentin Rothberg [email protected]
Date: Thu Mar 7 13:25:53 2019 +0100

fork github.com/docker/docker/builder/dockerfile                     

The package has been changed upstream quite a bit.  As imagebuilder  
relies on the old API, it is incompatible with newer versions of     
Moby.  This enforces users of imagebuilder to use an outdated version
of Moby.

Fork github.com/docker/docker/builder/dockerfile to overcome the     
incompatibility issue and to allow us to fix potential issues        
independently.

All code has been copied from github.docker/docker, so we need to    
mention that.  As the files don't have license headers, adding a     
NOTICE files seems like a suitable alternative.                      

Fixes: #116
Signed-off-by: Valentin Rothberg <[email protected]>               

commit 81424bf
Author: Valentin Rothberg [email protected]
Date: Thu Mar 7 13:10:38 2019 +0100

make test: don't recurse into vendor directory                       

Signed-off-by: Valentin Rothberg <[email protected]>               

commit 279dc3b
Author: Valentin Rothberg [email protected]
Date: Thu Mar 7 12:13:11 2019 +0100

use `vndr` for dependency management

Signed-off-by: Valentin Rothberg <[email protected]>               

[vrothberg]

@rhatdan @nalind @TomSweeneyRedHat PTAL (Cc @siretart)

The project seems to be using glide and godeps for dependency management. Do you think it's possible to move over to vndr?

[vrothberg]

The PR requires some more work to untangle Docker. I will give vndr a shot to see what we can do.

[vrothberg]

Things are green now but I opened containers/buildah#1390 to increase confidence.

[vrothberg]

containers/buildah#1390 is green besides a gofmt complaint.

@rhatdan @TomSweeneyRedHat PTAL

[rhatdan]

LGTM

[TomSweeneyRedHat]

OK, this is likely a question that will lead to Go learning for me. How did you pick this commit? It seems to be a couple years old now, is that the one that was in play currently prior to adding vendor.conf?

[TomSweeneyRedHat]

Probably more GoLang learnings for me, but here goes. So I'm curios as to why you forked /moby/builder/dockerfile into imagebuilder instead of vendoring it too? Would vendoring it give us headaches given we fork all of Docker already? Can/should we add a README.md file to the new dockerfile directory listing the date and commitID that had been forked?

I'd like a head nod from @nalind too.

[vrothberg]

Probably more GoLang learnings for me, but here goes. So I'm curios as to why you forked /moby/builder/dockerfile into imagebuilder instead of vendoring it too? Would vendoring it give us headaches given we fork all of Docker already?

The problem we had prior to this PR is that imagebuilder is using the dockerfile package from Docker. But over time, the dockerfile package has been changed upstream so that imagebuilder won't compile with any recent version of github.com/docker/docker. This is why Buildah and Podman vendor a very old version of docker/docker: they depend on imagebuilder which just won't compile.

@siretart mentioned this issue again recently as he is looking into the Debian packaging where all vendored dependencies must be put into their own, separate package. Since there is already a Docker package (including sources) in Debian it would be extremely unfortunate to have another Docker package just because some dependency of Podman/Buildah is incompatible. Actually, I am certain that the Debian packaging reviews would claim that upstream is broken and that it should be fixed there.

The fix in this PR is to "fork" the dockerfile package by bascically just copying the files from a known working state. However, imagebuilder itself is still limited with respect to using newer versions of docker/docker as it does not vendor upstream github.com/fsouza/go-dockerclient but a downstream one with the following patch which is (again) incompatible with newer versions of Docker: https://github.com/openshift/go-dockerclient/commits/openshift-4.0

Note that Buildah and Podman can use a newer version of docker/docker as we don't need the patch mentioned above (I also updated the Buildah PR to demonstrate that) as we can just update to a newer version of github.com/fsouza/go-dockerclient (we don't need the patch). But I cannot put enough emphasis on the fact that anybody who is trying to use imagebuilder or buildah or libpod - no matter if it's for integration or for packaging - has to go through this hairy ball of dependencies. Also, we cannot update the vendored libraries to fetch (bug) fixes which is a bit scary.

I think such situations should be future blockers from adding any dependency to our tools and libraries as we inherit the problems and pass them on to the users.

Can/should we add a README.md file to the new dockerfile directory listing the date and commitID that had been forked?

That's a good idea but I will add it to dockerfile/NOTICE.

[rhatdan]

/approve
/lgtm

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan, siretart, vrothberg

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[TomSweeneyRedHat]

@vrothberg thank you very much for the very thorough reply. Sounds a bit scary to me, we may want to set up a card (epic) in jira to really scrub this out at some point.

[siretart]

Valentin, I'm still running into issues with compiling this revision:

	cd obj-x86_64-linux-gnu && go install -gcflags=all=\"-trimpath=/<<BUILDDIR>>/golang-github-openshift-imagebuilder-1.0\+git20190308.705fe92/obj-x86_64-linux-gnu/src\" -asmflags=all=\"-trimpath=/<<BUILDDIR>>/golang-github-openshift-imagebuilder-1.0\+git20190308.705fe92/obj-x86_64-linux-gnu/src\" -v -p 4 github.com/openshift/imagebuilder github.com/openshift/imagebuilder/cmd/imagebuilder github.com/openshift/imagebuilder/dockerclient github.com/openshift/imagebuilder/dockerfile github.com/openshift/imagebuilder/dockerfile/command github.com/openshift/imagebuilder/dockerfile/parser github.com/openshift/imagebuilder/dockerfile/parser/dumper github.com/openshift/imagebuilder/imageprogress github.com/openshift/imagebuilder/signal github.com/openshift/imagebuilder/strslice
src/github.com/openshift/imagebuilder/dockerfile/builder.go:16:2: cannot find package "github.com/docker/docker/builder/dockerfile/command" in any of:
	/usr/lib/go-1.11/src/github.com/docker/docker/builder/dockerfile/command (from $GOROOT)
	/<<BUILDDIR>>/golang-github-openshift-imagebuilder-1.0+git20190308.705fe92/obj-x86_64-linux-gnu/src/github.com/docker/docker/builder/dockerfile/command (from $GOPATH)
src/github.com/openshift/imagebuilder/dockerfile/builder.go:17:2: cannot find package "github.com/docker/docker/builder/dockerfile/parser" in any of:
	/usr/lib/go-1.11/src/github.com/docker/docker/builder/dockerfile/parser (from $GOROOT)
	/<<BUILDDIR>>/golang-github-openshift-imagebuilder-1.0+git20190308.705fe92/obj-x86_64-linux-gnu/src/github.com/docker/docker/builder/dockerfile/parser (from $GOPATH)
src/github.com/docker/docker/container/container.go:22:2: cannot find package "github.com/docker/docker/daemon/exec" in any of:
	/<<BUILDDIR>>/golang-github-openshift-imagebuilder-1.0+git20190308.705fe92/obj-x86_64-linux-gnu/src/github.com/docker/docker/vendor/github.com/docker/docker/daemon/exec (vendor tree)
	/usr/lib/go-1.11/src/github.com/docker/docker/daemon/exec (from $GOROOT)
	/<<BUILDDIR>>/golang-github-openshift-imagebuilder-1.0+git20190308.705fe92/obj-x86_64-linux-gnu/src/github.com/docker/docker/daemon/exec (from $GOPATH)
src/github.com/docker/docker/container/container.go:23:2: cannot find package "github.com/docker/docker/daemon/logger" in any of:
	/<<BUILDDIR>>/golang-github-openshift-imagebuilder-1.0+git20190308.705fe92/obj-x86_64-linux-gnu/src/github.com/docker/docker/vendor/github.com/docker/docker/daemon/logger (vendor tree)
	/usr/lib/go-1.11/src/github.com/docker/docker/daemon/logger (from $GOROOT)
	/<<BUILDDIR>>/golang-github-openshift-imagebuilder-1.0+git20190308.705fe92/obj-x86_64-linux-gnu/src/github.com/docker/docker/daemon/logger (from $GOPATH)
src/github.com/docker/docker/container/container.go:24:2: cannot find package "github.com/docker/docker/daemon/logger/jsonfilelog" in any of:
	/<<BUILDDIR>>/golang-github-openshift-imagebuilder-1.0+git20190308.705fe92/obj-x86_64-linux-gnu/src/github.com/docker/docker/vendor/github.com/docker/docker/daemon/logger/jsonfilelog (vendor tree)
	/usr/lib/go-1.11/src/github.com/docker/docker/daemon/logger/jsonfilelog (from $GOROOT)
	/<<BUILDDIR>>/golang-github-openshift-imagebuilder-1.0+git20190308.705fe92/obj-x86_64-linux-gnu/src/github.com/docker/docker/daemon/logger/jsonfilelog (from $GOPATH)
src/github.com/docker/docker/container/container.go:25:2: cannot find package "github.com/docker/docker/daemon/logger/local" in any of:
	/<<BUILDDIR>>/golang-github-openshift-imagebuilder-1.0+git20190308.705fe92/obj-x86_64-linux-gnu/src/github.com/docker/docker/vendor/github.com/docker/docker/daemon/logger/local (vendor tree)
	/usr/lib/go-1.11/src/github.com/docker/docker/daemon/logger/local (from $GOROOT)
	/<<BUILDDIR>>/golang-github-openshift-imagebuilder-1.0+git20190308.705fe92/obj-x86_64-linux-gnu/src/github.com/docker/docker/daemon/logger/local (from $GOPATH)
[...]

And many similar instances. I notice that https://github.com/openshift/imagebuilder/blob/master/dockerfile/builder.go still references github.com/docker/docker/builder/dockerfile/* imports. Is that intentional or an oversight? Wasn't the goal to avoid the dependency on the github.com/docker/docker/builder/* imports?

BTW, I notice that http://github.com/docker/docker is redirecting to https://github.com/moby/moby. Doesn't that also imply that every import of anything underneath github.com/docker/docker would need updating? - I'm a bit confused right now.

[vrothberg]

And many similar instances. I notice that https://github.com/openshift/imagebuilder/blob/master/dockerfile/builder.go still references github.com/docker/docker/builder/dockerfile/* imports. Is that intentional or an oversight? Wasn't the goal to avoid the dependency on the github.com/docker/docker/builder/* imports?

That was pretty big oversight. Thanks for bringing this up! I'll have another shot.

BTW, I notice that http://github.com/docker/docker is redirecting to https://github.com/moby/moby. Doesn't that also imply that every import of anything underneath github.com/docker/docker would need updating? - I'm a bit confused right now.

Docker is redirecting to Moby while all packages in Moby are still using the "old" docker/docker references. Imagebuilder, however, is still trapped on an ancient version of Docker as mentioned in #125 (comment).

[vrothberg]

Here's a PR aiming at cleaning up the remaining bits in dockerfile: #127

I deleted all the files that we don't need in imagebuilder.