submodule: Fix 'submodule status' when called from a subdirectory

PATCH Superseded Manish Goregaokar via GitGitGadget 3 participants 11 messages Nov 23, 2019 – Nov 25, 2019

Share & tools

Manish Goregaokar via GitGitGadget

author

2019-11-23 06:28 UTC #0 lore raw

cover submodule: Fix 'submodule status' when called from a subdirectory

Previously, when calling git submodule status while in a subdirectory, it was incorrectly not detecting modified submodules and thus reporting that all of the submodules were unchanged.

This was because the submodule helper was calling diff-index with the submodule path assuming the path was relative to the current prefix directory, however the submodule path used is actually relative to the root.

This fixes the bug by setting the prefix to NULL when runningdiff-index from the helper, so that it correctly interprets the path as relative to the repository root.

Signed-off-by Manish Goregaokar [email protected]

[[email protected]]

Manish Goregaokar (1):
submodule: Fix 'submodule status' when called from a subdirectory

 builtin/submodule--helper.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

base-commit: 5fa0f5238b0cd46cfe7f6fa76c3f526ea98148d9
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-472%2FManishearth%2Fsubdir-status-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-472/Manishearth/subdir-status-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/472

Signature · 1 line

gitgitgadget

Manish Goregaokar via GitGitGadget

author

2019-11-23 06:28 UTC #1 lore raw

PATCH 1/1 submodule: Fix 'submodule status' when called from a subdirectory

From: Manish Goregaokar <[email protected]>

Previously, when calling git submodule status while in a subdirectory, it was incorrectly not detecting modified submodules and thus reporting that all of the submodules were unchanged.

This was because the submodule helper was calling diff-index with the submodule path assuming the path was relative to the current prefix directory, however the submodule path used is actually relative to the root.

This fixes the bug by setting the prefix to NULL when running diff-index from the helper, so that it correctly interprets the path as relative to the repository root.

Signed-off-by Manish Goregaokar <[email protected]>

---
 builtin/submodule--helper.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

builtin/submodule--helper.c +4 -1

index 909e77e802..abc5b46d46 100644
@@ -802,7 +802,10 @@ static void status_submodule(const char *path, const struct object_id *ce_oid,
 			 path, NULL);
 
 	git_config(git_diff_basic_config, NULL);
