threads / discuss / 2604

Re: please pull powerpc-merge.git

Subject: Re: please pull powerpc-merge.git

## tl;dr

8 messages between Nov 19, 2005 and Nov 21, 2005.

replies: 7people: 6as markdown or json

Linus Torvalds· Nov 19, 2005, 18:24 UTC · lore
[ 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
Junio C Hamano· Nov 20, 2005, 04:14 UTC · re: Linus Torvalds · lore

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

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
Linus Torvalds· Nov 20, 2005, 17:17 UTC · re: Junio C Hamano · lore

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

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
H. Peter Anvin· Nov 20, 2005, 18:08 UTC · re: Linus Torvalds · lore

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

Linus Torvalds wrote:
Show 5 quoted lines
> 
> 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
Alex Riesen· Nov 21, 2005, 14:22 UTC · re: H. Peter Anvin · lore

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

On 11/20/05, H. Peter Anvin <hpa@zytor.com> wrote:
Show 10 quoted lines
> 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.
Johannes Schindelin· Nov 21, 2005, 00:00 UTC · re: Linus Torvalds · lore

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

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?
Junio C Hamano· Nov 20, 2005, 04:14 UTC · re: Linus Torvalds · lore

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

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
Fredrik Kuivinen· Nov 20, 2005, 11:51 UTC · re: Junio C Hamano · lore

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

On Sat, Nov 19, 2005 at 08:14:48PM -0800, Junio C Hamano wrote:
Show 28 quoted lines
> 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

← back to recent threads