ls-files --exclude broken?

Am I missing something really obvious here?

kore:~/Repos/git (master)$ git ls-files | wc -l
    2009
kore:~/Repos/git (master)$ git ls-files -x \* | wc -l
    2009
kore:~/Repos/git (master)$ git ls-files -i -x \* | wc -l
    2009
kore:~/Repos/git (master)$ git version
git version 1.7.3.rc1.5.g95127

j.

On Thu, Sep 16, 2010 at 8:40 AM, Jay Soffian <jaysoffian@gmail.com> wrote:

excluded() is not called as far as I can tell, which explains why it did not exclude anything for you. That exclude code was touched by Jeff in commits b5227d8 (ls-files: excludes should not impact tracked files) and 500348a (ls-files: unbreak "ls-files -i").

Something like this works for me, but I haven't finished my morning tea yet, so..

Jay Soffian <jaysoffian@gmail.com> writes:

You asked it to show the cached paths (default). Your project currently tracks 2009 paths in the index.

You told that '*' is the exclude pattern for carrying out some operation, but you didn't tell what operation you want. You are shown the cached paths (default).

You told that '*' is the exclude pattern, you want only paths that match the exclude pattern, and chose to show files in the index (which again is the default) by not saying -o.

I've never found -i useful myself (actually I don't find many options the command has very useful anymore), but the above is how I read the ls-files manual page.

On Wed, Sep 15, 2010 at 7:12 PM, Junio C Hamano <gitster@pobox.com> wrote:

Correct.

I want cached paths, minus the exclude pattern. Using -c -x \* gives the same result.

There is no indication in the man page that -x doesn't apply to -c.

I don't care about -i myself, and maybe I should have been clearer. AFAICT, [-c] -x is broken:

$ git ls-files -c -x \* |wc -l
    2009

       -c, --cached
           Show cached files in the output (default)

       -x <pattern>, --exclude=<pattern>
           Skips files matching pattern. Note that pattern is a shell
wildcard pattern.

Oh, geez, here it is lower down in the man page:

  git ls-files can use a list of "exclude patterns" when traversing
the directory tree and finding files to show when the flags --others
or --ignored are specified. gitignore(5) specifies the format of
exclude patterns.

Bah, what use is that? Minimally ls-files shouldn't accept -x unless given either --ignored or --others if that's all it applies to. I guess this is my itch to scratch.

j.

On Thu, Sep 16, 2010 at 9:19 AM, Jay Soffian <jaysoffian@gmail.com> wrote:

I sense another use of negative pathspecs here..

On Wed, Sep 15, 2010 at 7:29 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:

Yeah, I'm really not sure I agree with the reasoning of b5227d8 (ls-files: excludes should not impact tracked files, 2009-10-12).

I thought about submitting a patch to revert b5227d8, but with an additional option to guard the behavior (--exclude-cached-files-too-pretty-please).

But I guess I don't see the harm in allowing excludes to apply to cached files when it is explicitly requested on the command-line. Here I think ls-files is slightly different than the other areas of git that don't apply excludes to cached files as it's about, well, listing files, and it's clear (I think) what the user wants.

But I'll go dig in the mailing list archives now to see if I can find what motivated Jeff's change beyond what he gave in the commit message.

j.

On Wed, Sep 15, 2010 at 7:41 PM, Jay Soffian <jaysoffian@gmail.com> wrote:

http://thread.gmane.org/gmane.comp.version-control.git/129889/focus=129976

Hmm.

j.

On Wed, Sep 15, 2010 at 07:41:50PM -0400, Jay Soffian wrote:

It's not just the command line. It's also what's in .gitignore files. If you disable just half of that, then you get the awful behavior that some excludes apply to index files, and some don't.

Yes, it's confusing that "-i" is not actually an inversion for index files. See

  http://article.gmane.org/gmane.comp.version-control.git/131719

It would help if I understood exactly what you're trying to accomplish.

-Peff

On Wed, Sep 15, 2010 at 8:16 PM, Jeff King <peff@peff.net> wrote:

Files matched by the standard excludes are not likely to be in the index in the first place. So in that sense, arguably -x is special.

But nonetheless, I agree with you, and since the user must specifically ask ls-files for the various exclusions, I think it makes sense to apply those even to cached files.

I'm building a project file for my editor and I want to exclude certain files that make no sense for it to care about even though they are part of the repo. So I tried:

$ git ls-files -x png -x jpg ...

and was confused by that not working. I've worked around this by just filtering the ls-files output through grep, but, ick.

j.

On Wed, Sep 15, 2010 at 08:33:29PM -0400, Jay Soffian wrote:

