Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> writes:
> diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
> index 9ca2da1..d9fe8f4 100644
> --- a/builtin/checkout-index.c
> +++ b/builtin/checkout-index.c
> @@ -48,6 +48,7 @@ static int checkout_file(const char *name, const char *prefix)
> int pos = cache_name_pos(name, namelen);
> int has_same_name = 0;
> int did_checkout = 0;
> + int has_intent_to_add = 0;
> int errs = 0;
>
> if (pos < 0)
> @@ -56,8 +57,11 @@ static int checkout_file(const char *name, const char *prefix)
> while (pos < active_nr) {
> struct cache_entry *ce = active_cache[pos];
> if (ce_namelen(ce) != namelen ||
> - memcmp(ce->name, name, namelen))
> + memcmp(ce->name, name, namelen)) {
> + if (ce_intent_to_add(ce))
> + has_intent_to_add = 1;
> break;
> + }
> has_same_name = 1;
> pos++;
> if (ce_stage(ce) != checkout_stage
Sorry, but I cannot see what this hunk is trying to do.
Earlier, 'pos' is set to where the 'name' would be inserted (or
would replace, if there is an existing entry with the same name).
We iterate over the index starting from that 'pos' until we see a
different name (i.e. the length is different, or the name does not
match) and then break out, because that entry is not something we
want to muck with, because the entry here you are checking with
ce_intent_to_add() is an entry that is entirely unrelated to the
path you are checking out.
So you do not check it out, just as before, which is correct, but
then this 'has_intent_to_add' flag is used to say something
nonsensical below ...
> @@ -77,7 +81,9 @@ static int checkout_file(const char *name, const char *prefix)
>
> if (!state.quiet) {
> fprintf(stderr, "git checkout-index: %s ", name);
> - if (!has_same_name)
> + if (has_intent_to_add)
> + fprintf(stderr, "is not yet in the cache");
... the reason why you are here does not have anything to do with the
entry that happened to come after the path you wanted to check out
was marked with the I-T-A bit. The reason why you are here is
because 'did_checkout' was unset, which in turn was because there
wasn't a path that match 'name' you are checking out in the index,
with or without the I-T-A bit.
$ rm .git/index
$ >file
$ git add -N file
$ git checkout-index fild
would hit this codepath, I think, and the code would give a message
that says "fild is not yet in the cache", which may be technically
correct but it is the same as !has_same_name's "fild is not in the
cache".
Probably the I-T-A check must go after the code decides that 'ce' is
an entry for the 'name' the caller asked us to check out (and of
course we shouldn't check it out in that case).
> @@ -99,6 +105,8 @@ static void checkout_all(const char *prefix, int prefix_length)
> if (ce_stage(ce) != checkout_stage
> && (CHECKOUT_ALL != checkout_stage || !ce_stage(ce)))
> continue;
> + if (ce_intent_to_add(ce))
> + continue;
This one is probably correct, I didn't check the context to fully
see if it is valid, though.
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index 3e141fc..ac37d92 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -328,12 +328,15 @@ static int checkout_paths(const struct checkout_opts *opts,
> if (opts->merge)
> unmerge_marked_index(&the_index);
>
> - /* Any unmerged paths? */
> for (pos = 0; pos < active_nr; pos++) {
> - const struct cache_entry *ce = active_cache[pos];
> + struct cache_entry *ce = active_cache[pos];
> if (ce->ce_flags & CE_MATCHED) {
> - if (!ce_stage(ce))
> + if (!ce_stage(ce)) {
> + if (ce_intent_to_add(ce))
> + ce->ce_flags &= ~CE_MATCHED;
> continue;
> + }
> + /* Unmerged paths */
> if (opts->force) {
> warning(_("path '%s' is unmerged"), ce->name);
> } else if (opts->writeout_stage) {
This loop has become dual-purpose; it is drops CE_MATCHED bit from
cache entries with the I-T-A bit, and it continues to handle errors
for an attempt to checkout unmerged paths.
$ rm .git/index
$ >file
$ git add -N file
$ git checkout file
would hit this codepath, and it will force-skip "file".
There is a small source of confusion for new people here, though.
$ git checkout filo
would complain that there is no 'filo'. But
$ git checkout file
would silently turn itself into a no-op. Don't we want to at least
give some indication that we are not checking out "file" because its
contents do not yet exist in the index, or something?
> diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
> index 96c8755..52e9f7f 100755
> --- a/t/t2203-add-intent.sh
> +++ b/t/t2203-add-intent.sh
> @@ -111,5 +111,39 @@ test_expect_success 'apply:check_preimage() not creating empty file' '
> )
> '
>
> +test_expect_success 'checkout ignores i-t-a' '
> + git init checkout &&
> + (
> + cd checkout &&
> + echo data >file &&
> + git add -N file &&
> + git checkout -- file &&
> + echo data >expected &&
> + test_cmp expected file
> + )
> +'
> +
> +test_expect_success 'checkout-index ignores i-t-a' '
> + (
> + cd checkout &&
> + git checkout-index file &&
> + echo data >expected &&
> + test_cmp expected file
> + )
> +'
> +
> +test_expect_success 'checkout-index --all ignores i-t-a' '
> + (
> + cd checkout &&
> + echo data >anotherfile &&
> + git add anotherfile &&
> + rm anotherfile &&
> + git checkout-index --all &&
> + echo data >expected &&
> + test_cmp expected file &&
> + test_cmp expected anotherfile
> + )
> +'
> +
> test_done
--
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