[PATCH] CI: select CC based on CC_PACKAGE (again)

[PATCH] CI: select CC based on CC_PACKAGE (again)

[Ævar Arnfjörð Bjarmason]

Fix a regression in 707d2f2fe86 (CI: use "$runs_on_pool", not
"$jobname" to select packages & config, 2021-11-23).

In that commit I changed CC=gcc from CC=gcc-9, but on OSX the "gcc" in
$PATH points to clang, we need to use gcc-9 instead. Likewise for the
linux-gcc job CC=gcc-8 was changed to the implicit CC=gcc, which would
select GCC 9.4.0 instead of GCC 8.4.0.

Furthermore in 25715419bf4 (CI: don't run "make test" twice in one
job, 2021-11-23) when the "linux-TEST-vars" job was split off from
"linux-gcc" the "cc_package: gcc-8" line was copied along with
it.

That wasn't a bug per-se, as that "make test" would have run under GCC
8 before the split into two jobs, but the point of selecting different
compiler for these jobs is to get better coverage, and to narrow down
any issues with a given compiler to the job that runs it. Since the
"linux-TEST-vars" job is already special in other ways (in running
with various GIT_TEST_* variables), and we've got the "linux-gcc" job
covering gcc-8 let's have it used the default system compiler instead.

Reported-by: Carlo Arenas <carenas@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Range-diff:
1:  94abb826627 < -:  ----------- CI: have osx-gcc use gcc, not clang
-:  ----------- > 1:  d89ad4d5b7c CI: select CC based on CC_PACKAGE (again)

The range-diff is against a relevant patch in v4 of a larger CI series
of mine[1]. This fix is independent of that, and on top of master. See
[2] for the original fix and discussion.

As noted in the updated commit message not only the OSX jobs were
affected, but also the linux-gcc job. This fixes both, along with a
small fix to the related linux-TEST-vars recipe.

1. https://lore.kernel.org/git/cover-v4-00.31-00000000000-20220418T132809Z-avarab@gmail.com/
2. https://lore.kernel.org/git/patch-v4-30.31-94abb826627-20220418T132809Z-avarab@gmail.com/

 .github/workflows/main.yml | 1 -
 ci/lib.sh                  | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index c35200defb9..f12819a00d7 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -236,7 +236,6 @@ jobs:
           - jobname: linux-TEST-vars
             cc: gcc
             os: ubuntu
-            cc_package: gcc-8
             pool: ubuntu-latest
           - jobname: osx-clang
             cc: clang
diff --git a/ci/lib.sh b/ci/lib.sh
index cbc2f8f1caa..44007dcf93b 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -122,7 +122,7 @@ then
 	test macos != "$CI_OS_NAME" || CI_OS_NAME=osx
 	CI_REPO_SLUG="$GITHUB_REPOSITORY"
 	CI_JOB_ID="$GITHUB_RUN_ID"
-	CC="${CC:-gcc}"
+	CC="${CC:-${CC_PACKAGE:-gcc}}"
 	DONT_SKIP_TAGS=t
 
 	cache_dir="$HOME/none"
-- 
2.36.0.876.g4bfefc07680

Re: [PATCH] CI: select CC based on CC_PACKAGE (again)

[Phillip Wood]

Hi Ævar

On 21/04/2022 14:03, Ævar Arnfjörð Bjarmason wrote:
> Fix a regression in 707d2f2fe86 (CI: use "$runs_on_pool", not
> "$jobname" to select packages & config, 2021-11-23).
> 
> In that commit I changed CC=gcc from CC=gcc-9, but on OSX the "gcc" in
> $PATH points to clang, we need to use gcc-9 instead. Likewise for the
> linux-gcc job CC=gcc-8 was changed to the implicit CC=gcc, which would
> select GCC 9.4.0 instead of GCC 8.4.0.
> 
> Furthermore in 25715419bf4 (CI: don't run "make test" twice in one
> job, 2021-11-23) when the "linux-TEST-vars" job was split off from
> "linux-gcc" the "cc_package: gcc-8" line was copied along with
> it.
> 
> That wasn't a bug per-se, as that "make test" would have run under GCC
> 8 before the split into two jobs, but the point of selecting different
> compiler for these jobs is to get better coverage, and to narrow down
> any issues with a given compiler to the job that runs it. Since the
> "linux-TEST-vars" job is already special in other ways (in running
> with various GIT_TEST_* variables), and we've got the "linux-gcc" job
> covering gcc-8 let's have it used the default system compiler instead.
> 
> Reported-by: Carlo Arenas <carenas@gmail.com>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> Range-diff:
> 1:  94abb826627 < -:  ----------- CI: have osx-gcc use gcc, not clang
> -:  ----------- > 1:  d89ad4d5b7c CI: select CC based on CC_PACKAGE (again)
> 
> The range-diff is against a relevant patch in v4 of a larger CI series
> of mine[1]. This fix is independent of that, and on top of master. See
> [2] for the original fix and discussion.
> 
> As noted in the updated commit message not only the OSX jobs were
> affected, but also the linux-gcc job. This fixes both, along with a
> small fix to the related linux-TEST-vars recipe.
> 
> 1. https://lore.kernel.org/git/cover-v4-00.31-00000000000-20220418T132809Z-avarab@gmail.com/
> 2. https://lore.kernel.org/git/patch-v4-30.31-94abb826627-20220418T132809Z-avarab@gmail.com/
> 
>   .github/workflows/main.yml | 1 -
>   ci/lib.sh                  | 2 +-
>   2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> index c35200defb9..f12819a00d7 100644
> --- a/.github/workflows/main.yml
> +++ b/.github/workflows/main.yml
> @@ -236,7 +236,6 @@ jobs:
>             - jobname: linux-TEST-vars
>               cc: gcc
>               os: ubuntu
> -            cc_package: gcc-8
>               pool: ubuntu-latest
>             - jobname: osx-clang
>               cc: clang
> diff --git a/ci/lib.sh b/ci/lib.sh
> index cbc2f8f1caa..44007dcf93b 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -122,7 +122,7 @@ then
>   	test macos != "$CI_OS_NAME" || CI_OS_NAME=osx
>   	CI_REPO_SLUG="$GITHUB_REPOSITORY"
>   	CI_JOB_ID="$GITHUB_RUN_ID"
> -	CC="${CC:-gcc}"
> +	CC="${CC:-${CC_PACKAGE:-gcc}}"

CC is set in .github/workflows/main.yaml for the ubuntu and macos jobs 
so I think they will not fallback to using CC_PACKAGE and therefore not 
pick up the correct compiler.

Best Wishes

Phillip

>   	DONT_SKIP_TAGS=t
>   
>   	cache_dir="$HOME/none"

[PATCH v2] CI: select CC based on CC_PACKAGE (again)