But then if somebody asks for both indexed and untracked files together, the behavior is quite confusing. There is no way to say "--exclude-standard just for the untracked files", and having exclusions impact cached files is unlike any other part of git. So the behavior of something like "git ls-files -s -o --exclude-standard" would be broken.

I have no idea if people are actually doing that.

So I think the best fix would be to leave the default behavior as-is, and add the --exclude-cached-files-too-pretty-please you mentioned.

I certainly have sympathy for you being confused by the behavior of ls-files. It undoubtedly is a mess.

But as for having to use grep, I would feel worse for you if you were actually trying to exclude patterns from your .gitignore and you needed to _convert_ them into a grep pattern. But it doesn't seem like:

  git ls-files | grep -v '\.png$'

is really much more work, and it's way more flexible.

At one point I proposed a "git check-ignore" similar to "git check-attr", but I never got around to coding it. If you really liked the gitignore-style patterns for some reason, it would be a nice flexible way of doing what you want.

-Peff

On Wed, Sep 15, 2010 at 5:19 PM, Jay Soffian <jaysoffian@gmail.com> wrote:

That caught me too.

<snip>

I ran into the same thing earlier this week and came up with a patch similar to Nguyen's (though I just restored it closer to what Johannes originally had there), before I discovered that this was an intentional change by Jeff. (And then I tabled it to get back to work on fixing up merge-recursive some more)

I can't say I really understand the changes from b5227d8 and 500348, though; I found (and find) it confusing as well. I did notice that restoring the old behavior didn't work very well with items below the current directory, so it'd take a bit more work than just restoring the original behavior.

Elijah

On Sep 15, 2010, at 4:12 PM, Junio C Hamano wrote:

My impression is that -x is pretty much only useful when you're using the -o flag, as anything that's already tracked isn't supposed to be affected by exclude patterns. The argument can be made that ls-files should indeed apply the exclude pattern to other requested display modes, and in fact a reading of the -i switch seems to imply that it should. Running `git ls-files -x '*.m' -c -i` displays all files ending in .m in the index. It would seem logical that the similar `git ls-files -x '*.m' -c` should display all files that don't end in .m in the index, but it doesn't appear to apply the exclude pattern in this case. As for -i being useful, I've never actually used it myself (though obviously it can be used as a way to grep through the index via the exclude pattern), but I can imagine a 
 scenario where I want to clean up all untracked files and end up running something like `git ls-files -z -o -i --exclude-standard | xargs -0 rm`.

-Kevin Ballard

On Wednesday 15 September 2010 18:40:20 Jay Soffian wrote:

For fun, I bisected this. This behavior changed in the following commit. Before this, it worked as you expected. The other replies seem to explain this more.

b5227d80aee5173bfda6aa43a890d03110b0df26 is the first bad commit
commit b5227d80aee5173bfda6aa43a890d03110b0df26
Author: Jeff King <peff@peff.net>
Date:   Mon Oct 12 01:11:57 2009 -0400

    ls-files: excludes should not impact tracked files
    
    In all parts of git, .gitignore and other exclude files
    impact only how we treat untracked files; they should have
    no effect on files listed in the index.
    
    This behavior was originally implemented very early on in
    9ff768e, but only for --exclude-from. Later, commit 63d285c
    accidentally caused us to trigger the behavior for
    --exclude-per-directory.
    
    This patch totally ignores excludes for files found in the
    index. This means we are reversing the original intent of
    9ff768e, while at the same time fixing the accidental
    behavior of 63d285c. This is a good thing, though, as the
    way that 9ff768e behaved does not really make sense with the
    way exclusions are used in modern git.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

Since b5227d8, -x/--exclude does not apply to cached files. This is easy to miss unless you read the discussion in the EXCLUDE PATTERNS section. Clarify that the option applies to untracked files and direct the reader to EXCLUDE PATTERNS.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 Documentation/git-ls-files.txt |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

Well, at least we can clarify the man page to reduce likelihood of future confusion.

On Wed, Sep 15, 2010 at 07:56:48PM -0400, Jay Soffian wrote:

Yeah, I think this is a good idea.

Acked-by: Jeff King <peff@peff.net>

-Peff

Jay Soffian <jaysoffian@gmail.com> writes:

Would be helpful; will queue directly on top of 500348a.

Thanks

By the way, I think the language in that file needs a bit of tweaking for readability. Perhaps this on top of your patch...

-- >8 --
Subject: [PATCH] ls-files documentation: reword for consistency

Similar to descriptions of other options, state what -x does in imperative mood. Start sentences for -X and --exclude-per-directory options in capital letters.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-ls-files.txt |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

On Wed, Sep 15, 2010 at 05:41:52PM -0700, Junio C Hamano wrote:

Also an improvement, IMHO.

-Peff