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

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

[Manish Goregaokar via GitGitGadget]

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 manishsmail@gmail.com
[manishsmail@gmail.com]

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
-- 
gitgitgadget

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

[Manish Goregaokar via GitGitGadget]

From: Manish Goregaokar <manishsmail@gmail.com>

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 <manishsmail@gmail.com>
---
 builtin/submodule--helper.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 909e77e802..abc5b46d46 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -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,
-- 
gitgitgadget

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

[Junio C Hamano]

"Manish Goregaokar via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Manish Goregaokar <manishsmail@gmail.com>
> 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).

> 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"

> 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.

> 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?

> Signed-off-by: Manish Goregaokar <manishsmail@gmail.com>
> ---
>  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.

> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 909e77e802..abc5b46d46 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -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.

[PATCH v2 0/1] submodule: Fix 'submodule status' when called from a subdirectory

[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 <manishsmail@gmail.com>
      
     -    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 <manishsmail@gmail.com>
      
     @@ -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" &&

-- 
gitgitgadget

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

[Manish Goregaokar via GitGitGadget]

From: Manish Goregaokar <manishsmail@gmail.com>

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 <manishsmail@gmail.com>
---
 builtin/submodule--helper.c |  3 ++-
 t/t7400-submodule-basic.sh  | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 909e77e802..eeea8dfa97 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -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,
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index a208cb26e1..4545b47ca4 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -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" &&
-- 
gitgitgadget

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

[Junio C Hamano]

"Manish Goregaokar via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Manish Goregaokar <manishsmail@gmail.com>
>
> 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 <manishsmail@gmail.com>
> ---
>  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.

> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index a208cb26e1..4545b47ca4 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -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.

> +		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?

> +		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.

> +		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 @^.

> +		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. 

> +	)
> +'
> +
>  test_expect_success 'setup - fetch commit name from submodule' '
>  	rev1=$(cd .subrepo && git rev-parse HEAD) &&
>  	printf "rev1: %s\n" "$rev1" &&

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

[Manish Goregaokar]

I'll make the changes and submit, thanks.

> 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

On Sun, Nov 24, 2019 at 5:56 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Manish Goregaokar via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > From: Manish Goregaokar <manishsmail@gmail.com>
> >
> > 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 <manishsmail@gmail.com>
> > ---
> >  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.
>
> > diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> > index a208cb26e1..4545b47ca4 100755
> > --- a/t/t7400-submodule-basic.sh
> > +++ b/t/t7400-submodule-basic.sh
> > @@ -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.
>
> > +             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?
>
> > +             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.
>
> > +             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 @^.
>
> > +             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.
>
> > +     )
> > +'
> > +
> >  test_expect_success 'setup - fetch commit name from submodule' '
> >       rev1=$(cd .subrepo && git rev-parse HEAD) &&
> >       printf "rev1: %s\n" "$rev1" &&

[PATCH v3 0/1] submodule: Fix 'submodule status' when called from a subdirectory

[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
      +	)
      +'
      +

-- 
gitgitgadget

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

[Manish Goregaokar via GitGitGadget]

From: Manish Goregaokar <manishsmail@gmail.com>

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 <manishsmail@gmail.com>
---
 builtin/submodule--helper.c |  3 ++-
 t/t7400-submodule-basic.sh  | 22 ++++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 909e77e802..eeea8dfa97 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -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,
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index a208cb26e1..09d8408f88 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -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" &&
-- 
gitgitgadget

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

[Junio C Hamano]

"Manish Goregaokar via GitGitGadget" <gitgitgadget@gmail.com>
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.

> +		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.

> +		! test_cmp actual actual2

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

> +	)
> +'

Thanks.

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

[Manish Goregaokar]

Thank you!
-Manish Goregaokar

On Sun, Nov 24, 2019 at 8:51 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Manish Goregaokar via GitGitGadget" <gitgitgadget@gmail.com>
> 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.
>
> > +             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.
>
> > +             ! test_cmp actual actual2
>
> And the order of this one actually does not matter ;-)
>
> > +     )
> > +'
>
> Thanks.