| Message ID | 20221128130323.8914-4-worldhello.net@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On Mon, Nov 28 2022, Jiang Xin wrote: > 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. > > Besides, all subsequent test cases assuming they are in the "sub" > directory do not run any destructive operations in their parent > directory (".."), and will not make damage out side of $TRASH_DIRECTORY. > > So it is a safe change for us to run the test case "shared=all" in > current repository instead of creating and changing to "sub". > > For the next test case, we no longer run it in the "sub" repository > which is initialized from an empty template, we should not assume the > path ".git/info" is missing. So add option "-p" to mkdir. > > Helped-by: Junio C Hamano <gitster@pobox.com> > Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> > --- > t/t1301-shared-repo.sh | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh > index 1225abbb6d..fd10c139f5 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) > ' > @@ -57,7 +55,7 @@ test_expect_success POSIXPERM 'update-server-info honors core.sharedRepository' > git add a1 && > test_tick && > git commit -m a1 && > - mkdir .git/info && > + mkdir -p .git/info && > umask 0277 && > git update-server-info && > actual="$(ls -l .git/info/refs)" && I think this approach goes against the effort to implicitly stop relying on templates. See 3d3874d537a (Merge branch 'ab/test-without-templates', 2022-07-18) for commits related to that. I think better thing to do here is to squash this in: diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh index 0b3722aa149..b7222b7bc07 100755 --- a/t/t1301-shared-repo.sh +++ b/t/t1301-shared-repo.sh @@ -8,6 +8,7 @@ test_description='Test shared repository initialization' GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +TEST_CREATE_REPO_NO_TEMPLATE=1 . ./test-lib.sh # Remove a default ACL from the test dir if possible. @@ -55,7 +56,7 @@ test_expect_success POSIXPERM 'update-server-info honors core.sharedRepository' git add a1 && test_tick && git commit -m a1 && - mkdir -p .git/info && + mkdir .git/info && umask 0277 && git update-server-info && actual="$(ls -l .git/info/refs)" && I.e. before we'd not reply on the template, as we created a directory manually, but now we're using the standard templated "git init", so AFAICT the first hunk here could be taken, and this could be squashed into the second hunk instead: diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh index 0b3722aa149..d4315b5ef5a 100755 --- a/t/t1301-shared-repo.sh +++ b/t/t1301-shared-repo.sh @@ -55,7 +55,6 @@ test_expect_success POSIXPERM 'update-server-info honors core.sharedRepository' git add a1 && test_tick && git commit -m a1 && - mkdir -p .git/info && umask 0277 && git update-server-info && actual="$(ls -l .git/info/refs)" && I.e. before your change we went from knowing that we're crafting a custom repo, to saying that we're unsure what we're doing by using the "mkdir -p". But can't we just use "TEST_CREATE_REPO_NO_TEMPLATE=1" then, and avoid the "cd sub?"
On Mon, Nov 28, 2022 at 9:23 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > I think this approach goes against the effort to implicitly stop relying > on templates. See 3d3874d537a (Merge branch 'ab/test-without-templates', > 2022-07-18) for commits related to that. As I said in the cover letter, it was the conflict of ".git/info" with our internal reference transactions that drew my attention to this test case. This is because our builtin reference-transaction hook which will automatically create a ".git/info" directory to create some files inside, such as ".git/info/checksum", I have to change "mkdir .git/info" to "mkdir -p .git/info" one by one. And I found in this test case, there is a wrong template dir. > I think better thing to do here is to squash this in: > > diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh > index 0b3722aa149..b7222b7bc07 100755 > --- a/t/t1301-shared-repo.sh > +++ b/t/t1301-shared-repo.sh > @@ -8,6 +8,7 @@ test_description='Test shared repository initialization' > GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main > export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME > > +TEST_CREATE_REPO_NO_TEMPLATE=1 > . ./test-lib.sh Will use this implementation instead. Thanks. -- Jiang Xin
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > I think this approach goes against the effort to implicitly stop relying > on templates. ... which I am perfectly OK with. If you removed something that is essential to the health of the respository from your templates, you deserve the breakage you caused yourself and can keep both halves.
diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh index 1225abbb6d..fd10c139f5 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) ' @@ -57,7 +55,7 @@ test_expect_success POSIXPERM 'update-server-info honors core.sharedRepository' git add a1 && test_tick && git commit -m a1 && - mkdir .git/info && + mkdir -p .git/info && umask 0277 && git update-server-info && actual="$(ls -l .git/info/refs)" &&