Re: [PATCH] Make git-clean a builtin

[Shawn Bohrer <shawn.bohrer@gmail.com>]

Thanks for the input.

On Sat, Oct 06, 2007 at 06:31:36PM -0700, Linus Torvalds wrote:
> This looks better, but I think you'd be even better off actually using the 
> "read_directory()" interface directly, instead of exec'ing off "git 
> ls-files" and parsing the line output.

Perhaps, I'll take a look at how git-ls-files does it and see if I can
do that directly.  Since I'm new to git (and C) it will probably take me
a while to re-implement though.

> I also would still worry a bit about 'chdir(x)' and 'chdir("..")', because 
> quite frankly, they are *not* mirrors of each other (think symlinks, but 
> also error behaviour due to directories that might be non-executable). 
> Now, admittedly, if a directory isn't executable, I can imagine other git 
> things having problems (anybody want to test?), but that whole pattern is 
> just very fragile and not very reliable.

Yes it does seem fragile, but 'chdir("-")' doesn't work in C and I
couldn't find any equivalents.  I actually did think about symlinks, and
my code does do the right thing since I test if it is a directory before
doing the 'chdir(x)'. Symlinks are therefore treated as normal files and
removed.

I did not think about non-executable directories, and you are correct
that my code will fail to remove a directory if it is non-executable. I
also tested a git-ls-files with non-executable directories, and it will
fail to show you any files that are more than one level deep for
example:

|-- docs
|   |-- contributing
|   |   `-- patches.txt
|   `-- manual.txt

If docs is non-executable it will only return 'docs/manual.txt'

--
Shawn