Re: please pull powerpc-merge.git

Re: please pull powerpc-merge.git

[Linus Torvalds]

[ Junio, Fredrik, you're cc'd because the "merge-one-file" logic doesn't 
  clean up empty directories after removing a file in merge ]

On Sat, 19 Nov 2005, Christoph Hellwig wrote:
> 
> Btw, in current Linus' tree arch/ppc64/ contains various empty
> directories, anyone care to git rm -r arch/ppc64 (and include/asm-ppc64
> now) ?

Actually, a git tree can never contain an empty subdirectory (well, it 
technically _can_, but it really shouldn't - the index only tracks 
_files_, and directories in git are created only as needed to contain 
those files. An empty directory tree - while technically possible in a 
data structure sense - would be a bug in git if git ever created one).

So when you say "in current Linus' tree", I suspect that in my tree it 
very much does not, but in _your_ tree the stale directories still exist 
because "git pull" just hasn't removed them.

It's a git bug/misfeature: git does not remove directories when the last 
file in it has been removed, simply because git doesn't actually really 
even _track_ directories at all. It only tracks files that just happen to 
be in directories.

[ Side note: this is not strictly true: git _will_ actually remove 
  directories that become empty when you switch between different branches 
  with "git checkout". However, even then it will not actually ever remove 
  a directory that contains any non-tracked files, so if you have old 
  object files etc, git will never remove that directory even if all the 
  tracked files got removed. ]

The reason I say "bug/misfeature" and not outright "bug" is the fact that 
removing directories is not something that git can really always do. 
Exactly because directories are often used for things that aren't tracked. 
Should it just remove old object files? Hell no. Stuff git doesn't know 
about (even if they have been marked in ".gitignore") it definitely 
shouldn't touch.

So in general, after you've merged somebody elses tree, the final pruning 
of your own old checked-out directories is really up to you.

But I agree that git should maybe at least _tell_ people about "there's a 
directory there that I'm not tracking". And it could perhaps remove the 
empty ones outright a bit more aggressively (ie now it effectively does it 
only for "git checkout" and for the _trivial_ cases of merges that happen 
with "git pull", but it doesn't do it if the merge ended up being of the 
more involved case that needed higher-level - but still automatic - help).

Frederik - I don't do perl, but I think for the recursive merge strategy, 
it's the "removeFile()" thing. And for the old git-merge-one-file.sh, it's 
that "deleted in both or deleted in one and unchanged in the other" case.

In both cases, I think it should do something like

	.. remove the file itself ..
	#
	# remove all directories leading up the path if they
	# have become non-empty
	#
	path=$(dirname $path)
	while [ "$path" ] && rmdir "$path"; do
		path=$(dirname $path)
	done

or similar.

But as mentioned, quite often, this still won't remove the directories, 
since it will have various old stale build-files in them. 

			Linus

[PATCH] merge-one-file: remove empty directories

[Junio C Hamano]

When the last file in a directory is removed as the result of a
merge, try to rmdir the now-empty directory.

[jc: We probably could use "rmdir -p", but for now we do that by
hand for portability.]

Signed-off-by: Junio C Hamano <junkio@cox.net>

---
    Linus Torvalds <torvalds@osdl.org> writes:

    > [ Junio, Fredrik, you're cc'd because the "merge-one-file" logic doesn't 
    >   clean up empty directories after removing a file in merge ]

    Here is the one for merge-one-file that is used by 'git pull
    -s resolve'.  Next patch will be the merge-recursive one.

 git-merge-one-file.sh |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

applies-to: fbe4060a1b8ba894be5767ffc27402f94f356882
397c76697f898e0341699fba8ef783f3342329c7
diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh
index b08597d..b285990 100755
--- a/git-merge-one-file.sh
+++ b/git-merge-one-file.sh
@@ -25,7 +25,12 @@ case "${1:-.}${2:-.}${3:-.}" in
 		echo "Removing $4"
 	fi
 	if test -f "$4"; then
-		rm -f -- "$4"
+		rm -f -- "$4" &&
+		dn="$4" &&
+		while dn=$(expr "$dn" : '\(.*\)/') && rmdir "$dn" 2>/dev/null
+		do
+			:;
+		done
 	fi &&
 		exec git-update-index --remove -- "$4"
 	;;
---
0.99.9.GIT

Re: [PATCH] merge-one-file: remove empty directories

[Linus Torvalds]

On Sat, 19 Nov 2005, Junio C Hamano wrote:
> 
> [jc: We probably could use "rmdir -p", but for now we do that by
> hand for portability.]

