| Message ID | 20221127145130.16155-4-worldhello.net@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1,1/4] t1301: fix wrong template dir for git-init | expand |
Jiang Xin <worldhello.net@gmail.com> writes: > From: Jiang Xin <zhiyou.jx@alibaba-inc.com> > > In test case "shared=all", the working directory is permanently changed > to the "sub" directory. This leads to a strange behavior that the > temporary repositories created by subsequent test cases are all in this > "sub" directory, such as "sub/new", "sub/child.git". If we bypass this > test case, all subsequent test cases will have different working > directory. > > Since the test case "shared=all" and all subsequent will work properly > in the default test repository, we don't need to create and change to > the "sub" directory in the test case "shared=all". It is much worse than that. If existing tests after this step were running destructive operations in their "..", because we have this extra "sub" directory and such a destructive test were running there, the damage would have been contained in $TRASH_DIRECTORY but with this change, it will touch t/ (or the parent directory of the $TRASH_DIRECTORY). So, "will work properly" may not be sufficient; we need to audit the rest of the script and make sure there is no such funny "step outside the test enviromnent" happening before we are sure that this is a "safe" change. It is the "right" thing to do, though. I do not see any reason why we want to have an extra level "sub" directory there. > > Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> > --- > t/t1301-shared-repo.sh | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh > index 3ca91bf504..c4f2f72f6b 100755 > --- a/t/t1301-shared-repo.sh > +++ b/t/t1301-shared-repo.sh > @@ -46,8 +46,6 @@ do > done > > test_expect_success 'shared=all' ' > - mkdir sub && > - cd sub && > git init --template= --shared=all && > test 2 = $(git config core.sharedrepository) > '
On Mon, Nov 28, 2022 at 12:41 PM Junio C Hamano <gitster@pobox.com> wrote: > > Jiang Xin <worldhello.net@gmail.com> writes: > > > From: Jiang Xin <zhiyou.jx@alibaba-inc.com> > > > > In test case "shared=all", the working directory is permanently changed > > to the "sub" directory. This leads to a strange behavior that the > > temporary repositories created by subsequent test cases are all in this > > "sub" directory, such as "sub/new", "sub/child.git". If we bypass this > > test case, all subsequent test cases will have different working > > directory. > > > > Since the test case "shared=all" and all subsequent will work properly > > in the default test repository, we don't need to create and change to > > the "sub" directory in the test case "shared=all". > > It is much worse than that. If existing tests after this step were > running destructive operations in their "..", because we have this > extra "sub" directory and such a destructive test were running > there, the damage would have been contained in $TRASH_DIRECTORY but > with this change, it will touch t/ (or the parent directory of the > $TRASH_DIRECTORY). So, "will work properly" may not be sufficient; > we need to audit the rest of the script and make sure there is no > such funny "step outside the test enviromnent" happening before we > are sure that this is a "safe" change. No such funny operations in the follow test cases, but will add some notes in commit log. Thanks. -- Jiang Xin > > It is the "right" thing to do, though. I do not see any reason why > we want to have an extra level "sub" directory there.
diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh index 3ca91bf504..c4f2f72f6b 100755 --- a/t/t1301-shared-repo.sh +++ b/t/t1301-shared-repo.sh @@ -46,8 +46,6 @@ do done test_expect_success 'shared=all' ' - mkdir sub && - cd sub && git init --template= --shared=all && test 2 = $(git config core.sharedrepository) '