* [PATCH v2 1/3] Remove unused var
@ 2021-08-02 17:00 David Turner
2021-08-02 17:00 ` [PATCH v2 2/3] diff --submodule=diff: do not fail on ever-initialied deleted submodules David Turner
2021-08-02 17:00 ` [PATCH v2 3/3] diff --submodule=diff: Don't print failure message twice David Turner
0 siblings, 2 replies; 5+ messages in thread
From: David Turner @ 2021-08-02 17:00 UTC (permalink / raw)
To: git, novalis; +Cc: David Turner
Signed-off-by: David Turner <dturner@twosigma.com>
---
t/t4060-diff-submodule-option-diff-format.sh | 1 -
1 file changed, 1 deletion(-)
diff --git a/t/t4060-diff-submodule-option-diff-format.sh b/t/t4060-diff-submodule-option-diff-format.sh
index dc7b242697..69b9946931 100755
--- a/t/t4060-diff-submodule-option-diff-format.sh
+++ b/t/t4060-diff-submodule-option-diff-format.sh
@@ -361,7 +361,6 @@ test_expect_success 'typechanged submodule(submodule->blob)' '
rm -f sm1 &&
test_create_repo sm1 &&
head6=$(add_file sm1 foo6 foo7)
-fullhead6=$(cd sm1; git rev-parse --verify HEAD)
test_expect_success 'nonexistent commit' '
git diff-index -p --submodule=diff HEAD >actual &&
cat >expected <<-EOF &&
--
2.11.GIT
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/3] diff --submodule=diff: do not fail on ever-initialied deleted submodules
2021-08-02 17:00 [PATCH v2 1/3] Remove unused var David Turner
@ 2021-08-02 17:00 ` David Turner
2021-08-31 6:19 ` Junio C Hamano
2021-08-02 17:00 ` [PATCH v2 3/3] diff --submodule=diff: Don't print failure message twice David Turner
1 sibling, 1 reply; 5+ messages in thread
From: David Turner @ 2021-08-02 17:00 UTC (permalink / raw)
To: git, novalis; +Cc: David Turner
If you have ever initialized a submodule, open_submodule will open it.
If you then delete the submodule's worktree directory (but don't
remove it from .gitmodules), git diff --submodule=diff would error out
as it attempted to chdir into the now-deleted working tree directory.
Instead, we chdir into the submodule's git directory and run the diff
from there.
Signed-off-by: David Turner <dturner@twosigma.com>
---
submodule.c | 10 ++
t/t4060-diff-submodule-option-diff-format.sh | 157 +++++++++++++++++++++++++--
2 files changed, 160 insertions(+), 7 deletions(-)
diff --git a/submodule.c b/submodule.c
index 0b1d9c1dde..d13d103975 100644
--- a/submodule.c
+++ b/submodule.c
@@ -710,6 +710,16 @@ void show_submodule_inline_diff(struct diff_options *o, const char *path,
strvec_push(&cp.args, oid_to_hex(new_oid));
prepare_submodule_repo_env(&cp.env_array);
+
+ if (!is_directory(path)) {
+ // fall back to absorbed git dir, if any
+ if (!sub)
+ goto done;
+ cp.dir = sub->gitdir;
+ strvec_push(&cp.env_array, GIT_DIR_ENVIRONMENT "=.");
+ strvec_push(&cp.env_array, GIT_WORK_TREE_ENVIRONMENT "=.");
+ }
+
if (start_command(&cp))
diff_emit_submodule_error(o, "(diff failed)\n");
diff --git a/t/t4060-diff-submodule-option-diff-format.sh b/t/t4060-diff-submodule-option-diff-format.sh
index 69b9946931..82a64e0f5d 100755
--- a/t/t4060-diff-submodule-option-diff-format.sh
+++ b/t/t4060-diff-submodule-option-diff-format.sh
@@ -703,10 +703,26 @@ test_expect_success 'path filter' '
diff_cmp expected actual
'
-commit_file sm2
+cat >.gitmodules <<-EOF
+[submodule "sm2"]
+ path = sm2
+ url = bogus_url
+EOF
+git add .gitmodules
+commit_file sm2 .gitmodules
+
test_expect_success 'given commit' '
git diff-index -p --submodule=diff HEAD^ >actual &&
cat >expected <<-EOF &&
+ diff --git a/.gitmodules b/.gitmodules
+ new file mode 100644
+ index 1234567..89abcde
+ --- /dev/null
+ +++ b/.gitmodules
+ @@ -0,0 +1,3 @@
+ +[submodule "sm2"]
+ +path = sm2
+ +url = bogus_url
Submodule sm1 $head7...0000000 (submodule deleted)
Submodule sm2 0000000...$head9 (new submodule)
diff --git a/sm2/foo8 b/sm2/foo8
@@ -728,15 +744,21 @@ test_expect_success 'given commit' '
'
test_expect_success 'setup .git file for sm2' '
- (cd sm2 &&
- REAL="$(pwd)/../.real" &&
- mv .git "$REAL" &&
- echo "gitdir: $REAL" >.git)
+ git submodule absorbgitdirs sm2
'
test_expect_success 'diff --submodule=diff with .git file' '
git diff --submodule=diff HEAD^ >actual &&
cat >expected <<-EOF &&
+ diff --git a/.gitmodules b/.gitmodules
+ new file mode 100644
+ index 1234567..89abcde
+ --- /dev/null
+ +++ b/.gitmodules
+ @@ -0,0 +1,3 @@
+ +[submodule "sm2"]
+ +path = sm2
+ +url = bogus_url
Submodule sm1 $head7...0000000 (submodule deleted)
Submodule sm2 0000000...$head9 (new submodule)
diff --git a/sm2/foo8 b/sm2/foo8
@@ -757,9 +779,67 @@ test_expect_success 'diff --submodule=diff with .git file' '
diff_cmp expected actual
'
+mv sm2 sm2-bak
+
+test_expect_success 'deleted submodule with .git file' '
+ git diff-index -p --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 $head7...0000000 (submodule deleted)
+ Submodule sm2 $head9...0000000 (submodule deleted)
+ diff --git a/sm2/foo8 b/sm2/foo8
+ deleted file mode 100644
+ index 1234567..89abcde
+ --- a/sm2/foo8
+ +++ /dev/null
+ @@ -1 +0,0 @@
+ -foo8
+ diff --git a/sm2/foo9 b/sm2/foo9
+ deleted file mode 100644
+ index 1234567..89abcde
+ --- a/sm2/foo9
+ +++ /dev/null
+ @@ -1 +0,0 @@
+ -foo9
+ EOF
+ diff_cmp expected actual
+'
+
+echo submodule-to-blob>sm2
+
+test_expect_success 'typechanged(submodule->blob) submodule with .git file' '
+ git diff-index -p --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 $head7...0000000 (submodule deleted)
+ Submodule sm2 $head9...0000000 (submodule deleted)
+ diff --git a/sm2/foo8 b/sm2/foo8
+ deleted file mode 100644
+ index 1234567..89abcde
+ --- a/sm2/foo8
+ +++ /dev/null
+ @@ -1 +0,0 @@
+ -foo8
+ diff --git a/sm2/foo9 b/sm2/foo9
+ deleted file mode 100644
+ index 1234567..89abcde
+ --- a/sm2/foo9
+ +++ /dev/null
+ @@ -1 +0,0 @@
+ -foo9
+ diff --git a/sm2 b/sm2
+ new file mode 100644
+ index 1234567..89abcde
+ --- /dev/null
+ +++ b/sm2
+ @@ -0,0 +1 @@
+ +submodule-to-blob
+ EOF
+ diff_cmp expected actual
+'
+
+rm sm2
+mv sm2-bak sm2
+
test_expect_success 'setup nested submodule' '
- git submodule add -f ./sm2 &&
- git commit -a -m "add sm2" &&
git -C sm2 submodule add ../sm2 nested &&
git -C sm2 commit -a -m "nested sub" &&
head10=$(git -C sm2 rev-parse --short --verify HEAD)
@@ -790,6 +870,7 @@ test_expect_success 'diff --submodule=diff with moved nested submodule HEAD' '
test_expect_success 'diff --submodule=diff recurses into nested submodules' '
cat >expected <<-EOF &&
+ Submodule sm1 17243c9...0000000 (submodule deleted)
Submodule sm2 contains modified content
Submodule sm2 $head9..$head10:
diff --git a/sm2/.gitmodules b/sm2/.gitmodules
@@ -829,4 +910,66 @@ test_expect_success 'diff --submodule=diff recurses into nested submodules' '
diff_cmp expected actual
'
+(cd sm2; commit_file nested)
+commit_file sm2
+
+mv sm2 sm2-bak
+
+test_expect_success 'diff --submodule=diff recurses into deleted nested submodules' '
+ cat >expected <<-EOF &&
+ Submodule sm1 17243c9...0000000 (submodule deleted)
+ Submodule sm2 67d632f...0000000 (submodule deleted)
+ diff --git a/sm2/.gitmodules b/sm2/.gitmodules
+ deleted file mode 100644
+ index 3a816b8..0000000
+ --- a/sm2/.gitmodules
+ +++ /dev/null
+ @@ -1,3 +0,0 @@
+ -[submodule "nested"]
+ - path = nested
+ - url = ../sm2
+ diff --git a/sm2/foo8 b/sm2/foo8
+ deleted file mode 100644
+ index db9916b..0000000
+ --- a/sm2/foo8
+ +++ /dev/null
+ @@ -1 +0,0 @@
+ -foo8
+ diff --git a/sm2/foo9 b/sm2/foo9
+ deleted file mode 100644
+ index 9c3b4f6..0000000
+ --- a/sm2/foo9
+ +++ /dev/null
+ @@ -1 +0,0 @@
+ -foo9
+ Submodule nested b55928c...0000000 (submodule deleted)
+ diff --git a/sm2/nested/file b/sm2/nested/file
+ deleted file mode 100644
+ index ca281f5..0000000
+ --- a/sm2/nested/file
+ +++ /dev/null
+ @@ -1 +0,0 @@
+ -nested content
+ diff --git a/sm2/nested/foo8 b/sm2/nested/foo8
+ deleted file mode 100644
+ index db9916b..0000000
+ --- a/sm2/nested/foo8
+ +++ /dev/null
+ @@ -1 +0,0 @@
+ -foo8
+ diff --git a/sm2/nested/foo9 b/sm2/nested/foo9
+ deleted file mode 100644
+ index 9c3b4f6..0000000
+ --- a/sm2/nested/foo9
+ +++ /dev/null
+ @@ -1 +0,0 @@
+ -foo9
+ EOF
+ git diff --submodule=diff >actual 2>err &&
+ test_must_be_empty err &&
+ diff_cmp expected actual
+'
+
+mv sm2-bak sm2
+
test_done
--
2.11.GIT
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 3/3] diff --submodule=diff: Don't print failure message twice
2021-08-02 17:00 [PATCH v2 1/3] Remove unused var David Turner
2021-08-02 17:00 ` [PATCH v2 2/3] diff --submodule=diff: do not fail on ever-initialied deleted submodules David Turner
@ 2021-08-02 17:00 ` David Turner
2021-08-31 6:19 ` Junio C Hamano
1 sibling, 1 reply; 5+ messages in thread
From: David Turner @ 2021-08-02 17:00 UTC (permalink / raw)
To: git, novalis; +Cc: David Turner
When we fail to start a diff command inside a submodule, immediately
exit the routine rather than trying to finish the command and printing
a second message.
Signed-off-by: David Turner <dturner@twosigma.com>
---
submodule.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/submodule.c b/submodule.c
index d13d103975..2e98e840af 100644
--- a/submodule.c
+++ b/submodule.c
@@ -720,8 +720,10 @@ void show_submodule_inline_diff(struct diff_options *o, const char *path,
strvec_push(&cp.env_array, GIT_WORK_TREE_ENVIRONMENT "=.");
}
- if (start_command(&cp))
+ if (start_command(&cp)) {
diff_emit_submodule_error(o, "(diff failed)\n");
+ goto done;
+ }
while (strbuf_getwholeline_fd(&sb, cp.out, '\n') != EOF)
diff_emit_submodule_pipethrough(o, sb.buf, sb.len);
--
2.11.GIT
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/3] diff --submodule=diff: do not fail on ever-initialied deleted submodules
2021-08-02 17:00 ` [PATCH v2 2/3] diff --submodule=diff: do not fail on ever-initialied deleted submodules David Turner
@ 2021-08-31 6:19 ` Junio C Hamano
0 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2021-08-31 6:19 UTC (permalink / raw)
To: David Turner; +Cc: git, novalis
David Turner <dturner@twosigma.com> writes:
> If you have ever initialized a submodule, open_submodule will open it.
> If you then delete the submodule's worktree directory (but don't
> remove it from .gitmodules), git diff --submodule=diff would error out
> as it attempted to chdir into the now-deleted working tree directory.
>
> Instead, we chdir into the submodule's git directory and run the diff
> from there.
>
> Signed-off-by: David Turner <dturner@twosigma.com>
> ---
> submodule.c | 10 ++
> t/t4060-diff-submodule-option-diff-format.sh | 157 +++++++++++++++++++++++++--
> 2 files changed, 160 insertions(+), 7 deletions(-)
>
> diff --git a/submodule.c b/submodule.c
> index 0b1d9c1dde..d13d103975 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -710,6 +710,16 @@ void show_submodule_inline_diff(struct diff_options *o, const char *path,
> strvec_push(&cp.args, oid_to_hex(new_oid));
>
> prepare_submodule_repo_env(&cp.env_array);
> +
> + if (!is_directory(path)) {
> + // fall back to absorbed git dir, if any
I'll locally amend this comment style. If there is no other
comments, that would be sufficient.
> + if (!sub)
> + goto done;
> + cp.dir = sub->gitdir;
> + strvec_push(&cp.env_array, GIT_DIR_ENVIRONMENT "=.");
> + strvec_push(&cp.env_array, GIT_WORK_TREE_ENVIRONMENT "=.");
> + }
> +
> if (start_command(&cp))
> diff_emit_submodule_error(o, "(diff failed)\n");
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 3/3] diff --submodule=diff: Don't print failure message twice
2021-08-02 17:00 ` [PATCH v2 3/3] diff --submodule=diff: Don't print failure message twice David Turner
@ 2021-08-31 6:19 ` Junio C Hamano
0 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2021-08-31 6:19 UTC (permalink / raw)
To: David Turner; +Cc: git, novalis
David Turner <dturner@twosigma.com> writes:
> When we fail to start a diff command inside a submodule, immediately
> exit the routine rather than trying to finish the command and printing
> a second message.
>
> Signed-off-by: David Turner <dturner@twosigma.com>
> ---
> submodule.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/submodule.c b/submodule.c
> index d13d103975..2e98e840af 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -720,8 +720,10 @@ void show_submodule_inline_diff(struct diff_options *o, const char *path,
> strvec_push(&cp.env_array, GIT_WORK_TREE_ENVIRONMENT "=.");
> }
>
> - if (start_command(&cp))
> + if (start_command(&cp)) {
> diff_emit_submodule_error(o, "(diff failed)\n");
> + goto done;
> + }
>
> while (strbuf_getwholeline_fd(&sb, cp.out, '\n') != EOF)
> diff_emit_submodule_pipethrough(o, sb.buf, sb.len);
Makes sense.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-08-31 6:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-02 17:00 [PATCH v2 1/3] Remove unused var David Turner
2021-08-02 17:00 ` [PATCH v2 2/3] diff --submodule=diff: do not fail on ever-initialied deleted submodules David Turner
2021-08-31 6:19 ` Junio C Hamano
2021-08-02 17:00 ` [PATCH v2 3/3] diff --submodule=diff: Don't print failure message twice David Turner
2021-08-31 6:19 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).