Re: Possible regression: overwriting untracked files in a fresh repo

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Aug 24, 2009 at 07:11:43PM -0700, Junio C Hamano wrote:

> This looks a lot saner; I like it.  Care to wrap it up with the usual
> supporting material?

Patch is below. I did a little digging to see the origin of the
current behavior. It looks like invoking "force" was probably the
easiest way to make the shell version work, and then the behavior just
got ported to C (see below for details).

> I think the "You appear to be" can just go, but I do not feel very
> strongly either way.

I don't feel strongly, either. I couldn't think of a reason a user would
care, and it is potentially confusing to new users (though most will
simply clone and never see it), so I ripped it out.

And now you can try your new scissors patch. ;)

-- >8 --
Subject: [PATCH] checkout: do not imply "-f" on unborn branches

When checkout sees that HEAD points to a non-existent ref,
it currently acts as if "-f" was given; this behavior dates
back to 5a03e7f, which enabled checkout from unborn branches
in the shell version of "git-checkout". The reasoning given
is to avoid the code path which tries to merge the tree
contents. When checkout was converted to C, this code
remained intact.

The unfortunate side effect of this strategy is that the
"force" code path will overwrite working tree and index
state that may be precious to the user. Instead of enabling
"force", this patch uses the normal "merge" codepath for an
unborn branch, but substitutes the empty tree for the "old"
commit.

This means that in the absence of an index, any files in the
working tree will be treated as untracked files, and a
checkout which would overwrite them is aborted. Similarly,
any paths in the index will be merged with an empty entry
as the base, meaning that unless the new branch's content is
identical to what's in the index, there will be a conflict
and the checkout will be aborted.

The user is then free to correct the situation or proceed
with "-f" as appropriate.

This patch also removes the "warning: you are on a branch
yet to be born" message. Its function was to warn the user
that we were enabling the "-f" option. Since we are no
longer doing that, there is no reason for the user to care
whether we are switching away from an unborn branch.

Signed-off-by: Jeff King <peff@xxxxxxxx>
---
 builtin-checkout.c         |   12 +++---------
 t/t2015-checkout-unborn.sh |   40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 9 deletions(-)
 create mode 100755 t/t2015-checkout-unborn.sh

diff --git a/builtin-checkout.c b/builtin-checkout.c
index 8a9a474..c6d6ac9 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -402,7 +402,9 @@ static int merge_working_tree(struct checkout_opts *opts,
 		topts.dir = xcalloc(1, sizeof(*topts.dir));
 		topts.dir->flags |= DIR_SHOW_IGNORED;
 		topts.dir->exclude_per_dir = ".gitignore";
-		tree = parse_tree_indirect(old->commit->object.sha1);
+		tree = parse_tree_indirect(old->commit ?
+					   old->commit->object.sha1 :
+					   (unsigned char *)EMPTY_TREE_SHA1_BIN);
 		init_tree_desc(&trees[0], tree->buffer, tree->size);
 		tree = parse_tree_indirect(new->commit->object.sha1);
 		init_tree_desc(&trees[1], tree->buffer, tree->size);
@@ -541,14 +543,6 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
 		parse_commit(new->commit);
 	}
 
-	if (!old.commit && !opts->force) {
-		if (!opts->quiet) {
-			warning("You appear to be on a branch yet to be born.");
-			warning("Forcing checkout of %s.", new->name);
-		}
-		opts->force = 1;
-	}
-
 	ret = merge_working_tree(opts, &old, new);
 	if (ret)
 		return ret;
diff --git a/t/t2015-checkout-unborn.sh b/t/t2015-checkout-unborn.sh
new file mode 100755
index 0000000..c551d39
--- /dev/null
+++ b/t/t2015-checkout-unborn.sh
@@ -0,0 +1,40 @@
+#!/bin/sh
+
+test_description='checkout from unborn branch protects contents'
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	mkdir parent &&
+	(cd parent &&
+	 git init &&
+	 echo content >file &&
+	 git add file &&
+	 git commit -m base
+	) &&
+	git fetch parent master:origin
+'
+
+test_expect_success 'checkout from unborn preserves untracked files' '
+	echo precious >expect &&
+	echo precious >file &&
+	test_must_fail git checkout -b new origin &&
+	test_cmp expect file
+'
+
+test_expect_success 'checkout from unborn preserves index contents' '
+	echo precious >expect &&
+	echo precious >file &&
+	git add file &&
+	test_must_fail git checkout -b new origin &&
+	test_cmp expect file &&
+	git show :file >file &&
+	test_cmp expect file
+'
+
+test_expect_success 'checkout from unborn merges identical index contents' '
+	echo content >file &&
+	git add file &&
+	git checkout -b new origin
+'
+
+test_done
-- 
1.6.4.1.330.g14cea.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]