[Ævar Arnfjörð Bjarmason]

Fix a regression in 707d2f2fe86 (CI: use "$runs_on_pool", not
"$jobname" to select packages & config, 2021-11-23).

In that commit I changed CC=gcc from CC=gcc-9, but on OSX the "gcc" in
$PATH points to clang, we need to use gcc-9 instead. Likewise for the
linux-gcc job CC=gcc-8 was changed to the implicit CC=gcc, which would
select GCC 9.4.0 instead of GCC 8.4.0.

Furthermore in 25715419bf4 (CI: don't run "make test" twice in one
job, 2021-11-23) when the "linux-TEST-vars" job was split off from
"linux-gcc" the "cc_package: gcc-8" line was copied along with
it.

That wasn't a bug per-se, as that "make test" would have run under GCC
8 before the split into two jobs, but the point of selecting different
compiler for these jobs is to get better coverage, and to narrow down
any issues with a given compiler to the job that runs it. Since the
"linux-TEST-vars" job is already special in other ways (in running
with various GIT_TEST_* variables), and we've got the "linux-gcc" job
covering gcc-8 let's have it used the default system compiler instead.

Reported-by: Carlo Arenas <carenas@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

On Thu, Apr 21 2022, Phillip Wood wrote:

> CC is set in .github/workflows/main.yaml for the ubuntu and macos jobs
> so I think they will not fallback to using CC_PACKAGE and therefore
> not pick up the correct compiler.

Urgh, sorry. I don't know how I got that turned around, but this
version works, and the (currently ongoing) CI run shows that the
linux-gcc job selects gcc-8 correctly, instead of gcc:

    https://github.com/avar/git/runs/6116735686?check_suite_focus=true#step:3:22

