Skip to content

Conversation

thomasboyt
Copy link

Closes #859.

The major limitation this fix has is that, if the dockerignore has any exception rules, the walk algorithm will descend into ignored directories, since of course if you have a dockerignore like:

foo/bar/baz/*
!*.txt

it needs to descend into foo/bar/baz to see if there are any files that match *.txt to include. This is the same behavior that Docker has, so I think it's acceptable - long-term, it may be possible to further optimize this by only descending if there is an exception rule that could actually match inside the directory.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably better to use a timestamp here instead of "current docker logic"

@aanand
Copy link
Contributor

aanand commented Nov 24, 2015

Nice. I agree with @kanzure's comments, plus it'd be good to document the logic of should_include.

@thomasboyt
Copy link
Author

@aanand cool, added some docs/comments!

@shin- shin- added this to the 1.7.0 milestone Nov 24, 2015
@aanand
Copy link
Contributor

aanand commented Dec 7, 2015

Thanks! I think this can be squashed to one commit.

Signed-off-by: Thomas Boyt <thomas@ledgerx.com>
@thomasboyt
Copy link
Author

@aanand rebased :)

@aanand
Copy link
Contributor

aanand commented Dec 8, 2015

Thanks. I just noticed that we're now excluding parent directories of files mentioned in !-style exceptions. Is that in line with Docker client behaviour?

@thomasboyt
Copy link
Author

So, while before it was adding the parent directories of exceptions to the set of included paths, it wasn't adding back the contents of those directories.

Let's say you have the following structure:

foo/
  a.py
  b.py
  bar/
    a.py

and the following rules:

foo
!foo/bar/a.py

Before, the final set of included paths would have looked like:

foo
foo/bar
foo/bar/a.py

Because the tar created in the build process adds each path non-recursively, this didn't actually include foo/a.py or foo/b.py, it just made empty directories for foo/ and foo/bar/.

Now, the final set of included paths just looks like:

foo/bar/a.py

Even though foo and foo/bar are no longer included in the set of paths, they'll still be created when foo/bar/a.py is added to the build context (which the updated integration test confirms, since I wanted to make sure it was creating the directories).

@aanand
Copy link
Contributor

aanand commented Dec 16, 2015

OK, so even if we're not passing the directories explicitly to tar, they'll still be created as needed? Sounds good. Glad the integration test now explicitly checks the "exception rule for file in ignored subdirectory" case.

LGTM

@dnephin
Copy link
Contributor

dnephin commented Dec 16, 2015

LGTM

aanand added a commit that referenced this pull request Dec 16, 2015
Don't descend into ignored directories when building context
@aanand aanand merged commit 9deffc4 into docker:master Dec 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dockerignore implementation is relatively slow compared to Docker's implementation

6 participants