{"thread":{"id":"53461","subject":"[PATCH 0/2] CI: Enable t1509 on GitHub Actions and Travis","startedAt":"2020-05-13T13:24:09Z","lastAt":"2020-05-13T23:59:47Z","messageCount":8,"participants":["Đoàn Trần Công Danh","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"397710","messageId":"cover.1589375923.git.congdanhqx@gmail.com","threadId":"53461","inReplyTo":null,"subject":"[PATCH 0/2] CI: Enable t1509 on GitHub Actions and Travis","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-05-13T13:23:05Z","receivedAt":"2020-05-13T13:24:09Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"It's our long desire to enable t1509 in CI system [1]\n\nThis is here to enable it.\n\n[1]: https://lore.kernel.org/git/1FEDCA48-FE77-44C3-8C4A-65B4C435E6B3@gmail.com/\n\nĐoàn Trần Công Danh (2):\n  t1509: correct i18n test\n  CI: enable t1509 in Travis and GitHub Actions\n\n ci/lib.sh                 | 13 +++++++++++++\n ci/run-docker-build.sh    |  3 +++\n t/t1509-root-work-tree.sh |  4 ++--\n 3 files changed, 18 insertions(+), 2 deletions(-)\n\n-- \n2.26.2.672.g232c24e857\n\n"},{"id":"397711","messageId":"6675cb952771a48a50b46bcf577e363fcf5d7b3b.1589375923.git.congdanhqx@gmail.com","threadId":"53461","inReplyTo":"cover.1589375923.git.congdanhqx@gmail.com","subject":"[PATCH 1/2] t1509: correct i18n test","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-05-13T13:23:06Z","receivedAt":"2020-05-13T13:24:09Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"git-init(1)'s messages is subjected to i18n.\nThey should be tested by test_i18n* family.\n\nFix them.\n\nSigned-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n---\n t/t1509-root-work-tree.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t1509-root-work-tree.sh b/t/t1509-root-work-tree.sh\nindex 553a3f601b..fd2f7abf1c 100755\n--- a/t/t1509-root-work-tree.sh\n+++ b/t/t1509-root-work-tree.sh\n@@ -221,7 +221,7 @@ test_expect_success 'setup' '\n \trm -rf /.git &&\n \techo \"Initialized empty Git repository in /.git/\" > expected &&\n \tgit init > result &&\n-\ttest_cmp expected result\n+\ttest_i18ncmp expected result\n '\n \n test_vars 'auto gitdir, root' \".git\" \"/\" \"\"\n@@ -246,7 +246,7 @@ test_expect_success 'setup' '\n \tcd / &&\n \techo \"Initialized empty Git repository in /\" > expected &&\n \tgit init --bare > result &&\n-\ttest_cmp expected result\n+\ttest_i18ncmp expected result\n '\n \n test_vars 'auto gitdir, root' \".\" \"\" \"\"\n-- \n2.26.2.672.g232c24e857\n\n"},{"id":"397712","messageId":"fa2e1f7b44262eac1fa26161fc5d3f3b9b6bdb47.1589375923.git.congdanhqx@gmail.com","threadId":"53461","inReplyTo":"cover.1589375923.git.congdanhqx@gmail.com","subject":"[PATCH 2/2] CI: enable t1509 in Travis and GitHub Actions","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-05-13T13:23:07Z","receivedAt":"2020-05-13T13:24:11Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n---\n ci/lib.sh              | 13 +++++++++++++\n ci/run-docker-build.sh |  3 +++\n 2 files changed, 16 insertions(+)\n\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex dac36886e3..e9c22ae718 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -113,6 +113,7 @@ then\n \texport GIT_PROVE_OPTS=\"--timer --jobs 3 --state=failed,slow,save\"\n \texport GIT_TEST_OPTS=\"--verbose-log -x --immediate\"\n \tMAKEFLAGS=\"$MAKEFLAGS --jobs=2\"\n+\tt1509_allowed=YES\n elif test -n \"$SYSTEM_COLLECTIONURI\" || test -n \"$SYSTEM_TASKDEFINITIONSURI\"\n then\n \tCI_TYPE=azure-pipelines\n@@ -162,6 +163,7 @@ then\n \techo \"::add-matcher::ci/git-problem-matcher.json\"\n \ttest linux-musl = \"$jobname\" ||\n \tMAKEFLAGS=\"$MAKEFLAGS TEST_SHELL_PATH=/bin/sh\"\n+\tt1509_allowed=YES\n else\n \techo \"Could not identify CI type\" >&2\n \tenv >&2\n@@ -184,6 +186,17 @@ export DEVELOPER=1\n export DEFAULT_TEST_TARGET=prove\n export GIT_TEST_CLONE_2GB=true\n \n+if test \"$t1509_allowed\" = YES\n+then\n+\tcase \"$jobname\" in\n+\tosx-*) ;;\n+\t*)\n+\t\tchmod a+w / || sudo chmod a+w / || true\n+\t\texport IKNOWWHATIAMDOING=YES\n+\t\t;;\n+\tesac\n+fi\n+\n case \"$jobname\" in\n linux-clang|linux-gcc)\n \tif [ \"$jobname\" = linux-gcc ]\ndiff --git a/ci/run-docker-build.sh b/ci/run-docker-build.sh\nindex 8d47a5fda3..026afe275a 100755\n--- a/ci/run-docker-build.sh\n+++ b/ci/run-docker-build.sh\n@@ -58,6 +58,8 @@ else\n \ttest -n \"$cache_dir\" && chown -R $HOST_UID:$HOST_UID \"$cache_dir\"\n fi\n \n+chmod a+w /\n+\n # Build and test\n command $switch_cmd su -m -l $CI_USER -c \"\n \tset -ex\n@@ -68,6 +70,7 @@ command $switch_cmd su -m -l $CI_USER -c \"\n \texport GIT_TEST_CLONE_2GB='$GIT_TEST_CLONE_2GB'\n \texport MAKEFLAGS='$MAKEFLAGS'\n \texport cache_dir='$cache_dir'\n+\texport IKNOWWHATIAMDOING=YES\n \tcd /usr/src/git\n \ttest -n '$cache_dir' && ln -s '$cache_dir/.prove' t/.prove\n \tmake\n-- \n2.26.2.672.g232c24e857\n\n"},{"id":"397721","messageId":"xmqqh7wjvlbm.fsf@gitster.c.googlers.com","threadId":"53461","inReplyTo":"cover.1589375923.git.congdanhqx@gmail.com","subject":"Re: [PATCH 0/2] CI: Enable t1509 on GitHub Actions and Travis","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-13T16:41:49Z","receivedAt":"2020-05-13T16:41:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:\n\n> It's our long desire to enable t1509 in CI system [1]\n>\n> This is here to enable it.\n\nYuck.\n\nIt's not like having a single repository at /.git that govern\neverything on the machine is a usecase we care about, but if it is\neasily ported with just two patches, and the task has already been\ndone, then why not?  ;-)\n\n\n\n\n>\n> [1]: https://lore.kernel.org/git/1FEDCA48-FE77-44C3-8C4A-65B4C435E6B3@gmail.com/\n>\n> Đoàn Trần Công Danh (2):\n>   t1509: correct i18n test\n>   CI: enable t1509 in Travis and GitHub Actions\n>\n>  ci/lib.sh                 | 13 +++++++++++++\n>  ci/run-docker-build.sh    |  3 +++\n>  t/t1509-root-work-tree.sh |  4 ++--\n>  3 files changed, 18 insertions(+), 2 deletions(-)\n"},{"id":"397722","messageId":"xmqqd077vl9c.fsf@gitster.c.googlers.com","threadId":"53461","inReplyTo":"6675cb952771a48a50b46bcf577e363fcf5d7b3b.1589375923.git.congdanhqx@gmail.com","subject":"Re: [PATCH 1/2] t1509: correct i18n test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-13T16:43:11Z","receivedAt":"2020-05-13T16:43:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:\n\n> git-init(1)'s messages is subjected to i18n.\n> They should be tested by test_i18n* family.\n>\n> Fix them.\n>\n> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n> ---\n>  t/t1509-root-work-tree.sh | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t1509-root-work-tree.sh b/t/t1509-root-work-tree.sh\n> index 553a3f601b..fd2f7abf1c 100755\n> --- a/t/t1509-root-work-tree.sh\n> +++ b/t/t1509-root-work-tree.sh\n> @@ -221,7 +221,7 @@ test_expect_success 'setup' '\n>  \trm -rf /.git &&\n>  \techo \"Initialized empty Git repository in /.git/\" > expected &&\n>  \tgit init > result &&\n> -\ttest_cmp expected result\n> +\ttest_i18ncmp expected result\n>  '\n>  \n>  test_vars 'auto gitdir, root' \".git\" \"/\" \"\"\n> @@ -246,7 +246,7 @@ test_expect_success 'setup' '\n>  \tcd / &&\n>  \techo \"Initialized empty Git repository in /\" > expected &&\n>  \tgit init --bare > result &&\n> -\ttest_cmp expected result\n> +\ttest_i18ncmp expected result\n>  '\n>  \n>  test_vars 'auto gitdir, root' \".\" \"\" \"\"\n\nOK.  It shows how rarely (if ever) this test script is actually\nexercised that we need fixes like these.  Well done.\n\nThanks.\n"},{"id":"397723","messageId":"xmqq8shvvkur.fsf@gitster.c.googlers.com","threadId":"53461","inReplyTo":"fa2e1f7b44262eac1fa26161fc5d3f3b9b6bdb47.1589375923.git.congdanhqx@gmail.com","subject":"Re: [PATCH 2/2] CI: enable t1509 in Travis and GitHub Actions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-13T16:51:56Z","receivedAt":"2020-05-13T16:52:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:\n\n> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n> ---\n>  ci/lib.sh              | 13 +++++++++++++\n>  ci/run-docker-build.sh |  3 +++\n>  2 files changed, 16 insertions(+)\n>\n> diff --git a/ci/lib.sh b/ci/lib.sh\n> index dac36886e3..e9c22ae718 100755\n> --- a/ci/lib.sh\n> +++ b/ci/lib.sh\n> @@ -113,6 +113,7 @@ then\n>  \texport GIT_PROVE_OPTS=\"--timer --jobs 3 --state=failed,slow,save\"\n>  \texport GIT_TEST_OPTS=\"--verbose-log -x --immediate\"\n>  \tMAKEFLAGS=\"$MAKEFLAGS --jobs=2\"\n> +\tt1509_allowed=YES\n>  elif test -n \"$SYSTEM_COLLECTIONURI\" || test -n \"$SYSTEM_TASKDEFINITIONSURI\"\n>  then\n>  \tCI_TYPE=azure-pipelines\n> @@ -162,6 +163,7 @@ then\n>  \techo \"::add-matcher::ci/git-problem-matcher.json\"\n>  \ttest linux-musl = \"$jobname\" ||\n>  \tMAKEFLAGS=\"$MAKEFLAGS TEST_SHELL_PATH=/bin/sh\"\n> +\tt1509_allowed=YES\n>  else\n>  \techo \"Could not identify CI type\" >&2\n>  \tenv >&2\n> @@ -184,6 +186,17 @@ export DEVELOPER=1\n>  export DEFAULT_TEST_TARGET=prove\n>  export GIT_TEST_CLONE_2GB=true\n>  \n> +if test \"$t1509_allowed\" = YES\n> +then\n> +\tcase \"$jobname\" in\n> +\tosx-*) ;;\n> +\t*)\n> +\t\tchmod a+w / || sudo chmod a+w / || true\n> +\t\texport IKNOWWHATIAMDOING=YES\n\nEeeww ;-)  This makes readers wonder where we did not enable the\ntest and why.  Perhaps throw in a matching\n\n\tt1509_allowed=NO\n\nin the azure thing for completeness?\n\nAlso, do we want to give a more descriptive name than t1509 to the\nvariable, say, ROOT_WORK_TREE_TEST_ALLOWED?\n\n> diff --git a/ci/run-docker-build.sh b/ci/run-docker-build.sh\n> index 8d47a5fda3..026afe275a 100755\n> --- a/ci/run-docker-build.sh\n> +++ b/ci/run-docker-build.sh\n> @@ -58,6 +58,8 @@ else\n>  \ttest -n \"$cache_dir\" && chown -R $HOST_UID:$HOST_UID \"$cache_dir\"\n>  fi\n>  \n> +chmod a+w /\n> +\n>  # Build and test\n>  command $switch_cmd su -m -l $CI_USER -c \"\n>  \tset -ex\n> @@ -68,6 +70,7 @@ command $switch_cmd su -m -l $CI_USER -c \"\n>  \texport GIT_TEST_CLONE_2GB='$GIT_TEST_CLONE_2GB'\n>  \texport MAKEFLAGS='$MAKEFLAGS'\n>  \texport cache_dir='$cache_dir'\n> +\texport IKNOWWHATIAMDOING=YES\n>  \tcd /usr/src/git\n>  \ttest -n '$cache_dir' && ln -s '$cache_dir/.prove' t/.prove\n>  \tmake\n\nBig EWWWWWWwwww.  Do we need to do this for _all_ tests, not\nselectively only while running t1509?  This makes me worried as a\ntest by mistake can easily corrupt the VM and invalidating the\ntests; I know we get a fresh one every time, so there is no\npermanent harm done by corrupting it, but having one fewer thing we\nhave to worry about is always better than having one more thing.\n"},{"id":"397796","messageId":"20200513235215.GA10618@danh.dev","threadId":"53461","inReplyTo":"xmqq8shvvkur.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 2/2] CI: enable t1509 in Travis and GitHub Actions","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-05-13T23:52:15Z","receivedAt":"2020-05-13T23:52:22Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2020-05-13 09:51:56-0700, Junio C Hamano <gitster@pobox.com> wrote:\n> Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:\n> > +if test \"$t1509_allowed\" = YES\n> > +then\n> > +\tcase \"$jobname\" in\n> > +\tosx-*) ;;\n> > +\t*)\n> > +\t\tchmod a+w / || sudo chmod a+w / || true\n> > +\t\texport IKNOWWHATIAMDOING=YES\n> \n> Eeeww ;-)  This makes readers wonder where we did not enable the\n> test and why.  Perhaps throw in a matching\n> \n> \tt1509_allowed=NO\n> \n> in the azure thing for completeness?\n\nI was thinking about allowing people set it via environment variable\nand check, but it seems too risky, now.\n\nPerhaps, always reset it to NO before the checking for $CI_TYPE,\nand enable it selectively for only Travis, and GitHub Actions.\n\nI didn't enable it for Azure because I can't assure it ;).\n\n> Also, do we want to give a more descriptive name than t1509 to the\n> variable, say, ROOT_WORK_TREE_TEST_ALLOWED?\n\nYeah, I think all caps is better for this risky variable.\nI think using T1509_ROOT_WORK_TREE_TEST_ALLOWED is better, to point\nout which test is risky. But it require future tests with root\nwork-tree must be written in t1509, since it's rare usecase, It'd be\nfine, I think.\n\n> \n> > diff --git a/ci/run-docker-build.sh b/ci/run-docker-build.sh\n> > index 8d47a5fda3..026afe275a 100755\n> > --- a/ci/run-docker-build.sh\n> > +++ b/ci/run-docker-build.sh\n> > @@ -58,6 +58,8 @@ else\n> >  \ttest -n \"$cache_dir\" && chown -R $HOST_UID:$HOST_UID \"$cache_dir\"\n> >  fi\n> >  \n> > +chmod a+w /\n> > +\n> >  # Build and test\n> >  command $switch_cmd su -m -l $CI_USER -c \"\n> >  \tset -ex\n> > @@ -68,6 +70,7 @@ command $switch_cmd su -m -l $CI_USER -c \"\n> >  \texport GIT_TEST_CLONE_2GB='$GIT_TEST_CLONE_2GB'\n> >  \texport MAKEFLAGS='$MAKEFLAGS'\n> >  \texport cache_dir='$cache_dir'\n> > +\texport IKNOWWHATIAMDOING=YES\n> >  \tcd /usr/src/git\n> >  \ttest -n '$cache_dir' && ln -s '$cache_dir/.prove' t/.prove\n> >  \tmake\n> \n> Big EWWWWWWwwww.  Do we need to do this for _all_ tests, not\n> selectively only while running t1509?  This makes me worried as a\n> test by mistake can easily corrupt the VM and invalidating the\n> tests; I know we get a fresh one every time, so there is no\n> permanent harm done by corrupting it, but having one fewer thing we\n> have to worry about is always better than having one more thing.\n\nPerhaps pass this variable all the way down from ci/lib.sh?\nAdding another variable into t1509 (except T1509_*) doesn't make it\nless risky.\nOr should we add T1509_ prefix to this env var?\n\n-- \nDanh\n"},{"id":"397798","messageId":"xmqqo8qrqtch.fsf@gitster.c.googlers.com","threadId":"53461","inReplyTo":"20200513235215.GA10618@danh.dev","subject":"Re: [PATCH 2/2] CI: enable t1509 in Travis and GitHub Actions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-13T23:59:42Z","receivedAt":"2020-05-13T23:59:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:\n\n>> >  \n>> > +chmod a+w /\n>> > +\n>> >  # Build and test\n>> >  command $switch_cmd su -m -l $CI_USER -c \"\n>> >  \tset -ex\n>> > @@ -68,6 +70,7 @@ command $switch_cmd su -m -l $CI_USER -c \"\n>> >  \texport GIT_TEST_CLONE_2GB='$GIT_TEST_CLONE_2GB'\n>> >  \texport MAKEFLAGS='$MAKEFLAGS'\n>> >  \texport cache_dir='$cache_dir'\n>> > +\texport IKNOWWHATIAMDOING=YES\n>> >  \tcd /usr/src/git\n>> >  \ttest -n '$cache_dir' && ln -s '$cache_dir/.prove' t/.prove\n>> >  \tmake\n>> \n>> Big EWWWWWWwwww.  Do we need to do this for _all_ tests, not\n>> selectively only while running t1509?  This makes me worried as a\n>> test by mistake can easily corrupt the VM and invalidating the\n>> tests; I know we get a fresh one every time, so there is no\n>> permanent harm done by corrupting it, but having one fewer thing we\n>> have to worry about is always better than having one more thing.\n>\n> Perhaps pass this variable all the way down from ci/lib.sh?\n> Adding another variable into t1509 (except T1509_*) doesn't make it\n> less risky.\n> Or should we add T1509_ prefix to this env var?\n\nI was not worried about any environment variable, but the \"let's\nmake the root directory writable by anybody during _all_ tests\",\nwhen we need such a crazy permission bits on the filesystem only\nwhile running t1509 and not any other time, stood out as extremely\nyucky.\n\n\n\n"}]}