threads / discuss / 16285
Subject: Change in "git checkout" behaviour between 1.6.0.2 and 1.6.0.3
The following works fine with 1.6.0.2 and before, but not 1.6.0.3 or later:
git clone -n git git-test
cd git-test
git checkout -b work v1.6.0.2When it breaks, the error is:
error: Entry '.gitignore' would be overwritten by merge. Cannot merge.
I'm guessing it's a bug rather than a deliberate change?
Michael J GruberRe: Change in "git checkout" behaviour between 1.6.0.2 and 1.6.0.3
Bruce Stephens venit, vidit, dixit 12.11.2008 15:36:
> The following works fine with 1.6.0.2 and before, but not 1.6.0.3 or > later: > > git clone -n git git-test > cd git-test > git checkout -b work v1.6.0.2 > > When it breaks, the error is: > > error: Entry '.gitignore' would be overwritten by merge. Cannot merge. > > I'm guessing it's a bug rather than a deliberate change?
Bisecting gives:
5521883490e85f4d973141972cf16f89a79f1979 is first bad commit commit 5521883490e85f4d973141972cf16f89a79f1979 Author: Junio C Hamano <gitster@pobox.com> Date: Sun Sep 7 19:49:25 2008 -0700
checkout: do not lose staged removal
CCing Junio...
Michael
Re: Change in "git checkout" behaviour between 1.6.0.2 and 1.6.0.3
Michael J Gruber <git@drmicha.warpmail.net> writes:
[...]
> Bisecting gives: > > > 5521883490e85f4d973141972cf16f89a79f1979 is first bad commit > commit 5521883490e85f4d973141972cf16f89a79f1979 > Author: Junio C Hamano <gitster@pobox.com> > Date: Sun Sep 7 19:49:25 2008 -0700 > > checkout: do not lose staged removal
I got the same, which is reassuring.
Looks like a deliberate change with (what seems to me to be) an unfortunate interaction with "git clone -n"
Junio C HamanoRe: Change in "git checkout" behaviour between 1.6.0.2 and 1.6.0.3
Bruce Stephens <bruce.stephens@isode.com> writes:
> Michael J Gruber <git@drmicha.warpmail.net> writes: > > [...] > >> Bisecting gives: >> >> >> 5521883490e85f4d973141972cf16f89a79f1979 is first bad commit >> commit 5521883490e85f4d973141972cf16f89a79f1979 >> Author: Junio C Hamano <gitster@pobox.com> >> Date: Sun Sep 7 19:49:25 2008 -0700 >> >> checkout: do not lose staged removal > > I got the same, which is reassuring. > > Looks like a deliberate change with (what seems to me to be) an > unfortunate interaction with "git clone -n"
Yeah, it was meant to allow:
git clone -n $there $here
cd $here
git checkoutand was not taking care of the case to switch branches when the initial checkout is made.
Perhaps this would help.
builtin-checkout.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
diff --git c/builtin-checkout.c w/builtin-checkout.c index 05eee4e..d2265df 100644 --- c/builtin-checkout.c +++ w/builtin-checkout.c @@ -269,8 +269,7 @@ static int merge_working_tree(struct checkout_opts *opts, } /* 2-way merge to the new branch */ - topts.initial_checkout = (!active_nr && - (old->commit == new->commit)); + topts.initial_checkout = !active_nr; topts.update = 1; topts.merge = 1; topts.gently = opts->merge;
Re: Change in "git checkout" behaviour between 1.6.0.2 and 1.6.0.3
Junio C Hamano <gitster@pobox.com> writes:
[...]
> Yeah, it was meant to allow: > > git clone -n $there $here > cd $here > git checkout > > and was not taking care of the case to switch branches when the initial > checkout is made.
That specific sequence does work. I guess that's why I hadn't noticed the issue for so long (I guess git's test suite has some tests using "clone -n", and perhaps they're of that form).
> Perhaps this would help.
Works for me.
[...]
Junio C HamanoRe* Change in "git checkout" behaviour between 1.6.0.2 and 1.6.0.3
Junio C Hamano <gitster@pobox.com> writes:
>> Looks like a deliberate change with (what seems to me to be) an >> unfortunate interaction with "git clone -n" > > Yeah, it was meant to allow: > > git clone -n $there $here > cd $here > git checkout > > and was not taking care of the case to switch branches when the initial > checkout is made. > > Perhaps this would help. > ...
Here is a more involved but hopefully more maintainable fix.
-- >8 -- Subject: checkout: Fix "initial checkout" detection
Earlier commit 5521883 (checkout: do not lose staged removal, 2008-09-07) tightened the rule to prevent switching branches from losing local changes, so that staged removal of paths can be protected, while attempting to keep a loophole to still allow a special case of switching out of an un-checked-out state.
However, the loophole was made a bit too tight, and did not allow switching from one branch (in an un-checked-out state) to check out another branch.
The change to builtin-checkout.c in this commit loosens it to allow this, by not insisting the original commit and the new commit to be the same.
It also introduces a new function, is_index_unborn (and an associated macro, is_cache_unborn), to check if the repository is truly in an un-checked-out state more reliably, by making sure that $GIT_INDEX_FILE did not exist when populating the in-core index structure. A few places the earlier commit 5521883 added the check for the initial checkout condition are updated to use this function.
Signed-off-by: Junio C Hamano <gitster@pobox.com> --- builtin-checkout.c | 3 +-- builtin-read-tree.c | 2 +- cache.h | 2 ++ read-cache.c | 5 +++++ 4 files changed, 9 insertions(+), 3 deletions(-)
diff --git c/builtin-checkout.c w/builtin-checkout.c index 05eee4e..25845cd 100644 --- c/builtin-checkout.c +++ w/builtin-checkout.c @@ -269,8 +269,7 @@ static int merge_working_tree(struct checkout_opts *opts, } /* 2-way merge to the new branch */ - topts.initial_checkout = (!active_nr && - (old->commit == new->commit)); + topts.initial_checkout = is_cache_unborn(); topts.update = 1; topts.merge = 1; topts.gently = opts->merge; diff --git c/builtin-read-tree.c w/builtin-read-tree.c index 0706c95..38fef34 100644 --- c/builtin-read-tree.c +++ w/builtin-read-tree.c @@ -206,7 +206,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix) break; case 2: opts.fn = twoway_merge; - opts.initial_checkout = !active_nr; + opts.initial_checkout = is_cache_unborn(); break; case 3: default: diff --git c/cache.h w/cache.h index a1e4982..3960931 100644 --- c/cache.h +++ w/cache.h @@ -255,6 +255,7 @@ static inline void remove_name_hash(struct cache_entry *ce) #define read_cache() read_index(&the_index) #define read_cache_from(path) read_index_from(&the_index, (path)) +#define is_cache_unborn() is_index_unborn(&the_index) #define read_cache_unmerged() read_index_unmerged(&the_index) #define write_cache(newfd, cache, entries) write_index(&the_index, (newfd)) #define discard_cache() discard_index(&the_index) @@ -360,6 +361,7 @@ extern int init_db(const char *template_dir, unsigned int flags); /* Initialize and use the cache information */ extern int read_index(struct index_state *); extern int read_index_from(struct index_state *, const char *path); +extern int is_index_unborn(struct index_state *); extern int read_index_unmerged(struct index_state *); extern int write_index(const struct index_state *, int newfd); extern int discard_index(struct index_state *); diff --git c/read-cache.c w/read-cache.c index 967f483..525d138 100644 --- c/read-cache.c +++ w/read-cache.c @@ -1239,6 +1239,11 @@ unmap: die("index file corrupt"); } +int is_index_unborn(struct index_state *istate) +{ + return (!istate->cache_nr && !istate->alloc && !istate->timestamp); +} + int discard_index(struct index_state *istate) { istate->cache_nr = 0;
Re: Change in "git checkout" behaviour between 1.6.0.2 and 1.6.0.3
Bruce Stephens <bruce.stephens@isode.com> writes:
> The following works fine with 1.6.0.2 and before, but not 1.6.0.3 or > later: > > git clone -n git git-test > cd git-test > git checkout -b work v1.6.0.2 > > When it breaks, the error is: > > error: Entry '.gitignore' would be overwritten by merge. Cannot merge. > > I'm guessing it's a bug rather than a deliberate change?
According to "git bisect", the commit that caused this is the following one. Perhaps "git clone -n" doesn't start out with the index empty in the relevant sense?
commit 5521883490e85f4d973141972cf16f89a79f1979 Author: Junio C Hamano <gitster@pobox.com> Date: Sun Sep 7 19:49:25 2008 -0700
checkout: do not lose staged removal
The logic to checkout a different commit implements the safety to never
lose user's local changes. For example, switching from a commit to
another commit, when you have changed a path that is different between
them, need to merge your changes to the version from the switched-to
commit, which you may not necessarily be able to resolve easily. By
default, "git checkout" refused to switch branches, to give you a chance
to stash your local changes (or use "-m" to merge, accepting the risks of
getting conflicts).
This safety, however, had one deliberate hole since early June 2005. When
your local change was to remove a path (and optionally to stage that
removal), the command checked out the path from the switched-to commit
nevertheless.
This was to allow an initial checkout to happen smoothly (e.g. an initial
checkout is done by starting with an empty index and switching from the
commit at the HEAD to the same commit). We can tighten the rule slightly
to allow this special case to pass, without losing sight of removal
explicitly done by the user, by noticing if the index is truly empty when
the operation begins.
For historical background, see:
http://thread.gmane.org/gmane.comp.version-control.git/4641/focus=4646
This case is marked as *0* in the message, which both Linus and I said "it
feels somewhat wrong but otherwise we cannot start from an empty index".
Signed-off-by: Junio C Hamano <gitster@pobox.com>