{"thread":{"id":"57772","subject":"[PATCH] CI: select CC based on CC_PACKAGE (again)","startedAt":"2022-04-21T13:03:39Z","lastAt":"2022-04-22T23:10:39Z","messageCount":18,"participants":["Ævar Arnfjörð Bjarmason","Phillip Wood","Carlo Arenas","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"454069","messageId":"patch-1.1-d89ad4d5b7c-20220421T130310Z-avarab@gmail.com","threadId":"57772","inReplyTo":null,"subject":"[PATCH] CI: select CC based on CC_PACKAGE (again)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-21T13:03:26Z","receivedAt":"2022-04-21T13:03:39Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a regression in 707d2f2fe86 (CI: use \"$runs_on_pool\", not\n\"$jobname\" to select packages & config, 2021-11-23).\n\nIn that commit I changed CC=gcc from CC=gcc-9, but on OSX the \"gcc\" in\n$PATH points to clang, we need to use gcc-9 instead. Likewise for the\nlinux-gcc job CC=gcc-8 was changed to the implicit CC=gcc, which would\nselect GCC 9.4.0 instead of GCC 8.4.0.\n\nFurthermore in 25715419bf4 (CI: don't run \"make test\" twice in one\njob, 2021-11-23) when the \"linux-TEST-vars\" job was split off from\n\"linux-gcc\" the \"cc_package: gcc-8\" line was copied along with\nit.\n\nThat wasn't a bug per-se, as that \"make test\" would have run under GCC\n8 before the split into two jobs, but the point of selecting different\ncompiler for these jobs is to get better coverage, and to narrow down\nany issues with a given compiler to the job that runs it. Since the\n\"linux-TEST-vars\" job is already special in other ways (in running\nwith various GIT_TEST_* variables), and we've got the \"linux-gcc\" job\ncovering gcc-8 let's have it used the default system compiler instead.\n\nReported-by: Carlo Arenas \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\nRange-diff:\n1: 94abb826627 < -: ----------- CI: have osx-gcc use gcc, not clang\n-: ----------- > 1: d89ad4d5b7c CI: select CC based on CC_PACKAGE (again)\n\nThe range-diff is against a relevant patch in v4 of a larger CI series\nof mine[1]. This fix is independent of that, and on top of master. See\n[2] for the original fix and discussion.\n\nAs noted in the updated commit message not only the OSX jobs were\naffected, but also the linux-gcc job. This fixes both, along with a\nsmall fix to the related linux-TEST-vars recipe.\n\n1. https://lore.kernel.org/git/cover-v4-00.31-00000000000-20220418T132809Z-avarab@gmail.com/\n2. https://lore.kernel.org/git/patch-v4-30.31-94abb826627-20220418T132809Z-avarab@gmail.com/\n\n .github/workflows/main.yml | 1 -\n ci/lib.sh | 2 +-\n 2 files changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex c35200defb9..f12819a00d7 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -236,7 +236,6 @@ jobs:\n - jobname: linux-TEST-vars\n cc: gcc\n os: ubuntu\n- cc_package: gcc-8\n pool: ubuntu-latest\n - jobname: osx-clang\n cc: clang\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex cbc2f8f1caa..44007dcf93b 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -122,7 +122,7 @@ then\n \ttest macos != \"$CI_OS_NAME\" || CI_OS_NAME=osx\n \tCI_REPO_SLUG=\"$GITHUB_REPOSITORY\"\n \tCI_JOB_ID=\"$GITHUB_RUN_ID\"\n-\tCC=\"${CC:-gcc}\"\n+\tCC=\"${CC:-${CC_PACKAGE:-gcc}}\"\n \tDONT_SKIP_TAGS=t\n \n \tcache_dir=\"$HOME/none\"\n-- \n2.36.0.876.g4bfefc07680\n\n"},{"id":"454088","messageId":"db2dfab3-205e-036e-e9b5-f64bcf3650f6@gmail.com","threadId":"57772","inReplyTo":"patch-1.1-d89ad4d5b7c-20220421T130310Z-avarab@gmail.com","subject":"Re: [PATCH] CI: select CC based on CC_PACKAGE (again)","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-04-21T16:26:34Z","receivedAt":"2022-04-21T16:30:56Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Ævar\n\nOn 21/04/2022 14:03, Ævar Arnfjörð Bjarmason wrote:\n> Fix a regression in 707d2f2fe86 (CI: use \"$runs_on_pool\", not\n> \"$jobname\" to select packages & config, 2021-11-23).\n> \n> In that commit I changed CC=gcc from CC=gcc-9, but on OSX the \"gcc\" in\n> $PATH points to clang, we need to use gcc-9 instead. Likewise for the\n> linux-gcc job CC=gcc-8 was changed to the implicit CC=gcc, which would\n> select GCC 9.4.0 instead of GCC 8.4.0.\n> \n> Furthermore in 25715419bf4 (CI: don't run \"make test\" twice in one\n> job, 2021-11-23) when the \"linux-TEST-vars\" job was split off from\n> \"linux-gcc\" the \"cc_package: gcc-8\" line was copied along with\n> it.\n> \n> That wasn't a bug per-se, as that \"make test\" would have run under GCC\n> 8 before the split into two jobs, but the point of selecting different\n> compiler for these jobs is to get better coverage, and to narrow down\n> any issues with a given compiler to the job that runs it. Since the\n> \"linux-TEST-vars\" job is already special in other ways (in running\n> with various GIT_TEST_* variables), and we've got the \"linux-gcc\" job\n> covering gcc-8 let's have it used the default system compiler instead.\n> \n> Reported-by: Carlo Arenas \n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> Range-diff:\n> 1: 94abb826627 < -: ----------- CI: have osx-gcc use gcc, not clang\n> -: ----------- > 1: d89ad4d5b7c CI: select CC based on CC_PACKAGE (again)\n> \n> The range-diff is against a relevant patch in v4 of a larger CI series\n> of mine[1]. This fix is independent of that, and on top of master. See\n> [2] for the original fix and discussion.\n> \n> As noted in the updated commit message not only the OSX jobs were\n> affected, but also the linux-gcc job. This fixes both, along with a\n> small fix to the related linux-TEST-vars recipe.\n> \n> 1. https://lore.kernel.org/git/cover-v4-00.31-00000000000-20220418T132809Z-avarab@gmail.com/\n> 2. https://lore.kernel.org/git/patch-v4-30.31-94abb826627-20220418T132809Z-avarab@gmail.com/\n> \n> .github/workflows/main.yml | 1 -\n> ci/lib.sh | 2 +-\n> 2 files changed, 1 insertion(+), 2 deletions(-)\n> \n> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\n> index c35200defb9..f12819a00d7 100644\n> --- a/.github/workflows/main.yml\n> +++ b/.github/workflows/main.yml\n> @@ -236,7 +236,6 @@ jobs:\n> - jobname: linux-TEST-vars\n> cc: gcc\n> os: ubuntu\n> - cc_package: gcc-8\n> pool: ubuntu-latest\n> - jobname: osx-clang\n> cc: clang\n> diff --git a/ci/lib.sh b/ci/lib.sh\n> index cbc2f8f1caa..44007dcf93b 100755\n> --- a/ci/lib.sh\n> +++ b/ci/lib.sh\n> @@ -122,7 +122,7 @@ then\n> \ttest macos != \"$CI_OS_NAME\" || CI_OS_NAME=osx\n> \tCI_REPO_SLUG=\"$GITHUB_REPOSITORY\"\n> \tCI_JOB_ID=\"$GITHUB_RUN_ID\"\n> -\tCC=\"${CC:-gcc}\"\n> +\tCC=\"${CC:-${CC_PACKAGE:-gcc}}\"\n\nCC is set in .github/workflows/main.yaml for the ubuntu and macos jobs \nso I think they will not fallback to using CC_PACKAGE and therefore not \npick up the correct compiler.\n\nBest Wishes\n\nPhillip\n\n> \tDONT_SKIP_TAGS=t\n> \n> \tcache_dir=\"$HOME/none\"\n\n"},{"id":"454095","messageId":"patch-v2-1.1-92acf9420a9-20220421T174733Z-avarab@gmail.com","threadId":"57772","inReplyTo":"patch-1.1-d89ad4d5b7c-20220421T130310Z-avarab@gmail.com","subject":"[PATCH v2] CI: select CC based on CC_PACKAGE (again)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-21T17:48:28Z","receivedAt":"2022-04-21T17:48:38Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a regression in 707d2f2fe86 (CI: use \"$runs_on_pool\", not\n\"$jobname\" to select packages & config, 2021-11-23).\n\nIn that commit I changed CC=gcc from CC=gcc-9, but on OSX the \"gcc\" in\n$PATH points to clang, we need to use gcc-9 instead. Likewise for the\nlinux-gcc job CC=gcc-8 was changed to the implicit CC=gcc, which would\nselect GCC 9.4.0 instead of GCC 8.4.0.\n\nFurthermore in 25715419bf4 (CI: don't run \"make test\" twice in one\njob, 2021-11-23) when the \"linux-TEST-vars\" job was split off from\n\"linux-gcc\" the \"cc_package: gcc-8\" line was copied along with\nit.\n\nThat wasn't a bug per-se, as that \"make test\" would have run under GCC\n8 before the split into two jobs, but the point of selecting different\ncompiler for these jobs is to get better coverage, and to narrow down\nany issues with a given compiler to the job that runs it. Since the\n\"linux-TEST-vars\" job is already special in other ways (in running\nwith various GIT_TEST_* variables), and we've got the \"linux-gcc\" job\ncovering gcc-8 let's have it used the default system compiler instead.\n\nReported-by: Carlo Arenas \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n\nOn Thu, Apr 21 2022, Phillip Wood wrote:\n\n> CC is set in .github/workflows/main.yaml for the ubuntu and macos jobs\n> so I think they will not fallback to using CC_PACKAGE and therefore\n> not pick up the correct compiler.\n\nUrgh, sorry. I don't know how I got that turned around, but this\nversion works, and the (currently ongoing) CI run shows that the\nlinux-gcc job selects gcc-8 correctly, instead of gcc:\n\n https://github.com/avar/git/runs/6116735686?check_suite_focus=true#step:3:22\n\nThe osx-gcc job will be likewise fixed, but it's failing in that run\ndue to the unrelated perforce installation issue affecting all OSX\njobs (fix here:\nhttps://lore.kernel.org/git/cover-0.2-00000000000-20220421T124225Z-avarab@gmail.com/).\n\nRange-diff against v1:\n1: d89ad4d5b7c ! 1: 92acf9420a9 CI: select CC based on CC_PACKAGE (again)\n @@ ci/lib.sh: then\n \tCI_REPO_SLUG=\"$GITHUB_REPOSITORY\"\n \tCI_JOB_ID=\"$GITHUB_RUN_ID\"\n -\tCC=\"${CC:-gcc}\"\n -+\tCC=\"${CC:-${CC_PACKAGE:-gcc}}\"\n ++\tCC=\"${CC_PACKAGE:-${CC:-gcc}}\"\n \tDONT_SKIP_TAGS=t\n \n \tcache_dir=\"$HOME/none\"\n\n .github/workflows/main.yml | 1 -\n ci/lib.sh | 2 +-\n 2 files changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex c35200defb9..f12819a00d7 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -236,7 +236,6 @@ jobs:\n - jobname: linux-TEST-vars\n cc: gcc\n os: ubuntu\n- cc_package: gcc-8\n pool: ubuntu-latest\n - jobname: osx-clang\n cc: clang\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex cbc2f8f1caa..86e37da9bc5 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -122,7 +122,7 @@ then\n \ttest macos != \"$CI_OS_NAME\" || CI_OS_NAME=osx\n \tCI_REPO_SLUG=\"$GITHUB_REPOSITORY\"\n \tCI_JOB_ID=\"$GITHUB_RUN_ID\"\n-\tCC=\"${CC:-gcc}\"\n+\tCC=\"${CC_PACKAGE:-${CC:-gcc}}\"\n \tDONT_SKIP_TAGS=t\n \n \tcache_dir=\"$HOME/none\"\n-- \n2.36.0.879.g3659959fcca\n\n"},{"id":"454149","messageId":"CAPUEspggNfra_C8PBcVqjZrxxnswg27WFLg2kT6JOCMB8f0FGg@mail.gmail.com","threadId":"57772","inReplyTo":"patch-v2-1.1-92acf9420a9-20220421T174733Z-avarab@gmail.com","subject":"Re: [PATCH v2] CI: select CC based on CC_PACKAGE (again)","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2022-04-21T19:05:20Z","receivedAt":"2022-04-21T19:05:36Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Thu, Apr 21, 2022 at 10:48 AM Ævar Arnfjörð Bjarmason\n wrote:\n>\n> Reported-by: Carlo Arenas \n\nto avoid confusing the attribution scripts better use my full name:\n\"Carlo Marcelo Arenas Belón\"\n\n> diff --git a/ci/lib.sh b/ci/lib.sh\n> index cbc2f8f1caa..86e37da9bc5 100755\n> --- a/ci/lib.sh\n> +++ b/ci/lib.sh\n> @@ -122,7 +122,7 @@ then\n> test macos != \"$CI_OS_NAME\" || CI_OS_NAME=osx\n> CI_REPO_SLUG=\"$GITHUB_REPOSITORY\"\n> CI_JOB_ID=\"$GITHUB_RUN_ID\"\n> - CC=\"${CC:-gcc}\"\n> + CC=\"${CC_PACKAGE:-${CC:-gcc}}\"\n\nminor nitpick, but most likely still relevant considering your other\n\"bashism\" fixes.\nthe POSIX syntax doesn't use \":\" (documented in CodingGuidelines)\n\nnot sure if the compounded assignment is valid in POSIX, but at least\nthe following worked with a recent NetBSD 8 sh :\n\n CC=\"${CC_PACKAGE-${CC-gcc}}\"\n\nCarlo\n"},{"id":"454150","messageId":"xmqqzgkegspg.fsf@gitster.g","threadId":"57772","inReplyTo":"patch-v2-1.1-92acf9420a9-20220421T174733Z-avarab@gmail.com","subject":"Re: [PATCH v2] CI: select CC based on CC_PACKAGE (again)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-21T19:08:27Z","receivedAt":"2022-04-21T19:08:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Fix a regression in 707d2f2fe86 (CI: use \"$runs_on_pool\", not\n> \"$jobname\" to select packages & config, 2021-11-23).\n>\n> In that commit I changed CC=gcc from CC=gcc-9, but on OSX the \"gcc\" in\n> $PATH points to clang, we need to use gcc-9 instead. Likewise for the\n> linux-gcc job CC=gcc-8 was changed to the implicit CC=gcc, which would\n> select GCC 9.4.0 instead of GCC 8.4.0.\n\nThanks for diagnosing how things were broken.\n\n> On Thu, Apr 21 2022, Phillip Wood wrote:\n>\n>> CC is set in .github/workflows/main.yaml for the ubuntu and macos jobs\n>> so I think they will not fallback to using CC_PACKAGE and therefore\n>> not pick up the correct compiler.\n> ...\n> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\n> index c35200defb9..f12819a00d7 100644\n> --- a/.github/workflows/main.yml\n> +++ b/.github/workflows/main.yml\n> @@ -236,7 +236,6 @@ jobs:\n> - jobname: linux-TEST-vars\n> cc: gcc\n> os: ubuntu\n> - cc_package: gcc-8\n> pool: ubuntu-latest\n> - jobname: osx-clang\n> cc: clang\n> diff --git a/ci/lib.sh b/ci/lib.sh\n> index cbc2f8f1caa..86e37da9bc5 100755\n> --- a/ci/lib.sh\n> +++ b/ci/lib.sh\n> @@ -122,7 +122,7 @@ then\n> \ttest macos != \"$CI_OS_NAME\" || CI_OS_NAME=osx\n> \tCI_REPO_SLUG=\"$GITHUB_REPOSITORY\"\n> \tCI_JOB_ID=\"$GITHUB_RUN_ID\"\n> -\tCC=\"${CC:-gcc}\"\n> +\tCC=\"${CC_PACKAGE:-${CC:-gcc}}\"\n> \tDONT_SKIP_TAGS=t\n> \n> \tcache_dir=\"$HOME/none\"\n\nOK, so we favor CC_PACKAGE (from the matrix.vector.cc_package) if\nset, and then cc (again, from the matrix.vector.cc) if set, and then\nfinally use \"gcc\" as a fallback. In the osx-gcc job, cc_package is\nset to gcc-9 while in the osx-clang, cc is gcc that confusingly calls\nfor clang. That sounds like it would do the right thing for two\nmacs.\n\nFor other jobs with different settings for cc and cc_package, does\nthis have any effect? I do not think I saw any mention in the\nproposed log message.\n\n\t\tvector.cc\tvector.cc_package\told\tnew\nlinux-clang\tclang\t\t-\t\t\tclang\tclang\nlinux-sha256\tclang\t\t-\t\t\tclang\tclang\nlinux-gcc\tgcc\t\tgcc-8\t\t\tgcc\tgcc-8\nosx-clang\tclang\t\t-\t\t\tclang\tclang\nosx-gcc\t\tgcc\t\tgcc-9\t\t\tclang\tgcc-9\nlinux-gcc-default gcc\t\t-\t\t\tgcc\tgcc\n\nSo, linux-gcc job used to use whichever \"gcc\" the platform gave us,\nbut now it explicitly asks for gcc-8, which may or may not be\ndifferent from what linux-gcc-default uses, and there is no other\ndifference by this change. We may get a better test coverage (if\nthe default gcc is not gcc-8) or no improvement (if the default is\ngcc-8), so it is a strict improvement worth recording as an intended\nside effect in the proposed log message to help future developers.\n\nOther than that, looks good to me.\n\n"},{"id":"454152","messageId":"xmqqv8v2gsjl.fsf@gitster.g","threadId":"57772","inReplyTo":"CAPUEspggNfra_C8PBcVqjZrxxnswg27WFLg2kT6JOCMB8f0FGg@mail.gmail.com","subject":"Re: [PATCH v2] CI: select CC based on CC_PACKAGE (again)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-21T19:11:58Z","receivedAt":"2022-04-21T19:12:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlo Arenas writes:\n\n>> - CC=\"${CC:-gcc}\"\n>> + CC=\"${CC_PACKAGE:-${CC:-gcc}}\"\n>\n> minor nitpick, but most likely still relevant considering your other\n> \"bashism\" fixes.\n> the POSIX syntax doesn't use \":\" (documented in CodingGuidelines)\n\nYou are reading the guideline wrong, I am afraid.\n\nThis is not a substring substitution, ${foo:0:3} picking three\ncharacters from the beginning of foo.\n\nIt is \"If CC_PACKAGE is UNSET OR SET TO EMPTY, then use the fallback\nvalue that is ${CC:-gcc}\", which is a bog standard bourne shell that\npredates POSIX. The colon before \"-\" is what adds \"OR SET TO EMPTY\"\npart; without the colon, i.e. \"${CC_PACKAGE-${CC-gcc}}\", then you\nget the same without \"OR SET TO EMPTY\" part.\n\nAnd we do use that form.\n"},{"id":"454153","messageId":"220421.867d7i6ycr.gmgdl@evledraar.gmail.com","threadId":"57772","inReplyTo":"xmqqzgkegspg.fsf@gitster.g","subject":"Re: [PATCH v2] CI: select CC based on CC_PACKAGE (again)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-21T19:13:57Z","receivedAt":"2022-04-21T19:16:42Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Apr 21 2022, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> Fix a regression in 707d2f2fe86 (CI: use \"$runs_on_pool\", not\n>> \"$jobname\" to select packages & config, 2021-11-23).\n>>\n>> In that commit I changed CC=gcc from CC=gcc-9, but on OSX the \"gcc\" in\n>> $PATH points to clang, we need to use gcc-9 instead. Likewise for the\n>> linux-gcc job CC=gcc-8 was changed to the implicit CC=gcc, which would\n>> select GCC 9.4.0 instead of GCC 8.4.0.\n>\n> Thanks for diagnosing how things were broken.\n>\n>> On Thu, Apr 21 2022, Phillip Wood wrote:\n>>\n>>> CC is set in .github/workflows/main.yaml for the ubuntu and macos jobs\n>>> so I think they will not fallback to using CC_PACKAGE and therefore\n>>> not pick up the correct compiler.\n>> ...\n>> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\n>> index c35200defb9..f12819a00d7 100644\n>> --- a/.github/workflows/main.yml\n>> +++ b/.github/workflows/main.yml\n>> @@ -236,7 +236,6 @@ jobs:\n>> - jobname: linux-TEST-vars\n>> cc: gcc\n>> os: ubuntu\n>> - cc_package: gcc-8\n>> pool: ubuntu-latest\n>> - jobname: osx-clang\n>> cc: clang\n>> diff --git a/ci/lib.sh b/ci/lib.sh\n>> index cbc2f8f1caa..86e37da9bc5 100755\n>> --- a/ci/lib.sh\n>> +++ b/ci/lib.sh\n>> @@ -122,7 +122,7 @@ then\n>> \ttest macos != \"$CI_OS_NAME\" || CI_OS_NAME=osx\n>> \tCI_REPO_SLUG=\"$GITHUB_REPOSITORY\"\n>> \tCI_JOB_ID=\"$GITHUB_RUN_ID\"\n>> -\tCC=\"${CC:-gcc}\"\n>> +\tCC=\"${CC_PACKAGE:-${CC:-gcc}}\"\n>> \tDONT_SKIP_TAGS=t\n>> \n>> \tcache_dir=\"$HOME/none\"\n>\n> OK, so we favor CC_PACKAGE (from the matrix.vector.cc_package) if\n> set, and then cc (again, from the matrix.vector.cc) if set, and then\n> finally use \"gcc\" as a fallback. In the osx-gcc job, cc_package is\n> set to gcc-9 while in the osx-clang, cc is gcc that confusingly calls\n> for clang. That sounds like it would do the right thing for two\n> macs.\n\nYes.\n\n> For other jobs with different settings for cc and cc_package, does\n> this have any effect? I do not think I saw any mention in the\n> proposed log message.\n>\n> \t\tvector.cc\tvector.cc_package\told\tnew\n> linux-clang\tclang\t\t-\t\t\tclang\tclang\n> linux-sha256\tclang\t\t-\t\t\tclang\tclang\n> linux-gcc\tgcc\t\tgcc-8\t\t\tgcc\tgcc-8\n> osx-clang\tclang\t\t-\t\t\tclang\tclang\n> osx-gcc\t\tgcc\t\tgcc-9\t\t\tclang\tgcc-9\n> linux-gcc-default gcc\t\t-\t\t\tgcc\tgcc\n>\n> So, linux-gcc job used to use whichever \"gcc\" the platform gave us,\n> but now it explicitly asks for gcc-8, which may or may not be\n> different from what linux-gcc-default uses, and there is no other\n> difference by this change. We may get a better test coverage (if\n> the default gcc is not gcc-8) or no improvement (if the default is\n> gcc-8), so it is a strict improvement worth recording as an intended\n> side effect in the proposed log message to help future developers.\n>\n> Other than that, looks good to me.\n\nI'm happy to rephrase it however you'd like, but I'm a bit confused by\nthe \"saw any mention in the proposed log message\". I'm fairly sure\nparagraph 2 onwards covers this, i.e. how linux-gcc's behavior is\nchanged (as it also regressed).\n\nWhat I suppose is left undiscussed is that jobs that don't define\nCC_PACKAGE at all won't be impacted, is that what you wanted to be\nexplicitly mentioned?\n"},{"id":"454154","messageId":"CAPUEspgrk3+FAo9Y6VWcYddh0o4xCkqmQ=GLap1XomcMmnFCuA@mail.gmail.com","threadId":"57772","inReplyTo":"xmqqv8v2gsjl.fsf@gitster.g","subject":"Re: [PATCH v2] CI: select CC based on CC_PACKAGE (again)","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2022-04-21T19:22:21Z","receivedAt":"2022-04-21T19:22:46Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Thu, Apr 21, 2022 at 12:12 PM Junio C Hamano wrote:\n>\n> Carlo Arenas writes:\n>\n> >> - CC=\"${CC:-gcc}\"\n> >> + CC=\"${CC_PACKAGE:-${CC:-gcc}}\"\n> >\n> > minor nitpick, but most likely still relevant considering your other\n> > \"bashism\" fixes.\n> > the POSIX syntax doesn't use \":\" (documented in CodingGuidelines)\n>\n> You are reading the guideline wrong, I am afraid.\n\nIndeed; and I realized it as I was trying to answer my own question\nabout the compounded replacement being supported.\n\nWondering if something more explicit might be easier to understand anyway like :\n\n if test -n \"$CC_PACKAGE\"\n then\n CC=\"$CC_PACKAGE\"\n fi\n\nCarlo\n"},{"id":"454155","messageId":"xmqqr15qgqw1.fsf@gitster.g","threadId":"57772","inReplyTo":"CAPUEspgrk3+FAo9Y6VWcYddh0o4xCkqmQ=GLap1XomcMmnFCuA@mail.gmail.com","subject":"Re: [PATCH v2] CI: select CC based on CC_PACKAGE (again)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-21T19:47:42Z","receivedAt":"2022-04-21T19:47:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlo Arenas writes:\n\n> On Thu, Apr 21, 2022 at 12:12 PM Junio C Hamano wrote:\n>>\n>> Carlo Arenas writes:\n>>\n>> >> - CC=\"${CC:-gcc}\"\n>> >> + CC=\"${CC_PACKAGE:-${CC:-gcc}}\"\n>> > ...\n> Wondering if something more explicit might be easier to understand anyway like :\n>\n> if test -n \"$CC_PACKAGE\"\n> then\n> CC=\"$CC_PACKAGE\"\n> fi\n>\n> Carlo\n\nThe parameter substitution is commonly used part of the shell\nlanguage, and even if developers, when starting to work on parts of\nthe system that are written in shell, are initially unfamiliar with\nsuch language constructs, nobody will stay a newbie forever. \n\nTo them, idioms like ${A:-${B:-C}} would quickly become as explicit\nas a long-form written as if/then/elif/fi. Surely it takes practice\nto be familiar with the idiom, but idioms help those who know them\nto be more efficient and effetive, so, no, I do not think it is a\ngood idea to aim for \"easier to understand by newbies\".\n\nThanks.\n\n\n\n\n"},{"id":"454158","messageId":"xmqqlevygqqc.fsf@gitster.g","threadId":"57772","inReplyTo":"220421.867d7i6ycr.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v2] CI: select CC based on CC_PACKAGE (again)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-21T19:51:07Z","receivedAt":"2022-04-21T19:51:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> I'm happy to rephrase it however you'd like, but I'm a bit confused by\n> the \"saw any mention in the proposed log message\". I'm fairly sure\n> paragraph 2 onwards covers this, i.e. how linux-gcc's behavior is\n> changed (as it also regressed).\n\nYeah, true, \"\"\"Likewise for the linux-gcc job CC=gcc-8 was changed\nto the implicit CC=gcc, which would select GCC 9.4.0 instead of GCC\n8.4.0.\"\"\" that you wrote is exactly about it.\n\nI was confused because immediately after that you said it was not a\nbug, so I dismissed it as not part or the real issue. Perhaps\nstriking that \"not a bug\" part may make it less confusing? I dunno.\n\n> What I suppose is left undiscussed is that jobs that don't define\n> CC_PACKAGE at all won't be impacted, is that what you wanted to be\n> explicitly mentioned?\n\nYes, I agree that is a good idea, too. Or you can steal the \"before\nand after\" table I gave you in the message you are responding to, if\nyou think it helps.\n\nThanks.\n"},{"id":"454161","messageId":"220421.86y1zy5hzz.gmgdl@evledraar.gmail.com","threadId":"57772","inReplyTo":"xmqqlevygqqc.fsf@gitster.g","subject":"Re: [PATCH v2] CI: select CC based on CC_PACKAGE (again)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-21T19:52:57Z","receivedAt":"2022-04-21T19:55:18Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Apr 21 2022, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> I'm happy to rephrase it however you'd like, but I'm a bit confused by\n>> the \"saw any mention in the proposed log message\". I'm fairly sure\n>> paragraph 2 onwards covers this, i.e. how linux-gcc's behavior is\n>> changed (as it also regressed).\n>\n> Yeah, true, \"\"\"Likewise for the linux-gcc job CC=gcc-8 was changed\n> to the implicit CC=gcc, which would select GCC 9.4.0 instead of GCC\n> 8.4.0.\"\"\" that you wrote is exactly about it.\n>\n> I was confused because immediately after that you said it was not a\n> bug, so I dismissed it as not part or the real issue. Perhaps\n> striking that \"not a bug\" part may make it less confusing? I dunno.\n\nI'll rephrase it, but I probably won't get to it tonight, sorry.\n\nWhat I was trying to get at is that the linux-TEST-vars job was\naffected, and *does* have a CC_PACKAGE defined, but in that case (and I\nwas the one who split it out) the point was never to have it use its own\ncompiler. For any of these platform/compiler combinations it's enough if\nwe cover that platform/compiler verison once.\n\nSo the right thing to do with linux-TEST-vars is to just have it use\n\"vanilla\", because it's special in other ways. So if and when it fails\nwe don't want to worry about untangling the two variables. I.e. is it\nthe special-versioned compiler, or is it (and it almost always would be)\nthe GIT_TEST_* variables. In that case we should only need to worry\nabout the latter.\n\n>> What I suppose is left undiscussed is that jobs that don't define\n>> CC_PACKAGE at all won't be impacted, is that what you wanted to be\n>> explicitly mentioned?\n>\n> Yes, I agree that is a good idea, too. Or you can steal the \"before\n> and after\" table I gave you in the message you are responding to, if\n> you think it helps.\n\n*nod*\n"},{"id":"454163","messageId":"xmqqczhagqim.fsf@gitster.g","threadId":"57772","inReplyTo":"xmqqlevygqqc.fsf@gitster.g","subject":"Re: [PATCH v2] CI: select CC based on CC_PACKAGE (again)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-21T19:55:45Z","receivedAt":"2022-04-21T19:55:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> I'm happy to rephrase it however you'd like, but I'm a bit confused by\n>> the \"saw any mention in the proposed log message\". I'm fairly sure\n>> paragraph 2 onwards covers this, i.e. how linux-gcc's behavior is\n>> changed (as it also regressed).\n>\n> Yeah, true, \"\"\"Likewise for the linux-gcc job CC=gcc-8 was changed\n> to the implicit CC=gcc, which would select GCC 9.4.0 instead of GCC\n> 8.4.0.\"\"\" that you wrote is exactly about it.\n>\n> I was confused because immediately after that you said it was not a\n> bug, so I dismissed it as not part or the real issue. Perhaps\n> striking that \"not a bug\" part may make it less confusing? I dunno.\n\nAlso, the description above merely says we use gcc-9 instead of gcc-8,\nwithout saying if that is a good change or not, which is another thing\nthat needs to be said. In the larger context, if we aim to try different\ncompilers, we should know which other ones we are using, and if linux-gcc\njob is meant to test gcc-8 (because we know there is another job\nthat tests gcc-9), the change was a bug. But because you said it\nwas not a bug, it made it unclear if that is merely an observation\nor it was pointing out our earlier mistake that should be corrected.\n"},{"id":"454235","messageId":"patch-v3-1.1-8b3444ecc87-20220422T092015Z-avarab@gmail.com","threadId":"57772","inReplyTo":"patch-v2-1.1-92acf9420a9-20220421T174733Z-avarab@gmail.com","subject":"[PATCH v3] CI: select CC based on CC_PACKAGE (again)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-22T09:20:53Z","receivedAt":"2022-04-22T09:21:00Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a regression in 707d2f2fe86 (CI: use \"$runs_on_pool\", not\n\"$jobname\" to select packages & config, 2021-11-23).\n\nIn that commit I changed CC=gcc from CC=gcc-9, but on OSX the \"gcc\" in\n$PATH points to clang, we need to use gcc-9 instead. Likewise for the\nlinux-gcc job CC=gcc-8 was changed to the implicit CC=gcc, which would\nselect GCC 9.4.0 instead of GCC 8.4.0.\n\nFurthermore in 25715419bf4 (CI: don't run \"make test\" twice in one\njob, 2021-11-23) when the \"linux-TEST-vars\" job was split off from\n\"linux-gcc\" the \"cc_package: gcc-8\" line was copied along with\nit, so its \"cc_package\" line wasn't working as intended either.\n\nAs a table, this is what's changed by this commit, i.e. it only\naffects the linux-gcc, linux-TEST-vars and osx-gcc jobs:\n\n\t|-------------------+-----------+-------------------+-------+-------|\n\t| jobname | vector.cc | vector.cc_package | old | new |\n\t|-------------------+-----------+-------------------+-------+-------|\n\t| linux-clang | clang | - | clang | clang |\n\t| linux-sha256 | clang | - | clang | clang |\n\t| linux-gcc | gcc | gcc-8 | gcc | gcc-8 |\n\t| osx-clang | clang | - | clang | clang |\n\t| osx-gcc | gcc | gcc-9 | clang | gcc-9 |\n\t| linux-gcc-default | gcc | - | gcc | gcc |\n\t| linux-TEST-vars | gcc | gcc-8 | gcc | gcc-8 |\n\t|-------------------+-----------+-------------------+-------+-------|\n\nReported-by: Carlo Arenas \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n\nI just dropped the linux-TEST-vars change from the v2 in lieu of\ntrying to get the wording in the commit message right.\n\nRange-diff against v2:\n1: 92acf9420a9 ! 1: 8b3444ecc87 CI: select CC based on CC_PACKAGE (again)\n @@ Commit message\n Furthermore in 25715419bf4 (CI: don't run \"make test\" twice in one\n job, 2021-11-23) when the \"linux-TEST-vars\" job was split off from\n \"linux-gcc\" the \"cc_package: gcc-8\" line was copied along with\n - it.\n -\n - That wasn't a bug per-se, as that \"make test\" would have run under GCC\n - 8 before the split into two jobs, but the point of selecting different\n - compiler for these jobs is to get better coverage, and to narrow down\n - any issues with a given compiler to the job that runs it. Since the\n - \"linux-TEST-vars\" job is already special in other ways (in running\n - with various GIT_TEST_* variables), and we've got the \"linux-gcc\" job\n - covering gcc-8 let's have it used the default system compiler instead.\n + it, so its \"cc_package\" line wasn't working as intended either.\n +\n + As a table, this is what's changed by this commit, i.e. it only\n + affects the linux-gcc, linux-TEST-vars and osx-gcc jobs:\n +\n + |-------------------+-----------+-------------------+-------+-------|\n + | jobname | vector.cc | vector.cc_package | old | new |\n + |-------------------+-----------+-------------------+-------+-------|\n + | linux-clang | clang | - | clang | clang |\n + | linux-sha256 | clang | - | clang | clang |\n + | linux-gcc | gcc | gcc-8 | gcc | gcc-8 |\n + | osx-clang | clang | - | clang | clang |\n + | osx-gcc | gcc | gcc-9 | clang | gcc-9 |\n + | linux-gcc-default | gcc | - | gcc | gcc |\n + | linux-TEST-vars | gcc | gcc-8 | gcc | gcc-8 |\n + |-------------------+-----------+-------------------+-------+-------|\n \n Reported-by: Carlo Arenas \n Signed-off-by: Ævar Arnfjörð Bjarmason \n \n - ## .github/workflows/main.yml ##\n -@@ .github/workflows/main.yml: jobs:\n - - jobname: linux-TEST-vars\n - cc: gcc\n - os: ubuntu\n -- cc_package: gcc-8\n - pool: ubuntu-latest\n - - jobname: osx-clang\n - cc: clang\n -\n ## ci/lib.sh ##\n @@ ci/lib.sh: then\n \ttest macos != \"$CI_OS_NAME\" || CI_OS_NAME=osx\n\n ci/lib.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex cbc2f8f1caa..86e37da9bc5 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -122,7 +122,7 @@ then\n \ttest macos != \"$CI_OS_NAME\" || CI_OS_NAME=osx\n \tCI_REPO_SLUG=\"$GITHUB_REPOSITORY\"\n \tCI_JOB_ID=\"$GITHUB_RUN_ID\"\n-\tCC=\"${CC:-gcc}\"\n+\tCC=\"${CC_PACKAGE:-${CC:-gcc}}\"\n \tDONT_SKIP_TAGS=t\n \n \tcache_dir=\"$HOME/none\"\n-- \n2.36.0.879.g56a83971f3f\n\n"},{"id":"454239","messageId":"1654562506.42130.1650619559902@apps.talktalk.co.uk","threadId":"57772","inReplyTo":"patch-v3-1.1-8b3444ecc87-20220422T092015Z-avarab@gmail.com","subject":"Re: [PATCH v3] CI: select CC based on CC_PACKAGE (again)","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2022-04-22T09:25:59Z","receivedAt":"2022-04-22T09:26:11Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Ævar\n\n> On 22 April 2022 at 10:20 Ævar Arnfjörð Bjarmason wrote:\n> \n> \n> Fix a regression in 707d2f2fe86 (CI: use \"$runs_on_pool\", not\n> \"$jobname\" to select packages & config, 2021-11-23).\n> \n> In that commit I changed CC=gcc from CC=gcc-9, but on OSX the \"gcc\" in\n> $PATH points to clang, we need to use gcc-9 instead. Likewise for the\n> linux-gcc job CC=gcc-8 was changed to the implicit CC=gcc, which would\n> select GCC 9.4.0 instead of GCC 8.4.0.\n> \n> Furthermore in 25715419bf4 (CI: don't run \"make test\" twice in one\n> job, 2021-11-23) when the \"linux-TEST-vars\" job was split off from\n> \"linux-gcc\" the \"cc_package: gcc-8\" line was copied along with\n> it, so its \"cc_package\" line wasn't working as intended either.\n> \n> As a table, this is what's changed by this commit, i.e. it only\n> affects the linux-gcc, linux-TEST-vars and osx-gcc jobs:\n> \n> \t|-------------------+-----------+-------------------+-------+-------|\n> \t| jobname | vector.cc | vector.cc_package | old | new |\n> \t|-------------------+-----------+-------------------+-------+-------|\n> \t| linux-clang | clang | - | clang | clang |\n> \t| linux-sha256 | clang | - | clang | clang |\n> \t| linux-gcc | gcc | gcc-8 | gcc | gcc-8 |\n> \t| osx-clang | clang | - | clang | clang |\n> \t| osx-gcc | gcc | gcc-9 | clang | gcc-9 |\n> \t| linux-gcc-default | gcc | - | gcc | gcc |\n> \t| linux-TEST-vars | gcc | gcc-8 | gcc | gcc-8 |\n> \t|-------------------+-----------+-------------------+-------+-------|\n\nHaving this table is helpful. I do find it confusing that we're still setting \nCC in the main.yaml and then overriding it in lib.sh. I think it would make \nthings clearer if we got find of the cc settings in the job matrix.\n\nBest Wishes\n\nPhillip\n\n> Reported-by: Carlo Arenas \n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> \n> I just dropped the linux-TEST-vars change from the v2 in lieu of\n> trying to get the wording in the commit message right.\n> \n> Range-diff against v2:\n> 1: 92acf9420a9 ! 1: 8b3444ecc87 CI: select CC based on CC_PACKAGE (again)\n> @@ Commit message\n> Furthermore in 25715419bf4 (CI: don't run \"make test\" twice in one\n> job, 2021-11-23) when the \"linux-TEST-vars\" job was split off from\n> \"linux-gcc\" the \"cc_package: gcc-8\" line was copied along with\n> - it.\n> -\n> - That wasn't a bug per-se, as that \"make test\" would have run under GCC\n> - 8 before the split into two jobs, but the point of selecting different\n> - compiler for these jobs is to get better coverage, and to narrow down\n> - any issues with a given compiler to the job that runs it. Since the\n> - \"linux-TEST-vars\" job is already special in other ways (in running\n> - with various GIT_TEST_* variables), and we've got the \"linux-gcc\" job\n> - covering gcc-8 let's have it used the default system compiler instead.\n> + it, so its \"cc_package\" line wasn't working as intended either.\n> +\n> + As a table, this is what's changed by this commit, i.e. it only\n> + affects the linux-gcc, linux-TEST-vars and osx-gcc jobs:\n> +\n> + |-------------------+-----------+-------------------+-------+-------|\n> + | jobname | vector.cc | vector.cc_package | old | new |\n> + |-------------------+-----------+-------------------+-------+-------|\n> + | linux-clang | clang | - | clang | clang |\n> + | linux-sha256 | clang | - | clang | clang |\n> + | linux-gcc | gcc | gcc-8 | gcc | gcc-8 |\n> + | osx-clang | clang | - | clang | clang |\n> + | osx-gcc | gcc | gcc-9 | clang | gcc-9 |\n> + | linux-gcc-default | gcc | - | gcc | gcc |\n> + | linux-TEST-vars | gcc | gcc-8 | gcc | gcc-8 |\n> + |-------------------+-----------+-------------------+-------+-------|\n> \n> Reported-by: Carlo Arenas \n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> \n> - ## .github/workflows/main.yml ##\n> -@@ .github/workflows/main.yml: jobs:\n> - - jobname: linux-TEST-vars\n> - cc: gcc\n> - os: ubuntu\n> -- cc_package: gcc-8\n> - pool: ubuntu-latest\n> - - jobname: osx-clang\n> - cc: clang\n> -\n> ## ci/lib.sh ##\n> @@ ci/lib.sh: then\n> \ttest macos != \"$CI_OS_NAME\" || CI_OS_NAME=osx\n> \n> ci/lib.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/ci/lib.sh b/ci/lib.sh\n> index cbc2f8f1caa..86e37da9bc5 100755\n> --- a/ci/lib.sh\n> +++ b/ci/lib.sh\n> @@ -122,7 +122,7 @@ then\n> \ttest macos != \"$CI_OS_NAME\" || CI_OS_NAME=osx\n> \tCI_REPO_SLUG=\"$GITHUB_REPOSITORY\"\n> \tCI_JOB_ID=\"$GITHUB_RUN_ID\"\n> -\tCC=\"${CC:-gcc}\"\n> +\tCC=\"${CC_PACKAGE:-${CC:-gcc}}\"\n> \tDONT_SKIP_TAGS=t\n> \n> \tcache_dir=\"$HOME/none\"\n> -- \n> 2.36.0.879.g56a83971f3f\n>\n"},{"id":"454257","messageId":"220422.86r15p4a34.gmgdl@evledraar.gmail.com","threadId":"57772","inReplyTo":"1654562506.42130.1650619559902@apps.talktalk.co.uk","subject":"Re: [PATCH v3] CI: select CC based on CC_PACKAGE (again)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-22T11:43:01Z","receivedAt":"2022-04-22T11:43:51Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Apr 22 2022, Phillip Wood wrote:\n\n> Hi Ævar\n>\n>> On 22 April 2022 at 10:20 Ævar Arnfjörð Bjarmason wrote:\n>> \n>> \n>> Fix a regression in 707d2f2fe86 (CI: use \"$runs_on_pool\", not\n>> \"$jobname\" to select packages & config, 2021-11-23).\n>> \n>> In that commit I changed CC=gcc from CC=gcc-9, but on OSX the \"gcc\" in\n>> $PATH points to clang, we need to use gcc-9 instead. Likewise for the\n>> linux-gcc job CC=gcc-8 was changed to the implicit CC=gcc, which would\n>> select GCC 9.4.0 instead of GCC 8.4.0.\n>> \n>> Furthermore in 25715419bf4 (CI: don't run \"make test\" twice in one\n>> job, 2021-11-23) when the \"linux-TEST-vars\" job was split off from\n>> \"linux-gcc\" the \"cc_package: gcc-8\" line was copied along with\n>> it, so its \"cc_package\" line wasn't working as intended either.\n>> \n>> As a table, this is what's changed by this commit, i.e. it only\n>> affects the linux-gcc, linux-TEST-vars and osx-gcc jobs:\n>> \n>> \t|-------------------+-----------+-------------------+-------+-------|\n>> \t| jobname | vector.cc | vector.cc_package | old | new |\n>> \t|-------------------+-----------+-------------------+-------+-------|\n>> \t| linux-clang | clang | - | clang | clang |\n>> \t| linux-sha256 | clang | - | clang | clang |\n>> \t| linux-gcc | gcc | gcc-8 | gcc | gcc-8 |\n>> \t| osx-clang | clang | - | clang | clang |\n>> \t| osx-gcc | gcc | gcc-9 | clang | gcc-9 |\n>> \t| linux-gcc-default | gcc | - | gcc | gcc |\n>> \t| linux-TEST-vars | gcc | gcc-8 | gcc | gcc-8 |\n>> \t|-------------------+-----------+-------------------+-------+-------|\n>\n> Having this table is helpful. I do find it confusing that we're still setting \n> CC in the main.yaml and then overriding it in lib.sh. I think it would make \n> things clearer if we got find of the cc settings in the job matrix.\n\nYes, this stand-alone patch is just a band-aid.\n\nDo you like the state at the tip of my larger CI series? It's only set\nin ci/lib.sh there, or in the case of the package we need the\ninstallation script is tasked with it now.\n"},{"id":"454278","messageId":"xmqqee1p9dna.fsf@gitster.g","threadId":"57772","inReplyTo":"patch-v3-1.1-8b3444ecc87-20220422T092015Z-avarab@gmail.com","subject":"Re: [PATCH v3] CI: select CC based on CC_PACKAGE (again)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-22T18:27:53Z","receivedAt":"2022-04-22T22:06:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Fix a regression in 707d2f2fe86 (CI: use \"$runs_on_pool\", not\n> \"$jobname\" to select packages & config, 2021-11-23).\n>\n> In that commit I changed CC=gcc from CC=gcc-9, but on OSX the \"gcc\" in\n> $PATH points to clang, we need to use gcc-9 instead. Likewise for the\n> linux-gcc job CC=gcc-8 was changed to the implicit CC=gcc, which would\n> select GCC 9.4.0 instead of GCC 8.4.0.\n>\n> Furthermore in 25715419bf4 (CI: don't run \"make test\" twice in one\n> job, 2021-11-23) when the \"linux-TEST-vars\" job was split off from\n> \"linux-gcc\" the \"cc_package: gcc-8\" line was copied along with\n> it, so its \"cc_package\" line wasn't working as intended either.\n>\n> As a table, this is what's changed by this commit, i.e. it only\n> affects the linux-gcc, linux-TEST-vars and osx-gcc jobs:\n>\n> \t|-------------------+-----------+-------------------+-------+-------|\n> \t| jobname | vector.cc | vector.cc_package | old | new |\n> \t|-------------------+-----------+-------------------+-------+-------|\n> \t| linux-clang | clang | - | clang | clang |\n> \t| linux-sha256 | clang | - | clang | clang |\n> \t| linux-gcc | gcc | gcc-8 | gcc | gcc-8 |\n> \t| osx-clang | clang | - | clang | clang |\n> \t| osx-gcc | gcc | gcc-9 | clang | gcc-9 |\n> \t| linux-gcc-default | gcc | - | gcc | gcc |\n> \t| linux-TEST-vars | gcc | gcc-8 | gcc | gcc-8 |\n> \t|-------------------+-----------+-------------------+-------+-------|\n>\n> Reported-by: Carlo Arenas \n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n>\n> I just dropped the linux-TEST-vars change from the v2 in lieu of\n> trying to get the wording in the commit message right.\n\nOK. So linux-gcc and linux-gcc-default jobs use different compilers\n(if gcc and gcc-8 are different), osx-gcc uses gcc-9 instead of clang.\n\nThanks. Will queue.\n\n> diff --git a/ci/lib.sh b/ci/lib.sh\n> index cbc2f8f1caa..86e37da9bc5 100755\n> --- a/ci/lib.sh\n> +++ b/ci/lib.sh\n> @@ -122,7 +122,7 @@ then\n> \ttest macos != \"$CI_OS_NAME\" || CI_OS_NAME=osx\n> \tCI_REPO_SLUG=\"$GITHUB_REPOSITORY\"\n> \tCI_JOB_ID=\"$GITHUB_RUN_ID\"\n> -\tCC=\"${CC:-gcc}\"\n> +\tCC=\"${CC_PACKAGE:-${CC:-gcc}}\"\n> \tDONT_SKIP_TAGS=t\n> \n> \tcache_dir=\"$HOME/none\"\n"},{"id":"454311","messageId":"xmqqy1zw4u9p.fsf@gitster.g","threadId":"57772","inReplyTo":"patch-v3-1.1-8b3444ecc87-20220422T092015Z-avarab@gmail.com","subject":"Re: [PATCH v3] CI: select CC based on CC_PACKAGE (again)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-22T22:40:02Z","receivedAt":"2022-04-22T23:05:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Fix a regression in 707d2f2fe86 (CI: use \"$runs_on_pool\", not\n> \"$jobname\" to select packages & config, 2021-11-23).\n>\n> In that commit I changed CC=gcc from CC=gcc-9, but on OSX the \"gcc\" in\n> $PATH points to clang, we need to use gcc-9 instead. Likewise for the\n> linux-gcc job CC=gcc-8 was changed to the implicit CC=gcc, which would\n> select GCC 9.4.0 instead of GCC 8.4.0.\n>\n> Furthermore in 25715419bf4 (CI: don't run \"make test\" twice in one\n> job, 2021-11-23) when the \"linux-TEST-vars\" job was split off from\n> \"linux-gcc\" the \"cc_package: gcc-8\" line was copied along with\n> it, so its \"cc_package\" line wasn't working as intended either.\n>\n> As a table, this is what's changed by this commit, i.e. it only\n> affects the linux-gcc, linux-TEST-vars and osx-gcc jobs:\n>\n> \t|-------------------+-----------+-------------------+-------+-------|\n> \t| jobname | vector.cc | vector.cc_package | old | new |\n> \t|-------------------+-----------+-------------------+-------+-------|\n> \t| linux-clang | clang | - | clang | clang |\n> \t| linux-sha256 | clang | - | clang | clang |\n> \t| linux-gcc | gcc | gcc-8 | gcc | gcc-8 |\n> \t| osx-clang | clang | - | clang | clang |\n> \t| osx-gcc | gcc | gcc-9 | clang | gcc-9 |\n> \t| linux-gcc-default | gcc | - | gcc | gcc |\n> \t| linux-TEST-vars | gcc | gcc-8 | gcc | gcc-8 |\n> \t|-------------------+-----------+-------------------+-------+-------|\n>\n> Reported-by: Carlo Arenas \n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n>\n> I just dropped the linux-TEST-vars change from the v2 in lieu of\n> trying to get the wording in the commit message right.\n\nI plan to fast-track this; the revamp of ci/lib.sh in the other huge\nseries would probably need to be rebased on top, or independently\nfix it the same way.\n\nIn the meantime, here is what I'll throwing into today's 'seen'.\n\ndiff --cc ci/lib.sh\nindex 86e37da9bc,80e89f89b7..0000000000\n--- i/ci/lib.sh\n+++ w/ci/lib.sh\n@@@ -195,6 -224,79 +224,79 @@@ ubuntu-latest\n esac\n \n case \"$jobname\" in\n+ windows-build)\n+ \tsetenv --build NO_PERL NoThanks\n+ \tsetenv --build ARTIFACTS_DIRECTORY artifacts\n+ \t;;\n+ windows-test)\n+ \tsetenv --test MAKEFLAGS \"$COMMON_MAKEFLAGS\"\n+ \t;;\n+ vs-build)\n+ \tsetenv --build DEVELOPER $DEVELOPER\n+ \tsetenv --build SKIP_DASHED_BUILT_INS $SKIP_DASHED_BUILT_INS\n+ \n+ \tsetenv --build NO_PERL NoThanks\n+ \tsetenv --build NO_GETTEXT NoThanks\n+ \tsetenv --build ARTIFACTS_DIRECTORY artifacts\n+ \tsetenv --build INCLUDE_DLLS_IN_ARTIFACTS YesPlease\n+ \tsetenv --build MSVC YesPlease\n+ \n+ \tsetenv --build GIT_CONFIG_COUNT 2\n+ \tsetenv --build GIT_CONFIG_KEY_0 user.name\n+ \tsetenv --build GIT_CONFIG_VALUE_0 CI\n+ \tsetenv --build GIT_CONFIG_KEY_1 user.emailname\n+ \tsetenv --build GIT_CONFIG_VALUE_1 ci@git\n+ \tsetenv --build GIT_CONFIG_VALUE_1 ci@git\n+ \t;;\n+ vs-test)\n+ \tsetenv --test NO_SVN_TESTS YesPlease\n+ \tsetenv --test MAKEFLAGS \"$COMMON_MAKEFLAGS\"\n+ \t;;\n+ linux-gcc)\n -\tCC=gcc\n++\tCC=gcc-8\n+ \tsetenv --test GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME main\n+ \t;;\n+ linux-gcc-default)\n+ \tCC=gcc\n+ \t;;\n+ linux-TEST-vars)\n -\tCC=gcc\n++\tCC=gcc-8\n+ \tsetenv --test GIT_TEST_SPLIT_INDEX yes\n+ \tsetenv --test GIT_TEST_MERGE_ALGORITHM recursive\n+ \tsetenv --test GIT_TEST_FULL_IN_PACK_ARRAY true\n+ \tsetenv --test GIT_TEST_OE_SIZE 10\n+ \tsetenv --test GIT_TEST_OE_DELTA_SIZE 5\n+ \tsetenv --test GIT_TEST_COMMIT_GRAPH 1\n+ \tsetenv --test GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS 1\n+ \tsetenv --test GIT_TEST_MULTI_PACK_INDEX 1\n+ \tsetenv --test GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP 1\n+ \tsetenv --test GIT_TEST_ADD_I_USE_BUILTIN 1\n+ \tsetenv --test GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME master\n+ \tsetenv --test GIT_TEST_WRITE_REV_INDEX 1\n+ \tsetenv --test GIT_TEST_CHECKOUT_WORKERS 2\n+ \t;;\n+ osx-gcc)\n+ \tMAKEFLAGS=\"$MAKEFLAGS PYTHON_PATH=$(which python3)\"\n -\tCC=gcc\n++\tCC=gcc-9\n+ \t;;\n+ osx-clang)\n+ \tMAKEFLAGS=\"$MAKEFLAGS PYTHON_PATH=$(which python2)\"\n+ \tCC=clang\n+ \t;;\n+ linux-clang)\n+ \tCC=clang\n+ \tsetenv --test GIT_TEST_DEFAULT_HASH sha1\n+ \t;;\n+ linux-sha256)\n+ \tCC=clang\n+ \tsetenv --test GIT_TEST_DEFAULT_HASH sha256\n+ \t;;\n+ pedantic)\n+ \tCC=gcc\n+ \t# Don't run the tests; we only care about whether Git can be\n+ \t# built.\n+ \tsetenv --build DEVOPTS pedantic\n+ \t;;\n linux32)\n \tCC=gcc\n \t;;\n\n"},{"id":"454312","messageId":"220423.86fsm4raze.gmgdl@evledraar.gmail.com","threadId":"57772","inReplyTo":"xmqqy1zw4u9p.fsf@gitster.g","subject":"Re: [PATCH v3] CI: select CC based on CC_PACKAGE (again)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-22T22:46:48Z","receivedAt":"2022-04-22T23:10:39Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Apr 22 2022, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> Fix a regression in 707d2f2fe86 (CI: use \"$runs_on_pool\", not\n>> \"$jobname\" to select packages & config, 2021-11-23).\n>>\n>> In that commit I changed CC=gcc from CC=gcc-9, but on OSX the \"gcc\" in\n>> $PATH points to clang, we need to use gcc-9 instead. Likewise for the\n>> linux-gcc job CC=gcc-8 was changed to the implicit CC=gcc, which would\n>> select GCC 9.4.0 instead of GCC 8.4.0.\n>>\n>> Furthermore in 25715419bf4 (CI: don't run \"make test\" twice in one\n>> job, 2021-11-23) when the \"linux-TEST-vars\" job was split off from\n>> \"linux-gcc\" the \"cc_package: gcc-8\" line was copied along with\n>> it, so its \"cc_package\" line wasn't working as intended either.\n>>\n>> As a table, this is what's changed by this commit, i.e. it only\n>> affects the linux-gcc, linux-TEST-vars and osx-gcc jobs:\n>>\n>> \t|-------------------+-----------+-------------------+-------+-------|\n>> \t| jobname | vector.cc | vector.cc_package | old | new |\n>> \t|-------------------+-----------+-------------------+-------+-------|\n>> \t| linux-clang | clang | - | clang | clang |\n>> \t| linux-sha256 | clang | - | clang | clang |\n>> \t| linux-gcc | gcc | gcc-8 | gcc | gcc-8 |\n>> \t| osx-clang | clang | - | clang | clang |\n>> \t| osx-gcc | gcc | gcc-9 | clang | gcc-9 |\n>> \t| linux-gcc-default | gcc | - | gcc | gcc |\n>> \t| linux-TEST-vars | gcc | gcc-8 | gcc | gcc-8 |\n>> \t|-------------------+-----------+-------------------+-------+-------|\n>>\n>> Reported-by: Carlo Arenas \n>> Signed-off-by: Ævar Arnfjörð Bjarmason \n>> ---\n>>\n>> I just dropped the linux-TEST-vars change from the v2 in lieu of\n>> trying to get the wording in the commit message right.\n>\n> I plan to fast-track this; the revamp of ci/lib.sh in the other huge\n> series would probably need to be rebased on top, or independently\n> fix it the same way.\n>\n> In the meantime, here is what I'll throwing into today's 'seen'.\n>\n> diff --cc ci/lib.sh\n> index 86e37da9bc,80e89f89b7..0000000000\n> --- i/ci/lib.sh\n> +++ w/ci/lib.sh\n> @@@ -195,6 -224,79 +224,79 @@@ ubuntu-latest\n> esac\n> \n> case \"$jobname\" in\n> + windows-build)\n> + \tsetenv --build NO_PERL NoThanks\n> + \tsetenv --build ARTIFACTS_DIRECTORY artifacts\n> + \t;;\n> + windows-test)\n> + \tsetenv --test MAKEFLAGS \"$COMMON_MAKEFLAGS\"\n> + \t;;\n> + vs-build)\n> + \tsetenv --build DEVELOPER $DEVELOPER\n> + \tsetenv --build SKIP_DASHED_BUILT_INS $SKIP_DASHED_BUILT_INS\n> + \n> + \tsetenv --build NO_PERL NoThanks\n> + \tsetenv --build NO_GETTEXT NoThanks\n> + \tsetenv --build ARTIFACTS_DIRECTORY artifacts\n> + \tsetenv --build INCLUDE_DLLS_IN_ARTIFACTS YesPlease\n> + \tsetenv --build MSVC YesPlease\n> + \n> + \tsetenv --build GIT_CONFIG_COUNT 2\n> + \tsetenv --build GIT_CONFIG_KEY_0 user.name\n> + \tsetenv --build GIT_CONFIG_VALUE_0 CI\n> + \tsetenv --build GIT_CONFIG_KEY_1 user.emailname\n> + \tsetenv --build GIT_CONFIG_VALUE_1 ci@git\n> + \tsetenv --build GIT_CONFIG_VALUE_1 ci@git\n> + \t;;\n> + vs-test)\n> + \tsetenv --test NO_SVN_TESTS YesPlease\n> + \tsetenv --test MAKEFLAGS \"$COMMON_MAKEFLAGS\"\n> + \t;;\n> + linux-gcc)\n> -\tCC=gcc\n> ++\tCC=gcc-8\n> + \tsetenv --test GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME main\n> + \t;;\n> + linux-gcc-default)\n> + \tCC=gcc\n> + \t;;\n> + linux-TEST-vars)\n> -\tCC=gcc\n> ++\tCC=gcc-8\n> + \tsetenv --test GIT_TEST_SPLIT_INDEX yes\n> + \tsetenv --test GIT_TEST_MERGE_ALGORITHM recursive\n> + \tsetenv --test GIT_TEST_FULL_IN_PACK_ARRAY true\n> + \tsetenv --test GIT_TEST_OE_SIZE 10\n> + \tsetenv --test GIT_TEST_OE_DELTA_SIZE 5\n> + \tsetenv --test GIT_TEST_COMMIT_GRAPH 1\n> + \tsetenv --test GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS 1\n> + \tsetenv --test GIT_TEST_MULTI_PACK_INDEX 1\n> + \tsetenv --test GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP 1\n> + \tsetenv --test GIT_TEST_ADD_I_USE_BUILTIN 1\n> + \tsetenv --test GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME master\n> + \tsetenv --test GIT_TEST_WRITE_REV_INDEX 1\n> + \tsetenv --test GIT_TEST_CHECKOUT_WORKERS 2\n> + \t;;\n> + osx-gcc)\n> + \tMAKEFLAGS=\"$MAKEFLAGS PYTHON_PATH=$(which python3)\"\n> -\tCC=gcc\n> ++\tCC=gcc-9\n> + \t;;\n> + osx-clang)\n> + \tMAKEFLAGS=\"$MAKEFLAGS PYTHON_PATH=$(which python2)\"\n> + \tCC=clang\n> + \t;;\n> + linux-clang)\n> + \tCC=clang\n> + \tsetenv --test GIT_TEST_DEFAULT_HASH sha1\n> + \t;;\n> + linux-sha256)\n> + \tCC=clang\n> + \tsetenv --test GIT_TEST_DEFAULT_HASH sha256\n> + \t;;\n> + pedantic)\n> + \tCC=gcc\n> + \t# Don't run the tests; we only care about whether Git can be\n> + \t# built.\n> + \tsetenv --build DEVOPTS pedantic\n> + \t;;\n> linux32)\n> \tCC=gcc\n> \t;;\n\nThis matches the resolution I have for it. noted here:\nhttps://lore.kernel.org/git/cover-v5-00.29-00000000000-20220421T181526Z-avarab@gmail.com/\n\nI.e. it was just those CC=gcc lines for those two jobs that needed to be\ntweaked to have CC=gcc-8 and CC=gcc-9, respectively. Then likewise for\nlinux-TEST-vars (not in my version, since that was against the v2 of the\nseries, but correct here).\n\nThanks!\n"}]}