Some googling seems to imply that "-p" is pretty much globally available.

It's in POSIX.1, but more importantly, it seems to actually be actively 
used in BSD projects, and I found a man-page with copyright date in -92 
(SCO of all places) which mentiones AT&T SVID2 - but not the more modern 
standards.

Even VMS seems to have it (although if git is ever ported to VMS, I'll 
just have to shoot myself. I used VMS in -88, and the scars are _still_ 
fresh).

			Linus

Re: [PATCH] merge-one-file: remove empty directories

[H. Peter Anvin]

Linus Torvalds wrote:
> 
> Even VMS seems to have it (although if git is ever ported to VMS, I'll 
> just have to shoot myself. I used VMS in -88, and the scars are _still_ 
> fresh).
> 

Yeah, well, WinNT is basically the successor to VMS.  (I have heard from 
several ex-DECers that WNT was done on top of a stolen copy of what was 
supposed-to-have-been the successor to VMS.)

	-hpa

Re: [PATCH] merge-one-file: remove empty directories

[Alex Riesen]

On 11/20/05, H. Peter Anvin <hpa@zytor.com> wrote:
> Linus Torvalds wrote:
> >
> > Even VMS seems to have it (although if git is ever ported to VMS, I'll
> > just have to shoot myself. I used VMS in -88, and the scars are _still_
> > fresh).
> >
>
> Yeah, well, WinNT is basically the successor to VMS.  (I have heard from
> several ex-DECers that WNT was done on top of a stolen copy of what was
> supposed-to-have-been the successor to VMS.)

...and yes, the users of NT are almost ready to shoot themselves.

Re: [PATCH] merge-one-file: remove empty directories

[Johannes Schindelin]

On Sun, 20 Nov 2005, Linus Torvalds wrote:

> [...] although if git is ever ported to VMS, I'll just have to shoot 
> myself. I used VMS in -88, and the scars are _still_ fresh.

Does that mean you are opposed to my plans on porting git to C64? Hmm?

[PATCH] merge-recursive::removeFile: remove empty directories

[Junio C Hamano]

When the last file in a directory is removed as the result of a
merge, try to rmdir the now-empty directory.

Signed-off-by: Junio C Hamano <junkio@cox.net>

---

 And this is the one for 'git pull -s recursive' which is the
 default these days.

 git-merge-recursive.py |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

applies-to: ea62f7003bb2769fa23d5ca371d84cee9d2ec46f
80e21a9ed809d98788ff6fb705d911bee37d460b
diff --git a/git-merge-recursive.py b/git-merge-recursive.py
index d7d36aa..37258ad 100755
--- a/git-merge-recursive.py
+++ b/git-merge-recursive.py
@@ -293,6 +293,10 @@ def removeFile(clean, path):
         except OSError, e:
             if e.errno != errno.ENOENT and e.errno != errno.EISDIR:
                 raise
+        try:
+            os.removedirs(os.path.dirname(path))
+        except:
+            pass
 
 def uniquePath(path, branch):
     def fileExists(path):
---
0.99.9.GIT

Re: [PATCH] merge-recursive::removeFile: remove empty directories

[Fredrik Kuivinen]

On Sat, Nov 19, 2005 at 08:14:48PM -0800, Junio C Hamano wrote:
> When the last file in a directory is removed as the result of a
> merge, try to rmdir the now-empty directory.
> 
> Signed-off-by: Junio C Hamano <junkio@cox.net>
> 
> ---
> 
>  And this is the one for 'git pull -s recursive' which is the
>  default these days.
> 
>  git-merge-recursive.py |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> applies-to: ea62f7003bb2769fa23d5ca371d84cee9d2ec46f
> 80e21a9ed809d98788ff6fb705d911bee37d460b
> diff --git a/git-merge-recursive.py b/git-merge-recursive.py
> index d7d36aa..37258ad 100755
> --- a/git-merge-recursive.py
> +++ b/git-merge-recursive.py
> @@ -293,6 +293,10 @@ def removeFile(clean, path):
>          except OSError, e:
>              if e.errno != errno.ENOENT and e.errno != errno.EISDIR:
>                  raise
> +        try:
> +            os.removedirs(os.path.dirname(path))
> +        except:
> +            pass
>  

Looks good, but I would prefer to have 'except OSError:' instead of
the except without any exception class. A plain 'except:' will catch
every kind of exception which includes things such as NameError and
AttributeError and we really don't want to silently ignore those two.

A plain 'except:' exists in one other place (actually two, but one of
them is ok) in git-merge-recursive.py. I will send a patch to remove
that one.

- Fredrik