From: Junio C Hamano <gitster@pobox.com>
To: Duy Nguyen <pclouds@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 2/2] reset: support "--mixed --intent-to-add" mode
Date: Wed, 05 Feb 2014 10:25:33 -0800 [thread overview]
Message-ID: <xmqqbnyl5sde.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <xmqqob2l5vl2.fsf@gitster.dls.corp.google.com> (Junio C. Hamano's message of "Wed, 05 Feb 2014 09:16:09 -0800")
Junio C Hamano <gitster@pobox.com> writes:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> On Tue, Feb 04, 2014 at 02:25:25PM -0800, Junio C Hamano wrote:
>>> Junio C Hamano <gitster@pobox.com> writes:
>>>
>>> > While I do not have any problem with adding an optional "keep lost
>>> > paths as intent-to-add entries" feature, I am not sure why this has
>>> > to be so different from the usual add-cache-entry codepath. The
>>> > if/elseif chain you are touching inside this loop does:
>>> >
>>> > - If the tree you are resetting to has something at the path
>>> > (which is different from the current index, obviously), create
>>> > a cache entry to represent that state from the tree and stuff
>>> > it in the index;
>>> >
>>> > - Otherwise, the tree you are resetting to does not have that
>>> > path. We used to say "remove it from the index", but now we have
>>> > an option to instead add it as an intent-to-add entry.
>>> >
>>> > So, why doesn't the new codepath do exactly the same thing as the
>>> > first branch of the if/else chain and call add_cache_entry but with
>>> > a ce marked with CE_INTENT_TO_ADD? That would parallel what happens
>>> > in "git add -N" better, I would think, no?
>>>
>>> In other words, something along this line, perhaps?
>>
>> <snip>
>>
>> Yes. But you need something like this on top to actually set
>> CE_INTENT_TO_ADD
>
> Yes, indeed. I wonder why your new test did not notice it, though
> ;-)
... and the answer turns out to be that it was not testing the right
thing. On top of that faulty version, this will fix it.
Your suggestion to move CE_INTENT_TO_ADD to mark-intent-to-add makes
sense but a caller needs to be adjusted to drop the duplicated flag
manipulation.
read-cache.c | 3 +--
t/t7102-reset.sh | 6 ++++++
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index 325d193..5b8102a 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -584,6 +584,7 @@ void mark_intent_to_add(struct cache_entry *ce)
unsigned char sha1[20];
if (write_sha1_file("", 0, blob_type, sha1))
die("cannot create an empty blob in the object database");
+ ce->ce_flags |= CE_INTENT_TO_ADD;
hashcpy(ce->sha1, sha1);
}
@@ -613,8 +614,6 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
ce->ce_namelen = namelen;
if (!intent_only)
fill_stat_cache_info(ce, st);
- else
- ce->ce_flags |= CE_INTENT_TO_ADD;
if (trust_executable_bit && has_symlinks)
ce->ce_mode = create_ce_mode(st_mode);
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 642920a..bc0846f 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -539,6 +539,12 @@ test_expect_success 'reset -N keeps removed files as intent-to-add' '
echo new-file >new-file &&
git add new-file &&
git reset -N HEAD &&
+
+ tree=$(git write-tree) &&
+ git ls-tree $tree new-file >actual &&
+ >expect &&
+ test_cmp expect actual &&
+
git diff --name-only >actual &&
echo new-file >expect &&
test_cmp expect actual
next prev parent reply other threads:[~2014-02-05 18:25 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-01 10:48 splitting a commit that adds new files Duy Nguyen
2014-02-02 18:15 ` Junio C Hamano
2014-02-02 23:11 ` Jeff King
2014-02-03 18:11 ` Junio C Hamano
2014-02-04 0:54 ` Duy Nguyen
2014-02-04 2:20 ` [PATCH 1/2] t7101, t7014: rename test files to indicate what that file is for Nguyễn Thái Ngọc Duy
2014-02-04 2:20 ` [PATCH 2/2] reset: support "--mixed --intent-to-add" mode Nguyễn Thái Ngọc Duy
2014-02-04 19:09 ` Junio C Hamano
2014-02-04 22:25 ` Junio C Hamano
2014-02-05 0:27 ` Duy Nguyen
2014-02-05 17:16 ` Junio C Hamano
2014-02-05 18:25 ` Junio C Hamano [this message]
2014-02-05 23:48 ` Duy Nguyen
2014-02-06 0:08 ` Junio C Hamano
2014-02-06 0:43 ` Junio C Hamano
2014-02-04 16:05 ` [PATCH 1/2] t7101, t7014: rename test files to indicate what that file is for Jonathan Nieder
2014-02-06 1:58 ` Duy Nguyen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqbnyl5sde.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=pclouds@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.