Carry #27834 — Do not require `.git` in the build context by vdemeester · Pull Request #30290 · moby/moby

vdemeester · 2017-01-19T18:20:57Z

Carry #27834 and hopefully fixes builds 👼.

Add .git to .dockerignore
export DOCKER_GITCOMMIT in the Makefile
prioritize DOCKER_GITCOMMIT against the git command in
./hack/make.sh
Also add integration-cli/bundles to gitignore (it's generated when
using integration-cli shell)

Note that, on my laptop for example, I'm sending 30M of context with this, and 150M without 🙀, so I really think this can be a huge win 👼

/cc @tianon @jhowardmsft @icecrime @thaJeztah @cpuguy83

🐸

vdemeester · 2017-01-19T18:22:11Z

Had to re-open because I force-pushed the branch before re-opening the PR... 😅
Now that 1.13.0 is out, I'll update the Janky builds to take care of that

lowenna · 2017-01-20T23:51:19Z

@vdemeester - Think I've got all the remaining bits for Windows done. Can you cherry-pick microsoft@e969eab from https://github.com/Microsoft/docker/tree/jjh/gitignore?

I am just doing the final validations to the CI script itself so that it works in the current mode (with .git present in the docker image) and this new mode (without .git in the docker image). Once I've done that, I'll update production Jenkins and things should be good :)

Signed-off-by: John Howard <jhoward@microsoft.com> Signed-off-by: Vincent Demeester <vincent@sbr.pm>

