Re: [PATCH] status: do not get confused by submodules in excluded directories
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.
Re: [PATCH] status: do not get confused by submodules in excluded directories
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
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
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 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
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
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
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