git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kyle Meyer <kyle@kyleam.com>
To: Elijah Newren <newren@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: [BUG?] ls-files -o now traverses nested repo when given multiple pathspecs
Date: Wed, 04 Dec 2019 15:04:48 -0500	[thread overview]
Message-ID: <874kyf6en3.fsf@kyleam.com> (raw)
In-Reply-To: <CABPp-BFG3FkTkC=L1v97LUksndkOmCN8ZhNJh5eoNdquE7v9DA@mail.gmail.com>

Hi Elijah,

Thanks for the detailed and helpful reply.

Elijah Newren <newren@gmail.com> writes:

[...]

> As you mentioned, it won't traverse into it even when specified...
>
> $ git ls-files -o untracked_repo/
> untracked_repo/empty
>
> ...except that it does traverse into this directory if the user tab
> completes the name or otherwise manually adds a trailing slash.

Ah yes, I recall encountering what I think is the same underlying issue
when working on a previous series [0,1].  In the context of 'git add
untracked_repo/', there's been some discussion related to this trailing
slash discrepancy at

  https://lore.kernel.org/git/20180618111919.GA10085@book.hvoigt.net/T/#u

> Weird, let's try multiple pathspecs:
>
> $ git ls-files -o untracked_dir untracked_repo
> untracked_dir/empty
> untracked_repo/
>
> $ git ls-files -o untracked_dir untracked_repo/
> untracked_dir/empty
> untracked_repo/
>
> So it will traverse into the untracked_repo when specified as
> 'untracked_repo/' but not if there are more than one pathspec given?!?

Eh, indeed.

>  And it traverses into an untracked directory regardless of the
> trailing slash?  <sarcasm>What a paragon of consistency...</sarcasm>
>
>
> At least my changes in git-2.24.0 made the behavior consistent; it'll
> always traverse into a directory that matches a given pathspec.

I might be getting mixed up, but the changes in 2.24.0 did introduce
some inconsistent behavior (in the no trailing slash case) with respect
to giving a single pathspec and giving multiple pathspecs, no?  Using
your example:

    $ git --version
    git version 2.24.0
    $ git ls-files -o untracked_repo
    untracked_repo/
    $ git ls-files -o untracked_repo empty
    empty
    untracked_repo/
    untracked_repo/empty

> As for whether that's desirable or not when the pathspec is a submodule,
> I'm not certain. [...]
>
> But here are some possibilities that at least sound sane:
>
> A) ls-files -o should traverse into untracked submodules.  This case
> is easy; the code already does that.

Hmm, but as shown in the last example, ls-files -o doesn't traverse into
untracked submodules for the single pathspec case.

> B) ls-files -o should NOT traverse into untracked submodules AND
> should not even report them.
>
> C) ls-files -o should NOT traverse into untracked submodules, but
> should at least report their directory name.  If so, the fix is
> [...]

This behavior---which matches the no-slash behavior when no patchspec or
a single pathspec is given (on both v2.24.0 and previous version) as
well as when multiple pathspecs are given (before v2.24.0)---is the one
I prefer.  My biased reason for this preference is that in the DataLad
project we identify untracked nested repositories based on `ls-files -o
<untracked directory>...` reporting only the directory name for
repositories.  (Looking into one of our tests that fails with Git
v2.24.0 is how I ran into the reported change in behavior [2].)

That some external project relies on unintended ls-files output of
course doesn't mean that Git should keep reporting things that way, but
it does mean that I _hope_ that not traversing into untracked
repositories is the intended behavior and that traversing (either
because a slash is appended or as of 89a1f4aaf7 because multiple
pathspecs are given) is not intended :>


[0]: https://lore.kernel.org/git/20190409230737.26809-1-kyle@kyleam.com
[1]: https://lore.kernel.org/git/87bm1mbua4.fsf@kyleam.com/
[2]: https://github.com/datalad/datalad/issues/3890#issuecomment-561722194

  parent reply	other threads:[~2019-12-04 20:04 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-03 22:08 [BUG?] ls-files -o now traverses nested repo when given multiple pathspecs Kyle Meyer
2019-12-04 17:30 ` Elijah Newren
2019-12-04 19:42   ` Junio C Hamano
2019-12-04 20:04   ` Kyle Meyer [this message]
2019-12-08  5:31     ` Kyle Meyer
2019-12-08  5:42       ` Elijah Newren
2019-12-08  7:46         ` Elijah Newren
2019-12-08 22:59           ` Kyle Meyer

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=874kyf6en3.fsf@kyleam.com \
    --to=kyle@kyleam.com \
    --cc=git@vger.kernel.org \
    --cc=newren@gmail.com \
    /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).