The osx-gcc job will be likewise fixed, but it's failing in that run
due to the unrelated perforce installation issue affecting all OSX
jobs (fix here:
https://lore.kernel.org/git/cover-0.2-00000000000-20220421T124225Z-avarab@gmail.com/).

Range-diff against v1:
1:  d89ad4d5b7c ! 1:  92acf9420a9 CI: select CC based on CC_PACKAGE (again)
    @@ ci/lib.sh: then
      	CI_REPO_SLUG="$GITHUB_REPOSITORY"
      	CI_JOB_ID="$GITHUB_RUN_ID"
     -	CC="${CC:-gcc}"
    -+	CC="${CC:-${CC_PACKAGE:-gcc}}"
    ++	CC="${CC_PACKAGE:-${CC:-gcc}}"
      	DONT_SKIP_TAGS=t
      
      	cache_dir="$HOME/none"

 .github/workflows/main.yml | 1 -
 ci/lib.sh                  | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index c35200defb9..f12819a00d7 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -236,7 +236,6 @@ jobs:
           - jobname: linux-TEST-vars
             cc: gcc
             os: ubuntu
-            cc_package: gcc-8
             pool: ubuntu-latest
           - jobname: osx-clang
             cc: clang
diff --git a/ci/lib.sh b/ci/lib.sh
index cbc2f8f1caa..86e37da9bc5 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -122,7 +122,7 @@ then
 	test macos != "$CI_OS_NAME" || CI_OS_NAME=osx
 	CI_REPO_SLUG="$GITHUB_REPOSITORY"
 	CI_JOB_ID="$GITHUB_RUN_ID"
-	CC="${CC:-gcc}"
+	CC="${CC_PACKAGE:-${CC:-gcc}}"
 	DONT_SKIP_TAGS=t
 
 	cache_dir="$HOME/none"
-- 
2.36.0.879.g3659959fcca

Re: [PATCH v2] CI: select CC based on CC_PACKAGE (again)

[Carlo Arenas]

On Thu, Apr 21, 2022 at 10:48 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
> Reported-by: Carlo Arenas <carenas@gmail.com>

to avoid confusing the attribution scripts better use my full name:
"Carlo Marcelo Arenas Belón"

> diff --git a/ci/lib.sh b/ci/lib.sh
> index cbc2f8f1caa..86e37da9bc5 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -122,7 +122,7 @@ then
>         test macos != "$CI_OS_NAME" || CI_OS_NAME=osx
>         CI_REPO_SLUG="$GITHUB_REPOSITORY"
>         CI_JOB_ID="$GITHUB_RUN_ID"
> -       CC="${CC:-gcc}"
> +       CC="${CC_PACKAGE:-${CC:-gcc}}"

minor nitpick, but most likely still relevant considering your other
"bashism" fixes.
the POSIX syntax doesn't use ":" (documented in CodingGuidelines)

not sure if the compounded assignment is valid in POSIX, but at least
the following worked with a recent NetBSD 8 sh :

  CC="${CC_PACKAGE-${CC-gcc}}"

Carlo

Re: [PATCH v2] CI: select CC based on CC_PACKAGE (again)

[Junio C Hamano]

Carlo Arenas <carenas@gmail.com> writes:

>> -       CC="${CC:-gcc}"
>> +       CC="${CC_PACKAGE:-${CC:-gcc}}"
>
> minor nitpick, but most likely still relevant considering your other
> "bashism" fixes.
> the POSIX syntax doesn't use ":" (documented in CodingGuidelines)

You are reading the guideline wrong, I am afraid.

This is not a substring substitution, ${foo:0:3} picking three
characters from the beginning of foo.

It is "If CC_PACKAGE is UNSET OR SET TO EMPTY, then use the fallback
value that is ${CC:-gcc}", which is a bog standard bourne shell that
predates POSIX.  The colon before "-" is what adds "OR SET TO EMPTY"
part; without the colon, i.e. "${CC_PACKAGE-${CC-gcc}}", then you
get the same without "OR SET TO EMPTY" part.

And we do use that form.

Re: [PATCH v2] CI: select CC based on CC_PACKAGE (again)

[Carlo Arenas]

On Thu, Apr 21, 2022 at 12:12 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Carlo Arenas <carenas@gmail.com> writes:
>
> >> -       CC="${CC:-gcc}"
> >> +       CC="${CC_PACKAGE:-${CC:-gcc}}"
> >
> > minor nitpick, but most likely still relevant considering your other
> > "bashism" fixes.
> > the POSIX syntax doesn't use ":" (documented in CodingGuidelines)
>
> You are reading the guideline wrong, I am afraid.

Indeed; and I realized it as I was trying to answer my own question
about the compounded replacement being supported.

Wondering if something more explicit might be easier to understand anyway like :

  if test -n "$CC_PACKAGE"
  then
     CC="$CC_PACKAGE"
  fi

Carlo

Re: [PATCH v2] CI: select CC based on CC_PACKAGE (again)

[Junio C Hamano]

Carlo Arenas <carenas@gmail.com> writes:

> On Thu, Apr 21, 2022 at 12:12 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Carlo Arenas <carenas@gmail.com> writes:
>>
>> >> -       CC="${CC:-gcc}"
>> >> +       CC="${CC_PACKAGE:-${CC:-gcc}}"
>> > ...
> Wondering if something more explicit might be easier to understand anyway like :
>
>   if test -n "$CC_PACKAGE"
>   then
>      CC="$CC_PACKAGE"
>   fi
>
> Carlo

The parameter substitution is commonly used part of the shell
language, and even if developers, when starting to work on parts of
the system that are written in shell, are initially unfamiliar with
such language constructs, nobody will stay a newbie forever.  

To them, idioms like ${A:-${B:-C}} would quickly become as explicit
as a long-form written as if/then/elif/fi.  Surely it takes practice
to be familiar with the idiom, but idioms help those who know them
to be more efficient and effetive, so, no, I do not think it is a
good idea to aim for "easier to understand by newbies".

Thanks.

Re: [PATCH v2] CI: select CC based on CC_PACKAGE (again)

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Fix a regression in 707d2f2fe86 (CI: use "$runs_on_pool", not
> "$jobname" to select packages & config, 2021-11-23).
>
> In that commit I changed CC=gcc from CC=gcc-9, but on OSX the "gcc" in
> $PATH points to clang, we need to use gcc-9 instead. Likewise for the
> linux-gcc job CC=gcc-8 was changed to the implicit CC=gcc, which would
> select GCC 9.4.0 instead of GCC 8.4.0.

Thanks for diagnosing how things were broken.

> On Thu, Apr 21 2022, Phillip Wood wrote:
>
>> CC is set in .github/workflows/main.yaml for the ubuntu and macos jobs
>> so I think they will not fallback to using CC_PACKAGE and therefore
>> not pick up the correct compiler.
> ...
> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> index c35200defb9..f12819a00d7 100644
> --- a/.github/workflows/main.yml
> +++ b/.github/workflows/main.yml
> @@ -236,7 +236,6 @@ jobs:
>            - jobname: linux-TEST-vars
>              cc: gcc
>              os: ubuntu
> -            cc_package: gcc-8
>              pool: ubuntu-latest
>            - jobname: osx-clang
>              cc: clang
> diff --git a/ci/lib.sh b/ci/lib.sh
> index cbc2f8f1caa..86e37da9bc5 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -122,7 +122,7 @@ then
>  	test macos != "$CI_OS_NAME" || CI_OS_NAME=osx
>  	CI_REPO_SLUG="$GITHUB_REPOSITORY"
>  	CI_JOB_ID="$GITHUB_RUN_ID"
> -	CC="${CC:-gcc}"
> +	CC="${CC_PACKAGE:-${CC:-gcc}}"
>  	DONT_SKIP_TAGS=t
>  
>  	cache_dir="$HOME/none"

OK, so we favor CC_PACKAGE (from the matrix.vector.cc_package) if
set, and then cc (again, from the matrix.vector.cc) if set, and then
finally use "gcc" as a fallback.  In the osx-gcc job, cc_package is
set to gcc-9 while in the osx-clang, cc is gcc that confusingly calls
for clang.  That sounds like it would do the right thing for two
macs.

For other jobs with different settings for cc and cc_package, does
this have any effect?  I do not think I saw any mention in the
proposed log message.

		vector.cc	vector.cc_package	old	new
linux-clang	clang		-			clang	clang
linux-sha256	clang		-			clang	clang
linux-gcc	gcc		gcc-8			gcc	gcc-8
osx-clang	clang		-			clang	clang
osx-gcc		gcc		gcc-9			clang	gcc-9
linux-gcc-default gcc		-			gcc	gcc

So, linux-gcc job used to use whichever "gcc" the platform gave us,
but now it explicitly asks for gcc-8, which may or may not be
different from what linux-gcc-default uses, and there is no other
difference by this change.  We may get a better test coverage (if
the default gcc is not gcc-8) or no improvement (if the default is
gcc-8), so it is a strict improvement worth recording as an intended
side effect in the proposed log message to help future developers.

Other than that, looks good to me.

Re: [PATCH v2] CI: select CC based on CC_PACKAGE (again)

[Ævar Arnfjörð Bjarmason]

On Thu, Apr 21 2022, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> Fix a regression in 707d2f2fe86 (CI: use "$runs_on_pool", not
>> "$jobname" to select packages & config, 2021-11-23).
>>
>> In that commit I changed CC=gcc from CC=gcc-9, but on OSX the "gcc" in
>> $PATH points to clang, we need to use gcc-9 instead. Likewise for the
>> linux-gcc job CC=gcc-8 was changed to the implicit CC=gcc, which would
>> select GCC 9.4.0 instead of GCC 8.4.0.
>
> Thanks for diagnosing how things were broken.
>
>> On Thu, Apr 21 2022, Phillip Wood wrote:
>>
>>> CC is set in .github/workflows/main.yaml for the ubuntu and macos jobs
>>> so I think they will not fallback to using CC_PACKAGE and therefore
>>> not pick up the correct compiler.
>> ...
>> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
>> index c35200defb9..f12819a00d7 100644
>> --- a/.github/workflows/main.yml
>> +++ b/.github/workflows/main.yml
>> @@ -236,7 +236,6 @@ jobs:
>>            - jobname: linux-TEST-vars
>>              cc: gcc
>>              os: ubuntu
>> -            cc_package: gcc-8
>>              pool: ubuntu-latest
>>            - jobname: osx-clang
>>              cc: clang
>> diff --git a/ci/lib.sh b/ci/lib.sh
>> index cbc2f8f1caa..86e37da9bc5 100755
>> --- a/ci/lib.sh
>> +++ b/ci/lib.sh
>> @@ -122,7 +122,7 @@ then
>>  	test macos != "$CI_OS_NAME" || CI_OS_NAME=osx
>>  	CI_REPO_SLUG="$GITHUB_REPOSITORY"
>>  	CI_JOB_ID="$GITHUB_RUN_ID"
>> -	CC="${CC:-gcc}"
>> +	CC="${CC_PACKAGE:-${CC:-gcc}}"
>>  	DONT_SKIP_TAGS=t
>>  
>>  	cache_dir="$HOME/none"
>
> OK, so we favor CC_PACKAGE (from the matrix.vector.cc_package) if
> set, and then cc (again, from the matrix.vector.cc) if set, and then
> finally use "gcc" as a fallback.  In the osx-gcc job, cc_package is
> set to gcc-9 while in the osx-clang, cc is gcc that confusingly calls
> for clang.  That sounds like it would do the right thing for two
> macs.

Yes.

> For other jobs with different settings for cc and cc_package, does
> this have any effect?  I do not think I saw any mention in the
> proposed log message.
>
> 		vector.cc	vector.cc_package	old	new
> linux-clang	clang		-			clang	clang
> linux-sha256	clang		-			clang	clang
> linux-gcc	gcc		gcc-8			gcc	gcc-8
> osx-clang	clang		-			clang	clang
> osx-gcc		gcc		gcc-9			clang	gcc-9
> linux-gcc-default gcc		-			gcc	gcc
>
> So, linux-gcc job used to use whichever "gcc" the platform gave us,
> but now it explicitly asks for gcc-8, which may or may not be
> different from what linux-gcc-default uses, and there is no other
> difference by this change.  We may get a better test coverage (if
> the default gcc is not gcc-8) or no improvement (if the default is
> gcc-8), so it is a strict improvement worth recording as an intended
> side effect in the proposed log message to help future developers.
>
> Other than that, looks good to me.

I'm happy to rephrase it however you'd like, but I'm a bit confused by
the "saw any mention in the proposed log message". I'm fairly sure
paragraph 2 onwards covers this, i.e. how linux-gcc's behavior is
changed (as it also regressed).

What I suppose is left undiscussed is that jobs that don't define
CC_PACKAGE at all won't be impacted, is that what you wanted to be
explicitly mentioned?

Re: [PATCH v2] CI: select CC based on CC_PACKAGE (again)

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> I'm happy to rephrase it however you'd like, but I'm a bit confused by
> the "saw any mention in the proposed log message". I'm fairly sure
> paragraph 2 onwards covers this, i.e. how linux-gcc's behavior is
> changed (as it also regressed).

Yeah, true, """Likewise for the linux-gcc job CC=gcc-8 was changed
to the implicit CC=gcc, which would select GCC 9.4.0 instead of GCC
8.4.0.""" that you wrote is exactly about it.

I was confused because immediately after that you said it was not a
bug, so I dismissed it as not part or the real issue.  Perhaps
striking that "not a bug" part may make it less confusing?  I dunno.

> What I suppose is left undiscussed is that jobs that don't define
> CC_PACKAGE at all won't be impacted, is that what you wanted to be
> explicitly mentioned?

Yes, I agree that is a good idea, too.  Or you can steal the "before
and after" table I gave you in the message you are responding to, if
you think it helps.

Thanks.

Re: [PATCH v2] CI: select CC based on CC_PACKAGE (again)

[Ævar Arnfjörð Bjarmason]

On Thu, Apr 21 2022, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> I'm happy to rephrase it however you'd like, but I'm a bit confused by
>> the "saw any mention in the proposed log message". I'm fairly sure
>> paragraph 2 onwards covers this, i.e. how linux-gcc's behavior is
>> changed (as it also regressed).
>
> Yeah, true, """Likewise for the linux-gcc job CC=gcc-8 was changed
> to the implicit CC=gcc, which would select GCC 9.4.0 instead of GCC
> 8.4.0.""" that you wrote is exactly about it.
>
> I was confused because immediately after that you said it was not a
> bug, so I dismissed it as not part or the real issue.  Perhaps
> striking that "not a bug" part may make it less confusing?  I dunno.

I'll rephrase it, but I probably won't get to it tonight, sorry.

What I was trying to get at is that the linux-TEST-vars job was
affected, and *does* have a CC_PACKAGE defined, but in that case (and I
was the one who split it out) the point was never to have it use its own
compiler. For any of these platform/compiler combinations it's enough if
we cover that platform/compiler verison once.

So the right thing to do with linux-TEST-vars is to just have it use
"vanilla", because it's special in other ways. So if and when it fails
we don't want to worry about untangling the two variables. I.e. is it
the special-versioned compiler, or is it (and it almost always would be)
the GIT_TEST_* variables. In that case we should only need to worry
about the latter.

>> What I suppose is left undiscussed is that jobs that don't define
>> CC_PACKAGE at all won't be impacted, is that what you wanted to be
>> explicitly mentioned?
>
> Yes, I agree that is a good idea, too.  Or you can steal the "before
> and after" table I gave you in the message you are responding to, if
> you think it helps.

*nod*

Re: [PATCH v2] CI: select CC based on CC_PACKAGE (again)

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> I'm happy to rephrase it however you'd like, but I'm a bit confused by
>> the "saw any mention in the proposed log message". I'm fairly sure
>> paragraph 2 onwards covers this, i.e. how linux-gcc's behavior is
>> changed (as it also regressed).
>
> Yeah, true, """Likewise for the linux-gcc job CC=gcc-8 was changed
> to the implicit CC=gcc, which would select GCC 9.4.0 instead of GCC
> 8.4.0.""" that you wrote is exactly about it.
>
> I was confused because immediately after that you said it was not a
> bug, so I dismissed it as not part or the real issue.  Perhaps
> striking that "not a bug" part may make it less confusing?  I dunno.

Also, the description above merely says we use gcc-9 instead of gcc-8,
without saying if that is a good change or not, which is another thing
that needs to be said.  In the larger context, if we aim to try different
compilers, we should know which other ones we are using, and if linux-gcc
job is meant to test gcc-8 (because we know there is another job
that tests gcc-9), the change was a bug.  But because you said it
was not a bug, it made it unclear if that is merely an observation
or it was pointing out our earlier mistake that should be corrected.

[PATCH v3] CI: select CC based on CC_PACKAGE (again)

[Ævar Arnfjörð Bjarmason]

Fix a regression in 707d2f2fe86 (CI: use "$runs_on_pool", not
"$jobname" to select packages & config, 2021-11-23).

In that commit I changed CC=gcc from CC=gcc-9, but on OSX the "gcc" in
$PATH points to clang, we need to use gcc-9 instead. Likewise for the
linux-gcc job CC=gcc-8 was changed to the implicit CC=gcc, which would
select GCC 9.4.0 instead of GCC 8.4.0.

Furthermore in 25715419bf4 (CI: don't run "make test" twice in one
job, 2021-11-23) when the "linux-TEST-vars" job was split off from
"linux-gcc" the "cc_package: gcc-8" line was copied along with
it, so its "cc_package" line wasn't working as intended either.

As a table, this is what's changed by this commit, i.e. it only
affects the linux-gcc, linux-TEST-vars and osx-gcc jobs:

	|-------------------+-----------+-------------------+-------+-------|
	| jobname           | vector.cc | vector.cc_package | old   | new   |
	|-------------------+-----------+-------------------+-------+-------|
	| linux-clang       | clang     | -                 | clang | clang |
	| linux-sha256      | clang     | -                 | clang | clang |
	| linux-gcc         | gcc       | gcc-8             | gcc   | gcc-8 |
	| osx-clang         | clang     | -                 | clang | clang |
	| osx-gcc           | gcc       | gcc-9             | clang | gcc-9 |
	| linux-gcc-default | gcc       | -                 | gcc   | gcc   |
	| linux-TEST-vars   | gcc       | gcc-8             | gcc   | gcc-8 |
	|-------------------+-----------+-------------------+-------+-------|

Reported-by: Carlo Arenas <carenas@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

I just dropped the linux-TEST-vars change from the v2 in lieu of
trying to get the wording in the commit message right.

Range-diff against v2:
1:  92acf9420a9 ! 1:  8b3444ecc87 CI: select CC based on CC_PACKAGE (again)
    @@ Commit message
         Furthermore in 25715419bf4 (CI: don't run "make test" twice in one
         job, 2021-11-23) when the "linux-TEST-vars" job was split off from
         "linux-gcc" the "cc_package: gcc-8" line was copied along with
    -    it.
    -
    -    That wasn't a bug per-se, as that "make test" would have run under GCC
    -    8 before the split into two jobs, but the point of selecting different
    -    compiler for these jobs is to get better coverage, and to narrow down
    -    any issues with a given compiler to the job that runs it. Since the
    -    "linux-TEST-vars" job is already special in other ways (in running
    -    with various GIT_TEST_* variables), and we've got the "linux-gcc" job
    -    covering gcc-8 let's have it used the default system compiler instead.
    +    it, so its "cc_package" line wasn't working as intended either.
    +
    +    As a table, this is what's changed by this commit, i.e. it only
    +    affects the linux-gcc, linux-TEST-vars and osx-gcc jobs:
    +
    +            |-------------------+-----------+-------------------+-------+-------|
    +            | jobname           | vector.cc | vector.cc_package | old   | new   |
    +            |-------------------+-----------+-------------------+-------+-------|
    +            | linux-clang       | clang     | -                 | clang | clang |
    +            | linux-sha256      | clang     | -                 | clang | clang |
    +            | linux-gcc         | gcc       | gcc-8             | gcc   | gcc-8 |
    +            | osx-clang         | clang     | -                 | clang | clang |
    +            | osx-gcc           | gcc       | gcc-9             | clang | gcc-9 |
    +            | linux-gcc-default | gcc       | -                 | gcc   | gcc   |
    +            | linux-TEST-vars   | gcc       | gcc-8             | gcc   | gcc-8 |
    +            |-------------------+-----------+-------------------+-------+-------|
     
         Reported-by: Carlo Arenas <carenas@gmail.com>
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
    - ## .github/workflows/main.yml ##
    -@@ .github/workflows/main.yml: jobs:
    -           - jobname: linux-TEST-vars
    -             cc: gcc
    -             os: ubuntu
    --            cc_package: gcc-8
    -             pool: ubuntu-latest
    -           - jobname: osx-clang
    -             cc: clang
    -
      ## ci/lib.sh ##
     @@ ci/lib.sh: then
      	test macos != "$CI_OS_NAME" || CI_OS_NAME=osx

 ci/lib.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ci/lib.sh b/ci/lib.sh
index cbc2f8f1caa..86e37da9bc5 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -122,7 +122,7 @@ then
 	test macos != "$CI_OS_NAME" || CI_OS_NAME=osx
 	CI_REPO_SLUG="$GITHUB_REPOSITORY"
 	CI_JOB_ID="$GITHUB_RUN_ID"
-	CC="${CC:-gcc}"
+	CC="${CC_PACKAGE:-${CC:-gcc}}"
 	DONT_SKIP_TAGS=t
 
 	cache_dir="$HOME/none"
-- 
2.36.0.879.g56a83971f3f

Re: [PATCH v3] CI: select CC based on CC_PACKAGE (again)

[Phillip Wood]

Hi Ævar

> On 22 April 2022 at 10:20 Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> 
> 
> Fix a regression in 707d2f2fe86 (CI: use "$runs_on_pool", not
> "$jobname" to select packages & config, 2021-11-23).
> 
> In that commit I changed CC=gcc from CC=gcc-9, but on OSX the "gcc" in
> $PATH points to clang, we need to use gcc-9 instead. Likewise for the
> linux-gcc job CC=gcc-8 was changed to the implicit CC=gcc, which would
> select GCC 9.4.0 instead of GCC 8.4.0.
> 
> Furthermore in 25715419bf4 (CI: don't run "make test" twice in one
> job, 2021-11-23) when the "linux-TEST-vars" job was split off from
> "linux-gcc" the "cc_package: gcc-8" line was copied along with
> it, so its "cc_package" line wasn't working as intended either.
> 
> As a table, this is what's changed by this commit, i.e. it only
> affects the linux-gcc, linux-TEST-vars and osx-gcc jobs:
> 
> 	|-------------------+-----------+-------------------+-------+-------|
> 	| jobname           | vector.cc | vector.cc_package | old   | new   |
> 	|-------------------+-----------+-------------------+-------+-------|
> 	| linux-clang       | clang     | -                 | clang | clang |
> 	| linux-sha256      | clang     | -                 | clang | clang |
> 	| linux-gcc         | gcc       | gcc-8             | gcc   | gcc-8 |
> 	| osx-clang         | clang     | -                 | clang | clang |
> 	| osx-gcc           | gcc       | gcc-9             | clang | gcc-9 |
> 	| linux-gcc-default | gcc       | -                 | gcc   | gcc   |
> 	| linux-TEST-vars   | gcc       | gcc-8             | gcc   | gcc-8 |
> 	|-------------------+-----------+-------------------+-------+-------|

Having this table is helpful. I do find it confusing that we're still setting 
CC in the main.yaml and then overriding it in lib.sh. I think it would make 
things clearer if we got find of the cc settings in the job matrix.

Best Wishes

Phillip

> Reported-by: Carlo Arenas <carenas@gmail.com>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> 
> I just dropped the linux-TEST-vars change from the v2 in lieu of
> trying to get the wording in the commit message right.
> 
> Range-diff against v2:
> 1:  92acf9420a9 ! 1:  8b3444ecc87 CI: select CC based on CC_PACKAGE (again)
>     @@ Commit message
>          Furthermore in 25715419bf4 (CI: don't run "make test" twice in one
>          job, 2021-11-23) when the "linux-TEST-vars" job was split off from
>          "linux-gcc" the "cc_package: gcc-8" line was copied along with
>     -    it.
>     -
>     -    That wasn't a bug per-se, as that "make test" would have run under GCC
>     -    8 before the split into two jobs, but the point of selecting different
>     -    compiler for these jobs is to get better coverage, and to narrow down
>     -    any issues with a given compiler to the job that runs it. Since the
>     -    "linux-TEST-vars" job is already special in other ways (in running
>     -    with various GIT_TEST_* variables), and we've got the "linux-gcc" job
>     -    covering gcc-8 let's have it used the default system compiler instead.
>     +    it, so its "cc_package" line wasn't working as intended either.
>     +
>     +    As a table, this is what's changed by this commit, i.e. it only
>     +    affects the linux-gcc, linux-TEST-vars and osx-gcc jobs:
>     +
>     +            |-------------------+-----------+-------------------+-------+-------|
>     +            | jobname           | vector.cc | vector.cc_package | old   | new   |
>     +            |-------------------+-----------+-------------------+-------+-------|
>     +            | linux-clang       | clang     | -                 | clang | clang |
>     +            | linux-sha256      | clang     | -                 | clang | clang |
>     +            | linux-gcc         | gcc       | gcc-8             | gcc   | gcc-8 |
>     +            | osx-clang         | clang     | -                 | clang | clang |
>     +            | osx-gcc           | gcc       | gcc-9             | clang | gcc-9 |
>     +            | linux-gcc-default | gcc       | -                 | gcc   | gcc   |
>     +            | linux-TEST-vars   | gcc       | gcc-8             | gcc   | gcc-8 |
>     +            |-------------------+-----------+-------------------+-------+-------|
>      
>          Reported-by: Carlo Arenas <carenas@gmail.com>
>          Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>      
>     - ## .github/workflows/main.yml ##
>     -@@ .github/workflows/main.yml: jobs:
>     -           - jobname: linux-TEST-vars
>     -             cc: gcc
>     -             os: ubuntu
>     --            cc_package: gcc-8
>     -             pool: ubuntu-latest
>     -           - jobname: osx-clang
>     -             cc: clang
>     -
>       ## ci/lib.sh ##
>      @@ ci/lib.sh: then
>       	test macos != "$CI_OS_NAME" || CI_OS_NAME=osx
> 
>  ci/lib.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ci/lib.sh b/ci/lib.sh
> index cbc2f8f1caa..86e37da9bc5 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -122,7 +122,7 @@ then
>  	test macos != "$CI_OS_NAME" || CI_OS_NAME=osx
>  	CI_REPO_SLUG="$GITHUB_REPOSITORY"
>  	CI_JOB_ID="$GITHUB_RUN_ID"
> -	CC="${CC:-gcc}"
> +	CC="${CC_PACKAGE:-${CC:-gcc}}"
>  	DONT_SKIP_TAGS=t
>  
>  	cache_dir="$HOME/none"
> -- 
> 2.36.0.879.g56a83971f3f
>

Re: [PATCH v3] CI: select CC based on CC_PACKAGE (again)

[Ævar Arnfjörð Bjarmason]

On Fri, Apr 22 2022, Phillip Wood wrote:

> Hi Ævar
>
>> On 22 April 2022 at 10:20 Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>> 
>> 
>> Fix a regression in 707d2f2fe86 (CI: use "$runs_on_pool", not
>> "$jobname" to select packages & config, 2021-11-23).
>> 
>> In that commit I changed CC=gcc from CC=gcc-9, but on OSX the "gcc" in
>> $PATH points to clang, we need to use gcc-9 instead. Likewise for the
>> linux-gcc job CC=gcc-8 was changed to the implicit CC=gcc, which would
>> select GCC 9.4.0 instead of GCC 8.4.0.
>> 
>> Furthermore in 25715419bf4 (CI: don't run "make test" twice in one
>> job, 2021-11-23) when the "linux-TEST-vars" job was split off from
>> "linux-gcc" the "cc_package: gcc-8" line was copied along with
>> it, so its "cc_package" line wasn't working as intended either.
>> 
>> As a table, this is what's changed by this commit, i.e. it only
>> affects the linux-gcc, linux-TEST-vars and osx-gcc jobs:
>> 
>> 	|-------------------+-----------+-------------------+-------+-------|
>> 	| jobname           | vector.cc | vector.cc_package | old   | new   |
>> 	|-------------------+-----------+-------------------+-------+-------|
>> 	| linux-clang       | clang     | -                 | clang | clang |
>> 	| linux-sha256      | clang     | -                 | clang | clang |
>> 	| linux-gcc         | gcc       | gcc-8             | gcc   | gcc-8 |
>> 	| osx-clang         | clang     | -                 | clang | clang |
>> 	| osx-gcc           | gcc       | gcc-9             | clang | gcc-9 |
>> 	| linux-gcc-default | gcc       | -                 | gcc   | gcc   |
>> 	| linux-TEST-vars   | gcc       | gcc-8             | gcc   | gcc-8 |
>> 	|-------------------+-----------+-------------------+-------+-------|
>
> Having this table is helpful. I do find it confusing that we're still setting 
> CC in the main.yaml and then overriding it in lib.sh. I think it would make 
> things clearer if we got find of the cc settings in the job matrix.

Yes, this stand-alone patch is just a band-aid.

Do you like the state at the tip of my larger CI series? It's only set
in ci/lib.sh there, or in the case of the package we need the
installation script is tasked with it now.

Re: [PATCH v3] CI: select CC based on CC_PACKAGE (again)

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Fix a regression in 707d2f2fe86 (CI: use "$runs_on_pool", not
> "$jobname" to select packages & config, 2021-11-23).
>
> In that commit I changed CC=gcc from CC=gcc-9, but on OSX the "gcc" in
> $PATH points to clang, we need to use gcc-9 instead. Likewise for the
> linux-gcc job CC=gcc-8 was changed to the implicit CC=gcc, which would
> select GCC 9.4.0 instead of GCC 8.4.0.
>
> Furthermore in 25715419bf4 (CI: don't run "make test" twice in one
> job, 2021-11-23) when the "linux-TEST-vars" job was split off from
> "linux-gcc" the "cc_package: gcc-8" line was copied along with
> it, so its "cc_package" line wasn't working as intended either.
>
> As a table, this is what's changed by this commit, i.e. it only
> affects the linux-gcc, linux-TEST-vars and osx-gcc jobs:
>
> 	|-------------------+-----------+-------------------+-------+-------|
> 	| jobname           | vector.cc | vector.cc_package | old   | new   |
> 	|-------------------+-----------+-------------------+-------+-------|
> 	| linux-clang       | clang     | -                 | clang | clang |
> 	| linux-sha256      | clang     | -                 | clang | clang |
> 	| linux-gcc         | gcc       | gcc-8             | gcc   | gcc-8 |
> 	| osx-clang         | clang     | -                 | clang | clang |
> 	| osx-gcc           | gcc       | gcc-9             | clang | gcc-9 |
> 	| linux-gcc-default | gcc       | -                 | gcc   | gcc   |
> 	| linux-TEST-vars   | gcc       | gcc-8             | gcc   | gcc-8 |
> 	|-------------------+-----------+-------------------+-------+-------|
>
> Reported-by: Carlo Arenas <carenas@gmail.com>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>
> I just dropped the linux-TEST-vars change from the v2 in lieu of
> trying to get the wording in the commit message right.

OK.  So linux-gcc and linux-gcc-default jobs use different compilers
(if gcc and gcc-8 are different), osx-gcc uses gcc-9 instead of clang.

Thanks.  Will queue.

> diff --git a/ci/lib.sh b/ci/lib.sh
> index cbc2f8f1caa..86e37da9bc5 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -122,7 +122,7 @@ then
>  	test macos != "$CI_OS_NAME" || CI_OS_NAME=osx
>  	CI_REPO_SLUG="$GITHUB_REPOSITORY"
>  	CI_JOB_ID="$GITHUB_RUN_ID"
> -	CC="${CC:-gcc}"
> +	CC="${CC_PACKAGE:-${CC:-gcc}}"
>  	DONT_SKIP_TAGS=t
>  
>  	cache_dir="$HOME/none"

Re: [PATCH v3] CI: select CC based on CC_PACKAGE (again)

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Fix a regression in 707d2f2fe86 (CI: use "$runs_on_pool", not
> "$jobname" to select packages & config, 2021-11-23).
>
> In that commit I changed CC=gcc from CC=gcc-9, but on OSX the "gcc" in
> $PATH points to clang, we need to use gcc-9 instead. Likewise for the
> linux-gcc job CC=gcc-8 was changed to the implicit CC=gcc, which would
> select GCC 9.4.0 instead of GCC 8.4.0.
>
> Furthermore in 25715419bf4 (CI: don't run "make test" twice in one
> job, 2021-11-23) when the "linux-TEST-vars" job was split off from
> "linux-gcc" the "cc_package: gcc-8" line was copied along with
> it, so its "cc_package" line wasn't working as intended either.
>
> As a table, this is what's changed by this commit, i.e. it only
> affects the linux-gcc, linux-TEST-vars and osx-gcc jobs:
>
> 	|-------------------+-----------+-------------------+-------+-------|
> 	| jobname           | vector.cc | vector.cc_package | old   | new   |
> 	|-------------------+-----------+-------------------+-------+-------|
> 	| linux-clang       | clang     | -                 | clang | clang |
> 	| linux-sha256      | clang     | -                 | clang | clang |
> 	| linux-gcc         | gcc       | gcc-8             | gcc   | gcc-8 |
> 	| osx-clang         | clang     | -                 | clang | clang |
> 	| osx-gcc           | gcc       | gcc-9             | clang | gcc-9 |
> 	| linux-gcc-default | gcc       | -                 | gcc   | gcc   |
> 	| linux-TEST-vars   | gcc       | gcc-8             | gcc   | gcc-8 |
> 	|-------------------+-----------+-------------------+-------+-------|
>
> Reported-by: Carlo Arenas <carenas@gmail.com>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>
> I just dropped the linux-TEST-vars change from the v2 in lieu of
> trying to get the wording in the commit message right.

I plan to fast-track this; the revamp of ci/lib.sh in the other huge
series would probably need to be rebased on top, or independently
fix it the same way.

In the meantime, here is what I'll throwing into today's 'seen'.

diff --cc ci/lib.sh
index 86e37da9bc,80e89f89b7..0000000000
--- i/ci/lib.sh
+++ w/ci/lib.sh
@@@ -195,6 -224,79 +224,79 @@@ ubuntu-latest
  esac
  
  case "$jobname" in
+ windows-build)
+ 	setenv --build NO_PERL NoThanks
+ 	setenv --build ARTIFACTS_DIRECTORY artifacts
+ 	;;
+ windows-test)
+ 	setenv --test MAKEFLAGS "$COMMON_MAKEFLAGS"
+ 	;;
+ vs-build)
+ 	setenv --build DEVELOPER $DEVELOPER
+ 	setenv --build SKIP_DASHED_BUILT_INS $SKIP_DASHED_BUILT_INS
+ 
+ 	setenv --build NO_PERL NoThanks
+ 	setenv --build NO_GETTEXT NoThanks
+ 	setenv --build ARTIFACTS_DIRECTORY artifacts
+ 	setenv --build INCLUDE_DLLS_IN_ARTIFACTS YesPlease
+ 	setenv --build MSVC YesPlease
+ 
+ 	setenv --build GIT_CONFIG_COUNT 2
+ 	setenv --build GIT_CONFIG_KEY_0 user.name
+ 	setenv --build GIT_CONFIG_VALUE_0 CI
+ 	setenv --build GIT_CONFIG_KEY_1 user.emailname
+ 	setenv --build GIT_CONFIG_VALUE_1 ci@git
+ 	setenv --build GIT_CONFIG_VALUE_1 ci@git
+ 	;;
+ vs-test)
+ 	setenv --test NO_SVN_TESTS YesPlease
+ 	setenv --test MAKEFLAGS "$COMMON_MAKEFLAGS"
+ 	;;
+ linux-gcc)
 -	CC=gcc
