From: Jeff King <peff@peff.net>
To: "René Scharfe" <l.s.r@web.de>
Cc: git@vger.kernel.org, David Adam <zanchey@ucc.gu.uwa.edu.au>,
Duy Nguyen <pclouds@gmail.com>,
Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 4/4] archive: queue directories for all types of pathspecs
Date: Sun, 20 Aug 2017 05:06:30 -0400 [thread overview]
Message-ID: <20170820090629.tumvqwzkromcykjf@sigill.intra.peff.net> (raw)
In-Reply-To: <33fa4f08-8f06-5a98-e492-3f05cc742555@web.de>
On Sat, Aug 19, 2017 at 06:53:26PM +0200, René Scharfe wrote:
> Am 19.08.2017 um 07:33 schrieb René Scharfe:
> > When read_tree_recursive() encounters a directory excluded by a pathspec
> > then it enters it anyway because it might contain included entries. It
> > calls the callback function before it is able to decide if the directory
> > is actually needed.
> >
> > For that reason git archive adds directories to a queue and writes
> > entries for them only when it encounters the first child item -- but
> > only if pathspecs with wildcards are used. Do the same for literal
> > pathspecs as well, as the reasoning above applies to them, too. This
> > prevents git archive from writing entries for excluded directories.
>
> This breaks the test "archive empty subtree with no pathspec" in t5004 by
> omitting the empty directory from the archive. Sorry for missing that!
>
> This is kind of a bonus patch, so please discard it for now; the first
> three are OK IMHO.
>
> A better version of this patch would at least update t5004 as well, but
> there might be a better way.
I actually think it would be reasonable to omit the empty directory in
t5004. The main thing we care about there is that we produce an archive
with no files rather than barfing. Checking that the empty directory is
actually there was mostly "this is what it happens to produce" rather
than any conscious decision, I think.
If the new rule is "we omit empty directories", then it would make sense
for us to follow that, regardless of whether it happened by pathspec
limiting or if the tree was empty in the first place (and such an
empty tree is insane anyway; Git tries hard not to create them).
-Peff
next prev parent reply other threads:[~2017-08-20 9:06 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-13 4:53 Bug?: git archive exclude pathspec and gitattributes export-ignore David Adam
2017-08-14 16:43 ` René Scharfe
2017-08-15 2:39 ` David Adam
2017-08-19 5:26 ` René Scharfe
2017-08-19 5:28 ` [PATCH 1/4] t5001: add tests for export-ignore attributes and exclude pathspecs René Scharfe
2017-08-19 5:29 ` [PATCH 2/4] archive: factor out helper functions for handling attributes René Scharfe
2017-08-19 5:32 ` [PATCH 3/4] archive: don't queue excluded directories René Scharfe
2017-08-19 5:33 ` [PATCH 4/4] archive: queue directories for all types of pathspecs René Scharfe
2017-08-19 16:53 ` René Scharfe
2017-08-19 16:58 ` René Scharfe
2017-08-19 17:42 ` Junio C Hamano
2017-08-20 9:06 ` Jeff King [this message]
2017-09-12 22:43 ` René Scharfe
2017-09-13 12:53 ` Jeff King
2017-09-13 14:56 ` René Scharfe
2017-08-21 18:17 ` Stefan Beller
2017-08-19 16:58 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170820090629.tumvqwzkromcykjf@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=l.s.r@web.de \
--cc=pclouds@gmail.com \
--cc=zanchey@ucc.gu.uwa.edu.au \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).