* [PATCH 1/4] test: use $_z40 from test-lib
2011-04-24 20:51 [PATCH 0/4] Fix diff-files output for unmerged paths Junio C Hamano
@ 2011-04-24 20:51 ` Junio C Hamano
2011-04-24 20:51 ` [PATCH 2/4] diff.c: return filepair from diff_unmerge() Junio C Hamano
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2011-04-24 20:51 UTC (permalink / raw)
To: git
There is no need to duplicate the definition of $_z40 and $_x40 that
test-lib.sh supplies the test scripts.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t1400-update-ref.sh | 2 +-
t/t1501-worktree.sh | 7 +++----
t/t2011-checkout-invalid-head.sh | 2 +-
t/t2201-add-update-typechange.sh | 2 --
t/t3200-branch.sh | 4 ++--
t/t3600-rm.sh | 3 +--
t/t4002-diff-basic.sh | 5 +----
t/t4020-diff-external.sh | 2 --
t/t4027-diff-submodule.sh | 1 -
t/t7011-skip-worktree-reading.sh | 4 ++--
t/t7012-skip-worktree-writing.sh | 2 +-
t/test-lib.sh | 3 +++
12 files changed, 15 insertions(+), 22 deletions(-)
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 54ba3df..6b39915 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -6,7 +6,7 @@
test_description='Test git update-ref and basic ref logging'
. ./test-lib.sh
-Z=0000000000000000000000000000000000000000
+Z=$_z40
test_expect_success setup '
diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh
index 2c8f01f..dc7c992 100755
--- a/t/t1501-worktree.sh
+++ b/t/t1501-worktree.sh
@@ -7,7 +7,6 @@ test_expect_success 'setup' '
EMPTY_TREE=$(git write-tree) &&
EMPTY_BLOB=$(git hash-object -t blob --stdin </dev/null) &&
CHANGED_BLOB=$(echo changed | git hash-object -t blob --stdin) &&
- ZEROES=0000000000000000000000000000000000000000 &&
EMPTY_BLOB7=$(echo $EMPTY_BLOB | sed "s/\(.......\).*/\1/") &&
CHANGED_BLOB7=$(echo $CHANGED_BLOB | sed "s/\(.......\).*/\1/") &&
@@ -239,10 +238,10 @@ test_expect_success '_gently() groks relative GIT_DIR & GIT_WORK_TREE' '
test_expect_success 'diff-index respects work tree under .git dir' '
cat >diff-index-cached.expected <<-EOF &&
- :000000 100644 $ZEROES $EMPTY_BLOB A sub/dir/tracked
+ :000000 100644 $_z40 $EMPTY_BLOB A sub/dir/tracked
EOF
cat >diff-index.expected <<-EOF &&
- :000000 100644 $ZEROES $ZEROES A sub/dir/tracked
+ :000000 100644 $_z40 $_z40 A sub/dir/tracked
EOF
(
@@ -258,7 +257,7 @@ test_expect_success 'diff-index respects work tree under .git dir' '
test_expect_success 'diff-files respects work tree under .git dir' '
cat >diff-files.expected <<-EOF &&
- :100644 100644 $EMPTY_BLOB $ZEROES M sub/dir/tracked
+ :100644 100644 $EMPTY_BLOB $_z40 M sub/dir/tracked
EOF
(
diff --git a/t/t2011-checkout-invalid-head.sh b/t/t2011-checkout-invalid-head.sh
index 15ebdc2..300f8bf 100755
--- a/t/t2011-checkout-invalid-head.sh
+++ b/t/t2011-checkout-invalid-head.sh
@@ -15,7 +15,7 @@ test_expect_success 'checkout should not start branch from a tree' '
'
test_expect_success 'checkout master from invalid HEAD' '
- echo 0000000000000000000000000000000000000000 >.git/HEAD &&
+ echo $_z40 >.git/HEAD &&
git checkout master --
'
diff --git a/t/t2201-add-update-typechange.sh b/t/t2201-add-update-typechange.sh
index 2e8f702..954fc51 100755
--- a/t/t2201-add-update-typechange.sh
+++ b/t/t2201-add-update-typechange.sh
@@ -4,8 +4,6 @@ test_description='more git add -u'
. ./test-lib.sh
-_z40=0000000000000000000000000000000000000000
-
test_expect_success setup '
>xyzzy &&
_empty=$(git hash-object --stdin <xyzzy) &&
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index f54a533..4c4cff1 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -35,7 +35,7 @@ test_expect_success \
'git branch a/b/c && test -f .git/refs/heads/a/b/c'
cat >expect <<EOF
-0000000000000000000000000000000000000000 $HEAD $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000 branch: Created from master
+$_z40 $HEAD $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000 branch: Created from master
EOF
test_expect_success \
'git branch -l d/e/f should create a branch and a log' \
@@ -214,7 +214,7 @@ test_expect_success \
# Keep this test last, as it changes the current branch
cat >expect <<EOF
-0000000000000000000000000000000000000000 $HEAD $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000 branch: Created from master
+$_z40 $HEAD $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000 branch: Created from master
EOF
test_expect_success \
'git checkout -b g/h/i -l should create a branch and a log' \
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index b26cabd..66523bd 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -240,11 +240,10 @@ test_expect_success 'refresh index before checking if it is up-to-date' '
test_expect_success 'choking "git rm" should not let it die with cruft' '
git reset -q --hard &&
- H=0000000000000000000000000000000000000000 &&
i=0 &&
while test $i -lt 12000
do
- echo "100644 $H 0 some-file-$i"
+ echo "100644 $_z40 0 some-file-$i"
i=$(( $i + 1 ))
done | git update-index --index-info &&
git rm -n "some-file-*" | :;
diff --git a/t/t4002-diff-basic.sh b/t/t4002-diff-basic.sh
index 73441a5..66e1a52 100755
--- a/t/t4002-diff-basic.sh
+++ b/t/t4002-diff-basic.sh
@@ -126,15 +126,12 @@ cat >.test-recursive-AB <<\EOF
:100644 100644 3fdbe17fd013303a2e981e1ca1c6cd6e72789087 7e09d6a3a14bd630913e8c75693cea32157b606d M Z/NM
EOF
-x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
-x40="$x40$x40$x40$x40$x40$x40$x40$x40"
-z40='0000000000000000000000000000000000000000'
cmp_diff_files_output () {
# diff-files never reports additions. Also it does not fill in the
# object ID for the changed files because it wants you to look at the
# filesystem.
sed <"$2" >.test-tmp \
- -e '/^:000000 /d;s/'$x40'\( [MCRNDU][0-9]*\) /'$z40'\1 /' &&
+ -e '/^:000000 /d;s/'$_x40'\( [MCRNDU][0-9]*\) /'$_z40'\1 /' &&
test_cmp "$1" .test-tmp
}
diff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh
index a7602cf..083f62d 100755
--- a/t/t4020-diff-external.sh
+++ b/t/t4020-diff-external.sh
@@ -4,8 +4,6 @@ test_description='external diff interface test'
. ./test-lib.sh
-_z40=0000000000000000000000000000000000000000
-
test_expect_success setup '
test_tick &&
diff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh
index d99814a..fbe44a3 100755
--- a/t/t4027-diff-submodule.sh
+++ b/t/t4027-diff-submodule.sh
@@ -5,7 +5,6 @@ test_description='difference in submodules'
. ./test-lib.sh
. "$TEST_DIRECTORY"/diff-lib.sh
-_z40=0000000000000000000000000000000000000000
test_expect_success setup '
test_tick &&
test_create_repo sub &&
diff --git a/t/t7011-skip-worktree-reading.sh b/t/t7011-skip-worktree-reading.sh
index bb4066f..8f3b54d 100755
--- a/t/t7011-skip-worktree-reading.sh
+++ b/t/t7011-skip-worktree-reading.sh
@@ -24,7 +24,7 @@ H sub/2
EOF
NULL_SHA1=e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
-ZERO_SHA0=0000000000000000000000000000000000000000
+
setup_absent() {
test -f 1 && rm 1
git update-index --remove 1 &&
@@ -120,7 +120,7 @@ test_expect_success 'grep with skip-worktree file' '
test "$(git grep --no-ext-grep test)" = "1:test"
'
-echo ":000000 100644 $ZERO_SHA0 $NULL_SHA1 A 1" > expected
+echo ":000000 100644 $_z40 $NULL_SHA1 A 1" > expected
test_expect_success 'diff-index does not examine skip-worktree absent entries' '
setup_absent &&
git diff-index HEAD -- 1 > result &&
diff --git a/t/t7012-skip-worktree-writing.sh b/t/t7012-skip-worktree-writing.sh
index 582d0b5..d70fe2f 100755
--- a/t/t7012-skip-worktree-writing.sh
+++ b/t/t7012-skip-worktree-writing.sh
@@ -54,7 +54,7 @@ test_expect_success 'read-tree removes worktree, dirty case' '
'
NULL_SHA1=e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
-ZERO_SHA0=0000000000000000000000000000000000000000
+
setup_absent() {
test -f 1 && rm 1
git update-index --remove 1 &&
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 830e5e7..7afa25f 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -97,6 +97,9 @@ esac
_x05='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
_x40="$_x05$_x05$_x05$_x05$_x05$_x05$_x05$_x05"
+# Zero SHA-1
+_z40=0000000000000000000000000000000000000000
+
# Each test should start with something like this, after copyright notices:
#
# test_description='Description of this test...
--
1.7.5
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 2/4] diff.c: return filepair from diff_unmerge()
2011-04-24 20:51 [PATCH 0/4] Fix diff-files output for unmerged paths Junio C Hamano
2011-04-24 20:51 ` [PATCH 1/4] test: use $_z40 from test-lib Junio C Hamano
@ 2011-04-24 20:51 ` Junio C Hamano
2011-04-24 22:18 ` Thiago Farina
2011-04-24 20:51 ` [PATCH 3/4] diff: remove often unused parameters " Junio C Hamano
` (2 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Junio C Hamano @ 2011-04-24 20:51 UTC (permalink / raw)
To: git
The underlying diff_queue() returns diff_filepair so that the caller can
further add information to it, and the helper function diff_unmerge()
utilizes the feature itself, but does not expose it to its callers, which
was kind of selfish.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
diff.c | 13 ++++++++-----
diff.h | 2 +-
2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/diff.c b/diff.c
index 9a5c77c..4c34c64 100644
--- a/diff.c
+++ b/diff.c
@@ -4308,20 +4308,23 @@ void diff_change(struct diff_options *options,
DIFF_OPT_SET(options, HAS_CHANGES);
}
-void diff_unmerge(struct diff_options *options,
- const char *path,
- unsigned mode, const unsigned char *sha1)
+struct diff_filepair *diff_unmerge(struct diff_options *options,
+ const char *path,
+ unsigned mode, const unsigned char *sha1)
{
+ struct diff_filepair *pair;
struct diff_filespec *one, *two;
if (options->prefix &&
strncmp(path, options->prefix, options->prefix_length))
- return;
+ return NULL;
one = alloc_filespec(path);
two = alloc_filespec(path);
fill_filespec(one, sha1, mode);
- diff_queue(&diff_queued_diff, one, two)->is_unmerged = 1;
+ pair = diff_queue(&diff_queued_diff, one, two);
+ pair->is_unmerged = 1;
+ return pair;
}
static char *run_textconv(const char *pgm, struct diff_filespec *spec,
diff --git a/diff.h b/diff.h
index bf2f44d..f51a8ee 100644
--- a/diff.h
+++ b/diff.h
@@ -209,7 +209,7 @@ extern void diff_change(struct diff_options *,
const char *fullpath,
unsigned dirty_submodule1, unsigned dirty_submodule2);
-extern void diff_unmerge(struct diff_options *,
+extern struct diff_filepair *diff_unmerge(struct diff_options *,
const char *path,
unsigned mode,
const unsigned char *sha1);
--
1.7.5
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 2/4] diff.c: return filepair from diff_unmerge()
2011-04-24 20:51 ` [PATCH 2/4] diff.c: return filepair from diff_unmerge() Junio C Hamano
@ 2011-04-24 22:18 ` Thiago Farina
2011-04-25 1:18 ` Junio C Hamano
0 siblings, 1 reply; 8+ messages in thread
From: Thiago Farina @ 2011-04-24 22:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Some unrelated style comments below.
On Sun, Apr 24, 2011 at 5:51 PM, Junio C Hamano <gitster@pobox.com> wrote:
> The underlying diff_queue() returns diff_filepair so that the caller can
> further add information to it, and the helper function diff_unmerge()
> utilizes the feature itself, but does not expose it to its callers, which
> was kind of selfish.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> diff.c | 13 ++++++++-----
> diff.h | 2 +-
> 2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index 9a5c77c..4c34c64 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -4308,20 +4308,23 @@ void diff_change(struct diff_options *options,
> DIFF_OPT_SET(options, HAS_CHANGES);
> }
>
> -void diff_unmerge(struct diff_options *options,
> - const char *path,
> - unsigned mode, const unsigned char *sha1)
> +struct diff_filepair *diff_unmerge(struct diff_options *options,
> + const char *path,
> + unsigned mode, const unsigned char *sha1)
> {
While you are here, why not write one arg per line?
> + struct diff_filepair *pair;
> struct diff_filespec *one, *two;
>
> if (options->prefix &&
> strncmp(path, options->prefix, options->prefix_length))
> - return;
> + return NULL;
>
> one = alloc_filespec(path);
> two = alloc_filespec(path);
> fill_filespec(one, sha1, mode);
> - diff_queue(&diff_queued_diff, one, two)->is_unmerged = 1;
> + pair = diff_queue(&diff_queued_diff, one, two);
> + pair->is_unmerged = 1;
> + return pair;
> }
>
> static char *run_textconv(const char *pgm, struct diff_filespec *spec,
> diff --git a/diff.h b/diff.h
> index bf2f44d..f51a8ee 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -209,7 +209,7 @@ extern void diff_change(struct diff_options *,
> const char *fullpath,
> unsigned dirty_submodule1, unsigned dirty_submodule2);
>
> -extern void diff_unmerge(struct diff_options *,
> +extern struct diff_filepair *diff_unmerge(struct diff_options *,
While you are here, why not add the argument name |options| here too?
> const char *path,
> unsigned mode,
> const unsigned char *sha1);
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 2/4] diff.c: return filepair from diff_unmerge()
2011-04-24 22:18 ` Thiago Farina
@ 2011-04-25 1:18 ` Junio C Hamano
0 siblings, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2011-04-25 1:18 UTC (permalink / raw)
To: Thiago Farina; +Cc: git
Thiago Farina <tfransosi@gmail.com> writes:
> Some unrelated style comments below.
> ...
>> -void diff_unmerge(struct diff_options *options,
>> - const char *path,
>> - unsigned mode, const unsigned char *sha1)
>> +struct diff_filepair *diff_unmerge(struct diff_options *options,
>> + const char *path,
>> + unsigned mode, const unsigned char *sha1)
>> {
>
> While you are here, why not write one arg per line?
No, actually I reindented the two lines by accident; I didn't mean to.
>> diff --git a/diff.h b/diff.h
>> index bf2f44d..f51a8ee 100644
>> --- a/diff.h
>> +++ b/diff.h
>> @@ -209,7 +209,7 @@ extern void diff_change(struct diff_options *,
>> const char *fullpath,
>> unsigned dirty_submodule1, unsigned dirty_submodule2);
>>
>> -extern void diff_unmerge(struct diff_options *,
>> +extern struct diff_filepair *diff_unmerge(struct diff_options *,
>
> While you are here, why not add the argument name |options| here too?
>
>> const char *path,
>> unsigned mode,
>> const unsigned char *sha1);
The patch was meant to be minimum to begin with. Besides, there is no
risk of confusion from not naming the option in this case, so even if we
were shooting for clean-up, we wouldn't be adding such noise here. The
other parameters do benefit from descriptive names, so there is no need to
remove the names either.
In other case, your "why not" is totally unwarranted for this hunk.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/4] diff: remove often unused parameters from diff_unmerge()
2011-04-24 20:51 [PATCH 0/4] Fix diff-files output for unmerged paths Junio C Hamano
2011-04-24 20:51 ` [PATCH 1/4] test: use $_z40 from test-lib Junio C Hamano
2011-04-24 20:51 ` [PATCH 2/4] diff.c: return filepair from diff_unmerge() Junio C Hamano
@ 2011-04-24 20:51 ` Junio C Hamano
2011-04-24 20:51 ` [PATCH 4/4] diff-files: show unmerged entries correctly Junio C Hamano
2011-04-24 20:53 ` [PATCH] Fix "add -u" that sometimes fails to resolve unmerged paths Junio C Hamano
4 siblings, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2011-04-24 20:51 UTC (permalink / raw)
To: git
e9c8409 (diff-index --cached --raw: show tree entry on the LHS for
unmerged entries., 2007-01-05) added a <mode, object name> pair as
parameters to this function, to store them in the pre-image side of an
unmerged file pair. Now the function is fixed to return the filepair it
queued, we can make the caller on the special case codepath to do so.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
diff-lib.c | 7 ++++---
diff.c | 5 +----
diff.h | 5 +----
3 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/diff-lib.c b/diff-lib.c
index 392ce2b..a983855 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -183,7 +183,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
* Show the diff for the 'ce' if we found the one
* from the desired stage.
*/
- diff_unmerge(&revs->diffopt, ce->name, 0, null_sha1);
+ diff_unmerge(&revs->diffopt, ce->name);
if (ce_stage(ce) != diff_unmerged_stage)
continue;
}
@@ -372,8 +372,9 @@ static void do_oneway_diff(struct unpack_trees_options *o,
match_missing = !revs->ignore_merges;
if (cached && idx && ce_stage(idx)) {
- diff_unmerge(&revs->diffopt, idx->name, idx->ce_mode,
- idx->sha1);
+ struct diff_filepair *pair;
+ pair = diff_unmerge(&revs->diffopt, idx->name);
+ fill_filespec(pair->one, idx->sha1, idx->ce_mode);
return;
}
diff --git a/diff.c b/diff.c
index 4c34c64..d2c5c56 100644
--- a/diff.c
+++ b/diff.c
@@ -4308,9 +4308,7 @@ void diff_change(struct diff_options *options,
DIFF_OPT_SET(options, HAS_CHANGES);
}
-struct diff_filepair *diff_unmerge(struct diff_options *options,
- const char *path,
- unsigned mode, const unsigned char *sha1)
+struct diff_filepair *diff_unmerge(struct diff_options *options, const char *path)
{
struct diff_filepair *pair;
struct diff_filespec *one, *two;
@@ -4321,7 +4319,6 @@ struct diff_filepair *diff_unmerge(struct diff_options *options,
one = alloc_filespec(path);
two = alloc_filespec(path);
- fill_filespec(one, sha1, mode);
pair = diff_queue(&diff_queued_diff, one, two);
pair->is_unmerged = 1;
return pair;
diff --git a/diff.h b/diff.h
index f51a8ee..3edb705 100644
--- a/diff.h
+++ b/diff.h
@@ -209,10 +209,7 @@ extern void diff_change(struct diff_options *,
const char *fullpath,
unsigned dirty_submodule1, unsigned dirty_submodule2);
-extern struct diff_filepair *diff_unmerge(struct diff_options *,
- const char *path,
- unsigned mode,
- const unsigned char *sha1);
+extern struct diff_filepair *diff_unmerge(struct diff_options *, const char *path);
#define DIFF_SETUP_REVERSE 1
#define DIFF_SETUP_USE_CACHE 2
--
1.7.5
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 4/4] diff-files: show unmerged entries correctly
2011-04-24 20:51 [PATCH 0/4] Fix diff-files output for unmerged paths Junio C Hamano
` (2 preceding siblings ...)
2011-04-24 20:51 ` [PATCH 3/4] diff: remove often unused parameters " Junio C Hamano
@ 2011-04-24 20:51 ` Junio C Hamano
2011-04-24 20:53 ` [PATCH] Fix "add -u" that sometimes fails to resolve unmerged paths Junio C Hamano
4 siblings, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2011-04-24 20:51 UTC (permalink / raw)
To: git
Earlier, e9c8409 (diff-index --cached --raw: show tree entry on the LHS
for unmerged entries., 2007-01-05) taught the command to show the object
name and the mode from the entry coming from the tree side when comparing
a tree with an unmerged index.
This is a belated companion patch that teaches diff-files to show the mode
from the entry coming from the working tree side, when comparing an
unmerged index and the working tree.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
diff-lib.c | 10 ++++-
t/t4046-diff-unmerged.sh | 87 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 95 insertions(+), 2 deletions(-)
create mode 100755 t/t4046-diff-unmerged.sh
diff --git a/diff-lib.c b/diff-lib.c
index a983855..b782476 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -111,6 +111,8 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
if (ce_stage(ce)) {
struct combine_diff_path *dpath;
+ struct diff_filepair *pair;
+ unsigned int wt_mode = 0;
int num_compare_stages = 0;
size_t path_len;
@@ -129,7 +131,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
changed = check_removed(ce, &st);
if (!changed)
- dpath->mode = ce_mode_from_stat(ce, st.st_mode);
+ wt_mode = ce_mode_from_stat(ce, st.st_mode);
else {
if (changed < 0) {
perror(ce->name);
@@ -137,7 +139,9 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
}
if (silent_on_removed)
continue;
+ wt_mode = 0;
}
+ dpath->mode = wt_mode;
while (i < entries) {
struct cache_entry *nce = active_cache[i];
@@ -183,7 +187,9 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
* Show the diff for the 'ce' if we found the one
* from the desired stage.
*/
- diff_unmerge(&revs->diffopt, ce->name);
+ pair = diff_unmerge(&revs->diffopt, ce->name);
+ if (wt_mode)
+ pair->two->mode = wt_mode;
if (ce_stage(ce) != diff_unmerged_stage)
continue;
}
diff --git a/t/t4046-diff-unmerged.sh b/t/t4046-diff-unmerged.sh
new file mode 100755
index 0000000..25d50a6
--- /dev/null
+++ b/t/t4046-diff-unmerged.sh
@@ -0,0 +1,87 @@
+#!/bin/sh
+
+test_description='diff with unmerged index entries'
+. ./test-lib.sh
+
+test_expect_success setup '
+ for i in 0 1 2 3
+ do
+ blob=$(echo $i | git hash-object --stdin) &&
+ eval "blob$i=$blob" &&
+ eval "m$i=\"100644 \$blob$i $i\"" || break
+ done &&
+ paths= &&
+ for b in o x
+ do
+ for o in o x
+ do
+ for t in o x
+ do
+ path="$b$o$t" &&
+ case "$path" in ooo) continue ;; esac
+ paths="$paths$path " &&
+ p=" $path" &&
+ case "$b" in x) echo "$m1$p" ;; esac &&
+ case "$o" in x) echo "$m2$p" ;; esac &&
+ case "$t" in x) echo "$m3$p" ;; esac ||
+ break
+ done || break
+ done || break
+ done >ls-files-s.expect &&
+ git update-index --index-info <ls-files-s.expect &&
+ git ls-files -s >ls-files-s.actual &&
+ test_cmp ls-files-s.expect ls-files-s.actual
+'
+
+test_expect_success 'diff-files -0' '
+ for path in $paths
+ do
+ >"$path" &&
+ echo ":000000 100644 $_z40 $_z40 U $path"
+ done >diff-files-0.expect &&
+ git diff-files -0 >diff-files-0.actual &&
+ test_cmp diff-files-0.expect diff-files-0.actual
+'
+
+test_expect_success 'diff-files -1' '
+ for path in $paths
+ do
+ >"$path" &&
+ echo ":000000 100644 $_z40 $_z40 U $path" &&
+ case "$path" in
+ x??) echo ":100644 100644 $blob1 $_z40 M $path"
+ esac
+ done >diff-files-1.expect &&
+ git diff-files -1 >diff-files-1.actual &&
+ test_cmp diff-files-1.expect diff-files-1.actual
+'
+
+test_expect_success 'diff-files -2' '
+ for path in $paths
+ do
+ >"$path" &&
+ echo ":000000 100644 $_z40 $_z40 U $path" &&
+ case "$path" in
+ ?x?) echo ":100644 100644 $blob2 $_z40 M $path"
+ esac
+ done >diff-files-2.expect &&
+ git diff-files -2 >diff-files-2.actual &&
+ test_cmp diff-files-2.expect diff-files-2.actual &&
+ git diff-files >diff-files-default-2.actual &&
+ test_cmp diff-files-2.expect diff-files-default-2.actual
+'
+
+test_expect_success 'diff-files -3' '
+ for path in $paths
+ do
+ >"$path" &&
+ echo ":000000 100644 $_z40 $_z40 U $path" &&
+ case "$path" in
+ ??x) echo ":100644 100644 $blob3 $_z40 M $path"
+ esac
+ done >diff-files-3.expect &&
+ git diff-files -3 >diff-files-3.actual &&
+ test_cmp diff-files-3.expect diff-files-3.actual
+'
+
+test_done
--
1.7.5
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH] Fix "add -u" that sometimes fails to resolve unmerged paths
2011-04-24 20:51 [PATCH 0/4] Fix diff-files output for unmerged paths Junio C Hamano
` (3 preceding siblings ...)
2011-04-24 20:51 ` [PATCH 4/4] diff-files: show unmerged entries correctly Junio C Hamano
@ 2011-04-24 20:53 ` Junio C Hamano
4 siblings, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2011-04-24 20:53 UTC (permalink / raw)
To: git
"git add -u" updates the index with the updated contents from the working
tree by internally running "diff-files" to grab the set of paths that are
different from the index. Then it updates the index entries for the paths
that are modified in the working tree, and deletes the index entries for
the paths that are deleted in the working tree.
It ignored the output from the diff-files that indicated that a path is
unmerged. For these paths, it instead relied on the fact that an unmerged
path is followed by the result of comparison between stage #2 (ours) and
the working tree, and used that to update or delete such a path when it is
used to record the resolution of a conflict.
As the result, when a path did not have stage #2 (e.g. "we deleted while
the other side added"), these unmerged stages were left behind, instead of
recording what the user resolved in the working tree.
Since we recently fixed "diff-files" to indicate if the corresponding path
exists on the working tree for an unmerged path, we do not have to rely on
the comparison with stage #2 anymore. We can instead tell the diff-files
not to compare with higher stages, and use the unmerged output to update
the index to reflect the state of the working tree.
The changes to the test vector in t2200 illustrates the nature of the bug
and the fix. The test expected stage #1 and #3 entries be left behind,
but it was codifying the buggy behaviour.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* This comes on top of the 4-patch series to fix "diff-files" output for
unmerged paths.
builtin/add.c | 45 +++++++++++++++++++++++----------------------
t/t2200-add-update.sh | 24 +++++++-----------------
2 files changed, 30 insertions(+), 39 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index 56a4e0a..027ca3b 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -27,6 +27,27 @@ struct update_callback_data
int add_errors;
};
+static int fix_unmerged_status(struct diff_filepair *p,
+ struct update_callback_data *data)
+{
+ if (p->status != DIFF_STATUS_UNMERGED)
+ return p->status;
+ if (!(data->flags & ADD_CACHE_IGNORE_REMOVAL) && !p->two->mode)
+ /*
+ * This is not an explicit add request, and the
+ * path is missing from the working tree (deleted)
+ */
+ return DIFF_STATUS_DELETED;
+ else
+ /*
+ * Either an explicit add request, or path exists
+ * in the working tree. An attempt to explicitly
+ * add a path that does not exist in the working tree
+ * will be caught as an error by the caller immediately.
+ */
+ return DIFF_STATUS_MODIFIED;
+}
+
static void update_callback(struct diff_queue_struct *q,
struct diff_options *opt, void *cbdata)
{
@@ -36,30 +57,9 @@ static void update_callback(struct diff_queue_struct *q,
for (i = 0; i < q->nr; i++) {
struct diff_filepair *p = q->queue[i];
const char *path = p->one->path;
- switch (p->status) {
+ switch (fix_unmerged_status(p, data)) {
default:
die("unexpected diff status %c", p->status);
- case DIFF_STATUS_UNMERGED:
- /*
- * ADD_CACHE_IGNORE_REMOVAL is unset if "git
- * add -u" is calling us, In such a case, a
- * missing work tree file needs to be removed
- * if there is an unmerged entry at stage #2,
- * but such a diff record is followed by
- * another with DIFF_STATUS_DELETED (and if
- * there is no stage #2, we won't see DELETED
- * nor MODIFIED). We can simply continue
- * either way.
- */
- if (!(data->flags & ADD_CACHE_IGNORE_REMOVAL))
- continue;
- /*
- * Otherwise, it is "git add path" is asking
- * to explicitly add it; we fall through. A
- * missing work tree file is an error and is
- * caught by add_file_to_index() in such a
- * case.
- */
case DIFF_STATUS_MODIFIED:
case DIFF_STATUS_TYPE_CHANGED:
if (add_file_to_index(&the_index, path, data->flags)) {
@@ -92,6 +92,7 @@ int add_files_to_cache(const char *prefix, const char **pathspec, int flags)
data.flags = flags;
data.add_errors = 0;
rev.diffopt.format_callback_data = &data;
+ rev.max_count = 0; /* do not compare unmerged paths with stage #2 */
run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
return !!data.add_errors;
}
diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh
index 2ad2819..d3bb9fa 100755
--- a/t/t2200-add-update.sh
+++ b/t/t2200-add-update.sh
@@ -149,31 +149,21 @@ test_expect_success 'add -u resolves unmerged paths' '
echo 3 >path1 &&
echo 2 >path3 &&
echo 2 >path5 &&
- git add -u &&
- git ls-files -s path1 path2 path3 path4 path5 path6 >actual &&
- {
- echo "100644 $three 0 path1"
- echo "100644 $one 1 path3"
- echo "100644 $one 1 path4"
- echo "100644 $one 3 path5"
- echo "100644 $one 3 path6"
- } >expect &&
- test_cmp expect actual &&
- # Bonus tests. Explicit resolving
- git add path3 path5 &&
+ # Explicit resolving by adding removed paths should fail
test_must_fail git add path4 &&
test_must_fail git add path6 &&
- git rm path4 &&
- git rm path6 &&
- git ls-files -s "path?" >actual &&
+ # "add -u" should notice removals no matter what stages
+ # the index entries are in.
+ git add -u &&
+ git ls-files -s path1 path2 path3 path4 path5 path6 >actual &&
{
echo "100644 $three 0 path1"
echo "100644 $two 0 path3"
echo "100644 $two 0 path5"
- } >expect
-
+ } >expect &&
+ test_cmp expect actual
'
test_expect_success '"add -u non-existent" should fail' '
--
1.7.5
^ permalink raw reply related [flat|nested] 8+ messages in thread