- export DOCKER_GITCOMMIT in the Makefile - prioritize DOCKER_GITCOMMIT against the `git` command in `./hack/make.sh` - Also add `integration-cli/bundles` to gitignore (it's generated when using integration-cli shell) Signed-off-by: Vincent Demeester <vincent@sbr.pm>

Signed-off-by: John Howard jhoward@microsoft.com Signed-off-by: Vincent Demeester <vincent@sbr.pm>

vdemeester · 2017-01-24T14:03:25Z

Updated.. Something bad though, is that some of the validate scripts (lots) are using git to validate only changes 😓 I wonder what to do there /cc @tiborvass @tianon

dnephin · 2017-01-24T16:15:02Z

Bind mount .git! Really we should be bind mounting . I think.

vdemeester · 2017-01-30T15:41:17Z

Ok so it's green, I did 2 things:

On the CI, I am binding .git when validate (and only when validate)
On the Makefile I always bind .git (not a huge fan be…) — this means validation won't work if remote daemon. I should probably make it "conditionnal" but …

dnephin

LGTM

Bind mounts are necessary.

thaJeztah

tested this, and looks to all work, so LGM

but, would like to have @tianon @andrewhsu @cpuguy83 @tiborvass double check if there's possible issues with bind-mounting

AkihiroSuda · 2017-02-02T01:07:25Z

.dockerignore

I thought this is already gone via #29121 ...

Ah you are right 👼 I'll update 😉

tianon · 2017-02-02T17:41:30Z

Makefile

This line could be written more concisely by taking the second -v part completely out of the $(if...), ie:

DOCKER_MOUNT := $(if $(DOCKER_MOUNT),$(DOCKER_MOUNT),-v /go/src/github.com/docker/docker/bundles) -v $(CURDIR)/.git:/go/src/github.com/docker/docker/.git

I'm definitely concerned about what this might mean for remote daemons (since DOCKER_MOUNT already takes DOCKER_HOST into account), but I'm not sure that there's a good solution for that. Perhaps it'd be fine if those validate scripts simply are not supported with a remote daemon unless one explictly removes .git from the .dockerignore?

ah right. Yeah we might print a warning if validate is called on a remote daemon, wdyt ?

tianon · 2017-02-02T17:45:28Z

hack/make.sh

This line already exists above (line 72) -- any reason it needs to be repeated here? (we need BUILDTIME set regardless, so I think the one up on line 72 is correct)

Hum, I don't think there is a reason for that, I'll remove 👼

tianon · 2017-02-03T16:58:56Z

SGTM 👍

LK4D4 · 2017-02-03T19:38:16Z

ping @cpuguy83

Signed-off-by: Vincent Demeester <vincent@sbr.pm>

cpuguy83

LGTM

This actually works out well because now I can build with separate worktrees (git worktree add ../other-docker my_branch)

thaJeztah · 2017-02-05T03:19:12Z

I'll go ahead and merge this, looks like we have enough LGTM's 🏎 🐎

GordonTheTurtle added the status/0-triage label Jan 19, 2017

vdemeester added status/2-code-review and removed status/0-triage labels Jan 19, 2017

thaJeztah added the status/failing-ci Indicates that the PR in its current state fails the test suite label Jan 19, 2017

vdemeester force-pushed the carry-pr-27834 branch from b9cbca4 to 9193c90 Compare January 24, 2017 13:40

GordonTheTurtle added the dco/no Automatically set by a bot when one of the commits lacks proper signature label Jan 24, 2017

John Howard and others added 3 commits January 24, 2017 14:42

Add .git to .dockerignore

a3b3e61

Signed-off-by: John Howard <jhoward@microsoft.com> Signed-off-by: Vincent Demeester <vincent@sbr.pm>

Windows updates for .git in .dockerignore

f32b267

Signed-off-by: John Howard jhoward@microsoft.com Signed-off-by: Vincent Demeester <vincent@sbr.pm>

vdemeester force-pushed the carry-pr-27834 branch from 9193c90 to f32b267 Compare January 24, 2017 13:43

GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label Jan 24, 2017

vdemeester added rebuild/janky and removed rebuild/janky labels Jan 30, 2017

vdemeester removed the status/failing-ci Indicates that the PR in its current state fails the test suite label Jan 30, 2017

dnephin approved these changes Jan 30, 2017

View reviewed changes

thaJeztah approved these changes Jan 31, 2017

View reviewed changes

thaJeztah mentioned this pull request Feb 2, 2017

Speedup build by skipping what's bind-mounted #29298

Closed

AkihiroSuda reviewed Feb 2, 2017

View reviewed changes

vdemeester requested review from andrewhsu, cpuguy83, tianon and tiborvass February 2, 2017 13:36

vdemeester force-pushed the carry-pr-27834 branch from 348857c to 8e6d8eb Compare February 2, 2017 13:43

tianon reviewed Feb 2, 2017

View reviewed changes

Bind mount .git 👼

617be0e

Signed-off-by: Vincent Demeester <vincent@sbr.pm>

vdemeester force-pushed the carry-pr-27834 branch from 8e6d8eb to 617be0e Compare February 3, 2017 20:00

cpuguy83 approved these changes Feb 3, 2017

View reviewed changes

thaJeztah merged commit 4af2555 into moby:master Feb 5, 2017

GordonTheTurtle added this to the 1.14.0 milestone Feb 5, 2017

vdemeester deleted the carry-pr-27834 branch February 5, 2017 07:18

andrewhsu mentioned this pull request Feb 17, 2017

add .git dir to build container docker-archive/docker-bb#2

Merged

twang2218 mentioned this pull request Mar 2, 2017

Add checksum information on the release page #29632

Closed

Conversation

vdemeester commented Jan 19, 2017

Uh oh!

vdemeester commented Jan 19, 2017

Uh oh!

lowenna commented Jan 20, 2017

Uh oh!

vdemeester commented Jan 24, 2017

Uh oh!

dnephin commented Jan 24, 2017

Uh oh!

vdemeester commented Jan 30, 2017

Uh oh!

dnephin left a comment • 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!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda Feb 2, 2017

Choose a reason for hiding this comment

Uh oh!

vdemeester Feb 2, 2017

Choose a reason for hiding this comment

Uh oh!

tianon Feb 2, 2017

Choose a reason for hiding this comment

Uh oh!

vdemeester Feb 3, 2017

Choose a reason for hiding this comment

Uh oh!

tianon Feb 2, 2017

Choose a reason for hiding this comment

Uh oh!

vdemeester Feb 3, 2017

Choose a reason for hiding this comment

Uh oh!

tianon commented Feb 3, 2017 via email

Uh oh!

LK4D4 commented Feb 3, 2017

Uh oh!

cpuguy83 left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Feb 5, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

dnephin left a comment •

edited

Loading