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

Rebased Dockerignore #6579

Merged
merged 5 commits into from Jun 27, 2014

Conversation

Projects
None yet
10 participants
@vieux
Collaborator

vieux commented Jun 20, 2014

Close #4165
Fixes #2224

@unclejack

This comment has been minimized.

Show comment
Hide comment
@unclejack

unclejack Jun 20, 2014

Contributor

LGTM

Contributor

unclejack commented Jun 20, 2014

LGTM

@vieux

This comment has been minimized.

Show comment
Hide comment
@vieux

vieux Jun 20, 2014

Collaborator

ping @tianon can you please check last one time that is the best .dockerignore we can have in our repo ?

Collaborator

vieux commented Jun 20, 2014

ping @tianon can you please check last one time that is the best .dockerignore we can have in our repo ?

@tianon

This comment has been minimized.

Show comment
Hide comment
@tianon

tianon Jun 20, 2014

Member

LGTM ❤️ Do you want to add me to the MAINTAINERS for that file since it dovetails nicely with the Dockerfile?

Member

tianon commented Jun 20, 2014

LGTM ❤️ Do you want to add me to the MAINTAINERS for that file since it dovetails nicely with the Dockerfile?

@crosbymichael

This comment has been minimized.

Show comment
Hide comment
@crosbymichael

crosbymichael Jun 20, 2014

Contributor

don't give @tianon the power

Contributor

crosbymichael commented Jun 20, 2014

don't give @tianon the power

Show outdated Hide outdated api/client/commands.go Outdated
If a file named ``.dockerignore`` exists in the root of ``PATH`` then it is
interpreted as a newline-separated list of exclusion patterns. Exclusion
patterns match files or directories relative to ``PATH`` that will be excluded
from the context. Globbing is done using Go's

This comment has been minimized.

@tiborvass

tiborvass Jun 20, 2014

Collaborator

I thought there was consensus in the original PR that we would not support globbing. Is everybody okay with supporting globbing now?

@tiborvass

tiborvass Jun 20, 2014

Collaborator

I thought there was consensus in the original PR that we would not support globbing. Is everybody okay with supporting globbing now?

This comment has been minimized.

@crosbymichael

crosbymichael Jun 23, 2014

Contributor

The last thing I remembered was no globbing to keep it simple.

@crosbymichael

crosbymichael Jun 23, 2014

Contributor

The last thing I remembered was no globbing to keep it simple.

This comment has been minimized.

@tianon

tianon Jun 23, 2014

Member

Booo :)

@tianon

tianon Jun 23, 2014

Member

Booo :)

This comment has been minimized.

@LK4D4

LK4D4 Jun 23, 2014

Contributor

This is very-very-very simple globbing. Not a globbing at all :D I vote for it. I am for example need to exclude tons of sql and php files.

@LK4D4

LK4D4 Jun 23, 2014

Contributor

This is very-very-very simple globbing. Not a globbing at all :D I vote for it. I am for example need to exclude tons of sql and php files.

This comment has been minimized.

@unclejack

unclejack Jun 23, 2014

Contributor

The code which handles this isn't that complicated, so I'm OK with having globbing.

@unclejack

unclejack Jun 23, 2014

Contributor

The code which handles this isn't that complicated, so I'm OK with having globbing.

This comment has been minimized.

@tiborvass

tiborvass Jun 23, 2014

Collaborator

@unclejack I don't think the complexity was referring to the code complexity but to behavior complexity and unknown weird side effects that we would have to maintain. The idea was to stay conservative and later add globbing if really necessary.

@LK4D4 Do you need the folder in which those sql and php files are, in the Dockerfile context?

@tiborvass

tiborvass Jun 23, 2014

Collaborator

@unclejack I don't think the complexity was referring to the code complexity but to behavior complexity and unknown weird side effects that we would have to maintain. The idea was to stay conservative and later add globbing if really necessary.

@LK4D4 Do you need the folder in which those sql and php files are, in the Dockerfile context?

This comment has been minimized.

@LK4D4

LK4D4 Jun 24, 2014

Contributor

I have a lot of dirs with them, and they autogenerated and laid near my precious python file.
Also, behavior of this "globbing" well described in go docs, on which we have link here.

@LK4D4

LK4D4 Jun 24, 2014

Contributor

I have a lot of dirs with them, and they autogenerated and laid near my precious python file.
Also, behavior of this "globbing" well described in go docs, on which we have link here.

This comment has been minimized.

@vieux

vieux Jun 26, 2014

Collaborator

@shykes can you confirm your view on the subject ? I think it's almost useless without simple globbing

@vieux

vieux Jun 26, 2014

Collaborator

@shykes can you confirm your view on the subject ? I think it's almost useless without simple globbing

This comment has been minimized.

@LK4D4

LK4D4 Jun 26, 2014

Contributor

@vieux I believe you meant without :)

@LK4D4

LK4D4 Jun 26, 2014

Contributor

@vieux I believe you meant without :)

This comment has been minimized.

@vieux

vieux Jun 26, 2014

Collaborator

@LK4D4 yes thanks.

@vieux

vieux Jun 26, 2014

Collaborator

@LK4D4 yes thanks.

@@ -6,3 +6,4 @@ Michael Crosby <michael@crosbymichael.com> (@crosbymichael)
AUTHORS: Tianon Gravi <admwiggin@gmail.com> (@tianon)
Dockerfile: Tianon Gravi <admwiggin@gmail.com> (@tianon)
Makefile: Tianon Gravi <admwiggin@gmail.com> (@tianon)
.dockerignore: Tianon Gravi <admwiggin@gmail.com> (@tianon)