-	repo_init_revisions(the_repository, &rev, prefix);
+	/*
+	 * prefix is NULL since path is an absolute path
+	 */
+	repo_init_revisions(the_repository, &rev, NULL);
 	rev.abbrev = 0;
 	diff_files_args.argc = setup_revisions(diff_files_args.argc,
 					       diff_files_args.argv,

Signature · 1 line

gitgitgadget

Junio C Hamano

2019-11-24 05:17 UTC #2 lore raw

"Manish Goregaokar via GitGitGadget" <[email protected]>

Quoted 2 lines · writes:

From: Manish Goregaokar <[email protected]>
Subject: [PATCH 1/1] submodule: Fix 'submodule status' when called from a subdirectory

At least, downcase 'fix' to match the project convention on commit titles (you can see "git shortlog -n 100 --no-merges", for example).

Quoted 3 lines

Previously, when calling git submodule status while in a subdirectory, it was incorrectly not detecting modified submodules and thus reporting that all of the submodules were unchanged.

We do not have to "Previously, ", as the norm for our log messages is to talk about the status quo (the state before applying your patch) in the current tense. The word used like this is fine,
though: "Previously commit X changed the behaviour of feature A, but it was wrong for such and such reasons; let's address the issue commit X wanted to change by doing this"

Quoted 3 lines

This was because the submodule helper was calling diff-index with the submodule path assuming the path was relative to the current prefix directory, however the submodule path used is actually relative to the root.

OK; modulo tense, nicely explained.

Quoted 3 lines

This fixes the bug by setting the prefix to NULL when running diff-index from the helper, so that it correctly interprets the path as relative to the repository root.

And then we outline the fix as if we are giving an order to the codebase "to be like so", in imperative mood. I.e.

Always pass NULL as the prefix when running the diff-index command inside the submodule, to make sure that we see changes to submodules anywhere in the superproject's tree, not limited to the current directory.

or something like that. By the way, aren't we running diff-files, not diff-index, in this context?

Quoted 4 lines

Signed-off-by Manish Goregaokar <[email protected]>
---
 builtin/submodule--helper.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

This fix should be protected by an additional test or two to make sure others won't break it again.

builtin/submodule--helper.c static void status_submodule(const char *path, const struct object_id *ce_oid, · line 808

Quoted 13 lines

builtin/submodule--helper.c +4 -1

index 909e77e802..abc5b46d46 100644
@@ -802,7 +802,10 @@ static void status_submodule(const char *path, const struct object_id *ce_oid,
 			 path, NULL);
 
 	git_config(git_diff_basic_config, NULL);
-	repo_init_revisions(the_repository, &rev, prefix);
+	/*
+	 * prefix is NULL since path is an absolute path
+	 */
+	repo_init_revisions(the_repository, &rev, NULL);

Absolute??? Isn't 'path' relative to the top-level of the superproject's working tree?

In any case, I do not think this does not deserve a multi-line in-code comment, once we get the code right. Explaining why this must be NULL instead of prefix is better done in the commit log message.

Thanks.

Manish Goregaokar via GitGitGadget

author

2019-11-24 08:01 UTC #3 lore raw

v2 cover submodule: Fix 'submodule status' when called from a subdirectory

In reply to Manish Goregaokar via GitGitGadget

Updated to include a test and a more standard commit message.

Manish Goregaokar (1):
submodule: fix 'submodule status' when called from a subdirectory

 builtin/submodule--helper.c |  3 ++-
 t/t7400-submodule-basic.sh  | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

base-commit: 5fa0f5238b0cd46cfe7f6fa76c3f526ea98148d9
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-472%2FManishearth%2Fsubdir-status-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-472/Manishearth/subdir-status-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/472

Range-diff vs v1:

1: 2004f4aaa9 ! 1: e4c932bd09 submodule: Fix 'submodule status' when called from a subdirectory
@@ -1,18 +1,18 @@
Author: Manish Goregaokar <[email protected]>

- submodule: Fix 'submodule status' when called from a subdirectory
+ submodule: fix 'submodule status' when called from a subdirectory

- Previously, when calling git submodule status while in a
- subdirectory, it was incorrectly not detecting modified submodules and
- thus reporting that all of the submodules were unchanged.
+ When calling git submodule status while in a subdirectory, we are
+ incorrectly not detecting modified submodules and
+ thus reporting that all of the submodules are unchanged.

- This was because the submodule helper was calling diff-index with the
- submodule path assuming the path was relative to the current prefix
+ This is because the submodule helper is calling diff-index with the
+ submodule path assuming the path is relative to the current prefix
directory, however the submodule path used is actually relative to the root.

- This fixes the bug by setting the prefix to NULL when running
- diff-index from the helper, so that it correctly interprets the path
- as relative to the repository root.
+ Always pass NULL as the prefix when running diff-files on the
+ submodule, to make sure the submodule's path is interpreted as relative
+ to the superproject's repository root.

Signed-off-by Manish Goregaokar <[email protected]>

@@ -24,10 +24,38 @@

git_config(git_diff_basic_config, NULL);
- repo_init_revisions(the_repository, &rev, prefix);
-+ /*
-+ * prefix is NULL since path is an absolute path
-+ */
++
+ repo_init_revisions(the_repository, &rev, NULL);
rev.abbrev = 0;
diff_files_args.argc = setup_revisions(diff_files_args.argc,
diff_files_args.argv,
+
+ diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
+ --- a/t/t7400-submodule-basic.sh
+ +++ b/t/t7400-submodule-basic.sh
+@@
+ test_line_count = 1 lines
+ '
+
++test_expect_success 'status from subdirectory should have the same SHA1' '
++ test_when_finished "rmdir addtest/subdir" &&
++ (
++ cd addtest &&
++ git status > /tmp/foo &&
++ git submodule status | awk "{print \$1}" >expected &&
++ mkdir subdir &&
++ cd subdir &&
++ git submodule status | awk "{print \$1}" >../actual &&
++ test_cmp ../expected ../actual &&
++ git -C ../submod checkout @^ &&
++ git submodule status | awk "{print \$1}" >../actual2 &&
++ cd .. &&
++ git submodule status | awk "{print \$1}" >expected2 &&
++ test_cmp actual2 expected2 &&
++ test_must_fail test_cmp actual actual2
++ )
++'
++
+ test_expect_success 'setup - fetch commit name from submodule' '
+ rev1=$(cd .subrepo && git rev-parse HEAD) &&
+ printf "rev1: %s\n" "$rev1" &&

Signature · 1 line

gitgitgadget

Manish Goregaokar via GitGitGadget

author

2019-11-24 08:01 UTC #4 lore raw

PATCH v2 1/1 submodule: fix 'submodule status' when called from a subdirectory

From: Manish Goregaokar <[email protected]>

When calling git submodule status while in a subdirectory, we are incorrectly not detecting modified submodules and
thus reporting that all of the submodules are unchanged.

This is because the submodule helper is calling diff-index with the submodule path assuming the path is relative to the current prefix directory, however the submodule path used is actually relative to the root.

Always pass NULL as the prefix when running diff-files on the submodule, to make sure the submodule's path is interpreted as relative to the superproject's repository root.

Signed-off-by Manish Goregaokar <[email protected]>

---
 builtin/submodule--helper.c |  3 ++-
 t/t7400-submodule-basic.sh  | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

builtin/submodule--helper.c +2 -1

index 909e77e802..eeea8dfa97 100644
@@ -802,7 +802,8 @@ static void status_submodule(const char *path, const struct object_id *ce_oid,
 			 path, NULL);
 
 	git_config(git_diff_basic_config, NULL);
-	repo_init_revisions(the_repository, &rev, prefix);
+
+	repo_init_revisions(the_repository, &rev, NULL);
 	rev.abbrev = 0;
 	diff_files_args.argc = setup_revisions(diff_files_args.argc,
 					       diff_files_args.argv,

t/t7400-submodule-basic.sh +19 -0

index a208cb26e1..4545b47ca4 100755
@@ -356,6 +356,25 @@ test_expect_success 'status should only print one line' '
 	test_line_count = 1 lines
 '
 
+test_expect_success 'status from subdirectory should have the same SHA1' '
+	test_when_finished "rmdir addtest/subdir" &&
+	(
+		cd addtest &&
+		git status > /tmp/foo &&
+		git submodule status | awk "{print \$1}" >expected &&
+		mkdir subdir &&
+		cd subdir &&
+		git submodule status | awk "{print \$1}" >../actual &&
+		test_cmp ../expected ../actual &&
+		git -C ../submod checkout @^ &&
+		git submodule status | awk "{print \$1}" >../actual2 &&
+		cd .. &&
+		git submodule status | awk "{print \$1}" >expected2 &&
+		test_cmp actual2 expected2 &&
+		test_must_fail test_cmp actual actual2
+	)
+'
+
 test_expect_success 'setup - fetch commit name from submodule' '
 	rev1=$(cd .subrepo && git rev-parse HEAD) &&
 	printf "rev1: %s\n" "$rev1" &&

Signature · 1 line

gitgitgadget

Junio C Hamano

2019-11-25 02:02 UTC #5 lore raw

Re: [PATCH v2 1/1] submodule: fix 'submodule status' when called from a subdirectory

"Manish Goregaokar via GitGitGadget" <[email protected]>

Quoted 14 lines · writes:

From: Manish Goregaokar <[email protected]>
When calling git submodule status while in a subdirectory, we are incorrectly not detecting modified submodules and
thus reporting that all of the submodules are unchanged.
This is because the submodule helper is calling diff-index with the submodule path assuming the path is relative to the current prefix directory, however the submodule path used is actually relative to the root.
Always pass NULL as the prefix when running diff-files on the submodule, to make sure the submodule's path is interpreted as relative to the superproject's repository root.

Quoted 5 lines

Signed-off-by Manish Goregaokar <[email protected]>

---
 builtin/submodule--helper.c |  3 ++-
 t/t7400-submodule-basic.sh  | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

Thanks.

Will queue as-is for now, but others may have comments on the patch (and certainly the test part would see a few issues pointed out), which we may want to address before this hits the 'next' and lower branches.

Quoted 5 lines

t/t7400-submodule-basic.sh +0 -0
index a208cb26e1..4545b47ca4 100755

t/t7400-submodule-basic.sh test_expect_success 'status should only print one line' ' · line 364

Quoted 9 lines

t/t7400-submodule-basic.sh +5 -0

@@ -356,6 +356,25 @@ test_expect_success 'status should only print one line' '
 	test_line_count = 1 lines
 '
 
+test_expect_success 'status from subdirectory should have the same SHA1' '
+	test_when_finished "rmdir addtest/subdir" &&
+	(
+		cd addtest &&
 
+		git status > /tmp/foo &&

I think that you added this line for debugging the test; because what it does has no effect on anything in the test, let's remove it.

t/t7400-submodule-basic.sh test_expect_success 'status should only print one line' ' · line 364

Quoted 1 line

t/t7400-submodule-basic.sh +1 -0
+		git submodule status | awk "{print \$1}" >expected &&

This construct to have "git submodule status" on the left hand side of a pipe hides its exit status. We wouldn't notice even if it crashes with a segfault, which is bad especially if it does so after showing the output we expect. This instance is doubly bad because the output is not even compared against a known-good copy. In fact, this is to create a known-good copy, so if "git submodule status" gets broken so badly that it crashes even before emitting anything, we would get an empty "expected" file (by the way, we tend to compare 'expect' and 'actual', not 'expected' and 'actual', especially in our newer tests) here, which would be compared with outputs from other invocations of "git submodule status" later in the test. If "git ubmodule status" is so broken that it crashes immediately, these later invocations would die without showing any output, so all the actual* files would also be empty and out test_cmp would be very happy to report that all tests are good.

Not so good.

	git submodule status >output &&
	sed -e "s/ .*// >expect &&

perhaps?

t/t7400-submodule-basic.sh test_expect_success 'status should only print one line' ' · line 365

Quoted 1 line

t/t7400-submodule-basic.sh +1 -0
+		mkdir subdir &&

If the test fails before reaching this line for whatever reason, addtest/subdir directory won't exist, and test-when-finished would not be so happy.

t/t7400-submodule-basic.sh test_expect_success 'status should only print one line' ' · line 369

Quoted 4 lines

t/t7400-submodule-basic.sh +4 -0

+		cd subdir &&
+		git submodule status | awk "{print \$1}" >../actual &&
+		test_cmp ../expected ../actual &&
+		git -C ../submod checkout @^ &&

Gahh. Please stick to human language HEAD^ and avoid line noise @^.

t/t7400-submodule-basic.sh test_expect_success 'status should only print one line' ' · line 374

Quoted 5 lines

t/t7400-submodule-basic.sh +5 -0

+		git submodule status | awk "{print \$1}" >../actual2 &&
+		cd .. &&
+		git submodule status | awk "{print \$1}" >expected2 &&
+		test_cmp actual2 expected2 &&
+		test_must_fail test_cmp actual actual2

Please do not apply test_must_fail to anything but "git subcmd". For now,

! test_cmp actual actual2

is a safer alternative. Right now we are cooking a topic to allow us to write it as

test_cmp ! actual actual2

but it hasn't been merged to 'master' yet.

Quoted 6 lines

t/t7400-submodule-basic.sh +3 -0

+	)
+'
+
 test_expect_success 'setup - fetch commit name from submodule' '
 	rev1=$(cd .subrepo && git rev-parse HEAD) &&

printf "rev1: %s\n" "$rev1" &&

Manish Goregaokar

2019-11-25 04:11 UTC #6 lore raw

Re: [PATCH v2 1/1] submodule: fix 'submodule status' when called from a subdirectory

I'll make the changes and submit, thanks.

Quoted 1 line

sed -e "s/ .*// >expect &&

This won't work since the output may start with a space, but I'll do this as a two-step thing with awk

-Manish Goregaokar

Quoted 111 lines · Junio C Hamano

"Manish Goregaokar via GitGitGadget" <[email protected]>
Quoted 14 lines · writes:
From: Manish Goregaokar <[email protected]>
When calling git submodule status while in a subdirectory, we are incorrectly not detecting modified submodules and
thus reporting that all of the submodules are unchanged.
This is because the submodule helper is calling diff-index with the submodule path assuming the path is relative to the current prefix directory, however the submodule path used is actually relative to the root.
Always pass NULL as the prefix when running diff-files on the submodule, to make sure the submodule's path is interpreted as relative to the superproject's repository root.
Quoted 5 lines
Signed-off-by Manish Goregaokar <[email protected]>
---
 builtin/submodule--helper.c |  3 ++-
 t/t7400-submodule-basic.sh  | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)
Thanks.
Will queue as-is for now, but others may have comments on the patch (and certainly the test part would see a few issues pointed out), which we may want to address before this hits the 'next' and lower branches.
Quoted 5 lines
t/t7400-submodule-basic.sh +0 -0
index a208cb26e1..4545b47ca4 100755
t/t7400-submodule-basic.sh test_expect_success 'status should only print one line' ' · line 364
Quoted 9 lines
t/t7400-submodule-basic.sh +5 -0
@@ -356,6 +356,25 @@ test_expect_success 'status should only print one line' '
      test_line_count = 1 lines
 '
 
+test_expect_success 'status from subdirectory should have the same SHA1' '
+     test_when_finished "rmdir addtest/subdir" &&
+     (
+             cd addtest &&
 
+             git status > /tmp/foo &&
I think that you added this line for debugging the test; because what it does has no effect on anything in the test, let's remove it.
t/t7400-submodule-basic.sh test_expect_success 'status should only print one line' ' · line 364
Quoted 1 line
t/t7400-submodule-basic.sh +1 -0
+             git submodule status | awk "{print \$1}" >expected &&
This construct to have "git submodule status" on the left hand side of a pipe hides its exit status. We wouldn't notice even if it crashes with a segfault, which is bad especially if it does so after showing the output we expect. This instance is doubly bad because the output is not even compared against a known-good copy. In fact, this is to create a known-good copy, so if "git submodule status" gets broken so badly that it crashes even before emitting anything, we would get an empty "expected" file (by the way, we tend to compare 'expect' and 'actual', not 'expected' and 'actual', especially in our newer tests) here, which would be compared with outputs from other invocations of "git submodule status" later in the test. If "git ubmodule status" is so broken that it crashes immediately, these later invocations would die without showing any output, so all the actual* files would also be empty and out test_cmp would be very happy to report that all tests are good.
Not so good.
        git submodule status >output &&
        sed -e "s/ .*// >expect &&
perhaps?
t/t7400-submodule-basic.sh test_expect_success 'status should only print one line' ' · line 365
Quoted 1 line
t/t7400-submodule-basic.sh +1 -0
+             mkdir subdir &&
If the test fails before reaching this line for whatever reason, addtest/subdir directory won't exist, and test-when-finished would not be so happy.
t/t7400-submodule-basic.sh test_expect_success 'status should only print one line' ' · line 369
Quoted 4 lines
t/t7400-submodule-basic.sh +4 -0
+             cd subdir &&
+             git submodule status | awk "{print \$1}" >../actual &&
+             test_cmp ../expected ../actual &&
+             git -C ../submod checkout @^ &&
Gahh. Please stick to human language HEAD^ and avoid line noise @^.
t/t7400-submodule-basic.sh test_expect_success 'status should only print one line' ' · line 374
Quoted 5 lines
t/t7400-submodule-basic.sh +5 -0
+             git submodule status | awk "{print \$1}" >../actual2 &&
+             cd .. &&
+             git submodule status | awk "{print \$1}" >expected2 &&
+             test_cmp actual2 expected2 &&
+             test_must_fail test_cmp actual actual2
Please do not apply test_must_fail to anything but "git subcmd". For now,
! test_cmp actual actual2
is a safer alternative. Right now we are cooking a topic to allow us to write it as
test_cmp ! actual actual2
but it hasn't been merged to 'master' yet.
Quoted 6 lines
t/t7400-submodule-basic.sh +3 -0
+     )
+'
+
 test_expect_success 'setup - fetch commit name from submodule' '
      rev1=$(cd .subrepo && git rev-parse HEAD) &&
      printf "rev1: %s\n" "$rev1" &&

Manish Goregaokar via GitGitGadget

author

2019-11-25 04:15 UTC #7 lore raw

v3 cover submodule: Fix 'submodule status' when called from a subdirectory

In reply to Manish Goregaokar via GitGitGadget

Updated with an improved test.

Manish Goregaokar (1):
submodule: fix 'submodule status' when called from a subdirectory

 builtin/submodule--helper.c |  3 ++-
 t/t7400-submodule-basic.sh  | 22 ++++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

base-commit: 5fa0f5238b0cd46cfe7f6fa76c3f526ea98148d9
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-472%2FManishearth%2Fsubdir-status-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-472/Manishearth/subdir-status-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/472

Range-diff vs v2:

1: e4c932bd09 ! 1: 8dcea6b399 submodule: fix 'submodule status' when called from a subdirectory

     @@ -41,18 +41,21 @@
      +	test_when_finished "rmdir addtest/subdir" &&
      +	(
      +		cd addtest &&
     -+		git status > /tmp/foo &&
     -+		git submodule status | awk "{print \$1}" >expected &&
      +		mkdir subdir &&
     ++		git submodule status >output &&
     ++		awk "{print \$1}" <output >expect &&
      +		cd subdir &&
     -+		git submodule status | awk "{print \$1}" >../actual &&
     -+		test_cmp ../expected ../actual &&
     -+		git -C ../submod checkout @^ &&
     -+		git submodule status | awk "{print \$1}" >../actual2 &&
     ++		git submodule status >../output &&
     ++		awk "{print \$1}" <../output >../actual &&
     ++		test_cmp ../expect ../actual &&
     ++		git -C ../submod checkout HEAD^ &&
     ++		git submodule status >../output &&
     ++		awk "{print \$1}" <../output >../actual2 &&
      +		cd .. &&
     -+		git submodule status | awk "{print \$1}" >expected2 &&
     -+		test_cmp actual2 expected2 &&
     -+		test_must_fail test_cmp actual actual2
     ++		git submodule status >output &&
     ++		awk "{print \$1}" <output >expect2 &&
     ++		test_cmp actual2 expect2 &&
     ++		! test_cmp actual actual2
      +	)
      +'
      +

Signature · 1 line

gitgitgadget

Manish Goregaokar via GitGitGadget

author

2019-11-25 04:15 UTC #8 lore raw

PATCH v3 1/1 submodule: fix 'submodule status' when called from a subdirectory

From: Manish Goregaokar <[email protected]>

When calling git submodule status while in a subdirectory, we are incorrectly not detecting modified submodules and
thus reporting that all of the submodules are unchanged.

Always pass NULL as the prefix when running diff-files on the submodule, to make sure the submodule's path is interpreted as relative to the superproject's repository root.

Signed-off-by Manish Goregaokar <[email protected]>

---
 builtin/submodule--helper.c |  3 ++-
 t/t7400-submodule-basic.sh  | 22 ++++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

builtin/submodule--helper.c +2 -1

index 909e77e802..eeea8dfa97 100644
@@ -802,7 +802,8 @@ static void status_submodule(const char *path, const struct object_id *ce_oid,
 			 path, NULL);
 
 	git_config(git_diff_basic_config, NULL);
-	repo_init_revisions(the_repository, &rev, prefix);
+
+	repo_init_revisions(the_repository, &rev, NULL);
 	rev.abbrev = 0;
 	diff_files_args.argc = setup_revisions(diff_files_args.argc,
 					       diff_files_args.argv,

t/t7400-submodule-basic.sh +22 -0

index a208cb26e1..09d8408f88 100755
@@ -356,6 +356,28 @@ test_expect_success 'status should only print one line' '
 	test_line_count = 1 lines
 '
 
+test_expect_success 'status from subdirectory should have the same SHA1' '
+	test_when_finished "rmdir addtest/subdir" &&
+	(
+		cd addtest &&
+		mkdir subdir &&
+		git submodule status >output &&
+		awk "{print \$1}" <output >expect &&
+		cd subdir &&
+		git submodule status >../output &&
+		awk "{print \$1}" <../output >../actual &&
+		test_cmp ../expect ../actual &&
+		git -C ../submod checkout HEAD^ &&
+		git submodule status >../output &&
+		awk "{print \$1}" <../output >../actual2 &&
+		cd .. &&
+		git submodule status >output &&
+		awk "{print \$1}" <output >expect2 &&
+		test_cmp actual2 expect2 &&
+		! test_cmp actual actual2
+	)
+'
+
 test_expect_success 'setup - fetch commit name from submodule' '
 	rev1=$(cd .subrepo && git rev-parse HEAD) &&
 	printf "rev1: %s\n" "$rev1" &&

Signature · 1 line

gitgitgadget

Junio C Hamano

2019-11-25 04:51 UTC #9 lore raw

Re: [PATCH v3 1/1] submodule: fix 'submodule status' when called from a subdirectory

"Manish Goregaokar via GitGitGadget" <[email protected]>

t/t7400-submodule-basic.sh test_expect_success 'status should only print one line' ' · line 369

Quoted 1 line · writes:

t/t7400-submodule-basic.sh +1 -0
+		test_cmp ../expect ../actual &&

Note that "test_cmp expect actual" highlights a breakage, when it finds one, by showing a diff that shows "what we saw is different from what we expected to see in this way". So this invocation is good.

t/t7400-submodule-basic.sh test_expect_success 'status should only print one line' ' · line 376

Quoted 7 lines

t/t7400-submodule-basic.sh +7 -0

+		git -C ../submod checkout HEAD^ &&
+		git submodule status >../output &&
+		awk "{print \$1}" <../output >../actual2 &&
+		cd .. &&
+		git submodule status >output &&
+		awk "{print \$1}" <output >expect2 &&
+		test_cmp actual2 expect2 &&

But this one is probably the other way around. I'll fix this up while queuing, so if there is no other changes necessary, there is no need to resend the patch.

t/t7400-submodule-basic.sh test_expect_success 'status should only print one line' ' · line 377

Quoted 1 line

t/t7400-submodule-basic.sh +1 -0
+		! test_cmp actual actual2

And the order of this one actually does not matter ;-)

t/t7400-submodule-basic.sh test_expect_success 'status should only print one line' ' · line 379

Quoted 2 lines

t/t7400-submodule-basic.sh +2 -0
+	)
+'

Thanks.

Manish Goregaokar

2019-11-25 06:32 UTC #10 lore raw

Re: [PATCH v3 1/1] submodule: fix 'submodule status' when called from a subdirectory

Thank you!
-Manish Goregaokar

Quoted 31 lines · Junio C Hamano

"Manish Goregaokar via GitGitGadget" <[email protected]>
Quoted 1 line · writes:
+ test_cmp ../expect ../actual &&
Note that "test_cmp expect actual" highlights a breakage, when it finds one, by showing a diff that shows "what we saw is different from what we expected to see in this way". So this invocation is good.
t/t7400-submodule-basic.sh test_expect_success 'status should only print one line' ' · line 376
Quoted 7 lines
t/t7400-submodule-basic.sh +7 -0
+             git -C ../submod checkout HEAD^ &&
+             git submodule status >../output &&
+             awk "{print \$1}" <../output >../actual2 &&
+             cd .. &&
+             git submodule status >output &&
+             awk "{print \$1}" <output >expect2 &&
+             test_cmp actual2 expect2 &&
But this one is probably the other way around. I'll fix this up while queuing, so if there is no other changes necessary, there is no need to resend the patch.
Quoted 1 line
+ ! test_cmp actual actual2
And the order of this one actually does not matter ;-)
Quoted 2 lines
+ )
+'
Thanks.