Re: [PATCH] status: do not get confused by submodules in excluded directories

[Johannes Schindelin]

Hi,

On Wed, 25 Oct 2017, Heiko Voigt wrote:

> On Wed, Oct 25, 2017 at 10:28:25AM +0900, Junio C Hamano wrote:
> > Heiko Voigt  writes:
> > 
> > > On Tue, Oct 24, 2017 at 02:18:49PM +0900, Junio C Hamano wrote:
> > >> Johannes Schindelin  writes:
> > >> 
> > >> > We meticulously pass the `exclude` flag to the `treat_directory()`
> > >> > function so that we can indicate that files in it are excluded rather
> > >> > than untracked when recursing.
> > >> >
> > >> > But we did not yet treat submodules the same way.
> > >> 
> > >> ... "because of that, we ended up showing < > >> what situation>>" would be a nice thing to have here, so that it can
> > >> be copied to the release notes for the bugfix.  
> > >
> > > Yes I agree that would be nice here. It was not immediately obvious that
> > > this only applies when using both flags: -u and --ignored.
> > 
> > Does any of you care to fill in the <> then? ;-)
> 
> How about:
> 
> Because of that, we ended up showing the submodule as untracked and its
> content as ignored files when using the --ignored and -u flags with git
> status.
> 
> ? But maybe Dscho also has some more information to add about his
> situation?

He has... as part of v2, a substantially more detailed commit message will
reach your inbox Real Soon Now.

Ciao,
Dscho

Re: [PATCH] status: do not get confused by submodules in excluded directories

[Heiko Voigt]

On Wed, Oct 25, 2017 at 10:28:25AM +0900, Junio C Hamano wrote:
> Heiko Voigt  writes:
> 
> > On Tue, Oct 24, 2017 at 02:18:49PM +0900, Junio C Hamano wrote:
> >> Johannes Schindelin  writes:
> >> 
> >> > We meticulously pass the `exclude` flag to the `treat_directory()`
> >> > function so that we can indicate that files in it are excluded rather
> >> > than untracked when recursing.
> >> >
> >> > But we did not yet treat submodules the same way.
> >> 
> >> ... "because of that, we ended up showing < >> what situation>>" would be a nice thing to have here, so that it can
> >> be copied to the release notes for the bugfix.  
> >
> > Yes I agree that would be nice here. It was not immediately obvious that
> > this only applies when using both flags: -u and --ignored.
> 
> Does any of you care to fill in the <> then? ;-)

How about:

Because of that, we ended up showing the submodule as untracked and its
content as ignored files when using the --ignored and -u flags with git
status.

? But maybe Dscho also has some more information to add about his
situation?

Cheers Heiko

Re: [PATCH] status: do not get confused by submodules in excluded directories

[Johannes Schindelin]

Hi Kevin,

On Tue, 24 Oct 2017, Kevin Daudt wrote:

> On Tue, Oct 17, 2017 at 03:10:11PM +0200, Johannes Schindelin wrote:
> > diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh
> > index fc6013ba3c8..8c849a4cd2f 100755
> > --- a/t/t7061-wtstatus-ignore.sh
> > +++ b/t/t7061-wtstatus-ignore.sh
> > @@ -272,4 +272,18 @@ test_expect_success 'status ignored tracked directory 
> > with uncommitted file in t
> > test_cmp expected actual
> >  '
> >  
> > +cat >expected <<\EOF
> > +!! tracked/submodule/
> > +EOF
> > +
> > +test_expect_success 'status ignores submodule in excluded directory' '
> > +   git init tracked/submodule &&
> > +   (
> > +   cd tracked/submodule &&
> > +   test_commit initial
> > +   ) &&
> 
> Could this use test_commit -C tracked/submodule initial?

Yes! Thanks. For some reason, I did not even think that test_commit would
accept the -C option.

Ciao,
Dscho

Re: [PATCH] status: do not get confused by submodules in excluded directories

[Junio C Hamano]

Heiko Voigt  writes:

> On Tue, Oct 24, 2017 at 02:18:49PM +0900, Junio C Hamano wrote:
>> Johannes Schindelin  writes:
>> 
>> > We meticulously pass the `exclude` flag to the `treat_directory()`
>> > function so that we can indicate that files in it are excluded rather
>> > than untracked when recursing.
>> >
>> > But we did not yet treat submodules the same way.
>> 
>> ... "because of that, we ended up showing <> what situation>>" would be a nice thing to have here, so that it can
>> be copied to the release notes for the bugfix.  
>
> Yes I agree that would be nice here. It was not immediately obvious that
> this only applies when using both flags: -u and --ignored.

Does any of you care to fill in the <> then? ;-)

> Looks good to me.
>
> Cheers Heiko

Re: [PATCH] status: do not get confused by submodules in excluded directories

[Stefan Beller]

On Tue, Oct 24, 2017 at 5:15 AM, Heiko Voigt  wrote:

> Looks good to me.

Same here,

Thanks,
Stefan

Re: [PATCH] status: do not get confused by submodules in excluded directories

[Heiko Voigt]

On Tue, Oct 24, 2017 at 02:18:49PM +0900, Junio C Hamano wrote:
> Johannes Schindelin  writes:
> 
> > We meticulously pass the `exclude` flag to the `treat_directory()`
> > function so that we can indicate that files in it are excluded rather
> > than untracked when recursing.
> >
> > But we did not yet treat submodules the same way.
> 
> ... "because of that, we ended up showing < what situation>>" would be a nice thing to have here, so that it can
> be copied to the release notes for the bugfix.  

Yes I agree that would be nice here. It was not immediately obvious that
this only applies when using both flags: -u and --ignored.