++	CC=gcc-8
+ 	setenv --test GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME main
+ 	;;
+ linux-gcc-default)
+ 	CC=gcc
+ 	;;
+ linux-TEST-vars)
 -	CC=gcc
++	CC=gcc-8
+ 	setenv --test GIT_TEST_SPLIT_INDEX yes
+ 	setenv --test GIT_TEST_MERGE_ALGORITHM recursive
+ 	setenv --test GIT_TEST_FULL_IN_PACK_ARRAY true
+ 	setenv --test GIT_TEST_OE_SIZE 10
+ 	setenv --test GIT_TEST_OE_DELTA_SIZE 5
+ 	setenv --test GIT_TEST_COMMIT_GRAPH 1
+ 	setenv --test GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS 1
+ 	setenv --test GIT_TEST_MULTI_PACK_INDEX 1
+ 	setenv --test GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP 1
+ 	setenv --test GIT_TEST_ADD_I_USE_BUILTIN 1
+ 	setenv --test GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME master
+ 	setenv --test GIT_TEST_WRITE_REV_INDEX 1
+ 	setenv --test GIT_TEST_CHECKOUT_WORKERS 2
+ 	;;
+ osx-gcc)
+ 	MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python3)"
 -	CC=gcc
++	CC=gcc-9
+ 	;;
+ osx-clang)
+ 	MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python2)"
+ 	CC=clang
+ 	;;
+ linux-clang)
+ 	CC=clang
+ 	setenv --test GIT_TEST_DEFAULT_HASH sha1
+ 	;;
+ linux-sha256)
+ 	CC=clang
+ 	setenv --test GIT_TEST_DEFAULT_HASH sha256
+ 	;;
+ pedantic)
+ 	CC=gcc
+ 	# Don't run the tests; we only care about whether Git can be
+ 	# built.
+ 	setenv --build DEVOPTS pedantic
+ 	;;
  linux32)
  	CC=gcc
  	;;