This comment has been minimized.

@crosbymichael

crosbymichael Jun 21, 2014

Contributor

dont ignore the .git dir i needs it

@crosbymichael

crosbymichael Jun 21, 2014

Contributor

dont ignore the .git dir i needs it

This comment has been minimized.

@tianon

tianon Jun 21, 2014

Member

Don't worry, we won't; our build script needs it too. :)

@tianon

tianon Jun 21, 2014

Member

Don't worry, we won't; our build script needs it too. :)

@vieux vieux added the /project/doc label Jun 23, 2014

@adambiggs

This comment has been minimized.

Show comment
Hide comment
@adambiggs

adambiggs Jun 23, 2014

Another +1 for this.

I'm developing with Vagrant, and deploying via Docker to AWS Elastic Beanstalk. Problem is, AWS needs the Dockerfile to be in the root of the Git repo being deployed. This means the .git and .vagrant directories get copied in the context, and I can't use the current workaround of putting the Docker context in a subdirectory...

Unless there's another workaround I am stuck manually deploying .zip archives to AWS until this feature drops. 😢

adambiggs commented Jun 23, 2014

Another +1 for this.

I'm developing with Vagrant, and deploying via Docker to AWS Elastic Beanstalk. Problem is, AWS needs the Dockerfile to be in the root of the Git repo being deployed. This means the .git and .vagrant directories get copied in the context, and I can't use the current workaround of putting the Docker context in a subdirectory...

Unless there's another workaround I am stuck manually deploying .zip archives to AWS until this feature drops. 😢

@vieux vieux added this to the 1.0.2 milestone Jun 26, 2014

@crosbymichael

This comment has been minimized.

Show comment
Hide comment
@crosbymichael

crosbymichael Jun 26, 2014

Contributor

@vieux

+ go test github.com/dotcloud/docker/archive
# github.com/dotcloud/docker/archive
./archive_test.go:180: undefined: TarFilter
FAIL    github.com/dotcloud/docker/archive [build failed]

Tests failed: ./archive
Contributor

crosbymichael commented Jun 26, 2014

@vieux

+ go test github.com/dotcloud/docker/archive
# github.com/dotcloud/docker/archive
./archive_test.go:180: undefined: TarFilter
FAIL    github.com/dotcloud/docker/archive [build failed]

Tests failed: ./archive

tmc and others added some commits Feb 14, 2014

Change misnamed TarFilter to TarWithOptions
Docker-DCO-1.1-Signed-off-by: Travis Cline <travis.cline@gmail.com> (github: tmc)
Add .dockerignore support
Fixes #2224

Docker-DCO-1.1-Signed-off-by: Travis Cline <travis.cline@gmail.com> (github: tmc)
add @tianon as maintainer of .dockerignore
Docker-DCO-1.1-Signed-off-by: Victor Vieux <vieux@docker.com> (github: vieux)
abort on error and fix debug
Docker-DCO-1.1-Signed-off-by: Victor Vieux <vieux@docker.com> (github: vieux)
fix rebase
Docker-DCO-1.1-Signed-off-by: Victor Vieux <vieux@docker.com> (github: vieux)
@vieux

This comment has been minimized.

Show comment
Hide comment
@vieux

vieux Jun 26, 2014

Collaborator

@crosbymichael please retry

Collaborator

vieux commented Jun 26, 2014

@crosbymichael please retry

@vieux

This comment has been minimized.

Show comment
Hide comment
@vieux

vieux Jun 26, 2014

Collaborator

@tmc can you add some docs ?

Collaborator

vieux commented Jun 26, 2014

@tmc can you add some docs ?

@crosbymichael

This comment has been minimized.

Show comment
Hide comment
@crosbymichael

crosbymichael Jun 26, 2014

Contributor

LGTM

Contributor

crosbymichael commented Jun 26, 2014

LGTM

1 similar comment
@tiborvass

This comment has been minimized.

Show comment
Hide comment
@tiborvass

tiborvass Jun 27, 2014

Collaborator

LGTM

Collaborator

tiborvass commented Jun 27, 2014

LGTM

tiborvass added a commit that referenced this pull request Jun 27, 2014

@tiborvass tiborvass merged commit 6e3fe93 into moby:master Jun 27, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@tiborvass

This comment has been minimized.

Show comment
Hide comment
@tiborvass

tiborvass Jun 27, 2014

Collaborator

ping @SvenDowideit Sorry I merged a little too quickly. Can we do another PR for the docs?

Collaborator

tiborvass commented Jun 27, 2014

ping @SvenDowideit Sorry I merged a little too quickly. Can we do another PR for the docs?

@benjamine

This comment has been minimized.

Show comment
Hide comment
@benjamine

benjamine Jun 27, 2014

😍 anxious to see that released, thanks!

benjamine commented Jun 27, 2014

😍 anxious to see that released, thanks!

@vieux vieux deleted the vieux:dockerignore branch Jun 27, 2014

@@ -215,6 +215,12 @@ temporary directory on your local host, and then this is sent to the
Docker daemon as the context. This way, your local user credentials and
vpn's etc can be used to access private repositories.
If a file named ``.dockerignore`` exists in the root of ``PATH`` then it is

This comment has been minimized.

@fredlf

fredlf Jul 1, 2014

Contributor

Change to single back-ticks. Also below.

@fredlf

fredlf Jul 1, 2014

Contributor

Change to single back-ticks. Also below.

This comment has been minimized.

@tiborvass

tiborvass Jul 1, 2014

Collaborator

I think this has been fixed in #6766

@tiborvass

tiborvass Jul 1, 2014

Collaborator

I think this has been fixed in #6766

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment