Allow one-way tree merge to remove old files

Allow one-way tree merge to remove old files

[Linus Torvalds]

For some random reason (probably just because nobody noticed), the one-way 
merge strategy didn't mark deleted files as deleted, so if you used

	git-read-tree -m -u <newtree>

it would update the files that got changed in the index, but it would not 
delete the files that got deleted.

This should fix it, and I can't imagine that anybody depends on the old 
strange "update only existing files" behaviour.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---
diff --git a/read-tree.c b/read-tree.c
index e926e4c..11157f4 100644
--- a/read-tree.c
+++ b/read-tree.c
@@ -684,7 +684,7 @@ static int oneway_merge(struct cache_ent
 			     merge_size);
 
 	if (!a)
-		return 0;
+		return deleted_entry(old, NULL);
 	if (old && same(old, a)) {
 		return keep_entry(old);
 	}

Re: Allow one-way tree merge to remove old files

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> For some random reason (probably just because nobody noticed), the one-way 
> merge strategy didn't mark deleted files as deleted, so if you used
>
> 	git-read-tree -m -u <newtree>
>
> it would update the files that got changed in the index, but it would not 
> delete the files that got deleted.

Good catch.  I think it is a leftover from the days we did not
have -u there; returning 0 was a way to say "there is no cache
entry resulting from this round of path-merge".

Simplify "git reset --hard"

[Linus Torvalds]

Now that the one-way merge strategy does the right thing wrt files that do 
not exist in the result, just remove all the random crud we did in "git 
reset" to do this all by hand.

Instead, just pass in "-u" to git-read-tree when we do a hard reset, and 
depend on git-read-tree to update the working tree appropriately.

This basically means that git reset turns into

	# Always update the HEAD ref
	git update-ref HEAD "$rev"

	case "--soft"
		# do nothing to index/working tree
	case "--hard"
		# read index _and_ update working tree
		git-read-tree --reset -u "$rev"
	case "--mixed"
		# update just index, report on working tree differences
		git-read-tree --reset "$rev"
		git-update-index --refresh

which is what it was always semantically doing, it just did it in a
rather strange way because it was written to not expect git-read-tree to 
do anything to the working tree.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---

NOTE! The switch to use "git-read-tree -u" does actually result in a real 
change: we will now remove files that were in the index but not in HEAD 
before (ie files added with "git add"). I'd argue that this is a bug-fix.

 git-reset.sh |   50 ++++----------------------------------------------
 1 files changed, 4 insertions(+), 46 deletions(-)

diff --git a/git-reset.sh b/git-reset.sh
index 6cb073c..0ee3e3e 100755
--- a/git-reset.sh
+++ b/git-reset.sh
@@ -6,6 +6,7 @@ USAGE='[--mixed | --soft | --hard]  [<co
 tmp=${GIT_DIR}/reset.$$
 trap 'rm -f $tmp-*' 0 1 2 3 15
 
+update=
 reset_type=--mixed
 case "$1" in
 --mixed | --soft | --hard)
@@ -23,24 +24,7 @@ # We need to remember the set of paths t
 # behind before a hard reset, so that we can remove them.
 if test "$reset_type" = "--hard"
 then
-	{
-		git-ls-files --stage -z
-		git-rev-parse --verify HEAD 2>/dev/null &&
-		git-ls-tree -r -z HEAD
-	} | perl -e '
-	    use strict;
-	    my %seen;
-	    $/ = "\0";
-	    while (<>) {
-		chomp;
-		my ($info, $path) = split(/\t/, $_);
-		next if ($info =~ / tree /);
-		if (!$seen{$path}) {
-			$seen{$path} = 1;
-			print "$path\0";
-		}
-	    }
-	' >$tmp-exists
+	update=-u
 fi
 
 # Soft reset does not touch the index file nor the working tree
@@ -54,7 +38,7 @@ then
 		die "Cannot do a soft reset in the middle of a merge."
 	fi
 else
-	git-read-tree --reset "$rev" || exit
+	git-read-tree --reset $update "$rev" || exit
 fi
 
 # Any resets update HEAD to the head being switched to.
@@ -68,33 +52,7 @@ git-update-ref HEAD "$rev"
 
 case "$reset_type" in
 --hard )
-	# Hard reset matches the working tree to that of the tree
-	# being switched to.
-	git-checkout-index -f -u -q -a
-	git-ls-files --cached -z |
-	perl -e '
-		use strict;
-		my (%keep, $fh);
-		$/ = "\0";
-		while (<STDIN>) {
-			chomp;
-			$keep{$_} = 1;
-		}
-		open $fh, "<", $ARGV[0]
-			or die "cannot open $ARGV[0]";
-		while (<$fh>) {
-			chomp;
-			if (! exists $keep{$_}) {
-				# it is ok if this fails -- it may already
-				# have been culled by checkout-index.
-				unlink $_;
-				while (s|/[^/]*$||) {
-					rmdir($_) or last;
-				}
-			}
-		}
-	' $tmp-exists
-	;;
+	;; # Nothing else to do
 --soft )
 	;; # Nothing else to do
 --mixed )

Re: Simplify "git reset --hard"

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> Now that the one-way merge strategy does the right thing wrt files that do 
> not exist in the result, just remove all the random crud we did in "git 
> reset" to do this all by hand.
>
> Instead, just pass in "-u" to git-read-tree when we do a hard reset, and 
> depend on git-read-tree to update the working tree appropriately.

Well, this is wrong.  Local modifications remain after your
version of "git-reset --hard HEAD". which is not what we want
from a hard reset.

-- >8 --

diff --git a/git-reset.sh b/git-reset.sh
index 0ee3e3e..ecc111b 100755
--- a/git-reset.sh
+++ b/git-reset.sh
@@ -52,7 +52,7 @@ git-update-ref HEAD "$rev"
 
 case "$reset_type" in
 --hard )
-	;; # Nothing else to do
+	git-checkout-index -f -q -u -a ;;
 --soft )
 	;; # Nothing else to do
 --mixed )

Re: Simplify "git reset --hard"

[Junio C Hamano]

Junio C Hamano <junkio@cox.net> writes:

>> Instead, just pass in "-u" to git-read-tree when we do a hard reset, and 
>> depend on git-read-tree to update the working tree appropriately.
>
> Well, this is wrong.  Local modifications remain after your
> version of "git-reset --hard HEAD". which is not what we want
> from a hard reset.

... and attempting to paper it over in git-reset.sh is also
wrong.  Keep your "--hard is noop" change in git-reset.sh and
replace it with this would be the right fix.

-- >8 --

diff --git a/read-tree.c b/read-tree.c
index 11157f4..c135f08 100644
--- a/read-tree.c
+++ b/read-tree.c
@@ -686,6 +686,7 @@ static int oneway_merge(struct cache_ent
 	if (!a)
 		return deleted_entry(old, NULL);
 	if (old && same(old, a)) {
+		old->ce_flags |= htons(CE_UPDATE);
 		return keep_entry(old);
 	}
 	return merged_entry(a, NULL);

Re: Simplify "git reset --hard"

[Junio C Hamano]

Junio C Hamano <junkio@cox.net> writes:

> Junio C Hamano <junkio@cox.net> writes:
>
>>> Instead, just pass in "-u" to git-read-tree when we do a hard reset, and 
>>> depend on git-read-tree to update the working tree appropriately.
>>
>> Well, this is wrong.  Local modifications remain after your
>> version of "git-reset --hard HEAD". which is not what we want
>> from a hard reset.
>
> ... and attempting to paper it over in git-reset.sh is also
> wrong.  Keep your "--hard is noop" change in git-reset.sh and
> replace it with this would be the right fix.

-- >8 --
read-tree -u one-way merge fix to check out locally modified paths.

The "-u" flag means "update the working tree files", but to
other types of merges, it also implies "I want to keep my local
changes" -- because they prevent local changes from getting lost
by using verify_uptodate.  The one-way merge is different from
other merges in that its purpose is opposite of doing something
else while keeping unrelated local changes.  The point of
one-way merge is to nuke local changes.  So while it feels
somewhat wrong that this actively loses local changes, it is the
right thing to do.

The earlier one marked old->ce_flags to be updated
unconditionally, but that would cause 18,000 paths to be updated
when you have only a few paths different from the HEAD you are
switching to, which is far worse than what we used to do in
git-reset by hand.

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

---

 * Third time lucky ;-)

diff --git a/read-tree.c b/read-tree.c
index 11157f4..d847c6f 100644
--- a/read-tree.c
+++ b/read-tree.c
@@ -686,6 +698,9 @@ static int oneway_merge(struct cache_ent
 	if (!a)
 		return deleted_entry(old, NULL);
 	if (old && same(old, a)) {
+		struct stat st;
+		if (lstat(old->name, &st) || ce_match_stat(old, &st, 1))
+			old->ce_flags |= htons(CE_UPDATE);
 		return keep_entry(old);
 	}
 	return merged_entry(a, NULL);

Re: Simplify "git reset --hard"

[Linus Torvalds]

On Mon, 15 May 2006, Junio C Hamano wrote:
>
> read-tree -u one-way merge fix to check out locally modified paths.
> 
> The "-u" flag means "update the working tree files", but to
> other types of merges, it also implies "I want to keep my local
> changes" -- because they prevent local changes from getting lost
> by using verify_uptodate.  The one-way merge is different from
> other merges in that its purpose is opposite of doing something
> else while keeping unrelated local changes.  The point of
> one-way merge is to nuke local changes.  So while it feels
> somewhat wrong that this actively loses local changes, it is the
> right thing to do.

Ack. On the other hand, I wonder if it might not make sense to have this 
part potentially depend on the "--reset" flag.

That way you wouldn't even have to apologize for it.

		Linus

Re: Simplify "git reset --hard"

[Linus Torvalds]

On Mon, 15 May 2006, Linus Torvalds wrote:
> 
> Ack. On the other hand, I wonder if it might not make sense to have this 
> part potentially depend on the "--reset" flag.
> 
> That way you wouldn't even have to apologize for it.

Ie, something like this..

This should allow a two-way or three-way merge to akso ignore any dirty 
state when you use "--reset", because they use "verify_uptodate()", which 
now would set the CD_UPDATE flag on the ce rather than complain about it 
being different (if it survives the merge, of course - often it won't, but 
then we won't care).

This is all totally untested, of course.

		Linus

----
diff --git a/read-tree.c b/read-tree.c
index 7c83031..f2d674c 100644
--- a/read-tree.c
+++ b/read-tree.c
@@ -13,6 +13,7 @@ #include "cache-tree.h"
 #include <sys/time.h>
 #include <signal.h>
 
+static int reset = 0;
 static int merge = 0;
 static int update = 0;
 static int index_only = 0;
@@ -419,6 +420,10 @@ static void verify_uptodate(struct cache
 			return;
 		errno = 0;
 	}
+	if (reset) {
+		ce->ce_flags |= htons(CE_UPDATE);
+		return;
+	}
 	if (errno == ENOENT)
 		return;
 	die("Entry '%s' not uptodate. Cannot merge.", ce->name);
@@ -723,6 +728,11 @@ static int oneway_merge(struct cache_ent
 		return deleted_entry(old, NULL);
 	}
 	if (old && same(old, a)) {
+		if (reset) {
+			struct stat st;
+			if (lstat(old->name, &st) || ce_match_stat(old, &st, 1))
+				old->ce_flags |= htons(CE_UPDATE);
+		}
 		return keep_entry(old);
 	}
 	return merged_entry(a, NULL);
@@ -790,7 +800,7 @@ static struct cache_file cache_file;
 
 int main(int argc, char **argv)
 {
-	int i, newfd, reset, stage = 0;
+	int i, newfd, stage = 0;
 	unsigned char sha1[20];
 	merge_fn_t fn = NULL;