Re: [PATCH v3] CI: select CC based on CC_PACKAGE (again)

[Ævar Arnfjörð Bjarmason]

On Fri, Apr 22 2022, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> Fix a regression in 707d2f2fe86 (CI: use "$runs_on_pool", not
>> "$jobname" to select packages & config, 2021-11-23).
>>
>> In that commit I changed CC=gcc from CC=gcc-9, but on OSX the "gcc" in
>> $PATH points to clang, we need to use gcc-9 instead. Likewise for the
>> linux-gcc job CC=gcc-8 was changed to the implicit CC=gcc, which would
>> select GCC 9.4.0 instead of GCC 8.4.0.
>>
>> Furthermore in 25715419bf4 (CI: don't run "make test" twice in one
>> job, 2021-11-23) when the "linux-TEST-vars" job was split off from
>> "linux-gcc" the "cc_package: gcc-8" line was copied along with
>> it, so its "cc_package" line wasn't working as intended either.
>>
>> As a table, this is what's changed by this commit, i.e. it only
>> affects the linux-gcc, linux-TEST-vars and osx-gcc jobs:
>>
>> 	|-------------------+-----------+-------------------+-------+-------|
>> 	| jobname           | vector.cc | vector.cc_package | old   | new   |
>> 	|-------------------+-----------+-------------------+-------+-------|
>> 	| linux-clang       | clang     | -                 | clang | clang |
>> 	| linux-sha256      | clang     | -                 | clang | clang |
>> 	| linux-gcc         | gcc       | gcc-8             | gcc   | gcc-8 |
>> 	| osx-clang         | clang     | -                 | clang | clang |
>> 	| osx-gcc           | gcc       | gcc-9             | clang | gcc-9 |
>> 	| linux-gcc-default | gcc       | -                 | gcc   | gcc   |
>> 	| linux-TEST-vars   | gcc       | gcc-8             | gcc   | gcc-8 |
>> 	|-------------------+-----------+-------------------+-------+-------|
>>
>> Reported-by: Carlo Arenas <carenas@gmail.com>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>
>> I just dropped the linux-TEST-vars change from the v2 in lieu of
>> trying to get the wording in the commit message right.
>
> I plan to fast-track this; the revamp of ci/lib.sh in the other huge
> series would probably need to be rebased on top, or independently
> fix it the same way.
>
> In the meantime, here is what I'll throwing into today's 'seen'.
>
> diff --cc ci/lib.sh
> index 86e37da9bc,80e89f89b7..0000000000
> --- i/ci/lib.sh
> +++ w/ci/lib.sh
> @@@ -195,6 -224,79 +224,79 @@@ ubuntu-latest
>   esac
>   
>   case "$jobname" in
> + windows-build)
> + 	setenv --build NO_PERL NoThanks
> + 	setenv --build ARTIFACTS_DIRECTORY artifacts
> + 	;;
> + windows-test)
> + 	setenv --test MAKEFLAGS "$COMMON_MAKEFLAGS"
> + 	;;
> + vs-build)
> + 	setenv --build DEVELOPER $DEVELOPER
> + 	setenv --build SKIP_DASHED_BUILT_INS $SKIP_DASHED_BUILT_INS
> + 
> + 	setenv --build NO_PERL NoThanks
> + 	setenv --build NO_GETTEXT NoThanks
> + 	setenv --build ARTIFACTS_DIRECTORY artifacts
> + 	setenv --build INCLUDE_DLLS_IN_ARTIFACTS YesPlease
> + 	setenv --build MSVC YesPlease
> + 
> + 	setenv --build GIT_CONFIG_COUNT 2
> + 	setenv --build GIT_CONFIG_KEY_0 user.name
> + 	setenv --build GIT_CONFIG_VALUE_0 CI
> + 	setenv --build GIT_CONFIG_KEY_1 user.emailname
> + 	setenv --build GIT_CONFIG_VALUE_1 ci@git
> + 	setenv --build GIT_CONFIG_VALUE_1 ci@git
> + 	;;
> + vs-test)
> + 	setenv --test NO_SVN_TESTS YesPlease
> + 	setenv --test MAKEFLAGS "$COMMON_MAKEFLAGS"
> + 	;;
> + linux-gcc)
>  -	CC=gcc
> ++	CC=gcc-8
> + 	setenv --test GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME main
> + 	;;
> + linux-gcc-default)
> + 	CC=gcc
> + 	;;
> + linux-TEST-vars)
>  -	CC=gcc
> ++	CC=gcc-8
> + 	setenv --test GIT_TEST_SPLIT_INDEX yes
> + 	setenv --test GIT_TEST_MERGE_ALGORITHM recursive
> + 	setenv --test GIT_TEST_FULL_IN_PACK_ARRAY true
> + 	setenv --test GIT_TEST_OE_SIZE 10
> + 	setenv --test GIT_TEST_OE_DELTA_SIZE 5
> + 	setenv --test GIT_TEST_COMMIT_GRAPH 1
> + 	setenv --test GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS 1
> + 	setenv --test GIT_TEST_MULTI_PACK_INDEX 1
> + 	setenv --test GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP 1
> + 	setenv --test GIT_TEST_ADD_I_USE_BUILTIN 1
> + 	setenv --test GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME master
> + 	setenv --test GIT_TEST_WRITE_REV_INDEX 1
> + 	setenv --test GIT_TEST_CHECKOUT_WORKERS 2
> + 	;;
> + osx-gcc)
> + 	MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python3)"
>  -	CC=gcc
> ++	CC=gcc-9
> + 	;;
> + osx-clang)
> + 	MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python2)"
> + 	CC=clang
> + 	;;
> + linux-clang)
> + 	CC=clang
> + 	setenv --test GIT_TEST_DEFAULT_HASH sha1
> + 	;;
> + linux-sha256)
> + 	CC=clang
> + 	setenv --test GIT_TEST_DEFAULT_HASH sha256
> + 	;;
> + pedantic)
> + 	CC=gcc
> + 	# Don't run the tests; we only care about whether Git can be
> + 	# built.
> + 	setenv --build DEVOPTS pedantic
> + 	;;
>   linux32)
>   	CC=gcc
>   	;;

This matches the resolution I have for it. noted here:
https://lore.kernel.org/git/cover-v5-00.29-00000000000-20220421T181526Z-avarab@gmail.com/

I.e. it was just those CC=gcc lines for those two jobs that needed to be
tweaked to have CC=gcc-8 and CC=gcc-9, respectively. Then likewise for
linux-TEST-vars (not in my version, since that was against the v2 of the
series, but correct here).

Thanks!