Seems to be a corner that not many people are using. At first I thought
a plain 'git status' would show that behavior...

> How far back a release do we want to make this fix applicable?  It
> seems that it applies cleanly to maint-2.13 without breaking from my
> quick test, so that is probably where I'll queue this, even though
> we may no longer issue further maintenance releases on that track.
> 
> Any comment from submodule folks?
> 
> Sorry that I didn't notice this was left unattended by anybody til
> now.  Will queue while waiting for those who are into submodules to
> respond.

Looks good to me.

Cheers Heiko

Re: [PATCH] status: do not get confused by submodules in excluded directories

[Kevin Daudt]

On Tue, Oct 17, 2017 at 03:10:11PM +0200, Johannes Schindelin wrote:
> We meticulously pass the `exclude` flag to the `treat_directory()`
> function so that we can indicate that files in it are excluded rather
> than untracked when recursing.
> 
> But we did not yet treat submodules the same way.
> 
> Signed-off-by: Johannes Schindelin 
> ---
> Published-As: 
> https://github.com/dscho/git/releases/tag/submodule-in-excluded-v1
> Fetch-It-Via: git fetch https://github.com/dscho/git submodule-in-excluded-v1
>  dir.c  |  2 +-
>  t/t7061-wtstatus-ignore.sh | 14 ++
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/dir.c b/dir.c
> index 1d17b800cf3..9987011da57 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -1392,7 +1392,7 @@ static enum path_treatment treat_directory(struct 
> dir_struct *dir,
>   if (!(dir->flags & DIR_NO_GITLINKS)) {
>   unsigned char sha1[20];
>   if (resolve_gitlink_ref(dirname, "HEAD", sha1) == 0)
> - return path_untracked;
> + return exclude ? path_excluded : path_untracked;
>   }
>   return path_recurse;
>   }
> diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh
> index fc6013ba3c8..8c849a4cd2f 100755
> --- a/t/t7061-wtstatus-ignore.sh
> +++ b/t/t7061-wtstatus-ignore.sh
> @@ -272,4 +272,18 @@ test_expect_success 'status ignored tracked directory 
> with uncommitted file in t
>   test_cmp expected actual
>  '
>  
> +cat >expected <<\EOF
> +!! tracked/submodule/
> +EOF
> +
> +test_expect_success 'status ignores submodule in excluded directory' '
> + git init tracked/submodule &&
> + (
> + cd tracked/submodule &&
> + test_commit initial
> + ) &&

Could this use test_commit -C tracked/submodule initial?

> + git status --porcelain --ignored -u tracked/submodule >actual &&
> + test_cmp expected actual
> +'
> +
>  test_done
> 
> base-commit: 111ef79afe185f8731920569450f6a65320f5d5f
> -- 
> 2.14.2.windows.3

Re: [PATCH] status: do not get confused by submodules in excluded directories

[Junio C Hamano]

Johannes Schindelin  writes:

> We meticulously pass the `exclude` flag to the `treat_directory()`
> function so that we can indicate that files in it are excluded rather
> than untracked when recursing.
>
> But we did not yet treat submodules the same way.

... "because of that, we ended up showing <>" would be a nice thing to have here, so that it can
be copied to the release notes for the bugfix.  

How far back a release do we want to make this fix applicable?  It
seems that it applies cleanly to maint-2.13 without breaking from my
quick test, so that is probably where I'll queue this, even though
we may no longer issue further maintenance releases on that track.

Any comment from submodule folks?

Sorry that I didn't notice this was left unattended by anybody til
now.  Will queue while waiting for those who are into submodules to
respond.

Thanks.

[PATCH] status: do not get confused by submodules in excluded directories

[Johannes Schindelin]

We meticulously pass the `exclude` flag to the `treat_directory()`
function so that we can indicate that files in it are excluded rather
than untracked when recursing.

But we did not yet treat submodules the same way.

Signed-off-by: Johannes Schindelin 
---
Published-As: https://github.com/dscho/git/releases/tag/submodule-in-excluded-v1
Fetch-It-Via: git fetch https://github.com/dscho/git submodule-in-excluded-v1
 dir.c  |  2 +-
 t/t7061-wtstatus-ignore.sh | 14 ++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/dir.c b/dir.c
index 1d17b800cf3..9987011da57 100644
--- a/dir.c
+++ b/dir.c
@@ -1392,7 +1392,7 @@ static enum path_treatment treat_directory(struct 
dir_struct *dir,
if (!(dir->flags & DIR_NO_GITLINKS)) {
unsigned char sha1[20];
if (resolve_gitlink_ref(dirname, "HEAD", sha1) == 0)
-   return path_untracked;
+   return exclude ? path_excluded : path_untracked;
}
return path_recurse;
}
diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh
index fc6013ba3c8..8c849a4cd2f 100755
--- a/t/t7061-wtstatus-ignore.sh
+++ b/t/t7061-wtstatus-ignore.sh
@@ -272,4 +272,18 @@ test_expect_success 'status ignored tracked directory with 
uncommitted file in t
test_cmp expected actual
 '
 
+cat >expected <<\EOF
+!! tracked/submodule/
+EOF
+
+test_expect_success 'status ignores submodule in excluded directory' '
+   git init tracked/submodule &&
+   (
+   cd tracked/submodule &&
+   test_commit initial
+   ) &&
+   git status --porcelain --ignored -u tracked/submodule >actual &&
+   test_cmp expected actual
+'
+
 test_done

base-commit: 111ef79afe185f8731920569450f6a65320f5d5f
-- 
2.14.2.windows.3