Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> writes:
> The cached blob of i-t-a entries are empty blob. By checkout, we delete
> the content we have. Don't do it.
>
> This is done higher up instead of inside checkout_entry() because we
> would have limited options in there: silently ignore, loudly ignore,
> die. At higher level we can do better reporting. For example, "git
> checkout -- foo" will complain that "foo" does not match pathspec, just
> like when "foo" is not registered with "git add -N"
Hmm, I am not sure about the example, though. The user _told_ Git
that 'foo' is a path she cares about so "does not match pathspec" is
a very unfriendly thing to say. "foo does not exist in the index
hence we cannot check it out" is a very good thing to say, though
(and of course, I agree that not touching the working tree is a good
thing to do).
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
> builtin/checkout-index.c | 5 ++++-
> builtin/checkout.c | 2 ++
> t/t2203-add-intent.sh | 34 ++++++++++++++++++++++++++++++++++
> 3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
> index 8028c37..eca975d 100644
> --- a/builtin/checkout-index.c
> +++ b/builtin/checkout-index.c
> @@ -56,7 +56,8 @@ 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) ||
> + ce_intent_to_add(ce))
> break;
> has_same_name = 1;
> pos++;
'pos' here comes from cache_name_pos(), and it is either the
location an entry with the same name exists, or the location a new
entry with the given name would be inserted at.
When we detect that the entry at pos is different from the given
name, we break out of the loop, because we _know_ that an entry with
the same name cannot exist. Does the same hold for an i-t-a entry
that exactly records the given name?
The answer is yes, as you cannot have an unmerged i-t-a entry. If a
path marked as i-t-a is in the index, no other entry with the same
name would exist.
So this change is good. I would actually have introduced another
bool has_i_t_a to make the reporting richer, as it is your primary
reason why you are touching this part of the code instead of
checkout_entry(). The reporting part then would become
fprintf(stderr, "git checkout-index: %s ", name);
- if (!has_same_name)
+ if (has_i_t_a)
+ fprintf(stderr, "is not yet in the cache);
else if (!has_same_name)
fprintf(stderr, "is not in the cache");
;-)
> @@ -99,6 +100,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 obviously good ;-)
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index e1403be..02889d4 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -300,6 +300,8 @@ static int checkout_paths(const struct checkout_opts *opts,
> * anything to this entry at all.
> */
> continue;
> + if (ce_intent_to_add(ce))
> + continue;
> /*
> * Either this entry came from the tree-ish we are
> * checking the paths out of, or we are checking out
Hmm, while this does prevent the later code from checking it out, I
am not sure how well this interacts with ps_matched[] logic here.
If the user told Git that 'foo' is a path that she cares about with
"add -N foo", and said "git checkout -- foo", should we be somehow
saying that 'foo' did match but there is nothing to check out, or
something?
--
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