git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/2] CI: use shorter names for CI jobs, less truncation
@ 2021-11-19 13:56 Ævar Arnfjörð Bjarmason
  2021-11-19 13:56 ` [PATCH 1/2] CI: use shorter names that fit in UX tooltips Ævar Arnfjörð Bjarmason
                   ` (4 more replies)
  0 siblings, 5 replies; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-19 13:56 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, SZEDER Gábor,
	Ævar Arnfjörð Bjarmason

This changes the names used in GitHub CI to be shorter, because the
current ones are so long that they overflow the pop-up tooltips in the
GitHub UI.

New pop-up visible at: https://github.com/avar/git/tree/avar/ci-shorter-names

Full CI run at (currently pending, I had a trivial last-minute
update):
https://github.com/avar/git/runs/4264929546?check_suite_focus=true

Ævar Arnfjörð Bjarmason (2):
  CI: use shorter names that fit in UX tooltips
  CI: rename the "Linux32" job to lower-case "linux32"

 .github/workflows/main.yml        | 15 +++++++++++++--
 .travis.yml                       |  2 +-
 README.md                         |  2 +-
 ci/install-docker-dependencies.sh |  2 +-
 ci/lib.sh                         |  2 +-
 ci/run-docker-build.sh            |  2 +-
 ci/run-docker.sh                  |  2 +-
 7 files changed, 19 insertions(+), 8 deletions(-)

-- 
2.34.0.823.g5753b56b5c1


^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH 1/2] CI: use shorter names that fit in UX tooltips
  2021-11-19 13:56 [PATCH 0/2] CI: use shorter names for CI jobs, less truncation Ævar Arnfjörð Bjarmason
@ 2021-11-19 13:56 ` Ævar Arnfjörð Bjarmason
  2021-11-19 14:58   ` Johannes Schindelin
  2021-11-19 16:02   ` Victoria Dye
  2021-11-19 13:56 ` [PATCH 2/2] CI: rename the "Linux32" job to lower-case "linux32" Ævar Arnfjörð Bjarmason
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-19 13:56 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, SZEDER Gábor,
	Ævar Arnfjörð Bjarmason

Change the names used for the GitHub CI workflows to be short enough
to (mostly) fit in the pop-up tool-tips that GitHub shows in the
commit view. I.e. when mouse-clicking on the passing or failing
check-mark next to the commit subject.

That description is truncated to 24 characters, with the 3 at the end
being placed by "...".

E.g. the full job name (visible at [1]):

    "regular (linux-gcc-default, gcc, ubuntu-latest)"

Will, when shown in the tool-tip be truncated to:

    "CI/PR / regular (linu..."

There's then a further limit in the expanded view where the job names
are observably truncated to 44 characters (including "..."). I.e.:

    "regular (linux-gcc-default, gcc, ubuntu-l..."

With this change we shorten both the job names, and change the
top-level "name" from "CI/PR" to "CI", since it will be used as a
prefix in the tooltips. We also remove redundant or superfluous
information from the name, e.g. "ubuntu-latest" isn't really needed
for "linux-leaks", it'll suffice to say linux. For discovering what
image runs that specifically we can consult main.yml itself.

The above "regular (linux-gcc-default, gcc, ubuntu-latest)" job name
then becomes a 1=1 match to the "$jobname" used in
"ci/run-build-and-tests.sh". A "( push" or " (pull_request" is then
added implicitly as before (from the top-level "on" parameter in
"main.yml"). In the tooltip we'll now show:

    "CI / linux-leaks (pu..."

We then have no truncation in the expanded view. See [2] for a
currently visible CI run using this commit, and [3] for the GitHub
workflow syntax involved being changed here.

We could avoid even more truncation with more compact names,
e.g. changing "linux" to "lin" or "lnx", but I didn't do that since
any additional shortening seemed counterproductive, i.e. "w32" is a
well-known way of referring to "Windows", but "lin" isn't). We could
also shorten e.g. "::build" and "::test" to "+bld" and "+tst", but
those seem similarly to be overly obtuse.

1. https://github.com/git/git/tree/master/
2. https://github.com/avar/git/tree/avar/ci-shorter-names
3. https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 .github/workflows/main.yml | 13 ++++++++++++-
 README.md                  |  2 +-
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 6ed6a9e8076..8f4caa8f040 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -1,4 +1,4 @@
-name: CI/PR
+name: CI
 
 on: [push, pull_request]
 
@@ -7,6 +7,7 @@ env:
 
 jobs:
   ci-config:
+    name: config
     runs-on: ubuntu-latest
     outputs:
       enabled: ${{ steps.check-ref.outputs.enabled }}${{ steps.skip-if-redundant.outputs.enabled }}
@@ -77,6 +78,7 @@ jobs:
             }
 
   windows-build:
+    name: w32::build
     needs: ci-config
     if: needs.ci-config.outputs.enabled == 'yes'
     runs-on: windows-latest
@@ -97,6 +99,7 @@ jobs:
         name: windows-artifacts
         path: artifacts
   windows-test:
+    name: w32::test
     runs-on: windows-latest
     needs: [windows-build]
     strategy:
@@ -127,6 +130,7 @@ jobs:
         name: failed-tests-windows
         path: ${{env.FAILED_TEST_ARTIFACTS}}
   vs-build:
+    name: w32/VS::build
     needs: ci-config
     if: needs.ci-config.outputs.enabled == 'yes'
     env:
@@ -178,6 +182,7 @@ jobs:
         name: vs-artifacts
         path: artifacts
   vs-test:
+    name: w32/VS::test
     runs-on: windows-latest
     needs: vs-build
     strategy:
@@ -210,6 +215,7 @@ jobs:
         name: failed-tests-windows
         path: ${{env.FAILED_TEST_ARTIFACTS}}
   regular:
+    name: ${{matrix.vector.jobname}}
     needs: ci-config
     if: needs.ci-config.outputs.enabled == 'yes'
     strategy:
@@ -251,6 +257,7 @@ jobs:
         name: failed-tests-${{matrix.vector.jobname}}
         path: ${{env.FAILED_TEST_ARTIFACTS}}
   dockerized:
+    name: ${{matrix.vector.jobname}} (docker)
     needs: ci-config
     if: needs.ci-config.outputs.enabled == 'yes'
     strategy:
@@ -258,10 +265,13 @@ jobs:
       matrix:
         vector:
         - jobname: linux-musl
+          os: alpine
           image: alpine
         - jobname: Linux32
+          os: ubuntu32
           image: daald/ubuntu32:xenial
         - jobname: pedantic
+          os: fedora
           image: fedora
     env:
       jobname: ${{matrix.vector.jobname}}
@@ -310,6 +320,7 @@ jobs:
       run: ci/install-dependencies.sh
     - run: make sparse
   documentation:
+    name: documentation
     needs: ci-config
     if: needs.ci-config.outputs.enabled == 'yes'
     env:
diff --git a/README.md b/README.md
index eb8115e6b04..f6f43e78deb 100644
--- a/README.md
+++ b/README.md
@@ -1,4 +1,4 @@
-[![Build status](https://github.com/git/git/workflows/CI/PR/badge.svg)](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush)
+[![Build status](https://github.com/git/git/workflows/CI/badge.svg)](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush)
 
 Git - fast, scalable, distributed revision control system
 =========================================================
-- 
2.34.0.823.g5753b56b5c1


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 2/2] CI: rename the "Linux32" job to lower-case "linux32"
  2021-11-19 13:56 [PATCH 0/2] CI: use shorter names for CI jobs, less truncation Ævar Arnfjörð Bjarmason
  2021-11-19 13:56 ` [PATCH 1/2] CI: use shorter names that fit in UX tooltips Ævar Arnfjörð Bjarmason
@ 2021-11-19 13:56 ` Ævar Arnfjörð Bjarmason
  2021-11-19 14:57   ` Jeff King
  2021-11-19 15:03 ` [PATCH 0/2] CI: use shorter names for CI jobs, less truncation Jeff King
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-19 13:56 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, SZEDER Gábor,
	Ævar Arnfjörð Bjarmason

As a follow-up to the preceding commit's shortening of CI job names,
rename the only job that starts with an upper-case letter to be
consistent with the rest. It was added in 88dedd5e72c (Travis: also
test on 32-bit Linux, 2017-03-05).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 .github/workflows/main.yml        | 2 +-
 .travis.yml                       | 2 +-
 ci/install-docker-dependencies.sh | 2 +-
 ci/lib.sh                         | 2 +-
 ci/run-docker-build.sh            | 2 +-
 ci/run-docker.sh                  | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 8f4caa8f040..9c4af979559 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -267,7 +267,7 @@ jobs:
         - jobname: linux-musl
           os: alpine
           image: alpine
-        - jobname: Linux32
+        - jobname: linux32
           os: ubuntu32
           image: daald/ubuntu32:xenial
         - jobname: pedantic
diff --git a/.travis.yml b/.travis.yml
index 908330a0a3d..73f983776c7 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -25,7 +25,7 @@ matrix:
       os: linux
       dist: trusty
       compiler:
-    - env: jobname=Linux32
+    - env: jobname=linux32
       os: linux
       compiler:
       addons:
diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
index 07a8c6b199d..78b7e326da6 100755
--- a/ci/install-docker-dependencies.sh
+++ b/ci/install-docker-dependencies.sh
@@ -4,7 +4,7 @@
 #
 
 case "$jobname" in
-Linux32)
+linux32)
 	linux32 --32bit i386 sh -c '
 		apt update >/dev/null &&
 		apt install -y build-essential libcurl4-openssl-dev \
diff --git a/ci/lib.sh b/ci/lib.sh
index 82cb17f8eea..83c2e08be79 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -222,7 +222,7 @@ osx-clang|osx-gcc)
 	;;
 linux-gcc-default)
 	;;
-Linux32)
+linux32)
 	CC=gcc
 	;;
 linux-musl)
diff --git a/ci/run-docker-build.sh b/ci/run-docker-build.sh
index 8d47a5fda3b..886fa33e148 100755
--- a/ci/run-docker-build.sh
+++ b/ci/run-docker-build.sh
@@ -15,7 +15,7 @@ then
 fi
 
 case "$jobname" in
-Linux32)
+linux32)
 	switch_cmd="linux32 --32bit i386"
 	;;
 linux-musl)
diff --git a/ci/run-docker.sh b/ci/run-docker.sh
index 37fa372052d..6663d56ade6 100755
--- a/ci/run-docker.sh
+++ b/ci/run-docker.sh
@@ -6,7 +6,7 @@
 . ${0%/*}/lib.sh
 
 case "$jobname" in
-Linux32)
+linux32)
 	CI_CONTAINER="daald/ubuntu32:xenial"
 	;;
 linux-musl)
-- 
2.34.0.823.g5753b56b5c1


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCH 2/2] CI: rename the "Linux32" job to lower-case "linux32"
  2021-11-19 13:56 ` [PATCH 2/2] CI: rename the "Linux32" job to lower-case "linux32" Ævar Arnfjörð Bjarmason
@ 2021-11-19 14:57   ` Jeff King
  0 siblings, 0 replies; 38+ messages in thread
From: Jeff King @ 2021-11-19 14:57 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: git, Junio C Hamano, Johannes Schindelin, SZEDER Gábor

On Fri, Nov 19, 2021 at 02:56:07PM +0100, Ævar Arnfjörð Bjarmason wrote:

> As a follow-up to the preceding commit's shortening of CI job names,
> rename the only job that starts with an upper-case letter to be
> consistent with the rest. It was added in 88dedd5e72c (Travis: also
> test on 32-bit Linux, 2017-03-05).

This makes sense, though I had to wonder...

>  .travis.yml                       | 2 +-

Is this travis file even still useful? With travis-ci.org shutting down
last summer, I would think nobody is triggering these builds anymore.

Not a blocker, obviously, but maybe a possible cleanup on top.

-Peff

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/2] CI: use shorter names that fit in UX tooltips
  2021-11-19 13:56 ` [PATCH 1/2] CI: use shorter names that fit in UX tooltips Ævar Arnfjörð Bjarmason
@ 2021-11-19 14:58   ` Johannes Schindelin
  2021-11-19 20:39     ` Ævar Arnfjörð Bjarmason
  2021-11-19 16:02   ` Victoria Dye
  1 sibling, 1 reply; 38+ messages in thread
From: Johannes Schindelin @ 2021-11-19 14:58 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: git, Junio C Hamano, SZEDER Gábor

[-- Attachment #1: Type: text/plain, Size: 6154 bytes --]

Hi,

On Fri, 19 Nov 2021, Ævar Arnfjörð Bjarmason wrote:

> Change the names used for the GitHub CI workflows to be short enough
> to (mostly) fit in the pop-up tool-tips that GitHub shows in the
> commit view. I.e. when mouse-clicking on the passing or failing
> check-mark next to the commit subject.
>
> That description is truncated to 24 characters, with the 3 at the end
> being placed by "...".
>
> E.g. the full job name (visible at [1]):
>
>     "regular (linux-gcc-default, gcc, ubuntu-latest)"
>
> Will, when shown in the tool-tip be truncated to:
>
>     "CI/PR / regular (linu..."
>
> There's then a further limit in the expanded view where the job names
> are observably truncated to 44 characters (including "..."). I.e.:
>
>     "regular (linux-gcc-default, gcc, ubuntu-l..."
>
> With this change we shorten both the job names, and change the
> top-level "name" from "CI/PR" to "CI", since it will be used as a
> prefix in the tooltips. We also remove redundant or superfluous
> information from the name, e.g. "ubuntu-latest" isn't really needed
> for "linux-leaks", it'll suffice to say linux. For discovering what
> image runs that specifically we can consult main.yml itself.
>
> The above "regular (linux-gcc-default, gcc, ubuntu-latest)" job name
> then becomes a 1=1 match to the "$jobname" used in
> "ci/run-build-and-tests.sh". A "( push" or " (pull_request" is then
> added implicitly as before (from the top-level "on" parameter in
> "main.yml"). In the tooltip we'll now show:
>
>     "CI / linux-leaks (pu..."
>
> We then have no truncation in the expanded view. See [2] for a
> currently visible CI run using this commit, and [3] for the GitHub
> workflow syntax involved being changed here.
>
> We could avoid even more truncation with more compact names,
> e.g. changing "linux" to "lin" or "lnx", but I didn't do that since
> any additional shortening seemed counterproductive, i.e. "w32" is a
> well-known way of referring to "Windows", but "lin" isn't). We could
> also shorten e.g. "::build" and "::test" to "+bld" and "+tst", but
> those seem similarly to be overly obtuse.
>
> 1. https://github.com/git/git/tree/master/
> 2. https://github.com/avar/git/tree/avar/ci-shorter-names
> 3. https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

My vote is to drop this patch: it does a lot of things, but I do not see
any benefit. Names that are too long in one person's setup are not too
long in another one's.

Also, it drops the "PR" as if we would not do PR builds at all anymore
("CI" is for "Continuous Integration", i.e. the testing of the current
`main` branch).

And `w32`? Really? *Really*?

Ciao,
Johannes

> ---
>  .github/workflows/main.yml | 13 ++++++++++++-
>  README.md                  |  2 +-
>  2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> index 6ed6a9e8076..8f4caa8f040 100644
> --- a/.github/workflows/main.yml
> +++ b/.github/workflows/main.yml
> @@ -1,4 +1,4 @@
> -name: CI/PR
> +name: CI
>
>  on: [push, pull_request]
>
> @@ -7,6 +7,7 @@ env:
>
>  jobs:
>    ci-config:
> +    name: config
>      runs-on: ubuntu-latest
>      outputs:
>        enabled: ${{ steps.check-ref.outputs.enabled }}${{ steps.skip-if-redundant.outputs.enabled }}
> @@ -77,6 +78,7 @@ jobs:
>              }
>
>    windows-build:
> +    name: w32::build
>      needs: ci-config
>      if: needs.ci-config.outputs.enabled == 'yes'
>      runs-on: windows-latest
> @@ -97,6 +99,7 @@ jobs:
>          name: windows-artifacts
>          path: artifacts
>    windows-test:
> +    name: w32::test
>      runs-on: windows-latest
>      needs: [windows-build]
>      strategy:
> @@ -127,6 +130,7 @@ jobs:
>          name: failed-tests-windows
>          path: ${{env.FAILED_TEST_ARTIFACTS}}
>    vs-build:
> +    name: w32/VS::build
>      needs: ci-config
>      if: needs.ci-config.outputs.enabled == 'yes'
>      env:
> @@ -178,6 +182,7 @@ jobs:
>          name: vs-artifacts
>          path: artifacts
>    vs-test:
> +    name: w32/VS::test
>      runs-on: windows-latest
>      needs: vs-build
>      strategy:
> @@ -210,6 +215,7 @@ jobs:
>          name: failed-tests-windows
>          path: ${{env.FAILED_TEST_ARTIFACTS}}
>    regular:
> +    name: ${{matrix.vector.jobname}}
>      needs: ci-config
>      if: needs.ci-config.outputs.enabled == 'yes'
>      strategy:
> @@ -251,6 +257,7 @@ jobs:
>          name: failed-tests-${{matrix.vector.jobname}}
>          path: ${{env.FAILED_TEST_ARTIFACTS}}
>    dockerized:
> +    name: ${{matrix.vector.jobname}} (docker)
>      needs: ci-config
>      if: needs.ci-config.outputs.enabled == 'yes'
>      strategy:
> @@ -258,10 +265,13 @@ jobs:
>        matrix:
>          vector:
>          - jobname: linux-musl
> +          os: alpine
>            image: alpine
>          - jobname: Linux32
> +          os: ubuntu32
>            image: daald/ubuntu32:xenial
>          - jobname: pedantic
> +          os: fedora
>            image: fedora
>      env:
>        jobname: ${{matrix.vector.jobname}}
> @@ -310,6 +320,7 @@ jobs:
>        run: ci/install-dependencies.sh
>      - run: make sparse
>    documentation:
> +    name: documentation
>      needs: ci-config
>      if: needs.ci-config.outputs.enabled == 'yes'
>      env:
> diff --git a/README.md b/README.md
> index eb8115e6b04..f6f43e78deb 100644
> --- a/README.md
> +++ b/README.md
> @@ -1,4 +1,4 @@
> -[![Build status](https://github.com/git/git/workflows/CI/PR/badge.svg)](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush)
> +[![Build status](https://github.com/git/git/workflows/CI/badge.svg)](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush)
>
>  Git - fast, scalable, distributed revision control system
>  =========================================================
> --
> 2.34.0.823.g5753b56b5c1
>
>
>

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 0/2] CI: use shorter names for CI jobs, less truncation
  2021-11-19 13:56 [PATCH 0/2] CI: use shorter names for CI jobs, less truncation Ævar Arnfjörð Bjarmason
  2021-11-19 13:56 ` [PATCH 1/2] CI: use shorter names that fit in UX tooltips Ævar Arnfjörð Bjarmason
  2021-11-19 13:56 ` [PATCH 2/2] CI: rename the "Linux32" job to lower-case "linux32" Ævar Arnfjörð Bjarmason
@ 2021-11-19 15:03 ` Jeff King
  2021-11-19 19:57 ` Junio C Hamano
  2021-11-20  3:28 ` [PATCH v2 0/6] CI: Remove Travis CI, shorten names for GH tooltips, split jobs Ævar Arnfjörð Bjarmason
  4 siblings, 0 replies; 38+ messages in thread
From: Jeff King @ 2021-11-19 15:03 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: git, Junio C Hamano, Johannes Schindelin, SZEDER Gábor

On Fri, Nov 19, 2021 at 02:56:05PM +0100, Ævar Arnfjörð Bjarmason wrote:

> This changes the names used in GitHub CI to be shorter, because the
> current ones are so long that they overflow the pop-up tooltips in the
> GitHub UI.
> 
> New pop-up visible at: https://github.com/avar/git/tree/avar/ci-shorter-names
> 
> Full CI run at (currently pending, I had a trivial last-minute
> update):
> https://github.com/avar/git/runs/4264929546?check_suite_focus=true

Thanks for giving examples. I don't have a strong opinion on the first
patch, as I never look at those pop-ups anyway. ;) It does also shorten
the names in the main "runs" page, where we had room to show them fully.

To my mind, "linux-gcc", etc, are more readable than "regular
(linux-gcc, gcc, ubuntu-latest)", and that's a strict improvement. "w32"
versus "windows" is less obviously good, but I'm OK with it if the
shortness is important in some contexts.

-Peff

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/2] CI: use shorter names that fit in UX tooltips
  2021-11-19 13:56 ` [PATCH 1/2] CI: use shorter names that fit in UX tooltips Ævar Arnfjörð Bjarmason
  2021-11-19 14:58   ` Johannes Schindelin
@ 2021-11-19 16:02   ` Victoria Dye
  2021-11-19 20:33     ` Ævar Arnfjörð Bjarmason
  2021-11-19 22:14     ` Junio C Hamano
  1 sibling, 2 replies; 38+ messages in thread
From: Victoria Dye @ 2021-11-19 16:02 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason, git
  Cc: Junio C Hamano, Johannes Schindelin, SZEDER Gábor

Ævar Arnfjörð Bjarmason wrote:
> Change the names used for the GitHub CI workflows to be short enough
> to (mostly) fit in the pop-up tool-tips that GitHub shows in the
> commit view. I.e. when mouse-clicking on the passing or failing
> check-mark next to the commit subject.
> 
> That description is truncated to 24 characters, with the 3 at the end
> being placed by "...".
> 
> E.g. the full job name (visible at [1]):
> 
>     "regular (linux-gcc-default, gcc, ubuntu-latest)"
> 
> Will, when shown in the tool-tip be truncated to:
> 
>     "CI/PR / regular (linu..."
> 
> There's then a further limit in the expanded view where the job names
> are observably truncated to 44 characters (including "..."). I.e.:
> 
>     "regular (linux-gcc-default, gcc, ubuntu-l..."
> 

Tooltips like the ones you've pointed out here appear intended to be an "at
a glance" view of the jobs (mostly for showing pass/fail/skip status) - each
job in the tooltip has a "Details" link that takes you to the job summary
and logs. In the current state, although the names of the are truncated in
the tooltip, the information is still easily accessible in the full workflow
details (one click away). For example, the details for the "linux-leaks" job
[1] tell me the image, compiler, and job name right at the top of the page.

[1] https://github.com/git/git/runs/4214606314?check_suite_focus=true

> With this change we shorten both the job names, and change the
> top-level "name" from "CI/PR" to "CI", since it will be used as a
> prefix in the tooltips. We also remove redundant or superfluous
> information from the name, e.g. "ubuntu-latest" isn't really needed
> for "linux-leaks", it'll suffice to say linux. For discovering what
> image runs that specifically we can consult main.yml itself.
> 

By optimizing for the tooltip, this patch shortens names to the point that
they're more difficult to interpret (w32 vs. w32/VS) and/or removes valuable
context about platform/image/etc. When a user *does* want more information
on the job, they now have to: 

1) know that the "CI/PR" job definition is in ".github/workflows/main.yml"
2) parse through the file to find the job they want
3) correlate that back to the job in the workflow details they're
   investigating. 

That's a strictly worse experience for an extremely common use-case. What
use-case is this patch attempting to improve?

> The above "regular (linux-gcc-default, gcc, ubuntu-latest)" job name
> then becomes a 1=1 match to the "$jobname" used in
> "ci/run-build-and-tests.sh". A "( push" or " (pull_request" is then
> added implicitly as before (from the top-level "on" parameter in
> "main.yml"). In the tooltip we'll now show:
> 
>     "CI / linux-leaks (pu..."
> 
> We then have no truncation in the expanded view. See [2] for a
> currently visible CI run using this commit, and [3] for the GitHub
> workflow syntax involved being changed here.
> 

If the only problem this patch really "solves" is making some job names fit
a bit better into the tooltip and, I think the costs (namely the loss of
accessible contextual info) outweigh any potential benefits you gain. 

> We could avoid even more truncation with more compact names,
> e.g. changing "linux" to "lin" or "lnx", but I didn't do that since
> any additional shortening seemed counterproductive, i.e. "w32" is a
> well-known way of referring to "Windows", but "lin" isn't). We could
> also shorten e.g. "::build" and "::test" to "+bld" and "+tst", but
> those seem similarly to be overly obtuse.
> 
> 1. https://github.com/git/git/tree/master/
> 2. https://github.com/avar/git/tree/avar/ci-shorter-names
> 3. https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>> ---
>  .github/workflows/main.yml | 13 ++++++++++++-
>  README.md                  |  2 +-
>  2 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> index 6ed6a9e8076..8f4caa8f040 100644
> --- a/.github/workflows/main.yml
> +++ b/.github/workflows/main.yml
> @@ -1,4 +1,4 @@
> -name: CI/PR
> +name: CI
>  
>  on: [push, pull_request]
>  
> @@ -7,6 +7,7 @@ env:
>  
>  jobs:
>    ci-config:
> +    name: config
>      runs-on: ubuntu-latest
>      outputs:
>        enabled: ${{ steps.check-ref.outputs.enabled }}${{ steps.skip-if-redundant.outputs.enabled }}
> @@ -77,6 +78,7 @@ jobs:
>              }
>  
>    windows-build:
> +    name: w32::build
>      needs: ci-config
>      if: needs.ci-config.outputs.enabled == 'yes'
>      runs-on: windows-latest
> @@ -97,6 +99,7 @@ jobs:
>          name: windows-artifacts
>          path: artifacts
>    windows-test:
> +    name: w32::test
>      runs-on: windows-latest
>      needs: [windows-build]
>      strategy:
> @@ -127,6 +130,7 @@ jobs:
>          name: failed-tests-windows
>          path: ${{env.FAILED_TEST_ARTIFACTS}}
>    vs-build:
> +    name: w32/VS::build
>      needs: ci-config
>      if: needs.ci-config.outputs.enabled == 'yes'
>      env:
> @@ -178,6 +182,7 @@ jobs:
>          name: vs-artifacts
>          path: artifacts
>    vs-test:
> +    name: w32/VS::test
>      runs-on: windows-latest
>      needs: vs-build
>      strategy:
> @@ -210,6 +215,7 @@ jobs:
>          name: failed-tests-windows
>          path: ${{env.FAILED_TEST_ARTIFACTS}}
>    regular:
> +    name: ${{matrix.vector.jobname}}
>      needs: ci-config
>      if: needs.ci-config.outputs.enabled == 'yes'
>      strategy:
> @@ -251,6 +257,7 @@ jobs:
>          name: failed-tests-${{matrix.vector.jobname}}
>          path: ${{env.FAILED_TEST_ARTIFACTS}}
>    dockerized:
> +    name: ${{matrix.vector.jobname}} (docker)
>      needs: ci-config
>      if: needs.ci-config.outputs.enabled == 'yes'
>      strategy:
> @@ -258,10 +265,13 @@ jobs:
>        matrix:
>          vector:
>          - jobname: linux-musl
> +          os: alpine
>            image: alpine
>          - jobname: Linux32
> +          os: ubuntu32
>            image: daald/ubuntu32:xenial
>          - jobname: pedantic
> +          os: fedora
>            image: fedora
>      env:
>        jobname: ${{matrix.vector.jobname}}
> @@ -310,6 +320,7 @@ jobs:
>        run: ci/install-dependencies.sh
>      - run: make sparse
>    documentation:
> +    name: documentation
>      needs: ci-config
>      if: needs.ci-config.outputs.enabled == 'yes'
>      env:
> diff --git a/README.md b/README.md
> index eb8115e6b04..f6f43e78deb 100644
> --- a/README.md
> +++ b/README.md
> @@ -1,4 +1,4 @@
> -[![Build status](https://github.com/git/git/workflows/CI/PR/badge.svg)](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush)
> +[![Build status](https://github.com/git/git/workflows/CI/badge.svg)](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush)
>  
>  Git - fast, scalable, distributed revision control system
>  =========================================================
> 


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 0/2] CI: use shorter names for CI jobs, less truncation
  2021-11-19 13:56 [PATCH 0/2] CI: use shorter names for CI jobs, less truncation Ævar Arnfjörð Bjarmason
                   ` (2 preceding siblings ...)
  2021-11-19 15:03 ` [PATCH 0/2] CI: use shorter names for CI jobs, less truncation Jeff King
@ 2021-11-19 19:57 ` Junio C Hamano
  2021-11-19 20:48   ` Ævar Arnfjörð Bjarmason
  2021-11-20  3:28 ` [PATCH v2 0/6] CI: Remove Travis CI, shorten names for GH tooltips, split jobs Ævar Arnfjörð Bjarmason
  4 siblings, 1 reply; 38+ messages in thread
From: Junio C Hamano @ 2021-11-19 19:57 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: git, Johannes Schindelin, SZEDER Gábor

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

> This changes the names used in GitHub CI to be shorter, because the
> current ones are so long that they overflow the pop-up tooltips in the
> GitHub UI.
>
> New pop-up visible at: https://github.com/avar/git/tree/avar/ci-shorter-names
>
> Full CI run at (currently pending, I had a trivial last-minute
> update):
> https://github.com/avar/git/runs/4264929546?check_suite_focus=true

I have found the labels on "Jobs" on the left hand side pane
irritatingly unhelpful.  For example, "regular (linux-gcc-default,
gcc..."  does not tell me much about how it is different from
"regular (linux-gcc, gcc, ubunt...".

The question I ask most often is "which one of these ones is the job
that runs tests twice, the second time with nonstandard settings?",
or "Only windows-test(4) is failing, but not vs-test(4); what area
did we break?  What is in (4)?".

I do not think relabelling "windows" -> "w32" (why not "win", by the
way?), "vs" -> "w32/VS", or "regular (\(.*\))" -> "\1" helps me very
much in these questions.  I however think the blame for it lies
mostly on the original naming, not your effort in this series.

The job that is now called "linux-leaks" used to be "regular
(linux-leaks, gcc, ubu...", and there definitely is an improvement,
so "regular (\(.*\))" -> "\1" could help if the original was named
properly.  It is easier to spot what the job is about for that
particular one.

I find this of mixed value, ranging from "Meh" to "Hmm...nice?".

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/2] CI: use shorter names that fit in UX tooltips
  2021-11-19 16:02   ` Victoria Dye
@ 2021-11-19 20:33     ` Ævar Arnfjörð Bjarmason
  2021-11-19 21:55       ` Victoria Dye
  2021-11-19 22:14     ` Junio C Hamano
  1 sibling, 1 reply; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-19 20:33 UTC (permalink / raw)
  To: Victoria Dye; +Cc: git, Junio C Hamano, Johannes Schindelin, SZEDER Gábor


On Fri, Nov 19 2021, Victoria Dye wrote:

> Ævar Arnfjörð Bjarmason wrote:
>> Change the names used for the GitHub CI workflows to be short enough
>> to (mostly) fit in the pop-up tool-tips that GitHub shows in the
>> commit view. I.e. when mouse-clicking on the passing or failing
>> check-mark next to the commit subject.
>> 
>> That description is truncated to 24 characters, with the 3 at the end
>> being placed by "...".
>> 
>> E.g. the full job name (visible at [1]):
>> 
>>     "regular (linux-gcc-default, gcc, ubuntu-latest)"
>> 
>> Will, when shown in the tool-tip be truncated to:
>> 
>>     "CI/PR / regular (linu..."
>> 
>> There's then a further limit in the expanded view where the job names
>> are observably truncated to 44 characters (including "..."). I.e.:
>> 
>>     "regular (linux-gcc-default, gcc, ubuntu-l..."
>> 
>
> Tooltips like the ones you've pointed out here appear intended to be an "at
> a glance" view of the jobs (mostly for showing pass/fail/skip status) - each
> job in the tooltip has a "Details" link that takes you to the job summary
> and logs. In the current state, although the names of the are truncated in
> the tooltip, the information is still easily accessible in the full workflow
> details (one click away). For example, the details for the "linux-leaks" job
> [1] tell me the image, compiler, and job name right at the top of the page.
>
> [1] https://github.com/git/git/runs/4214606314?check_suite_focus=true
>
>> With this change we shorten both the job names, and change the
>> top-level "name" from "CI/PR" to "CI", since it will be used as a
>> prefix in the tooltips. We also remove redundant or superfluous
>> information from the name, e.g. "ubuntu-latest" isn't really needed
>> for "linux-leaks", it'll suffice to say linux. For discovering what
>> image runs that specifically we can consult main.yml itself.
>> 
>
> By optimizing for the tooltip, this patch shortens names to the point that
> they're more difficult to interpret (w32 vs. w32/VS) and/or removes valuable
> context about platform/image/etc. When a user *does* want more information
> on the job, they now have to: 
>
> 1) know that the "CI/PR" job definition is in ".github/workflows/main.yml"
> 2) parse through the file to find the job they want
> 3) correlate that back to the job in the workflow details they're
>    investigating. 
>
> That's a strictly worse experience for an extremely common use-case. What
> use-case is this patch attempting to improve?

That I can click on the button that your co-workers implemented and see
the relevant information about the job :)

Given that it's truncated we need to pick and choose what we display if
we're not going to force the user to have to go to the full view every
time.

I'll change s/w32/win/ etc, and there's room to move stuff around here,
but I think it's fine to just not display that it's e.g. "ubuntu" or
"fedora" at all. That's almost never been relevant.

If we were trying to do the opposite and lengthen the names to shove
every bit of useful information in there at a glance I can think of 5-10
things we'd put there before "fedora". libc/version, compiler/version,
kernel/version etc.

Whether it's a recent Gentoo or Ubuntu is something that's OK to omit.

But maybe I'm wrong, are there cases you can think of where we really
need "ubuntu" or "fedora" etc.?

>> The above "regular (linux-gcc-default, gcc, ubuntu-latest)" job name
>> then becomes a 1=1 match to the "$jobname" used in
>> "ci/run-build-and-tests.sh". A "( push" or " (pull_request" is then
>> added implicitly as before (from the top-level "on" parameter in
>> "main.yml"). In the tooltip we'll now show:
>> 
>>     "CI / linux-leaks (pu..."
>> 
>> We then have no truncation in the expanded view. See [2] for a
>> currently visible CI run using this commit, and [3] for the GitHub
>> workflow syntax involved being changed here.
>> 
>
> If the only problem this patch really "solves" is making some job names fit
> a bit better into the tooltip and, I think the costs (namely the loss of
> accessible contextual info) outweigh any potential benefits you gain. 

Yeah it's a trade-off for sure, but now you can't see some of the
relevant information at all in some views, so it's a gain for
accessibility too.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/2] CI: use shorter names that fit in UX tooltips
  2021-11-19 14:58   ` Johannes Schindelin
@ 2021-11-19 20:39     ` Ævar Arnfjörð Bjarmason
  0 siblings, 0 replies; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-19 20:39 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Junio C Hamano, SZEDER Gábor


On Fri, Nov 19 2021, Johannes Schindelin wrote:

> On Fri, 19 Nov 2021, Ævar Arnfjörð Bjarmason wrote:
>
>> Change the names used for the GitHub CI workflows to be short enough
>> to (mostly) fit in the pop-up tool-tips that GitHub shows in the
>> commit view. I.e. when mouse-clicking on the passing or failing
>> check-mark next to the commit subject.
>>
>> That description is truncated to 24 characters, with the 3 at the end
>> being placed by "...".
>>
>> E.g. the full job name (visible at [1]):
>>
>>     "regular (linux-gcc-default, gcc, ubuntu-latest)"
>>
>> Will, when shown in the tool-tip be truncated to:
>>
>>     "CI/PR / regular (linu..."
>>
>> There's then a further limit in the expanded view where the job names
>> are observably truncated to 44 characters (including "..."). I.e.:
>>
>>     "regular (linux-gcc-default, gcc, ubuntu-l..."
>>
>> With this change we shorten both the job names, and change the
>> top-level "name" from "CI/PR" to "CI", since it will be used as a
>> prefix in the tooltips. We also remove redundant or superfluous
>> information from the name, e.g. "ubuntu-latest" isn't really needed
>> for "linux-leaks", it'll suffice to say linux. For discovering what
>> image runs that specifically we can consult main.yml itself.
>>
>> The above "regular (linux-gcc-default, gcc, ubuntu-latest)" job name
>> then becomes a 1=1 match to the "$jobname" used in
>> "ci/run-build-and-tests.sh". A "( push" or " (pull_request" is then
>> added implicitly as before (from the top-level "on" parameter in
>> "main.yml"). In the tooltip we'll now show:
>>
>>     "CI / linux-leaks (pu..."
>>
>> We then have no truncation in the expanded view. See [2] for a
>> currently visible CI run using this commit, and [3] for the GitHub
>> workflow syntax involved being changed here.
>>
>> We could avoid even more truncation with more compact names,
>> e.g. changing "linux" to "lin" or "lnx", but I didn't do that since
>> any additional shortening seemed counterproductive, i.e. "w32" is a
>> well-known way of referring to "Windows", but "lin" isn't). We could
>> also shorten e.g. "::build" and "::test" to "+bld" and "+tst", but
>> those seem similarly to be overly obtuse.
>>
>> 1. https://github.com/git/git/tree/master/
>> 2. https://github.com/avar/git/tree/avar/ci-shorter-names
>> 3. https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>
> My vote is to drop this patch: it does a lot of things, but I do not see
> any benefit. Names that are too long in one person's setup are not too
> long in another one's.

What person's setup are we talking about here? Satya Nadella's? :)

This isn't a terminal I resized, it's your employer's website.

From the browsers I've tried (Firefox, Chrome, logged in & out) it's the
same for everyone. 

Do you not see the existing labels truncated in the same way?

> Also, it drops the "PR" as if we would not do PR builds at all anymore
> ("CI" is for "Continuous Integration", i.e. the testing of the current
> `main` branch).

I think "CI" is commonly understood to not mean that these days. It's
just "the test thing that runs on push" to most people. Is anyone
confused that CI is running in their topic branch without a merge back
to mainline?

What's the potential for confusion here? Do you really need to see "CI /
PR" there to be assured that the thing you're looking at running is
actually running?

I dropped it because omitting the " / PR" is worth it not truncate the
actual meaningful info that comes after, e.g. showing both of
"linux-gcc"and "linux-leaks" as "linu..." or whatever.

> And `w32`? Really? *Really*?

I didn't know what to pick there, "win" was suggested in a side-thread.

FWIW I just ran "find" on some repos that I've got checked out locally
of various software, many of which had something-w32.c, or a w32
directory, even if that software is obviously targeting Windows 64 bit
these days.

I thought it was like how my kernel says I'm running an "amd64", even
though it's an Intel :)

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 0/2] CI: use shorter names for CI jobs, less truncation
  2021-11-19 19:57 ` Junio C Hamano
@ 2021-11-19 20:48   ` Ævar Arnfjörð Bjarmason
  0 siblings, 0 replies; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-19 20:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johannes Schindelin, SZEDER Gábor


On Fri, Nov 19 2021, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> This changes the names used in GitHub CI to be shorter, because the
>> current ones are so long that they overflow the pop-up tooltips in the
>> GitHub UI.
>>
>> New pop-up visible at: https://github.com/avar/git/tree/avar/ci-shorter-names
>>
>> Full CI run at (currently pending, I had a trivial last-minute
>> update):
>> https://github.com/avar/git/runs/4264929546?check_suite_focus=true
>
> I have found the labels on "Jobs" on the left hand side pane
> irritatingly unhelpful.  For example, "regular (linux-gcc-default,
> gcc..."  does not tell me much about how it is different from
> "regular (linux-gcc, gcc, ubunt...".

Yeah, I've needed to look it up most times..

> The question I ask most often is "which one of these ones is the job
> that runs tests twice, the second time with nonstandard settings?",
> or "Only windows-test(4) is failing, but not vs-test(4); what area
> did we break?  What is in (4)?".

Because I had to look: It's a splitting method Johannes came up with,
first stat() all the tests, sort by size, then chunk them up, and use
the Nth as a way of dividing those chunks.

Maybe he feels strongly about it, but I think a better approach is just
to hardcode t0xxx, t1xxx or whatever, then if one is unusually slow have
a t1[0-4]xx & t1[5-9]xx or whatever, I.e. just manually partition them
as a one-off.

These jobs take ~30m anyway, so if one is a tad slower than another it
doesn't really matter as much as seeing at a glance where in the test
suite the failure is.

I nicely split these all up in a follow-up, along with removing the
travis CI, but anticipated the usual objections about too much of a
scatterbrain series.

But yeah, I think all of that would be great to have, I can submit that
as a v2, sound goood?

AFAICT the whole "stick this all into one job" way of doing the
GIT_TEST_* CI is a workaround for some Travis-specific thing.

Or a micro-optimization for trying to max out our total CPU time, but
anyway it doesn't seem worth it. By far most of the time is spent in the
tests themselves, not the build.

A WIP split I had of this, e.g. there's a linux-sha256 job,
linux-TEST-vars for the big GIT_TEST_* accumulation etc:
https://github.com/avar/git/runs/4265312207?check_suite_focus=true

We could (and I'd like to..., but not now) cache the build directory
between runs. Much faster compilations, and if it breaks, well then our
Makefile dependencies are broken, which is also nice to spot in CI (and
the cachewillexpire) ...

> I do not think relabelling "windows" -> "w32" (why not "win", by the
> way?), "vs" -> "w32/VS", or "regular (\(.*\))" -> "\1" helps me very
> much in these questions.  I however think the blame for it lies
> mostly on the original naming, not your effort in this series.

I'll pick "win" next time, as noted in some other follow-up replies.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/2] CI: use shorter names that fit in UX tooltips
  2021-11-19 20:33     ` Ævar Arnfjörð Bjarmason
@ 2021-11-19 21:55       ` Victoria Dye
  2021-11-20  2:51         ` Ævar Arnfjörð Bjarmason
  0 siblings, 1 reply; 38+ messages in thread
From: Victoria Dye @ 2021-11-19 21:55 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: git, Junio C Hamano, Johannes Schindelin, SZEDER Gábor

Ævar Arnfjörð Bjarmason wrote:
> 
> On Fri, Nov 19 2021, Victoria Dye wrote:
> 
>> Ævar Arnfjörð Bjarmason wrote:
>>> Change the names used for the GitHub CI workflows to be short enough
>>> to (mostly) fit in the pop-up tool-tips that GitHub shows in the
>>> commit view. I.e. when mouse-clicking on the passing or failing
>>> check-mark next to the commit subject.
>>>
>>> That description is truncated to 24 characters, with the 3 at the end
>>> being placed by "...".
>>>
>>> E.g. the full job name (visible at [1]):
>>>
>>>     "regular (linux-gcc-default, gcc, ubuntu-latest)"
>>>
>>> Will, when shown in the tool-tip be truncated to:
>>>
>>>     "CI/PR / regular (linu..."
>>>
>>> There's then a further limit in the expanded view where the job names
>>> are observably truncated to 44 characters (including "..."). I.e.:
>>>
>>>     "regular (linux-gcc-default, gcc, ubuntu-l..."
>>>
>>
>> Tooltips like the ones you've pointed out here appear intended to be an "at
>> a glance" view of the jobs (mostly for showing pass/fail/skip status) - each
>> job in the tooltip has a "Details" link that takes you to the job summary
>> and logs. In the current state, although the names of the are truncated in
>> the tooltip, the information is still easily accessible in the full workflow
>> details (one click away). For example, the details for the "linux-leaks" job
>> [1] tell me the image, compiler, and job name right at the top of the page.
>>
>> [1] https://github.com/git/git/runs/4214606314?check_suite_focus=true
>>
>>> With this change we shorten both the job names, and change the
>>> top-level "name" from "CI/PR" to "CI", since it will be used as a
>>> prefix in the tooltips. We also remove redundant or superfluous
>>> information from the name, e.g. "ubuntu-latest" isn't really needed
>>> for "linux-leaks", it'll suffice to say linux. For discovering what
>>> image runs that specifically we can consult main.yml itself.
>>>
>>
>> By optimizing for the tooltip, this patch shortens names to the point that
>> they're more difficult to interpret (w32 vs. w32/VS) and/or removes valuable
>> context about platform/image/etc. When a user *does* want more information
>> on the job, they now have to: 
>>
>> 1) know that the "CI/PR" job definition is in ".github/workflows/main.yml"
>> 2) parse through the file to find the job they want
>> 3) correlate that back to the job in the workflow details they're
>>    investigating. 
>>
>> That's a strictly worse experience for an extremely common use-case. What
>> use-case is this patch attempting to improve?
> 
> That I can click on the button that your co-workers implemented and see
> the relevant information about the job :)
> 

I'm sure you meant this in good faith, but I don't see how where I work is
relevant. GitHub is a tool you can use to develop Git, and I'm reviewing
because this patch would affect how I work on Git.

> Given that it's truncated we need to pick and choose what we display if
> we're not going to force the user to have to go to the full view every
> time.
> 

This is what I wanted to dig into by asking for a use-case. Which users do
you expect are using this tooltip view so heavily that what's displayed
there justifies this change? If you're one of those users, then waiting for
more feedback on this patch will hopefully provide a better idea of what the
"average" user finds helpful.

> I'll change s/w32/win/ etc, and there's room to move stuff around here,
> but I think it's fine to just not display that it's e.g. "ubuntu" or
> "fedora" at all. That's almost never been relevant.
> 
> If we were trying to do the opposite and lengthen the names to shove
> every bit of useful information in there at a glance I can think of 5-10
> things we'd put there before "fedora". libc/version, compiler/version,
> kernel/version etc.
> 

I'm sure there's plenty of information that would be helpful to have there;
however, that's not really a justification for removing some of what we
already have.

> Whether it's a recent Gentoo or Ubuntu is something that's OK to omit.
> 
> But maybe I'm wrong, are there cases you can think of where we really
> need "ubuntu" or "fedora" etc.?
> 

Yes, absolutely - knowing that `fedora-latest` was the image used in the
failing builds for v2.34.0-rc1 led to learning that the image was updated
the day before the build failed, which was instrumental in quickly
identifying the root cause of the bug [1]. In that case, I got the
information to start debugging from the web UI, *not* from digging into
`main.yml`. 

In general, easily finding what image a CI job was built on can help with
investigating bugs that arise from environmental differences. It's a minor
quality-of-life improvement, but it's no less significant than the benefit
you're suggesting this patch provides.

[1] https://lore.kernel.org/git/pull.1072.git.1635990465854.gitgitgadget@gmail.com/

>>> The above "regular (linux-gcc-default, gcc, ubuntu-latest)" job name
>>> then becomes a 1=1 match to the "$jobname" used in
>>> "ci/run-build-and-tests.sh". A "( push" or " (pull_request" is then
>>> added implicitly as before (from the top-level "on" parameter in
>>> "main.yml"). In the tooltip we'll now show:
>>>
>>>     "CI / linux-leaks (pu..."
>>>
>>> We then have no truncation in the expanded view. See [2] for a
>>> currently visible CI run using this commit, and [3] for the GitHub
>>> workflow syntax involved being changed here.
>>>
>>
>> If the only problem this patch really "solves" is making some job names fit
>> a bit better into the tooltip and, I think the costs (namely the loss of
>> accessible contextual info) outweigh any potential benefits you gain. 
> 
> Yeah it's a trade-off for sure, but now you can't see some of the
> relevant information at all in some views, so it's a gain for
> accessibility too.
> 

It may be a "gain for accessibility" in this one particular case but, in my
opinion, the relative decrease in accessibility by outright removing
information from the *all* CI views is greater. Of course, I don't represent
*all* users, so I'm interested in hearing what others have to say. For now,
though, I'm still unconvinced this patch is beneficial. 

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/2] CI: use shorter names that fit in UX tooltips
  2021-11-19 16:02   ` Victoria Dye
  2021-11-19 20:33     ` Ævar Arnfjörð Bjarmason
@ 2021-11-19 22:14     ` Junio C Hamano
  1 sibling, 0 replies; 38+ messages in thread
From: Junio C Hamano @ 2021-11-19 22:14 UTC (permalink / raw)
  To: Victoria Dye
  Cc: Ævar Arnfjörð Bjarmason, git, Johannes Schindelin,
	SZEDER Gábor

Victoria Dye <vdye@github.com> writes:

> Tooltips like the ones you've pointed out here appear intended to be an "at
> a glance" view of the jobs (mostly for showing pass/fail/skip status) - each
> job in the tooltip has a "Details" link that takes you to the job summary
> and logs. In the current state, although the names of the are truncated in
> the tooltip, the information is still easily accessible in the full workflow
> details (one click away). For example, the details for the "linux-leaks" job
> [1] tell me the image, compiler, and job name right at the top of the page.

While that is all true, if the truncated one does not allow viewers
to easily tell between linux-leaks and linux-gcc and what these are
about, there is not much value to have the "tooltips" in the first
place.  The user will be forced to always visit the details, that
defeats the whole point of having an "at a glance" view.

> By optimizing for the tooltip, this patch shortens names to the point that
> they're more difficult to interpret (w32 vs. w32/VS) and/or removes valuable
> context about platform/image/etc. When a user *does* want more information
> on the job, they now have to: 
>
> 1) know that the "CI/PR" job definition is in ".github/workflows/main.yml"
> 2) parse through the file to find the job they want
> 3) correlate that back to the job in the workflow details they're
>    investigating. 

That is something I have to do with the current scheme, too.  I do
not think "windows" -> "w32" makes it much worse.

> If the only problem this patch really "solves" is making some job names fit
> a bit better into the tooltip and, I think the costs (namely the loss of
> accessible contextual info) outweigh any potential benefits you gain. 

I think "fits in the limited space" is a mere approximation of what
renaming effort can achieve.  If we can cram what matters more in
the tight display space we have there, so that we do not have to go
to the details page or to (eek!) the YAML file all the time, that
would be a welcome change.  Between "vs-test (4)" and "w32/vs test
(4)", for example, I do not think there is much difference as both
are equally opaque and I cannot guess what the particular job is
testing.  But if we can have a more informative label, "at a glance"
view would become much more useful, no?

Having said that, I do agree with you that this iteration does a
fairly poor job at it. The only thing that I found very much better
in Ævar's sample run over the current one is the "linux-leaks" Job.
All the other changes were "Meh" to me.

If do you think "what matters" is purely personal taste, and we will
not be able to gain a concensus on which part of the whole string
(before truncation) is more important, then it is a different story.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/2] CI: use shorter names that fit in UX tooltips
  2021-11-19 21:55       ` Victoria Dye
@ 2021-11-20  2:51         ` Ævar Arnfjörð Bjarmason
  0 siblings, 0 replies; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-20  2:51 UTC (permalink / raw)
  To: Victoria Dye; +Cc: git, Junio C Hamano, Johannes Schindelin, SZEDER Gábor


On Fri, Nov 19 2021, Victoria Dye wrote:

> Ævar Arnfjörð Bjarmason wrote:
>> 
>> On Fri, Nov 19 2021, Victoria Dye wrote:
>> 
>>> Ævar Arnfjörð Bjarmason wrote:
>>>> Change the names used for the GitHub CI workflows to be short enough
>>>> to (mostly) fit in the pop-up tool-tips that GitHub shows in the
>>>> commit view. I.e. when mouse-clicking on the passing or failing
>>>> check-mark next to the commit subject.
>>>>
>>>> That description is truncated to 24 characters, with the 3 at the end
>>>> being placed by "...".
>>>>
>>>> E.g. the full job name (visible at [1]):
>>>>
>>>>     "regular (linux-gcc-default, gcc, ubuntu-latest)"
>>>>
>>>> Will, when shown in the tool-tip be truncated to:
>>>>
>>>>     "CI/PR / regular (linu..."
>>>>
>>>> There's then a further limit in the expanded view where the job names
>>>> are observably truncated to 44 characters (including "..."). I.e.:
>>>>
>>>>     "regular (linux-gcc-default, gcc, ubuntu-l..."
>>>>
>>>
>>> Tooltips like the ones you've pointed out here appear intended to be an "at
>>> a glance" view of the jobs (mostly for showing pass/fail/skip status) - each
>>> job in the tooltip has a "Details" link that takes you to the job summary
>>> and logs. In the current state, although the names of the are truncated in
>>> the tooltip, the information is still easily accessible in the full workflow
>>> details (one click away). For example, the details for the "linux-leaks" job
>>> [1] tell me the image, compiler, and job name right at the top of the page.
>>>
>>> [1] https://github.com/git/git/runs/4214606314?check_suite_focus=true
>>>
>>>> With this change we shorten both the job names, and change the
>>>> top-level "name" from "CI/PR" to "CI", since it will be used as a
>>>> prefix in the tooltips. We also remove redundant or superfluous
>>>> information from the name, e.g. "ubuntu-latest" isn't really needed
>>>> for "linux-leaks", it'll suffice to say linux. For discovering what
>>>> image runs that specifically we can consult main.yml itself.
>>>>
>>>
>>> By optimizing for the tooltip, this patch shortens names to the point that
>>> they're more difficult to interpret (w32 vs. w32/VS) and/or removes valuable
>>> context about platform/image/etc. When a user *does* want more information
>>> on the job, they now have to: 
>>>
>>> 1) know that the "CI/PR" job definition is in ".github/workflows/main.yml"
>>> 2) parse through the file to find the job they want
>>> 3) correlate that back to the job in the workflow details they're
>>>    investigating. 
>>>
>>> That's a strictly worse experience for an extremely common use-case. What
>>> use-case is this patch attempting to improve?
>> 
>> That I can click on the button that your co-workers implemented and see
>> the relevant information about the job :)
>> 
>
> I'm sure you meant this in good faith, but I don't see how where I work is
> relevant. GitHub is a tool you can use to develop Git, and I'm reviewing
> because this patch would affect how I work on Git.

Yes, sorry. That came off as quite snarky, it was just meant as a lame
joke.

>> Given that it's truncated we need to pick and choose what we display if
>> we're not going to force the user to have to go to the full view every
>> time.
>> 
>
> This is what I wanted to dig into by asking for a use-case. Which users do
> you expect are using this tooltip view so heavily that what's displayed
> there justifies this change? If you're one of those users, then waiting for
> more feedback on this patch will hopefully provide a better idea of what the
> "average" user finds helpful.

Will improve the description etc. in a re-roll.

>> I'll change s/w32/win/ etc, and there's room to move stuff around here,
>> but I think it's fine to just not display that it's e.g. "ubuntu" or
>> "fedora" at all. That's almost never been relevant.
>> 
>> If we were trying to do the opposite and lengthen the names to shove
>> every bit of useful information in there at a glance I can think of 5-10
>> things we'd put there before "fedora". libc/version, compiler/version,
>> kernel/version etc.
>> 
>
> I'm sure there's plenty of information that would be helpful to have there;
> however, that's not really a justification for removing some of what we
> already have.

I'll retain more information in an incoming re-roll.

>> Whether it's a recent Gentoo or Ubuntu is something that's OK to omit.
>> 
>> But maybe I'm wrong, are there cases you can think of where we really
>> need "ubuntu" or "fedora" etc.?
>> 
>
> Yes, absolutely - knowing that `fedora-latest` was the image used in the
> failing builds for v2.34.0-rc1 led to learning that the image was updated
> the day before the build failed, which was instrumental in quickly
> identifying the root cause of the bug [1]. In that case, I got the
> information to start debugging from the web UI, *not* from digging into
> `main.yml`. 
>
> In general, easily finding what image a CI job was built on can help with
> investigating bugs that arise from environmental differences. It's a minor
> quality-of-life improvement, but it's no less significant than the benefit
> you're suggesting this patch provides.
>
> [1] https://lore.kernel.org/git/pull.1072.git.1635990465854.gitgitgadget@gmail.com/

I thought that case was a good example of us creating a problem for
ourselves that we didn't need to have. as I noted downthread in:
https://lore.kernel.org/git/211104.86v918i78r.gmgdl@evledraar.gmail.com/

I.e. the "fedora" part didn't actually matter, it just so happened that
fedora had a relatively recent glibc/gcc, and when it was updated from
under us CI runs all over the place started failing.

Which is an issue that's entirely avoidable by pinning the dependency,
and not using *-latest.

In general I really don't see why who deploys software anywhere thinks
using this pattern of fluid dependencies is worth the hassle. It's
rather trivial to just run a cron script that bumps your dependencies,
commits and pushes.

It's not at all trivial to take software that downloads whatever
someone's random website has available at the moment and try to figure
out what state it was in last week, month or year when the state of that
third party website wasn't what it was today.

Anyway, I take your general point that these image names (or a derived
name) may be useful to someone somewhere.

I just thought this example was a bad one, because the only reason the
image name or update would have been unclear to anyone (as it was to me
when my CI started breaking due to this) is because we're using this
anti-pattern.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH v2 0/6] CI: Remove Travis CI, shorten names for GH tooltips, split jobs
  2021-11-19 13:56 [PATCH 0/2] CI: use shorter names for CI jobs, less truncation Ævar Arnfjörð Bjarmason
                   ` (3 preceding siblings ...)
  2021-11-19 19:57 ` Junio C Hamano
@ 2021-11-20  3:28 ` Ævar Arnfjörð Bjarmason
  2021-11-20  3:28   ` [PATCH v2 1/6] CI: remove Travis CI support Ævar Arnfjörð Bjarmason
                     ` (7 more replies)
  4 siblings, 8 replies; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-20  3:28 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, SZEDER Gábor,
	Victoria Dye, Ævar Arnfjörð Bjarmason

I thought I'd submit a smaller change to just address the GitHub CI
name truncation in tooltips (now 2/6), but part of the feedback was
why we needed to update Travis CI code, and can't we split up the
"default" job etc.

So here's a larger series I'd initially peeled v1[1] from a WIP
version of.

The end-state is that the job names are shorter, and some are now
split up, before:

    https://github.com/git/git/runs/4214600139

After:

    https://github.com/avar/git/runs/4271369035

1. https://lore.kernel.org/git/cover-0.2-00000000000-20211119T135343Z-avarab@gmail.com/

Ævar Arnfjörð Bjarmason (6):
  CI: remove Travis CI support
  CI: use shorter names that fit in UX tooltips
  CI: rename the "Linux32" job to lower-case "linux32"
  CI: use "$runs_on_pool", not "$jobname" to select packages & config
  CI: don't run "make test" twice in one job
  CI: run "documentation" via run-build-and-test.sh

 .github/workflows/main.yml        | 44 ++++++++++++++++++++--
 .travis.yml                       | 60 ------------------------------
 README.md                         |  2 +-
 ci/install-dependencies.sh        | 35 ++++++++---------
 ci/install-docker-dependencies.sh |  2 +-
 ci/lib.sh                         | 62 ++++++++-----------------------
 ci/print-test-failures.sh         | 10 -----
 ci/run-build-and-tests.sh         | 47 +++++++++++++++--------
 ci/run-docker-build.sh            | 11 +-----
 ci/run-docker.sh                  |  4 +-
 ci/test-documentation.sh          | 39 ++++++++-----------
 11 files changed, 123 insertions(+), 193 deletions(-)
 delete mode 100644 .travis.yml

Range-diff against v1:
-:  ----------- > 1:  cc94a353ccb CI: remove Travis CI support
1:  26f80c87c8d ! 2:  73981cedee8 CI: use shorter names that fit in UX tooltips
    @@ Commit message
         commit view. I.e. when mouse-clicking on the passing or failing
         check-mark next to the commit subject.
     
    -    That description is truncated to 24 characters, with the 3 at the end
    -    being placed by "...".
    -
    -    E.g. the full job name (visible at [1]):
    -
    -        "regular (linux-gcc-default, gcc, ubuntu-latest)"
    -
    -    Will, when shown in the tool-tip be truncated to:
    -
    -        "CI/PR / regular (linu..."
    -
    -    There's then a further limit in the expanded view where the job names
    -    are observably truncated to 44 characters (including "..."). I.e.:
    -
    -        "regular (linux-gcc-default, gcc, ubuntu-l..."
    -
    -    With this change we shorten both the job names, and change the
    -    top-level "name" from "CI/PR" to "CI", since it will be used as a
    -    prefix in the tooltips. We also remove redundant or superfluous
    -    information from the name, e.g. "ubuntu-latest" isn't really needed
    -    for "linux-leaks", it'll suffice to say linux. For discovering what
    -    image runs that specifically we can consult main.yml itself.
    -
    -    The above "regular (linux-gcc-default, gcc, ubuntu-latest)" job name
    -    then becomes a 1=1 match to the "$jobname" used in
    -    "ci/run-build-and-tests.sh". A "( push" or " (pull_request" is then
    -    added implicitly as before (from the top-level "on" parameter in
    -    "main.yml"). In the tooltip we'll now show:
    -
    -        "CI / linux-leaks (pu..."
    -
    -    We then have no truncation in the expanded view. See [2] for a
    -    currently visible CI run using this commit, and [3] for the GitHub
    -    workflow syntax involved being changed here.
    -
    -    We could avoid even more truncation with more compact names,
    -    e.g. changing "linux" to "lin" or "lnx", but I didn't do that since
    -    any additional shortening seemed counterproductive, i.e. "w32" is a
    -    well-known way of referring to "Windows", but "lin" isn't). We could
    -    also shorten e.g. "::build" and "::test" to "+bld" and "+tst", but
    -    those seem similarly to be overly obtuse.
    +    These names are seemingly truncated to 17-20 characters followed by
    +    three dots ("..."). Since a "CI/PR / " prefix is added to them the job
    +    names looked like this before (windows-test and vs-test jobs omitted):
    +
    +        CI/PR / ci-config (p...
    +        CI/PR / windows-buil...
    +        CI/PR / vs-build (pu...
    +        CI/PR / regular (lin...
    +        CI/PR / regular (lin...
    +        CI/PR / regular (os...
    +        CI/PR / regular (os...
    +        CI/PR / regular (lin...
    +        CI/PR / regular (lin...
    +        CI/PR / dockerized (...
    +        CI/PR / dockerized (...
    +        CI/PR / dockerized (...
    +        CI/PR / static-anal...
    +        CI/PR / sparse (pu...
    +        CI/PR / documenta...
    +
    +    By omitting the "/PR" from the top-level name, and pushing the
    +    $jobname to the front we'll now instead get:
    +
    +        CI / config (push)
    +        CI / win build (push...
    +        CI / win+VS build (...
    +        CI / linux-clang (ub...
    +        CI / linux-gcc (ubun...
    +        CI / osx-clang (osx)...
    +        CI / osx-gcc (osx) (...
    +        CI / linux-gcc-defau...
    +        CI / linux-leaks (ub...
    +        CI / linux-musl (alp...
    +        CI / Linux32 (daald/...
    +        CI / pedantic (fedor...
    +        CI / static-analysis...
    +        CI / sparse (push)...
    +        CI / documentation
    +
    +    We then have no truncation in the expanded view. See [1] for how it
    +    looked before, [2] for a currently visible CI run using this commit,
    +    and [3] for the GitHub workflow syntax involved being changed here.
    +
    +    Let's also add a field for the "os" and use it where appropriate, it's
    +    occasionally useful to know we're running on say ubuntu
    +    v.s. fedora (but the "-latest" suffix isn't very useful, that applies
    +    to almost all the jobs.
     
         1. https://github.com/git/git/tree/master/
    -    2. https://github.com/avar/git/tree/avar/ci-shorter-names
    +    2. https://github.com/avar/git/tree/avar/ci-rm-travis-cleanup-ci-names-2
         3. https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
    @@ .github/workflows/main.yml: jobs:
                  }
      
        windows-build:
    -+    name: w32::build
    ++    name: win build
          needs: ci-config
          if: needs.ci-config.outputs.enabled == 'yes'
          runs-on: windows-latest
    @@ .github/workflows/main.yml: jobs:
              name: windows-artifacts
              path: artifacts
        windows-test:
    -+    name: w32::test
    ++    name: win test
          runs-on: windows-latest
          needs: [windows-build]
          strategy:
    @@ .github/workflows/main.yml: jobs:
              name: failed-tests-windows
              path: ${{env.FAILED_TEST_ARTIFACTS}}
        vs-build:
    -+    name: w32/VS::build
    ++    name: win+VS build
          needs: ci-config
          if: needs.ci-config.outputs.enabled == 'yes'
          env:
    @@ .github/workflows/main.yml: jobs:
              name: vs-artifacts
              path: artifacts
        vs-test:
    -+    name: w32/VS::test
    ++    name: win+VS test
          runs-on: windows-latest
          needs: vs-build
          strategy:
    @@ .github/workflows/main.yml: jobs:
              name: failed-tests-windows
              path: ${{env.FAILED_TEST_ARTIFACTS}}
        regular:
    -+    name: ${{matrix.vector.jobname}}
    ++    name: ${{matrix.vector.jobname}} (${{matrix.vector.os}})
          needs: ci-config
          if: needs.ci-config.outputs.enabled == 'yes'
          strategy:
    +@@ .github/workflows/main.yml: jobs:
    +         vector:
    +           - jobname: linux-clang
    +             cc: clang
    ++            os: ubuntu
    +             pool: ubuntu-latest
    +           - jobname: linux-gcc
    +             cc: gcc
    ++            os: ubuntu
    +             pool: ubuntu-latest
    +           - jobname: osx-clang
    +             cc: clang
    ++            os: osx
    +             pool: macos-latest
    +           - jobname: osx-gcc
    +             cc: gcc
    ++            os: osx
    +             pool: macos-latest
    +           - jobname: linux-gcc-default
    +             cc: gcc
    ++            os: ubuntu
    +             pool: ubuntu-latest
    +           - jobname: linux-leaks
    +             cc: gcc
    ++            os: ubuntu
    +             pool: ubuntu-latest
    +     env:
    +       CC: ${{matrix.vector.cc}}
     @@ .github/workflows/main.yml: jobs:
              name: failed-tests-${{matrix.vector.jobname}}
              path: ${{env.FAILED_TEST_ARTIFACTS}}
        dockerized:
    -+    name: ${{matrix.vector.jobname}} (docker)
    ++    name: ${{matrix.vector.jobname}} (${{matrix.vector.image}})
          needs: ci-config
          if: needs.ci-config.outputs.enabled == 'yes'
          strategy:
    -@@ .github/workflows/main.yml: jobs:
    -       matrix:
    -         vector:
    -         - jobname: linux-musl
    -+          os: alpine
    -           image: alpine
    -         - jobname: Linux32
    -+          os: ubuntu32
    -           image: daald/ubuntu32:xenial
    -         - jobname: pedantic
    -+          os: fedora
    -           image: fedora
    -     env:
    -       jobname: ${{matrix.vector.jobname}}
     @@ .github/workflows/main.yml: jobs:
            run: ci/install-dependencies.sh
          - run: make sparse
2:  9b8a3f0cdc4 ! 3:  002c183fff4 CI: rename the "Linux32" job to lower-case "linux32"
    @@ Commit message
     
      ## .github/workflows/main.yml ##
     @@ .github/workflows/main.yml: jobs:
    +         vector:
              - jobname: linux-musl
    -           os: alpine
                image: alpine
     -        - jobname: Linux32
     +        - jobname: linux32
    -           os: ubuntu32
    ++          os: ubuntu32
                image: daald/ubuntu32:xenial
              - jobname: pedantic
    -
    - ## .travis.yml ##
    -@@ .travis.yml: matrix:
    -       os: linux
    -       dist: trusty
    -       compiler:
    --    - env: jobname=Linux32
    -+    - env: jobname=linux32
    -       os: linux
    -       compiler:
    -       addons:
    +           image: fedora
     
      ## ci/install-docker-dependencies.sh ##
     @@
-:  ----------- > 4:  eca0ad08d4b CI: use "$runs_on_pool", not "$jobname" to select packages & config
-:  ----------- > 5:  a113b8404ed CI: don't run "make test" twice in one job
-:  ----------- > 6:  7c423c8283d CI: run "documentation" via run-build-and-test.sh
-- 
2.34.0.823.gcc3243ae16c


^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH v2 1/6] CI: remove Travis CI support
  2021-11-20  3:28 ` [PATCH v2 0/6] CI: Remove Travis CI, shorten names for GH tooltips, split jobs Ævar Arnfjörð Bjarmason
@ 2021-11-20  3:28   ` Ævar Arnfjörð Bjarmason
  2021-11-20  3:28   ` [PATCH v2 2/6] CI: use shorter names that fit in UX tooltips Ævar Arnfjörð Bjarmason
                     ` (6 subsequent siblings)
  7 siblings, 0 replies; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-20  3:28 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, SZEDER Gábor,
	Victoria Dye, Ævar Arnfjörð Bjarmason

Remove support for running the CI in travis. The last builds in it are
from 5 months ago[1] (as of 2021-11-19), and our documentation has
referred to GitHub CI instead since f003a91f5c5 (SubmittingPatches:
replace discussion of Travis with GitHub Actions, 2021-07-22).

We'll now run the "t9810 t9816" and tests on OSX. We didn't before, as
we'd carried the Travis exclusion of them forward from
522354d70f4 (Add Travis CI support, 2015-11-27). Let's hope whatever
issue there was with them was either Travis specific, or fixed since
then (I'm not sure).

The "apt-add-repository" invocation (which we were doing in GitHub CI)
isn't needed, it was another Travis-only case that was carried forward
into more general code. See 0f0c51181df (travis-ci: install packages
in 'ci/install-dependencies.sh', 2018-11-01).

1. https://travis-ci.org/github/git/git/builds

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 .travis.yml                | 60 --------------------------------------
 ci/install-dependencies.sh |  1 -
 ci/lib.sh                  | 37 ++---------------------
 ci/print-test-failures.sh  | 10 -------
 ci/run-docker-build.sh     |  9 ------
 ci/run-docker.sh           |  2 +-
 6 files changed, 4 insertions(+), 115 deletions(-)
 delete mode 100644 .travis.yml

diff --git a/.travis.yml b/.travis.yml
deleted file mode 100644
index 908330a0a3d..00000000000
--- a/.travis.yml
+++ /dev/null
@@ -1,60 +0,0 @@
-language: c
-
-cache:
-  directories:
-    - $HOME/travis-cache
-
-os:
-  - linux
-  - osx
-
-osx_image: xcode10.1
-
-compiler:
-  - clang
-  - gcc
-
-matrix:
-  include:
-    - env: jobname=linux-gcc-default
-      os: linux
-      compiler:
-      addons:
-      before_install:
-    - env: jobname=linux-gcc-4.8
-      os: linux
-      dist: trusty
-      compiler:
-    - env: jobname=Linux32
-      os: linux
-      compiler:
-      addons:
-      services:
-        - docker
-      before_install:
-      script: ci/run-docker.sh
-    - env: jobname=linux-musl
-      os: linux
-      compiler:
-      addons:
-      services:
-        - docker
-      before_install:
-      script: ci/run-docker.sh
-    - env: jobname=StaticAnalysis
-      os: linux
-      compiler:
-      script: ci/run-static-analysis.sh
-      after_failure:
-    - env: jobname=Documentation
-      os: linux
-      compiler:
-      script: ci/test-documentation.sh
-      after_failure:
-
-before_install: ci/install-dependencies.sh
-script: ci/run-build-and-tests.sh
-after_failure: ci/print-test-failures.sh
-
-notifications:
-  email: false
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 1d0e48f4515..92e11c7198e 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -13,7 +13,6 @@ UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
 
 case "$jobname" in
 linux-clang|linux-gcc|linux-leaks)
-	sudo apt-add-repository -y "ppa:ubuntu-toolchain-r/test"
 	sudo apt-get -q update
 	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
 		$UBUNTU_COMMON_PKGS
diff --git a/ci/lib.sh b/ci/lib.sh
index 82cb17f8eea..73d959e87f7 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -34,7 +34,7 @@ save_good_tree () {
 # successfully before (e.g. because the branch got rebased, changing only
 # the commit messages).
 skip_good_tree () {
-	if test "$TRAVIS_DEBUG_MODE" = true || test true = "$GITHUB_ACTIONS"
+	if test true = "$GITHUB_ACTIONS"
 	then
 		return
 	fi
@@ -60,7 +60,7 @@ skip_good_tree () {
 			cat <<-EOF
 			$(tput setaf 2)Skipping build job for commit $CI_COMMIT.$(tput sgr0)
 			This commit's tree has already been built and tested successfully in build job $prev_good_job_number for commit $prev_good_commit.
-			The log of that build job is available at $(url_for_job_id $prev_good_job_id)
+			The log of that build job is available at $SYSTEM_TASKDEFINITIONSURI$SYSTEM_TEAMPROJECT/_build/results?buildId=$prev_good_job_id
 			To force a re-build delete the branch's cache and then hit 'Restart job'.
 			EOF
 		fi
@@ -91,29 +91,7 @@ export MAKEFLAGS=
 # and installing dependencies.
 set -ex
 
-if test true = "$TRAVIS"
-then
-	CI_TYPE=travis
-	# When building a PR, TRAVIS_BRANCH refers to the *target* branch. Not
-	# what we want here. We want the source branch instead.
-	CI_BRANCH="${TRAVIS_PULL_REQUEST_BRANCH:-$TRAVIS_BRANCH}"
-	CI_COMMIT="$TRAVIS_COMMIT"
-	CI_JOB_ID="$TRAVIS_JOB_ID"
-	CI_JOB_NUMBER="$TRAVIS_JOB_NUMBER"
-	CI_OS_NAME="$TRAVIS_OS_NAME"
-	CI_REPO_SLUG="$TRAVIS_REPO_SLUG"
-
-	cache_dir="$HOME/travis-cache"
-
-	url_for_job_id () {
-		echo "https://travis-ci.org/$CI_REPO_SLUG/jobs/$1"
-	}
-
-	BREW_INSTALL_PACKAGES="git-lfs gettext"
-	export GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save"
-	export GIT_TEST_OPTS="--verbose-log -x --immediate"
-	MAKEFLAGS="$MAKEFLAGS --jobs=2"
-elif test -n "$SYSTEM_COLLECTIONURI" || test -n "$SYSTEM_TASKDEFINITIONSURI"
+if test -n "$SYSTEM_COLLECTIONURI" || test -n "$SYSTEM_TASKDEFINITIONSURI"
 then
 	CI_TYPE=azure-pipelines
 	# We are running in Azure Pipelines
@@ -130,10 +108,6 @@ then
 	# among *all* phases)
 	cache_dir="$HOME/test-cache/$SYSTEM_PHASENAME"
 
-	url_for_job_id () {
-		echo "$SYSTEM_TASKDEFINITIONSURI$SYSTEM_TEAMPROJECT/_build/results?buildId=$1"
-	}
-
 	export GIT_PROVE_OPTS="--timer --jobs 10 --state=failed,slow,save"
 	export GIT_TEST_OPTS="--verbose-log -x --write-junit-xml"
 	MAKEFLAGS="$MAKEFLAGS --jobs=10"
@@ -214,11 +188,6 @@ osx-clang|osx-gcc)
 	else
 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python2)"
 	fi
-
-	# t9810 occasionally fails on Travis CI OS X
-	# t9816 occasionally fails with "TAP out of sequence errors" on
-	# Travis CI OS X
-	export GIT_SKIP_TESTS="t9810 t9816"
 	;;
 linux-gcc-default)
 	;;
diff --git a/ci/print-test-failures.sh b/ci/print-test-failures.sh
index c70d6cdbf24..57277eefcd0 100755
--- a/ci/print-test-failures.sh
+++ b/ci/print-test-failures.sh
@@ -39,8 +39,6 @@ do
 		test_name="${test_name##*/}"
 		trash_dir="trash directory.$test_name"
 		case "$CI_TYPE" in
-		travis)
-			;;
 		azure-pipelines)
 			mkdir -p failed-test-artifacts
 			mv "$trash_dir" failed-test-artifacts
@@ -88,11 +86,3 @@ do
 		fi
 	fi
 done
-
-if [ $combined_trash_size -gt 0 ]
-then
-	echo "------------------------------------------------------------------------"
-	echo "Trash directories embedded in this log can be extracted by running:"
-	echo
-	echo "  curl https://api.travis-ci.org/v3/job/$TRAVIS_JOB_ID/log.txt |./ci/util/extract-trash-dirs.sh"
-fi
diff --git a/ci/run-docker-build.sh b/ci/run-docker-build.sh
index 8d47a5fda3b..5d2764ad3a3 100755
--- a/ci/run-docker-build.sh
+++ b/ci/run-docker-build.sh
@@ -47,15 +47,6 @@ else
 	else
 		useradd -u $HOST_UID $CI_USER
 	fi
-
-	# Due to a bug the test suite was run as root in the past, so
-	# a prove state file created back then is only accessible by
-	# root.  Now that bug is fixed, the test suite is run as a
-	# regular user, but the prove state file coming from Travis
-	# CI's cache might still be owned by root.
-	# Make sure that this user has rights to any cached files,
-	# including an existing prove state file.
-	test -n "$cache_dir" && chown -R $HOST_UID:$HOST_UID "$cache_dir"
 fi
 
 # Build and test
diff --git a/ci/run-docker.sh b/ci/run-docker.sh
index 37fa372052d..b610dd4db84 100755
--- a/ci/run-docker.sh
+++ b/ci/run-docker.sh
@@ -25,7 +25,7 @@ docker pull "$CI_CONTAINER"
 # root@container:/# export jobname=<jobname>
 # root@container:/# /usr/src/git/ci/run-docker-build.sh <host-user-id>
 
-container_cache_dir=/tmp/travis-cache
+container_cache_dir=/tmp/container-cache
 
 docker run \
 	--interactive \
-- 
2.34.0.823.gcc3243ae16c


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v2 2/6] CI: use shorter names that fit in UX tooltips
  2021-11-20  3:28 ` [PATCH v2 0/6] CI: Remove Travis CI, shorten names for GH tooltips, split jobs Ævar Arnfjörð Bjarmason
  2021-11-20  3:28   ` [PATCH v2 1/6] CI: remove Travis CI support Ævar Arnfjörð Bjarmason
@ 2021-11-20  3:28   ` Ævar Arnfjörð Bjarmason
  2021-11-20  7:01     ` Victoria Dye
  2021-11-20  3:28   ` [PATCH v2 3/6] CI: rename the "Linux32" job to lower-case "linux32" Ævar Arnfjörð Bjarmason
                     ` (5 subsequent siblings)
  7 siblings, 1 reply; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-20  3:28 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, SZEDER Gábor,
	Victoria Dye, Ævar Arnfjörð Bjarmason

Change the names used for the GitHub CI workflows to be short enough
to (mostly) fit in the pop-up tool-tips that GitHub shows in the
commit view. I.e. when mouse-clicking on the passing or failing
check-mark next to the commit subject.

These names are seemingly truncated to 17-20 characters followed by
three dots ("..."). Since a "CI/PR / " prefix is added to them the job
names looked like this before (windows-test and vs-test jobs omitted):

    CI/PR / ci-config (p...
    CI/PR / windows-buil...
    CI/PR / vs-build (pu...
    CI/PR / regular (lin...
    CI/PR / regular (lin...
    CI/PR / regular (os...
    CI/PR / regular (os...
    CI/PR / regular (lin...
    CI/PR / regular (lin...
    CI/PR / dockerized (...
    CI/PR / dockerized (...
    CI/PR / dockerized (...
    CI/PR / static-anal...
    CI/PR / sparse (pu...
    CI/PR / documenta...

By omitting the "/PR" from the top-level name, and pushing the
$jobname to the front we'll now instead get:

    CI / config (push)
    CI / win build (push...
    CI / win+VS build (...
    CI / linux-clang (ub...
    CI / linux-gcc (ubun...
    CI / osx-clang (osx)...
    CI / osx-gcc (osx) (...
    CI / linux-gcc-defau...
    CI / linux-leaks (ub...
    CI / linux-musl (alp...
    CI / Linux32 (daald/...
    CI / pedantic (fedor...
    CI / static-analysis...
    CI / sparse (push)...
    CI / documentation

We then have no truncation in the expanded view. See [1] for how it
looked before, [2] for a currently visible CI run using this commit,
and [3] for the GitHub workflow syntax involved being changed here.

Let's also add a field for the "os" and use it where appropriate, it's
occasionally useful to know we're running on say ubuntu
v.s. fedora (but the "-latest" suffix isn't very useful, that applies
to almost all the jobs.

1. https://github.com/git/git/tree/master/
2. https://github.com/avar/git/tree/avar/ci-rm-travis-cleanup-ci-names-2
3. https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 .github/workflows/main.yml | 16 +++++++++++++++-
 README.md                  |  2 +-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 6ed6a9e8076..612b475fd0b 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -1,4 +1,4 @@
-name: CI/PR
+name: CI
 
 on: [push, pull_request]
 
@@ -7,6 +7,7 @@ env:
 
 jobs:
   ci-config:
+    name: config
     runs-on: ubuntu-latest
     outputs:
       enabled: ${{ steps.check-ref.outputs.enabled }}${{ steps.skip-if-redundant.outputs.enabled }}
@@ -77,6 +78,7 @@ jobs:
             }
 
   windows-build:
+    name: win build
     needs: ci-config
     if: needs.ci-config.outputs.enabled == 'yes'
     runs-on: windows-latest
@@ -97,6 +99,7 @@ jobs:
         name: windows-artifacts
         path: artifacts
   windows-test:
+    name: win test
     runs-on: windows-latest
     needs: [windows-build]
     strategy:
@@ -127,6 +130,7 @@ jobs:
         name: failed-tests-windows
         path: ${{env.FAILED_TEST_ARTIFACTS}}
   vs-build:
+    name: win+VS build
     needs: ci-config
     if: needs.ci-config.outputs.enabled == 'yes'
     env:
@@ -178,6 +182,7 @@ jobs:
         name: vs-artifacts
         path: artifacts
   vs-test:
+    name: win+VS test
     runs-on: windows-latest
     needs: vs-build
     strategy:
@@ -210,6 +215,7 @@ jobs:
         name: failed-tests-windows
         path: ${{env.FAILED_TEST_ARTIFACTS}}
   regular:
+    name: ${{matrix.vector.jobname}} (${{matrix.vector.os}})
     needs: ci-config
     if: needs.ci-config.outputs.enabled == 'yes'
     strategy:
@@ -218,21 +224,27 @@ jobs:
         vector:
           - jobname: linux-clang
             cc: clang
+            os: ubuntu
             pool: ubuntu-latest
           - jobname: linux-gcc
             cc: gcc
+            os: ubuntu
             pool: ubuntu-latest
           - jobname: osx-clang
             cc: clang
+            os: osx
             pool: macos-latest
           - jobname: osx-gcc
             cc: gcc
+            os: osx
             pool: macos-latest
           - jobname: linux-gcc-default
             cc: gcc
+            os: ubuntu
             pool: ubuntu-latest
           - jobname: linux-leaks
             cc: gcc
+            os: ubuntu
             pool: ubuntu-latest
     env:
       CC: ${{matrix.vector.cc}}
@@ -251,6 +263,7 @@ jobs:
         name: failed-tests-${{matrix.vector.jobname}}
         path: ${{env.FAILED_TEST_ARTIFACTS}}
   dockerized:
+    name: ${{matrix.vector.jobname}} (${{matrix.vector.image}})
     needs: ci-config
     if: needs.ci-config.outputs.enabled == 'yes'
     strategy:
@@ -310,6 +323,7 @@ jobs:
       run: ci/install-dependencies.sh
     - run: make sparse
   documentation:
+    name: documentation
     needs: ci-config
     if: needs.ci-config.outputs.enabled == 'yes'
     env:
diff --git a/README.md b/README.md
index eb8115e6b04..f6f43e78deb 100644
--- a/README.md
+++ b/README.md
@@ -1,4 +1,4 @@
-[![Build status](https://github.com/git/git/workflows/CI/PR/badge.svg)](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush)
+[![Build status](https://github.com/git/git/workflows/CI/badge.svg)](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush)
 
 Git - fast, scalable, distributed revision control system
 =========================================================
-- 
2.34.0.823.gcc3243ae16c


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v2 3/6] CI: rename the "Linux32" job to lower-case "linux32"
  2021-11-20  3:28 ` [PATCH v2 0/6] CI: Remove Travis CI, shorten names for GH tooltips, split jobs Ævar Arnfjörð Bjarmason
  2021-11-20  3:28   ` [PATCH v2 1/6] CI: remove Travis CI support Ævar Arnfjörð Bjarmason
  2021-11-20  3:28   ` [PATCH v2 2/6] CI: use shorter names that fit in UX tooltips Ævar Arnfjörð Bjarmason
@ 2021-11-20  3:28   ` Ævar Arnfjörð Bjarmason
  2021-11-20  3:28   ` [PATCH v2 4/6] CI: use "$runs_on_pool", not "$jobname" to select packages & config Ævar Arnfjörð Bjarmason
                     ` (4 subsequent siblings)
  7 siblings, 0 replies; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-20  3:28 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, SZEDER Gábor,
	Victoria Dye, Ævar Arnfjörð Bjarmason

As a follow-up to the preceding commit's shortening of CI job names,
rename the only job that starts with an upper-case letter to be
consistent with the rest. It was added in 88dedd5e72c (Travis: also
test on 32-bit Linux, 2017-03-05).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 .github/workflows/main.yml        | 3 ++-
 ci/install-docker-dependencies.sh | 2 +-
 ci/lib.sh                         | 2 +-
 ci/run-docker-build.sh            | 2 +-
 ci/run-docker.sh                  | 2 +-
 5 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 612b475fd0b..a91edec46d8 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -272,7 +272,8 @@ jobs:
         vector:
         - jobname: linux-musl
           image: alpine
-        - jobname: Linux32
+        - jobname: linux32
+          os: ubuntu32
           image: daald/ubuntu32:xenial
         - jobname: pedantic
           image: fedora
diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
index 07a8c6b199d..78b7e326da6 100755
--- a/ci/install-docker-dependencies.sh
+++ b/ci/install-docker-dependencies.sh
@@ -4,7 +4,7 @@
 #
 
 case "$jobname" in
-Linux32)
+linux32)
 	linux32 --32bit i386 sh -c '
 		apt update >/dev/null &&
 		apt install -y build-essential libcurl4-openssl-dev \
diff --git a/ci/lib.sh b/ci/lib.sh
index 73d959e87f7..0b3b0144882 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -191,7 +191,7 @@ osx-clang|osx-gcc)
 	;;
 linux-gcc-default)
 	;;
-Linux32)
+linux32)
 	CC=gcc
 	;;
 linux-musl)
diff --git a/ci/run-docker-build.sh b/ci/run-docker-build.sh
index 5d2764ad3a3..6cd832efb9c 100755
--- a/ci/run-docker-build.sh
+++ b/ci/run-docker-build.sh
@@ -15,7 +15,7 @@ then
 fi
 
 case "$jobname" in
-Linux32)
+linux32)
 	switch_cmd="linux32 --32bit i386"
 	;;
 linux-musl)
diff --git a/ci/run-docker.sh b/ci/run-docker.sh
index b610dd4db84..af89d1624a4 100755
--- a/ci/run-docker.sh
+++ b/ci/run-docker.sh
@@ -6,7 +6,7 @@
 . ${0%/*}/lib.sh
 
 case "$jobname" in
-Linux32)
+linux32)
 	CI_CONTAINER="daald/ubuntu32:xenial"
 	;;
 linux-musl)
-- 
2.34.0.823.gcc3243ae16c


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v2 4/6] CI: use "$runs_on_pool", not "$jobname" to select packages & config
  2021-11-20  3:28 ` [PATCH v2 0/6] CI: Remove Travis CI, shorten names for GH tooltips, split jobs Ævar Arnfjörð Bjarmason
                     ` (2 preceding siblings ...)
  2021-11-20  3:28   ` [PATCH v2 3/6] CI: rename the "Linux32" job to lower-case "linux32" Ævar Arnfjörð Bjarmason
@ 2021-11-20  3:28   ` Ævar Arnfjörð Bjarmason
  2021-11-20  3:28   ` [PATCH v2 5/6] CI: don't run "make test" twice in one job Ævar Arnfjörð Bjarmason
                     ` (3 subsequent siblings)
  7 siblings, 0 replies; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-20  3:28 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, SZEDER Gábor,
	Victoria Dye, Ævar Arnfjörð Bjarmason

Change the setup hooks for the CI to use "$runs_on_pool" for the
"$regular" job. Now we won't need as much boilerplate when adding new
jobs to the "regular" matrix, see 956d2e4639b (tests: add a test mode
for SANITIZE=leak, run it in CI, 2021-09-23) for the last such commit.

I.e. now instead of needing to enumerate each jobname when we select
packages we can install things depending on the pool we're running
in.

That we didn't do this dates back to the now gone dependency on Travis
CI, but even if we add a new CI target in the future this'll be easier
to port over, since we can probably treat "ubuntu-latest" as a
stand-in for some recent Linux that can run "apt" commands.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 .github/workflows/main.yml |  4 ++++
 ci/install-dependencies.sh | 32 ++++++++++++++------------------
 ci/lib.sh                  | 21 +++++++++++----------
 3 files changed, 29 insertions(+), 28 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index a91edec46d8..097be4c4405 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -229,6 +229,7 @@ jobs:
           - jobname: linux-gcc
             cc: gcc
             os: ubuntu
+            cc_package: gcc-8
             pool: ubuntu-latest
           - jobname: osx-clang
             cc: clang
@@ -237,6 +238,7 @@ jobs:
           - jobname: osx-gcc
             cc: gcc
             os: osx
+            cc_package: gcc-9
             pool: macos-latest
           - jobname: linux-gcc-default
             cc: gcc
@@ -248,7 +250,9 @@ jobs:
             pool: ubuntu-latest
     env:
       CC: ${{matrix.vector.cc}}
+      CC_PACKAGE: ${{matrix.vector.cc_package}}
       jobname: ${{matrix.vector.jobname}}
+      runs_on_pool: ${{matrix.vector.pool}}
     runs-on: ${{matrix.vector.pool}}
     steps:
     - uses: actions/checkout@v2
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 92e11c7198e..ca1eaa49c14 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -11,17 +11,11 @@ UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
  tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl
  libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl"
 
-case "$jobname" in
-linux-clang|linux-gcc|linux-leaks)
+case "$runs_on_pool" in
+ubuntu-latest)
 	sudo apt-get -q update
 	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
-		$UBUNTU_COMMON_PKGS
-	case "$jobname" in
-	linux-gcc)
-		sudo apt-get -q -y install gcc-8
-		;;
-	esac
-
+		$UBUNTU_COMMON_PKGS $CC_PACKAGE
 	mkdir --parents "$P4_PATH"
 	pushd "$P4_PATH"
 		wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d"
@@ -36,7 +30,7 @@ linux-clang|linux-gcc|linux-leaks)
 		cp git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs .
 	popd
 	;;
-osx-clang|osx-gcc)
+macos-latest)
 	export HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_INSTALL_CLEANUP=1
 	# Uncomment this if you want to run perf tests:
 	# brew install gnu-time
@@ -50,15 +44,17 @@ osx-clang|osx-gcc)
 		brew install --cask --no-quarantine perforce
 	} ||
 	brew install homebrew/cask/perforce
-	case "$jobname" in
-	osx-gcc)
-		brew install gcc@9
-		# Just in case the image is updated to contain gcc@9
-		# pre-installed but not linked.
-		brew link gcc@9
-		;;
-	esac
+
+	if test -n "$CC_PACKAGE"
+	then
+		BREW_PACKAGE=${CC_PACKAGE/-/@}
+		brew install "$BREW_PACKAGE"
+		brew link "$BREW_PACKAGE"
+	fi
 	;;
+esac
+
+case "$jobname" in
 StaticAnalysis)
 	sudo apt-get -q update
 	sudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \
diff --git a/ci/lib.sh b/ci/lib.sh
index 0b3b0144882..cbc2f8f1caa 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -156,11 +156,15 @@ export DEFAULT_TEST_TARGET=prove
 export GIT_TEST_CLONE_2GB=true
 export SKIP_DASHED_BUILT_INS=YesPlease
 
-case "$jobname" in
-linux-clang|linux-gcc|linux-leaks)
+case "$runs_on_pool" in
+ubuntu-latest)
+	if test "$jobname" = "linux-gcc-default"
+	then
+		break
+	fi
+
 	if [ "$jobname" = linux-gcc ]
 	then
-		export CC=gcc-8
 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python3"
 	else
 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python2"
@@ -180,17 +184,17 @@ linux-clang|linux-gcc|linux-leaks)
 	GIT_LFS_PATH="$HOME/custom/git-lfs"
 	export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH"
 	;;
-osx-clang|osx-gcc)
+macos-latest)
 	if [ "$jobname" = osx-gcc ]
 	then
-		export CC=gcc-9
 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python3)"
 	else
 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python2)"
 	fi
 	;;
-linux-gcc-default)
-	;;
+esac
+
+case "$jobname" in
 linux32)
 	CC=gcc
 	;;
@@ -200,9 +204,6 @@ linux-musl)
 	MAKEFLAGS="$MAKEFLAGS NO_REGEX=Yes ICONV_OMITS_BOM=Yes"
 	MAKEFLAGS="$MAKEFLAGS GIT_TEST_UTF8_LOCALE=C.UTF-8"
 	;;
-esac
-
-case "$jobname" in
 linux-leaks)
 	export SANITIZE=leak
 	export GIT_TEST_PASSING_SANITIZE_LEAK=true
-- 
2.34.0.823.gcc3243ae16c


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v2 5/6] CI: don't run "make test" twice in one job
  2021-11-20  3:28 ` [PATCH v2 0/6] CI: Remove Travis CI, shorten names for GH tooltips, split jobs Ævar Arnfjörð Bjarmason
                     ` (3 preceding siblings ...)
  2021-11-20  3:28   ` [PATCH v2 4/6] CI: use "$runs_on_pool", not "$jobname" to select packages & config Ævar Arnfjörð Bjarmason
@ 2021-11-20  3:28   ` Ævar Arnfjörð Bjarmason
  2021-11-20  3:28   ` [PATCH v2 6/6] CI: run "documentation" via run-build-and-test.sh Ævar Arnfjörð Bjarmason
                     ` (2 subsequent siblings)
  7 siblings, 0 replies; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-20  3:28 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, SZEDER Gábor,
	Victoria Dye, Ævar Arnfjörð Bjarmason

The "linux-clang" and "linux-gcc" jobs both run "make test" twice, but
with different environment variables. Running these in sequence seems
to have been done to work around some constraint on Travis, see
ae59a4e44f3 (travis: run tests with GIT_TEST_SPLIT_INDEX, 2018-01-07).

By having these run in parallel we'll get jobs that finish much sooner
than they otherwise would have.

We can also simplify the control flow in "ci/run-build-and-tests.sh"
as a result, since we won't run "make test" twice we don't need to run
"make" twice at all, let's default to "make all test" after setting
the variables, and then override it to just "all" for the compile-only
tests.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 .github/workflows/main.yml |  9 +++++++++
 ci/run-build-and-tests.sh  | 26 ++++++++++++--------------
 2 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 097be4c4405..63fff2744ad 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -226,11 +226,20 @@ jobs:
             cc: clang
             os: ubuntu
             pool: ubuntu-latest
+          - jobname: linux-sha256
+            cc: clang
+            os: ubuntu
+            pool: ubuntu-latest
           - jobname: linux-gcc
             cc: gcc
             os: ubuntu
             cc_package: gcc-8
             pool: ubuntu-latest
+          - jobname: linux-TEST-vars
+            cc: gcc
+            os: ubuntu
+            cc_package: gcc-8
+            pool: ubuntu-latest
           - jobname: osx-clang
             cc: clang
             os: osx
diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
index cc62616d806..16840b2065d 100755
--- a/ci/run-build-and-tests.sh
+++ b/ci/run-build-and-tests.sh
@@ -10,16 +10,13 @@ windows*) cmd //c mklink //j t\\.prove "$(cygpath -aw "$cache_dir/.prove")";;
 *) ln -s "$cache_dir/.prove" t/.prove;;
 esac
 
-if test "$jobname" = "pedantic"
-then
-	export DEVOPTS=pedantic
-fi
+export MAKE_TARGETS="all test"
 
-make
 case "$jobname" in
 linux-gcc)
 	export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
-	make test
+	;;
+linux-TEST-vars)
 	export GIT_TEST_SPLIT_INDEX=yes
 	export GIT_TEST_MERGE_ALGORITHM=recursive
 	export GIT_TEST_FULL_IN_PACK_ARRAY=true
@@ -33,23 +30,24 @@ linux-gcc)
 	export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
 	export GIT_TEST_WRITE_REV_INDEX=1
 	export GIT_TEST_CHECKOUT_WORKERS=2
-	make test
 	;;
 linux-clang)
 	export GIT_TEST_DEFAULT_HASH=sha1
-	make test
+	;;
+linux-sha256)
 	export GIT_TEST_DEFAULT_HASH=sha256
-	make test
 	;;
-linux-gcc-4.8|pedantic)
-	# Don't run the tests; we only care about whether Git can be
-	# built with GCC 4.8 or with pedantic
+pedantic)
+	export DEVOPTS=pedantic
+	export MAKE_TARGETS=all
 	;;
-*)
-	make test
+linux-gcc-4.8)
+	export MAKE_TARGETS=all
 	;;
 esac
 
+make $MAKE_TARGETS
+
 check_unignored_build_artifacts
 
 save_good_tree
-- 
2.34.0.823.gcc3243ae16c


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v2 6/6] CI: run "documentation" via run-build-and-test.sh
  2021-11-20  3:28 ` [PATCH v2 0/6] CI: Remove Travis CI, shorten names for GH tooltips, split jobs Ævar Arnfjörð Bjarmason
                     ` (4 preceding siblings ...)
  2021-11-20  3:28   ` [PATCH v2 5/6] CI: don't run "make test" twice in one job Ævar Arnfjörð Bjarmason
@ 2021-11-20  3:28   ` Ævar Arnfjörð Bjarmason
  2021-11-20  8:05   ` [PATCH v2 0/6] CI: Remove Travis CI, shorten names for GH tooltips, split jobs Johannes Schindelin
  2021-11-20 12:10   ` [PATCH v3 0/5] " Ævar Arnfjörð Bjarmason
  7 siblings, 0 replies; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-20  3:28 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, SZEDER Gábor,
	Victoria Dye, Ævar Arnfjörð Bjarmason

Now that we've taught the build and test script to accept arbitrary
"make" targets it becomes easy to split up the "asciidoc" and
"asciidoctor" jobs, and to have the "ci/run-build-and-tests.sh" do the
"make" test of building the documentation. I.e. we'll run both of:

    make check-builtins check-docs
    make check-builtins check-docs USE_ASCIIDOCTOR=Y

As noted in 505ad91304e (travis-ci: check AsciiDoc/AsciiDoctor stderr
output, 2017-04-26) we need to keep checking the stderr that's emitted
by these, so let's add that special-case to
"ci/run-build-and-tests.sh".

The other doc-specific tests added in b98712b9aa9 (travis-ci: build
documentation, 2016-05-04) and 159e6010c2d (travis-ci: build
documentation with AsciiDoc and Asciidoctor, 2017-04-11) should live
in "ci/test-documentation.sh".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 .github/workflows/main.yml | 14 +++++++++++---
 ci/install-dependencies.sh |  2 +-
 ci/lib.sh                  |  2 +-
 ci/run-build-and-tests.sh  | 29 ++++++++++++++++++++++------
 ci/test-documentation.sh   | 39 ++++++++++++++++----------------------
 5 files changed, 52 insertions(+), 34 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 63fff2744ad..c6aed3d2758 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -337,13 +337,21 @@ jobs:
       run: ci/install-dependencies.sh
     - run: make sparse
   documentation:
-    name: documentation
+    name: ${{matrix.vector.jobname}}
     needs: ci-config
     if: needs.ci-config.outputs.enabled == 'yes'
+    strategy:
+      fail-fast: false
+      matrix:
+        vector:
+          - jobname: doc-asciidoc
+          - jobname: doc-asciidoctor
+            makeflags: USE_ASCIIDOCTOR=Y
     env:
-      jobname: Documentation
+      jobname: ${{matrix.vector.jobname}}
+      MAKE_TARGETS: check-builtins check-docs doc ${{matrix.vector.makeflags}}
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v2
     - run: ci/install-dependencies.sh
-    - run: ci/test-documentation.sh
+    - run: ci/run-build-and-tests.sh
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index ca1eaa49c14..4a30713645a 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -65,7 +65,7 @@ sparse)
 	sudo apt-get -q -y install libssl-dev libcurl4-openssl-dev \
 		libexpat-dev gettext zlib1g-dev
 	;;
-Documentation)
+doc-*)
 	sudo apt-get -q update
 	sudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns make
 
diff --git a/ci/lib.sh b/ci/lib.sh
index cbc2f8f1caa..3afa95333ed 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -83,7 +83,7 @@ check_unignored_build_artifacts ()
 export TERM=${TERM:-dumb}
 
 # Clear MAKEFLAGS that may come from the outside world.
-export MAKEFLAGS=
+export MAKEFLAGS="${MAKEFLAGS:+$MAKEFLAGS }$EXTRA_MAKEFLAGS"
 
 # Set 'exit on error' for all CI scripts to let the caller know that
 # something went wrong.
diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
index 16840b2065d..a5f782db1f7 100755
--- a/ci/run-build-and-tests.sh
+++ b/ci/run-build-and-tests.sh
@@ -5,12 +5,10 @@
 
 . ${0%/*}/lib.sh
 
-case "$CI_OS_NAME" in
-windows*) cmd //c mklink //j t\\.prove "$(cygpath -aw "$cache_dir/.prove")";;
-*) ln -s "$cache_dir/.prove" t/.prove;;
-esac
-
-export MAKE_TARGETS="all test"
+if test -z "$MAKE_TARGETS"
+then
+	export MAKE_TARGETS="all test"
+fi
 
 case "$jobname" in
 linux-gcc)
@@ -46,6 +44,25 @@ linux-gcc-4.8)
 	;;
 esac
 
+case "$MAKE_TARGETS" in
+*test*)
+	case "$CI_OS_NAME" in
+	windows*) cmd //c mklink //j t\\.prove "$(cygpath -aw "$cache_dir/.prove")";;
+	*) ln -s "$cache_dir/.prove" t/.prove;;
+	esac
+	;;
+esac
+
+case "$jobname" in
+doc-*)
+	# This "make" command requireds bash-specific redirection
+	${0%/*}/test-documentation.sh
+	;;
+*)
+	make $MAKE_TARGETS
+	;;
+esac
+
 make $MAKE_TARGETS
 
 check_unignored_build_artifacts
diff --git a/ci/test-documentation.sh b/ci/test-documentation.sh
index de41888430a..898c33e74ba 100755
--- a/ci/test-documentation.sh
+++ b/ci/test-documentation.sh
@@ -14,32 +14,25 @@ filter_log () {
 	    "$1"
 }
 
-make check-builtins
-make check-docs
-
-# Build docs with AsciiDoc
-make doc > >(tee stdout.log) 2> >(tee stderr.raw >&2)
-cat stderr.raw
-filter_log stderr.raw >stderr.log
-test ! -s stderr.log
-test -s Documentation/git.html
-test -s Documentation/git.xml
-test -s Documentation/git.1
-grep '<meta name="generator" content="AsciiDoc ' Documentation/git.html
-
-rm -f stdout.log stderr.log stderr.raw
-check_unignored_build_artifacts
-
-# Build docs with AsciiDoctor
-make clean
-make USE_ASCIIDOCTOR=1 doc > >(tee stdout.log) 2> >(tee stderr.raw >&2)
+make $MAKE_TARGETS > >(tee stdout.log) 2> >(tee stderr.raw >&2)
 cat stderr.raw
 filter_log stderr.raw >stderr.log
 test ! -s stderr.log
-test -s Documentation/git.html
-grep '<meta name="generator" content="Asciidoctor ' Documentation/git.html
 
 rm -f stdout.log stderr.log stderr.raw
-check_unignored_build_artifacts
 
-save_good_tree
+case $jobname in
+doc-asciidoc)
+	test -s Documentation/git.html
+	test -s Documentation/git.xml
+	test -s Documentation/git.1
+	grep '<meta name="generator" content="AsciiDoc ' Documentation/git.html
+	;;
+doc-asciidoctor)
+	test -s Documentation/git.html
+	grep '<meta name="generator" content="Asciidoctor ' Documentation/git.html
+	;;
+*)
+	exit 1
+	;;
+esac
-- 
2.34.0.823.gcc3243ae16c


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCH v2 2/6] CI: use shorter names that fit in UX tooltips
  2021-11-20  3:28   ` [PATCH v2 2/6] CI: use shorter names that fit in UX tooltips Ævar Arnfjörð Bjarmason
@ 2021-11-20  7:01     ` Victoria Dye
  0 siblings, 0 replies; 38+ messages in thread
From: Victoria Dye @ 2021-11-20  7:01 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason, git
  Cc: Junio C Hamano, Johannes Schindelin, SZEDER Gábor

Ævar Arnfjörð Bjarmason wrote:
> Change the names used for the GitHub CI workflows to be short enough
> to (mostly) fit in the pop-up tool-tips that GitHub shows in the
> commit view. I.e. when mouse-clicking on the passing or failing
> check-mark next to the commit subject.
> 
> These names are seemingly truncated to 17-20 characters followed by
> three dots ("..."). Since a "CI/PR / " prefix is added to them the job
> names looked like this before (windows-test and vs-test jobs omitted):
> 
>     CI/PR / ci-config (p...
>     CI/PR / windows-buil...
>     CI/PR / vs-build (pu...
>     CI/PR / regular (lin...
>     CI/PR / regular (lin...
>     CI/PR / regular (os...
>     CI/PR / regular (os...
>     CI/PR / regular (lin...
>     CI/PR / regular (lin...
>     CI/PR / dockerized (...
>     CI/PR / dockerized (...
>     CI/PR / dockerized (...
>     CI/PR / static-anal...
>     CI/PR / sparse (pu...
>     CI/PR / documenta...
> 
> By omitting the "/PR" from the top-level name, and pushing the
> $jobname to the front we'll now instead get:
> 
>     CI / config (push)
>     CI / win build (push...
>     CI / win+VS build (...
>     CI / linux-clang (ub...
>     CI / linux-gcc (ubun...
>     CI / osx-clang (osx)...
>     CI / osx-gcc (osx) (...
>     CI / linux-gcc-defau...
>     CI / linux-leaks (ub...
>     CI / linux-musl (alp...
>     CI / Linux32 (daald/...
>     CI / pedantic (fedor...
>     CI / static-analysis...
>     CI / sparse (push)...
>     CI / documentation
> 
> We then have no truncation in the expanded view. See [1] for how it
> looked before, [2] for a currently visible CI run using this commit,
> and [3] for the GitHub workflow syntax involved being changed here.
> 
> Let's also add a field for the "os" and use it where appropriate, it's
> occasionally useful to know we're running on say ubuntu
> v.s. fedora (but the "-latest" suffix isn't very useful, that applies
> to almost all the jobs.
> 
> 1. https://github.com/git/git/tree/master/
> 2. https://github.com/avar/git/tree/avar/ci-rm-travis-cleanup-ci-names-2
> 3. https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  .github/workflows/main.yml | 16 +++++++++++++++-
>  README.md                  |  2 +-
>  2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> index 6ed6a9e8076..612b475fd0b 100644
> --- a/.github/workflows/main.yml
> +++ b/.github/workflows/main.yml
> @@ -1,4 +1,4 @@
> -name: CI/PR
> +name: CI
>  
>  on: [push, pull_request]
>  
> @@ -7,6 +7,7 @@ env:
>  
>  jobs:
>    ci-config:
> +    name: config
>      runs-on: ubuntu-latest
>      outputs:
>        enabled: ${{ steps.check-ref.outputs.enabled }}${{ steps.skip-if-redundant.outputs.enabled }}
> @@ -77,6 +78,7 @@ jobs:
>              }
>  
>    windows-build:
> +    name: win build
>      needs: ci-config
>      if: needs.ci-config.outputs.enabled == 'yes'
>      runs-on: windows-latest
> @@ -97,6 +99,7 @@ jobs:
>          name: windows-artifacts
>          path: artifacts
>    windows-test:
> +    name: win test
>      runs-on: windows-latest
>      needs: [windows-build]
>      strategy:
> @@ -127,6 +130,7 @@ jobs:
>          name: failed-tests-windows
>          path: ${{env.FAILED_TEST_ARTIFACTS}}
>    vs-build:
> +    name: win+VS build
>      needs: ci-config
>      if: needs.ci-config.outputs.enabled == 'yes'
>      env:
> @@ -178,6 +182,7 @@ jobs:
>          name: vs-artifacts
>          path: artifacts
>    vs-test:
> +    name: win+VS test
>      runs-on: windows-latest
>      needs: vs-build
>      strategy:
> @@ -210,6 +215,7 @@ jobs:
>          name: failed-tests-windows
>          path: ${{env.FAILED_TEST_ARTIFACTS}}
>    regular:
> +    name: ${{matrix.vector.jobname}} (${{matrix.vector.os}})

The consequence of explicitly removing `-latest` (as you mentioned in the
commit message) is the addition of the new `os` field just to remove that
suffix (+renaming 'macos' to 'osx' to - I assume - save a bit more space).

Keeping the `-latest` doesn't really seem to hurt your goal of improving the
tooltips, though; worst case, the `-latest` would be what's cut off in the
tooltip. The main reason I bring this up is because I'd generally prefer
reusing existing fields wherever possible - e.g. something like this: 

    name: ${{matrix.vector.jobname}} (${{matrix.vector.pool}})

...which has the added benefits of 1) fully reflecting the agents used
(potentially beneficial e.g., if we switched from `macos-latest` to
`macos-10.15`) and 2) better matching the way you've set up the docker
image-based jobs later on. 

>      needs: ci-config
>      if: needs.ci-config.outputs.enabled == 'yes'
>      strategy:
> @@ -218,21 +224,27 @@ jobs:
>          vector:
>            - jobname: linux-clang
>              cc: clang
> +            os: ubuntu
>              pool: ubuntu-latest
>            - jobname: linux-gcc
>              cc: gcc
> +            os: ubuntu
>              pool: ubuntu-latest
>            - jobname: osx-clang
>              cc: clang
> +            os: osx
>              pool: macos-latest
>            - jobname: osx-gcc
>              cc: gcc
> +            os: osx
>              pool: macos-latest
>            - jobname: linux-gcc-default
>              cc: gcc
> +            os: ubuntu
>              pool: ubuntu-latest
>            - jobname: linux-leaks
>              cc: gcc
> +            os: ubuntu
>              pool: ubuntu-latest
>      env:
>        CC: ${{matrix.vector.cc}}
> @@ -251,6 +263,7 @@ jobs:
>          name: failed-tests-${{matrix.vector.jobname}}
>          path: ${{env.FAILED_TEST_ARTIFACTS}}
>    dockerized:
> +    name: ${{matrix.vector.jobname}} (${{matrix.vector.image}})
>      needs: ci-config
>      if: needs.ci-config.outputs.enabled == 'yes'
>      strategy:
> @@ -310,6 +323,7 @@ jobs:
>        run: ci/install-dependencies.sh
>      - run: make sparse
>    documentation:
> +    name: documentation
>      needs: ci-config
>      if: needs.ci-config.outputs.enabled == 'yes'
>      env:
> diff --git a/README.md b/README.md
> index eb8115e6b04..f6f43e78deb 100644
> --- a/README.md
> +++ b/README.md
> @@ -1,4 +1,4 @@
> -[![Build status](https://github.com/git/git/workflows/CI/PR/badge.svg)](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush)
> +[![Build status](https://github.com/git/git/workflows/CI/badge.svg)](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush)
>  
>  Git - fast, scalable, distributed revision control system
>  =========================================================
> 

Overall, I like the fact that platform info is retained in this version
(while also managing to shorten names and/or make the non-truncated parts
more helpful). Thank you for the update! 

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v2 0/6] CI: Remove Travis CI, shorten names for GH tooltips, split jobs
  2021-11-20  3:28 ` [PATCH v2 0/6] CI: Remove Travis CI, shorten names for GH tooltips, split jobs Ævar Arnfjörð Bjarmason
                     ` (5 preceding siblings ...)
  2021-11-20  3:28   ` [PATCH v2 6/6] CI: run "documentation" via run-build-and-test.sh Ævar Arnfjörð Bjarmason
@ 2021-11-20  8:05   ` Johannes Schindelin
  2021-11-20 12:14     ` Ævar Arnfjörð Bjarmason
  2021-11-20 12:10   ` [PATCH v3 0/5] " Ævar Arnfjörð Bjarmason
  7 siblings, 1 reply; 38+ messages in thread
From: Johannes Schindelin @ 2021-11-20  8:05 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason, git
  Cc: Junio C Hamano, SZEDER Gábor, Victoria Dye

On November 20, 2021 4:28:30 AM GMT+01:00, "Ævar Arnfjörð Bjarmason" <avarab@gmail.com> wrote:
>
>  CI: remove Travis CI support

This is a patch I am in favor of, and would prefer in its own patch "series": separation of concern, and consideration in avoiding reviewer fatigue, are two aspects I would like to see you optimize for a bit more.

>  CI: use shorter names that fit in UX tooltips
>  CI: rename the "Linux32" job to lower-case "linux32"
>  CI: use "$runs_on_pool", not "$jobname" to select packages & config

These strike me as simply shuffling things around and ramping up commit count in git/git, without actually addressing any of the problems our GitHub workflow _definitely_ has.

One quite obvious problem is that any failing run is very cumbersome to diagnose, and you kind of have to know where you're looking. A troublesome and off-putting experience for newcomers (and even some oldtimers). You have to expand the print-failures step logs and search for "not ok" to get even close to the relevant part of the failing test case's details.

Yes, addressing this would be much harder and take more effort than just going ahead and renaming things. It would also be much more useful.

>  CI: don't run "make test" twice in one job

I am in favor of the idea. As is obvious from the fact that I already proposed this years ago.

The commit message, however, is mum about that. And about the reasons why my proposal was shot down. And why those reasons should somehow no longer apply (and I would strongly suggest to aim for providing convincing evidence over mere opinions, to back up the patch).

As has been mentioned before, this lack of diligence is disappointing. Reviewers should not be forced to look up previous related discussions on the Git mailing list. I would do that for a first-time contributor, but you are a long-term contributor who clearly has the ability, the knowledge, and the time, to accompany patches with such vital information.

>  CI: run "documentation" via run-build-and-test.sh

This patch has a commit message that explains what the patch does, and describes a little bit of related commit history.

It does not talk about any convincing reasons why the change should be desirable.

This is troubling, in particular since it counteracts the major benefit of the preceding patch: to reduce the jobs' runtime.

Also, while the preceding patch makes each job's focus more obvious, so that it no longer requires careful study of the entire test log merely to find out which `GIT_TEST_*` settings are set, _this_ patch crams the check-docs into the same job as the pretty unrelated test suite run. In other words, it combines even _more unrelated_ things into the same job.

So what does this patch series try to accomplish? To shorten the jobs' runtime? Or to extend it? To have a clearer separation of concerns of the individual jobs? Or to muddy the waters by once again doing several things in the same job?

It is quite confusing and strikes me as a patch series that could have used quite a bit more time and polishing and mailing list research and rearranging and splitting up and patch-dropping before unleashing it to the reviewers on the Git mailing list. It does not help that v2 seems to have been rushed out, and tripled in size, no less. If I had the task of wearing out reviewers, that is exactly the type of thing I would do. If I would accept the task.

To end on a positive note: I suggest to split off the Travis CI patch. It should be relatively uncontroversial. Further, I suggest to find the previous patch that split `linux-gcc` on the mailing list, summarize what the conclusion was, and either adjust the equivalent patch in this series accordingly in order to contribute it stand-alone, or drop it. Then, you could use the considerable time you seem to have at your disposal on working towards teaching our workflow to present diagnosable information about build/test failures in a much more immediately consumable manner than it does right now.

Ciao,
Johannes

Hi,

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH v3 0/5] CI: Remove Travis CI, shorten names for GH tooltips, split jobs
  2021-11-20  3:28 ` [PATCH v2 0/6] CI: Remove Travis CI, shorten names for GH tooltips, split jobs Ævar Arnfjörð Bjarmason
                     ` (6 preceding siblings ...)
  2021-11-20  8:05   ` [PATCH v2 0/6] CI: Remove Travis CI, shorten names for GH tooltips, split jobs Johannes Schindelin
@ 2021-11-20 12:10   ` Ævar Arnfjörð Bjarmason
  2021-11-20 12:10     ` [PATCH v3 1/5] CI: remove Travis CI support Ævar Arnfjörð Bjarmason
                       ` (5 more replies)
  7 siblings, 6 replies; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-20 12:10 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, SZEDER Gábor,
	Victoria Dye, Ævar Arnfjörð Bjarmason

An update to v3, now using the "pool" names instead of a new "os"
field for the job names. Before (on master):

    https://github.com/git/git/runs/4214600139

After:

    https://github.com/avar/git/actions/runs/1484426823

I peeled of the last "doc" patch based on Johannes's feedback. As with
the split-up TEST job we'll take more CPU time, but if we can run in
parallel take less wallclock time, but it's not really worth it for
the asciidoc/asciidoctor job with how long the install v.s. build/test
step takes (b.t.w. there was no change to "check-docs" running in
those jobs in v2).

1. https://lore.kernel.org/git/cover-v2-0.6-00000000000-20211120T030848Z-avarab@gmail.com/

Ævar Arnfjörð Bjarmason (5):
  CI: remove Travis CI support
  CI: use shorter names that fit in UX tooltips
  CI: rename the "Linux32" job to lower-case "linux32"
  CI: use "$runs_on_pool", not "$jobname" to select packages & config
  CI: don't run "make test" twice in one job

 .github/workflows/main.yml        | 26 ++++++++++++--
 .travis.yml                       | 60 -------------------------------
 README.md                         |  2 +-
 ci/install-dependencies.sh        | 33 ++++++++---------
 ci/install-docker-dependencies.sh |  2 +-
 ci/lib.sh                         | 60 ++++++++-----------------------
 ci/print-test-failures.sh         | 10 ------
 ci/run-build-and-tests.sh         | 26 +++++++-------
 ci/run-docker-build.sh            | 11 +-----
 ci/run-docker.sh                  |  4 +--
 10 files changed, 70 insertions(+), 164 deletions(-)
 delete mode 100644 .travis.yml

Range-diff against v2:
1:  cc94a353ccb = 1:  96433bcc02f CI: remove Travis CI support
2:  73981cedee8 ! 2:  b09cd076aeb CI: use shorter names that fit in UX tooltips
    @@ Commit message
         looked before, [2] for a currently visible CI run using this commit,
         and [3] for the GitHub workflow syntax involved being changed here.
     
    -    Let's also add a field for the "os" and use it where appropriate, it's
    -    occasionally useful to know we're running on say ubuntu
    -    v.s. fedora (but the "-latest" suffix isn't very useful, that applies
    -    to almost all the jobs.
    +    Let's also use the existing "pool" field as before. It's occasionally
    +    useful to know we're running on say ubuntu v.s. fedora. The "-latest"
    +    suffix is useful to some[4], and since it's now at the end it doesn't
    +    hurt readability in the short view compared to saying "ubuntu" or
    +    "macos".
     
         1. https://github.com/git/git/tree/master/
    -    2. https://github.com/avar/git/tree/avar/ci-rm-travis-cleanup-ci-names-2
    +    2. https://github.com/avar/git/tree/avar/ci-rm-travis-cleanup-ci-names-3
         3. https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions
    +    3. https://lore.kernel.org/git/d9b07ca5-b58d-a535-d25b-85d7f12e6295@github.com/
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
    @@ .github/workflows/main.yml: jobs:
              name: failed-tests-windows
              path: ${{env.FAILED_TEST_ARTIFACTS}}
        regular:
    -+    name: ${{matrix.vector.jobname}} (${{matrix.vector.os}})
    ++    name: ${{matrix.vector.jobname}} (${{matrix.vector.pool}})
          needs: ci-config
          if: needs.ci-config.outputs.enabled == 'yes'
          strategy:
    -@@ .github/workflows/main.yml: jobs:
    -         vector:
    -           - jobname: linux-clang
    -             cc: clang
    -+            os: ubuntu
    -             pool: ubuntu-latest
    -           - jobname: linux-gcc
    -             cc: gcc
    -+            os: ubuntu
    -             pool: ubuntu-latest
    -           - jobname: osx-clang
    -             cc: clang
    -+            os: osx
    -             pool: macos-latest
    -           - jobname: osx-gcc
    -             cc: gcc
    -+            os: osx
    -             pool: macos-latest
    -           - jobname: linux-gcc-default
    -             cc: gcc
    -+            os: ubuntu
    -             pool: ubuntu-latest
    -           - jobname: linux-leaks
    -             cc: gcc
    -+            os: ubuntu
    -             pool: ubuntu-latest
    -     env:
    -       CC: ${{matrix.vector.cc}}
     @@ .github/workflows/main.yml: jobs:
              name: failed-tests-${{matrix.vector.jobname}}
              path: ${{env.FAILED_TEST_ARTIFACTS}}
3:  002c183fff4 = 3:  fb1b0ecbadd CI: rename the "Linux32" job to lower-case "linux32"
4:  eca0ad08d4b ! 4:  54913e775c1 CI: use "$runs_on_pool", not "$jobname" to select packages & config
    @@ Commit message
     
      ## .github/workflows/main.yml ##
     @@ .github/workflows/main.yml: jobs:
    +             pool: ubuntu-latest
                - jobname: linux-gcc
                  cc: gcc
    -             os: ubuntu
     +            cc_package: gcc-8
                  pool: ubuntu-latest
                - jobname: osx-clang
                  cc: clang
    -@@ .github/workflows/main.yml: jobs:
    +             pool: macos-latest
                - jobname: osx-gcc
                  cc: gcc
    -             os: osx
     +            cc_package: gcc-9
                  pool: macos-latest
                - jobname: linux-gcc-default
5:  a113b8404ed ! 5:  877f27d847c CI: don't run "make test" twice in one job
    @@ Commit message
     
      ## .github/workflows/main.yml ##
     @@ .github/workflows/main.yml: jobs:
    +           - jobname: linux-clang
                  cc: clang
    -             os: ubuntu
                  pool: ubuntu-latest
     +          - jobname: linux-sha256
     +            cc: clang
    @@ .github/workflows/main.yml: jobs:
     +            pool: ubuntu-latest
                - jobname: linux-gcc
                  cc: gcc
    -             os: ubuntu
                  cc_package: gcc-8
                  pool: ubuntu-latest
     +          - jobname: linux-TEST-vars
    @@ .github/workflows/main.yml: jobs:
     +            pool: ubuntu-latest
                - jobname: osx-clang
                  cc: clang
    -             os: osx
    +             pool: macos-latest
     
      ## ci/run-build-and-tests.sh ##
     @@ ci/run-build-and-tests.sh: windows*) cmd //c mklink //j t\\.prove "$(cygpath -aw "$cache_dir/.prove")";;
6:  7c423c8283d < -:  ----------- CI: run "documentation" via run-build-and-test.sh
-- 
2.34.0.818.g0f23a581583


^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH v3 1/5] CI: remove Travis CI support
  2021-11-20 12:10   ` [PATCH v3 0/5] " Ævar Arnfjörð Bjarmason
@ 2021-11-20 12:10     ` Ævar Arnfjörð Bjarmason
  2021-11-20 12:10     ` [PATCH v3 2/5] CI: use shorter names that fit in UX tooltips Ævar Arnfjörð Bjarmason
                       ` (4 subsequent siblings)
  5 siblings, 0 replies; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-20 12:10 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, SZEDER Gábor,
	Victoria Dye, Ævar Arnfjörð Bjarmason

Remove support for running the CI in travis. The last builds in it are
from 5 months ago[1] (as of 2021-11-19), and our documentation has
referred to GitHub CI instead since f003a91f5c5 (SubmittingPatches:
replace discussion of Travis with GitHub Actions, 2021-07-22).

We'll now run the "t9810 t9816" and tests on OSX. We didn't before, as
we'd carried the Travis exclusion of them forward from
522354d70f4 (Add Travis CI support, 2015-11-27). Let's hope whatever
issue there was with them was either Travis specific, or fixed since
then (I'm not sure).

The "apt-add-repository" invocation (which we were doing in GitHub CI)
isn't needed, it was another Travis-only case that was carried forward
into more general code. See 0f0c51181df (travis-ci: install packages
in 'ci/install-dependencies.sh', 2018-11-01).

1. https://travis-ci.org/github/git/git/builds

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 .travis.yml                | 60 --------------------------------------
 ci/install-dependencies.sh |  1 -
 ci/lib.sh                  | 37 ++---------------------
 ci/print-test-failures.sh  | 10 -------
 ci/run-docker-build.sh     |  9 ------
 ci/run-docker.sh           |  2 +-
 6 files changed, 4 insertions(+), 115 deletions(-)
 delete mode 100644 .travis.yml

diff --git a/.travis.yml b/.travis.yml
deleted file mode 100644
index 908330a0a3d..00000000000
--- a/.travis.yml
+++ /dev/null
@@ -1,60 +0,0 @@
-language: c
-
-cache:
-  directories:
-    - $HOME/travis-cache
-
-os:
-  - linux
-  - osx
-
-osx_image: xcode10.1
-
-compiler:
-  - clang
-  - gcc
-
-matrix:
-  include:
-    - env: jobname=linux-gcc-default
-      os: linux
-      compiler:
-      addons:
-      before_install:
-    - env: jobname=linux-gcc-4.8
-      os: linux
-      dist: trusty
-      compiler:
-    - env: jobname=Linux32
-      os: linux
-      compiler:
-      addons:
-      services:
-        - docker
-      before_install:
-      script: ci/run-docker.sh
-    - env: jobname=linux-musl
-      os: linux
-      compiler:
-      addons:
-      services:
-        - docker
-      before_install:
-      script: ci/run-docker.sh
-    - env: jobname=StaticAnalysis
-      os: linux
-      compiler:
-      script: ci/run-static-analysis.sh
-      after_failure:
-    - env: jobname=Documentation
-      os: linux
-      compiler:
-      script: ci/test-documentation.sh
-      after_failure:
-
-before_install: ci/install-dependencies.sh
-script: ci/run-build-and-tests.sh
-after_failure: ci/print-test-failures.sh
-
-notifications:
-  email: false
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 1d0e48f4515..92e11c7198e 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -13,7 +13,6 @@ UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
 
 case "$jobname" in
 linux-clang|linux-gcc|linux-leaks)
-	sudo apt-add-repository -y "ppa:ubuntu-toolchain-r/test"
 	sudo apt-get -q update
 	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
 		$UBUNTU_COMMON_PKGS
diff --git a/ci/lib.sh b/ci/lib.sh
index 82cb17f8eea..73d959e87f7 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -34,7 +34,7 @@ save_good_tree () {
 # successfully before (e.g. because the branch got rebased, changing only
 # the commit messages).
 skip_good_tree () {
-	if test "$TRAVIS_DEBUG_MODE" = true || test true = "$GITHUB_ACTIONS"
+	if test true = "$GITHUB_ACTIONS"
 	then
 		return
 	fi
@@ -60,7 +60,7 @@ skip_good_tree () {
 			cat <<-EOF
 			$(tput setaf 2)Skipping build job for commit $CI_COMMIT.$(tput sgr0)
 			This commit's tree has already been built and tested successfully in build job $prev_good_job_number for commit $prev_good_commit.
-			The log of that build job is available at $(url_for_job_id $prev_good_job_id)
+			The log of that build job is available at $SYSTEM_TASKDEFINITIONSURI$SYSTEM_TEAMPROJECT/_build/results?buildId=$prev_good_job_id
 			To force a re-build delete the branch's cache and then hit 'Restart job'.
 			EOF
 		fi
@@ -91,29 +91,7 @@ export MAKEFLAGS=
 # and installing dependencies.
 set -ex
 
-if test true = "$TRAVIS"
-then
-	CI_TYPE=travis
-	# When building a PR, TRAVIS_BRANCH refers to the *target* branch. Not
-	# what we want here. We want the source branch instead.
-	CI_BRANCH="${TRAVIS_PULL_REQUEST_BRANCH:-$TRAVIS_BRANCH}"
-	CI_COMMIT="$TRAVIS_COMMIT"
-	CI_JOB_ID="$TRAVIS_JOB_ID"
-	CI_JOB_NUMBER="$TRAVIS_JOB_NUMBER"
-	CI_OS_NAME="$TRAVIS_OS_NAME"
-	CI_REPO_SLUG="$TRAVIS_REPO_SLUG"
-
-	cache_dir="$HOME/travis-cache"
-
-	url_for_job_id () {
-		echo "https://travis-ci.org/$CI_REPO_SLUG/jobs/$1"
-	}
-
-	BREW_INSTALL_PACKAGES="git-lfs gettext"
-	export GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save"
-	export GIT_TEST_OPTS="--verbose-log -x --immediate"
-	MAKEFLAGS="$MAKEFLAGS --jobs=2"
-elif test -n "$SYSTEM_COLLECTIONURI" || test -n "$SYSTEM_TASKDEFINITIONSURI"
+if test -n "$SYSTEM_COLLECTIONURI" || test -n "$SYSTEM_TASKDEFINITIONSURI"
 then
 	CI_TYPE=azure-pipelines
 	# We are running in Azure Pipelines
@@ -130,10 +108,6 @@ then
 	# among *all* phases)
 	cache_dir="$HOME/test-cache/$SYSTEM_PHASENAME"
 
-	url_for_job_id () {
-		echo "$SYSTEM_TASKDEFINITIONSURI$SYSTEM_TEAMPROJECT/_build/results?buildId=$1"
-	}
-
 	export GIT_PROVE_OPTS="--timer --jobs 10 --state=failed,slow,save"
 	export GIT_TEST_OPTS="--verbose-log -x --write-junit-xml"
 	MAKEFLAGS="$MAKEFLAGS --jobs=10"
@@ -214,11 +188,6 @@ osx-clang|osx-gcc)
 	else
 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python2)"
 	fi
-
-	# t9810 occasionally fails on Travis CI OS X
-	# t9816 occasionally fails with "TAP out of sequence errors" on
-	# Travis CI OS X
-	export GIT_SKIP_TESTS="t9810 t9816"
 	;;
 linux-gcc-default)
 	;;
diff --git a/ci/print-test-failures.sh b/ci/print-test-failures.sh
index c70d6cdbf24..57277eefcd0 100755
--- a/ci/print-test-failures.sh
+++ b/ci/print-test-failures.sh
@@ -39,8 +39,6 @@ do
 		test_name="${test_name##*/}"
 		trash_dir="trash directory.$test_name"
 		case "$CI_TYPE" in
-		travis)
-			;;
 		azure-pipelines)
 			mkdir -p failed-test-artifacts
 			mv "$trash_dir" failed-test-artifacts
@@ -88,11 +86,3 @@ do
 		fi
 	fi
 done
-
-if [ $combined_trash_size -gt 0 ]
-then
-	echo "------------------------------------------------------------------------"
-	echo "Trash directories embedded in this log can be extracted by running:"
-	echo
-	echo "  curl https://api.travis-ci.org/v3/job/$TRAVIS_JOB_ID/log.txt |./ci/util/extract-trash-dirs.sh"
-fi
diff --git a/ci/run-docker-build.sh b/ci/run-docker-build.sh
index 8d47a5fda3b..5d2764ad3a3 100755
--- a/ci/run-docker-build.sh
+++ b/ci/run-docker-build.sh
@@ -47,15 +47,6 @@ else
 	else
 		useradd -u $HOST_UID $CI_USER
 	fi
-
-	# Due to a bug the test suite was run as root in the past, so
-	# a prove state file created back then is only accessible by
-	# root.  Now that bug is fixed, the test suite is run as a
-	# regular user, but the prove state file coming from Travis
-	# CI's cache might still be owned by root.
-	# Make sure that this user has rights to any cached files,
-	# including an existing prove state file.
-	test -n "$cache_dir" && chown -R $HOST_UID:$HOST_UID "$cache_dir"
 fi
 
 # Build and test
diff --git a/ci/run-docker.sh b/ci/run-docker.sh
index 37fa372052d..b610dd4db84 100755
--- a/ci/run-docker.sh
+++ b/ci/run-docker.sh
@@ -25,7 +25,7 @@ docker pull "$CI_CONTAINER"
 # root@container:/# export jobname=<jobname>
 # root@container:/# /usr/src/git/ci/run-docker-build.sh <host-user-id>
 
-container_cache_dir=/tmp/travis-cache
+container_cache_dir=/tmp/container-cache
 
 docker run \
 	--interactive \
-- 
2.34.0.818.g0f23a581583


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v3 2/5] CI: use shorter names that fit in UX tooltips
  2021-11-20 12:10   ` [PATCH v3 0/5] " Ævar Arnfjörð Bjarmason
  2021-11-20 12:10     ` [PATCH v3 1/5] CI: remove Travis CI support Ævar Arnfjörð Bjarmason
@ 2021-11-20 12:10     ` Ævar Arnfjörð Bjarmason
  2021-11-20 19:06       ` Victoria Dye
  2021-11-20 12:10     ` [PATCH v3 3/5] CI: rename the "Linux32" job to lower-case "linux32" Ævar Arnfjörð Bjarmason
                       ` (3 subsequent siblings)
  5 siblings, 1 reply; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-20 12:10 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, SZEDER Gábor,
	Victoria Dye, Ævar Arnfjörð Bjarmason

Change the names used for the GitHub CI workflows to be short enough
to (mostly) fit in the pop-up tool-tips that GitHub shows in the
commit view. I.e. when mouse-clicking on the passing or failing
check-mark next to the commit subject.

These names are seemingly truncated to 17-20 characters followed by
three dots ("..."). Since a "CI/PR / " prefix is added to them the job
names looked like this before (windows-test and vs-test jobs omitted):

    CI/PR / ci-config (p...
    CI/PR / windows-buil...
    CI/PR / vs-build (pu...
    CI/PR / regular (lin...
    CI/PR / regular (lin...
    CI/PR / regular (os...
    CI/PR / regular (os...
    CI/PR / regular (lin...
    CI/PR / regular (lin...
    CI/PR / dockerized (...
    CI/PR / dockerized (...
    CI/PR / dockerized (...
    CI/PR / static-anal...
    CI/PR / sparse (pu...
    CI/PR / documenta...

By omitting the "/PR" from the top-level name, and pushing the
$jobname to the front we'll now instead get:

    CI / config (push)
    CI / win build (push...
    CI / win+VS build (...
    CI / linux-clang (ub...
    CI / linux-gcc (ubun...
    CI / osx-clang (osx)...
    CI / osx-gcc (osx) (...
    CI / linux-gcc-defau...
    CI / linux-leaks (ub...
    CI / linux-musl (alp...
    CI / Linux32 (daald/...
    CI / pedantic (fedor...
    CI / static-analysis...
    CI / sparse (push)...
    CI / documentation

We then have no truncation in the expanded view. See [1] for how it
looked before, [2] for a currently visible CI run using this commit,
and [3] for the GitHub workflow syntax involved being changed here.

Let's also use the existing "pool" field as before. It's occasionally
useful to know we're running on say ubuntu v.s. fedora. The "-latest"
suffix is useful to some[4], and since it's now at the end it doesn't
hurt readability in the short view compared to saying "ubuntu" or
"macos".

1. https://github.com/git/git/tree/master/
2. https://github.com/avar/git/tree/avar/ci-rm-travis-cleanup-ci-names-3
3. https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions
3. https://lore.kernel.org/git/d9b07ca5-b58d-a535-d25b-85d7f12e6295@github.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 .github/workflows/main.yml | 10 +++++++++-
 README.md                  |  2 +-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 6ed6a9e8076..c7c10456572 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -1,4 +1,4 @@
-name: CI/PR
+name: CI
 
 on: [push, pull_request]
 
@@ -7,6 +7,7 @@ env:
 
 jobs:
   ci-config:
+    name: config
     runs-on: ubuntu-latest
     outputs:
       enabled: ${{ steps.check-ref.outputs.enabled }}${{ steps.skip-if-redundant.outputs.enabled }}
@@ -77,6 +78,7 @@ jobs:
             }
 
   windows-build:
+    name: win build
     needs: ci-config
     if: needs.ci-config.outputs.enabled == 'yes'
     runs-on: windows-latest
@@ -97,6 +99,7 @@ jobs:
         name: windows-artifacts
         path: artifacts
   windows-test:
+    name: win test
     runs-on: windows-latest
     needs: [windows-build]
     strategy:
@@ -127,6 +130,7 @@ jobs:
         name: failed-tests-windows
         path: ${{env.FAILED_TEST_ARTIFACTS}}
   vs-build:
+    name: win+VS build
     needs: ci-config
     if: needs.ci-config.outputs.enabled == 'yes'
     env:
@@ -178,6 +182,7 @@ jobs:
         name: vs-artifacts
         path: artifacts
   vs-test:
+    name: win+VS test
     runs-on: windows-latest
     needs: vs-build
     strategy:
@@ -210,6 +215,7 @@ jobs:
         name: failed-tests-windows
         path: ${{env.FAILED_TEST_ARTIFACTS}}
   regular:
+    name: ${{matrix.vector.jobname}} (${{matrix.vector.pool}})
     needs: ci-config
     if: needs.ci-config.outputs.enabled == 'yes'
     strategy:
@@ -251,6 +257,7 @@ jobs:
         name: failed-tests-${{matrix.vector.jobname}}
         path: ${{env.FAILED_TEST_ARTIFACTS}}
   dockerized:
+    name: ${{matrix.vector.jobname}} (${{matrix.vector.image}})
     needs: ci-config
     if: needs.ci-config.outputs.enabled == 'yes'
     strategy:
@@ -310,6 +317,7 @@ jobs:
       run: ci/install-dependencies.sh
     - run: make sparse
   documentation:
+    name: documentation
     needs: ci-config
     if: needs.ci-config.outputs.enabled == 'yes'
     env:
diff --git a/README.md b/README.md
index eb8115e6b04..f6f43e78deb 100644
--- a/README.md
+++ b/README.md
@@ -1,4 +1,4 @@
-[![Build status](https://github.com/git/git/workflows/CI/PR/badge.svg)](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush)
+[![Build status](https://github.com/git/git/workflows/CI/badge.svg)](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush)
 
 Git - fast, scalable, distributed revision control system
 =========================================================
-- 
2.34.0.818.g0f23a581583


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v3 3/5] CI: rename the "Linux32" job to lower-case "linux32"
  2021-11-20 12:10   ` [PATCH v3 0/5] " Ævar Arnfjörð Bjarmason
  2021-11-20 12:10     ` [PATCH v3 1/5] CI: remove Travis CI support Ævar Arnfjörð Bjarmason
  2021-11-20 12:10     ` [PATCH v3 2/5] CI: use shorter names that fit in UX tooltips Ævar Arnfjörð Bjarmason
@ 2021-11-20 12:10     ` Ævar Arnfjörð Bjarmason
  2021-11-20 12:10     ` [PATCH v3 4/5] CI: use "$runs_on_pool", not "$jobname" to select packages & config Ævar Arnfjörð Bjarmason
                       ` (2 subsequent siblings)
  5 siblings, 0 replies; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-20 12:10 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, SZEDER Gábor,
	Victoria Dye, Ævar Arnfjörð Bjarmason

As a follow-up to the preceding commit's shortening of CI job names,
rename the only job that starts with an upper-case letter to be
consistent with the rest. It was added in 88dedd5e72c (Travis: also
test on 32-bit Linux, 2017-03-05).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 .github/workflows/main.yml        | 3 ++-
 ci/install-docker-dependencies.sh | 2 +-
 ci/lib.sh                         | 2 +-
 ci/run-docker-build.sh            | 2 +-
 ci/run-docker.sh                  | 2 +-
 5 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index c7c10456572..91b565f75bb 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -266,7 +266,8 @@ jobs:
         vector:
         - jobname: linux-musl
           image: alpine
-        - jobname: Linux32
+        - jobname: linux32
+          os: ubuntu32
           image: daald/ubuntu32:xenial
         - jobname: pedantic
           image: fedora
diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
index 07a8c6b199d..78b7e326da6 100755
--- a/ci/install-docker-dependencies.sh
+++ b/ci/install-docker-dependencies.sh
@@ -4,7 +4,7 @@
 #
 
 case "$jobname" in
-Linux32)
+linux32)
 	linux32 --32bit i386 sh -c '
 		apt update >/dev/null &&
 		apt install -y build-essential libcurl4-openssl-dev \
diff --git a/ci/lib.sh b/ci/lib.sh
index 73d959e87f7..0b3b0144882 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -191,7 +191,7 @@ osx-clang|osx-gcc)
 	;;
 linux-gcc-default)
 	;;
-Linux32)
+linux32)
 	CC=gcc
 	;;
 linux-musl)
diff --git a/ci/run-docker-build.sh b/ci/run-docker-build.sh
index 5d2764ad3a3..6cd832efb9c 100755
--- a/ci/run-docker-build.sh
+++ b/ci/run-docker-build.sh
@@ -15,7 +15,7 @@ then
 fi
 
 case "$jobname" in
-Linux32)
+linux32)
 	switch_cmd="linux32 --32bit i386"
 	;;
 linux-musl)
diff --git a/ci/run-docker.sh b/ci/run-docker.sh
index b610dd4db84..af89d1624a4 100755
--- a/ci/run-docker.sh
+++ b/ci/run-docker.sh
@@ -6,7 +6,7 @@
 . ${0%/*}/lib.sh
 
 case "$jobname" in
-Linux32)
+linux32)
 	CI_CONTAINER="daald/ubuntu32:xenial"
 	;;
 linux-musl)
-- 
2.34.0.818.g0f23a581583


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v3 4/5] CI: use "$runs_on_pool", not "$jobname" to select packages & config
  2021-11-20 12:10   ` [PATCH v3 0/5] " Ævar Arnfjörð Bjarmason
                       ` (2 preceding siblings ...)
  2021-11-20 12:10     ` [PATCH v3 3/5] CI: rename the "Linux32" job to lower-case "linux32" Ævar Arnfjörð Bjarmason
@ 2021-11-20 12:10     ` Ævar Arnfjörð Bjarmason
  2021-11-21  7:21       ` Junio C Hamano
  2021-11-20 12:10     ` [PATCH v3 5/5] CI: don't run "make test" twice in one job Ævar Arnfjörð Bjarmason
  2021-11-23 16:29     ` [PATCH v4 0/5] CI: Remove Travis CI, shorten names for GH tooltips, split jobs Ævar Arnfjörð Bjarmason
  5 siblings, 1 reply; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-20 12:10 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, SZEDER Gábor,
	Victoria Dye, Ævar Arnfjörð Bjarmason

Change the setup hooks for the CI to use "$runs_on_pool" for the
"$regular" job. Now we won't need as much boilerplate when adding new
jobs to the "regular" matrix, see 956d2e4639b (tests: add a test mode
for SANITIZE=leak, run it in CI, 2021-09-23) for the last such commit.

I.e. now instead of needing to enumerate each jobname when we select
packages we can install things depending on the pool we're running
in.

That we didn't do this dates back to the now gone dependency on Travis
CI, but even if we add a new CI target in the future this'll be easier
to port over, since we can probably treat "ubuntu-latest" as a
stand-in for some recent Linux that can run "apt" commands.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 .github/workflows/main.yml |  4 ++++
 ci/install-dependencies.sh | 32 ++++++++++++++------------------
 ci/lib.sh                  | 21 +++++++++++----------
 3 files changed, 29 insertions(+), 28 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 91b565f75bb..d402402a18b 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -227,12 +227,14 @@ jobs:
             pool: ubuntu-latest
           - jobname: linux-gcc
             cc: gcc
+            cc_package: gcc-8
             pool: ubuntu-latest
           - jobname: osx-clang
             cc: clang
             pool: macos-latest
           - jobname: osx-gcc
             cc: gcc
+            cc_package: gcc-9
             pool: macos-latest
           - jobname: linux-gcc-default
             cc: gcc
@@ -242,7 +244,9 @@ jobs:
             pool: ubuntu-latest
     env:
       CC: ${{matrix.vector.cc}}
+      CC_PACKAGE: ${{matrix.vector.cc_package}}
       jobname: ${{matrix.vector.jobname}}
+      runs_on_pool: ${{matrix.vector.pool}}
     runs-on: ${{matrix.vector.pool}}
     steps:
     - uses: actions/checkout@v2
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 92e11c7198e..ca1eaa49c14 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -11,17 +11,11 @@ UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
  tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl
  libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl"
 
-case "$jobname" in
-linux-clang|linux-gcc|linux-leaks)
+case "$runs_on_pool" in
+ubuntu-latest)
 	sudo apt-get -q update
 	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
-		$UBUNTU_COMMON_PKGS
-	case "$jobname" in
-	linux-gcc)
-		sudo apt-get -q -y install gcc-8
-		;;
-	esac
-
+		$UBUNTU_COMMON_PKGS $CC_PACKAGE
 	mkdir --parents "$P4_PATH"
 	pushd "$P4_PATH"
 		wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d"
@@ -36,7 +30,7 @@ linux-clang|linux-gcc|linux-leaks)
 		cp git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs .
 	popd
 	;;
-osx-clang|osx-gcc)
+macos-latest)
 	export HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_INSTALL_CLEANUP=1
 	# Uncomment this if you want to run perf tests:
 	# brew install gnu-time
@@ -50,15 +44,17 @@ osx-clang|osx-gcc)
 		brew install --cask --no-quarantine perforce
 	} ||
 	brew install homebrew/cask/perforce
-	case "$jobname" in
-	osx-gcc)
-		brew install gcc@9
-		# Just in case the image is updated to contain gcc@9
-		# pre-installed but not linked.
-		brew link gcc@9
-		;;
-	esac
+
+	if test -n "$CC_PACKAGE"
+	then
+		BREW_PACKAGE=${CC_PACKAGE/-/@}
+		brew install "$BREW_PACKAGE"
+		brew link "$BREW_PACKAGE"
+	fi
 	;;
+esac
+
+case "$jobname" in
 StaticAnalysis)
 	sudo apt-get -q update
 	sudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \
diff --git a/ci/lib.sh b/ci/lib.sh
index 0b3b0144882..cbc2f8f1caa 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -156,11 +156,15 @@ export DEFAULT_TEST_TARGET=prove
 export GIT_TEST_CLONE_2GB=true
 export SKIP_DASHED_BUILT_INS=YesPlease
 
-case "$jobname" in
-linux-clang|linux-gcc|linux-leaks)
+case "$runs_on_pool" in
+ubuntu-latest)
+	if test "$jobname" = "linux-gcc-default"
+	then
+		break
+	fi
+
 	if [ "$jobname" = linux-gcc ]
 	then
-		export CC=gcc-8
 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python3"
 	else
 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python2"
@@ -180,17 +184,17 @@ linux-clang|linux-gcc|linux-leaks)
 	GIT_LFS_PATH="$HOME/custom/git-lfs"
 	export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH"
 	;;
-osx-clang|osx-gcc)
+macos-latest)
 	if [ "$jobname" = osx-gcc ]
 	then
-		export CC=gcc-9
 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python3)"
 	else
 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python2)"
 	fi
 	;;
-linux-gcc-default)
-	;;
+esac
+
+case "$jobname" in
 linux32)
 	CC=gcc
 	;;
@@ -200,9 +204,6 @@ linux-musl)
 	MAKEFLAGS="$MAKEFLAGS NO_REGEX=Yes ICONV_OMITS_BOM=Yes"
 	MAKEFLAGS="$MAKEFLAGS GIT_TEST_UTF8_LOCALE=C.UTF-8"
 	;;
-esac
-
-case "$jobname" in
 linux-leaks)
 	export SANITIZE=leak
 	export GIT_TEST_PASSING_SANITIZE_LEAK=true
-- 
2.34.0.818.g0f23a581583


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v3 5/5] CI: don't run "make test" twice in one job
  2021-11-20 12:10   ` [PATCH v3 0/5] " Ævar Arnfjörð Bjarmason
                       ` (3 preceding siblings ...)
  2021-11-20 12:10     ` [PATCH v3 4/5] CI: use "$runs_on_pool", not "$jobname" to select packages & config Ævar Arnfjörð Bjarmason
@ 2021-11-20 12:10     ` Ævar Arnfjörð Bjarmason
  2021-11-23 16:29     ` [PATCH v4 0/5] CI: Remove Travis CI, shorten names for GH tooltips, split jobs Ævar Arnfjörð Bjarmason
  5 siblings, 0 replies; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-20 12:10 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, SZEDER Gábor,
	Victoria Dye, Ævar Arnfjörð Bjarmason

The "linux-clang" and "linux-gcc" jobs both run "make test" twice, but
with different environment variables. Running these in sequence seems
to have been done to work around some constraint on Travis, see
ae59a4e44f3 (travis: run tests with GIT_TEST_SPLIT_INDEX, 2018-01-07).

By having these run in parallel we'll get jobs that finish much sooner
than they otherwise would have.

We can also simplify the control flow in "ci/run-build-and-tests.sh"
as a result, since we won't run "make test" twice we don't need to run
"make" twice at all, let's default to "make all test" after setting
the variables, and then override it to just "all" for the compile-only
tests.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 .github/workflows/main.yml |  9 +++++++++
 ci/run-build-and-tests.sh  | 26 ++++++++++++--------------
 2 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index d402402a18b..628bcbf495e 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -225,10 +225,19 @@ jobs:
           - jobname: linux-clang
             cc: clang
             pool: ubuntu-latest
+          - jobname: linux-sha256
+            cc: clang
+            os: ubuntu
+            pool: ubuntu-latest
           - jobname: linux-gcc
             cc: gcc
             cc_package: gcc-8
             pool: ubuntu-latest
+          - jobname: linux-TEST-vars
+            cc: gcc
+            os: ubuntu
+            cc_package: gcc-8
+            pool: ubuntu-latest
           - jobname: osx-clang
             cc: clang
             pool: macos-latest
diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
index cc62616d806..16840b2065d 100755
--- a/ci/run-build-and-tests.sh
+++ b/ci/run-build-and-tests.sh
@@ -10,16 +10,13 @@ windows*) cmd //c mklink //j t\\.prove "$(cygpath -aw "$cache_dir/.prove")";;
 *) ln -s "$cache_dir/.prove" t/.prove;;
 esac
 
-if test "$jobname" = "pedantic"
-then
-	export DEVOPTS=pedantic
-fi
+export MAKE_TARGETS="all test"
 
-make
 case "$jobname" in
 linux-gcc)
 	export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
-	make test
+	;;
+linux-TEST-vars)
 	export GIT_TEST_SPLIT_INDEX=yes
 	export GIT_TEST_MERGE_ALGORITHM=recursive
 	export GIT_TEST_FULL_IN_PACK_ARRAY=true
@@ -33,23 +30,24 @@ linux-gcc)
 	export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
 	export GIT_TEST_WRITE_REV_INDEX=1
 	export GIT_TEST_CHECKOUT_WORKERS=2
-	make test
 	;;
 linux-clang)
 	export GIT_TEST_DEFAULT_HASH=sha1
-	make test
+	;;
+linux-sha256)
 	export GIT_TEST_DEFAULT_HASH=sha256
-	make test
 	;;
-linux-gcc-4.8|pedantic)
-	# Don't run the tests; we only care about whether Git can be
-	# built with GCC 4.8 or with pedantic
+pedantic)
+	export DEVOPTS=pedantic
+	export MAKE_TARGETS=all
 	;;
-*)
-	make test
+linux-gcc-4.8)
+	export MAKE_TARGETS=all
 	;;
 esac
 
+make $MAKE_TARGETS
+
 check_unignored_build_artifacts
 
 save_good_tree
-- 
2.34.0.818.g0f23a581583


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCH v2 0/6] CI: Remove Travis CI, shorten names for GH tooltips, split jobs
  2021-11-20  8:05   ` [PATCH v2 0/6] CI: Remove Travis CI, shorten names for GH tooltips, split jobs Johannes Schindelin
@ 2021-11-20 12:14     ` Ævar Arnfjörð Bjarmason
  0 siblings, 0 replies; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-20 12:14 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Junio C Hamano, SZEDER Gábor, Victoria Dye


On Sat, Nov 20 2021, Johannes Schindelin wrote:

> On November 20, 2021 4:28:30 AM GMT+01:00, "Ævar Arnfjörð Bjarmason" <avarab@gmail.com> wrote:
>>
>>  CI: remove Travis CI support
>
> This is a patch I am in favor of, and would prefer in its own patch
> "series": separation of concern, and consideration in avoiding
> reviewer fatigue, are two aspects I would like to see you optimize for
> a bit more.

It's included due to a comment on the v1 from Jeff King. I could have
gone either way, but I'm not going to change it around now.

I think it's fair enough to keep up the quality of series's, but I also
don't think it would help to go back & forth with including or not
including this travis removal.

If I don't include it I've got to patch "dead" code for the rest of the
series, if I re-roll and have two sets of patches depending on each
other that's its own reviewer fatigue etc.

For some counter-advice it would be nice to see you optimize for
responding to reviews, cf.:
https://lore.kernel.org/git/211118.86zgq14jp1.gmgdl@evledraar.gmail.com/
:)

>>  CI: use shorter names that fit in UX tooltips
>>  CI: rename the "Linux32" job to lower-case "linux32"
>>  CI: use "$runs_on_pool", not "$jobname" to select packages & config
>
> These strike me as simply shuffling things around and ramping up commit count in git/git, without actually addressing any of the problems our GitHub workflow _definitely_ has.
>
> One quite obvious problem is that any failing run is very cumbersome
> to diagnose, and you kind of have to know where you're looking. A
> troublesome and off-putting experience for newcomers (and even some
> oldtimers). You have to expand the print-failures step logs and search
> for "not ok" to get even close to the relevant part of the failing
> test case's details.
>
> Yes, addressing this would be much harder and take more effort than just going ahead and renaming things. It would also be much more useful.

FWIW I do have patches for that, but getting anything in takes time etc.

To do that properly needed to parse TAP, which I've got patches for in
pure-C (not Perl or whatever) locally.

One of the reasons I didn't submit that UX improvement yet is because I
know it'll run afoul of one of your hobby horses.

I.e. I'll either need to fix in-tree dead code relating to the test
suite's XML generation blindly (it's not used, so how am I going to test
it?), or argue with you about it being worth to remove it to move the
test suite UX forward without having to spending time on it.

Another is that someone (I think SZEDER, hopefully I'm not
misattributing there) pointed out that they liked to have the current
print-failure firehose of "set -x" output for both failures and
successes, and would object to e.g. something that just peeled out the
specific failure output.

I don't think that opinion is wrong (if I even understood it
correctl). I disagree, but I see how it's clearly a matter of
preference. Some would like that, some don't.

But whatever anyone's preference on that I think it clearly shows that
what you're suggesting here isn't as easy as you make it
out.

I.e. something to the effect of "instead of renaming things work on
<thing I consider a clear UX improvement>".

Even if I agree that it would be an improvement in this case others
won't, and getting that past review is a matter of arguing about that,
addressing feedback on that topic etc.

The reason I submitted this is because I thought "I'd like to rename
this thing, not because of bikeshedding, but because I literally cannot
see the relevant information" wouldn't be in any way contentious, but
here we are :)

If you'd like to test some of that referenced UX work out it's
avar/support-test-verbose-under-prove-2 in my git.git clone (the CI
changes aren't there yet, but they're made easy by the changes).

>>  CI: don't run "make test" twice in one job
>
> I am in favor of the idea. As is obvious from the fact that I already proposed this years ago.
>
> The commit message, however, is mum about that. And about the reasons
> why my proposal was shot down. And why those reasons should somehow no
> longer apply (and I would strongly suggest to aim for providing
> convincing evidence over mere opinions, to back up the patch).
>
> As has been mentioned before, this lack of diligence is
> disappointing. Reviewers should not be forced to look up previous
> related discussions on the Git mailing list. I would do that for a
> first-time contributor, but you are a long-term contributor who
> clearly has the ability, the knowledge, and the time, to accompany
> patches with such vital information.

I think in terms of over-explaining something in commit messages &
including references to past commit I'm doing better than most in terms
of commit messages to this project.

But you've got to stop somewhere, exhaustive explanations of past
caveats etc. are also its own fatigue.

In this case I think the explanations I've provided as they stand
suffice. Curious archaeologists can always dig in the archives for more.

>>  CI: run "documentation" via run-build-and-test.sh
>
> This patch has a commit message that explains what the patch does, and describes a little bit of related commit history.
>
> It does not talk about any convincing reasons why the change should be desirable.

I just ejected this in v3, yeah it's a bit of a mixed bag with the
runtime of the installation.

The benefit, which I thought was a bit too obvious to even point out, is
that you can plainly see which half of asciidoc/asciidoctor fails, or
both. So it's clear if it's a compatibility issue or doc ource issue.

> This is troubling, in particular since it counteracts the major benefit of the preceding patch: to reduce the jobs' runtime.
>
> Also, while the preceding patch makes each job's focus more obvious,
> so that it no longer requires careful study of the entire test log
> merely to find out which `GIT_TEST_*` settings are set, _this_ patch
> crams the check-docs into the same job as the pretty unrelated test
> suite run. In other words, it combines even _more unrelated_ things
> into the same job.

The "check-docs" run was already there in the pre-image, so nothing
changed there.

I agree that it would be beneficial to e.g. split out all this ci/
script wrapping into "make" targets at the top-level, but that's a
separate future improvement.

> [...]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3 2/5] CI: use shorter names that fit in UX tooltips
  2021-11-20 12:10     ` [PATCH v3 2/5] CI: use shorter names that fit in UX tooltips Ævar Arnfjörð Bjarmason
@ 2021-11-20 19:06       ` Victoria Dye
  0 siblings, 0 replies; 38+ messages in thread
From: Victoria Dye @ 2021-11-20 19:06 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason, git
  Cc: Junio C Hamano, Johannes Schindelin, SZEDER Gábor

Ævar Arnfjörð Bjarmason wrote:
> Change the names used for the GitHub CI workflows to be short enough
> to (mostly) fit in the pop-up tool-tips that GitHub shows in the
> commit view. I.e. when mouse-clicking on the passing or failing
> check-mark next to the commit subject.
> 
> These names are seemingly truncated to 17-20 characters followed by
> three dots ("..."). Since a "CI/PR / " prefix is added to them the job
> names looked like this before (windows-test and vs-test jobs omitted):
> 
>     CI/PR / ci-config (p...
>     CI/PR / windows-buil...
>     CI/PR / vs-build (pu...
>     CI/PR / regular (lin...
>     CI/PR / regular (lin...
>     CI/PR / regular (os...
>     CI/PR / regular (os...
>     CI/PR / regular (lin...
>     CI/PR / regular (lin...
>     CI/PR / dockerized (...
>     CI/PR / dockerized (...
>     CI/PR / dockerized (...
>     CI/PR / static-anal...
>     CI/PR / sparse (pu...
>     CI/PR / documenta...
> 
> By omitting the "/PR" from the top-level name, and pushing the
> $jobname to the front we'll now instead get:
> 
>     CI / config (push)
>     CI / win build (push...
>     CI / win+VS build (...
>     CI / linux-clang (ub...
>     CI / linux-gcc (ubun...
>     CI / osx-clang (osx)...
>     CI / osx-gcc (osx) (...
>     CI / linux-gcc-defau...
>     CI / linux-leaks (ub...
>     CI / linux-musl (alp...
>     CI / Linux32 (daald/...
>     CI / pedantic (fedor...
>     CI / static-analysis...
>     CI / sparse (push)...
>     CI / documentation
> 
> We then have no truncation in the expanded view. See [1] for how it
> looked before, [2] for a currently visible CI run using this commit,
> and [3] for the GitHub workflow syntax involved being changed here.
> 
> Let's also use the existing "pool" field as before. It's occasionally
> useful to know we're running on say ubuntu v.s. fedora. The "-latest"
> suffix is useful to some[4], and since it's now at the end it doesn't
> hurt readability in the short view compared to saying "ubuntu" or
> "macos".
> 
> 1. https://github.com/git/git/tree/master/
> 2. https://github.com/avar/git/tree/avar/ci-rm-travis-cleanup-ci-names-3
> 3. https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions
> 3. https://lore.kernel.org/git/d9b07ca5-b58d-a535-d25b-85d7f12e6295@github.com/
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  .github/workflows/main.yml | 10 +++++++++-
>  README.md                  |  2 +-
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> index 6ed6a9e8076..c7c10456572 100644
> --- a/.github/workflows/main.yml
> +++ b/.github/workflows/main.yml
> @@ -1,4 +1,4 @@
> -name: CI/PR
> +name: CI
>  
>  on: [push, pull_request]
>  
> @@ -7,6 +7,7 @@ env:
>  
>  jobs:
>    ci-config:
> +    name: config
>      runs-on: ubuntu-latest
>      outputs:
>        enabled: ${{ steps.check-ref.outputs.enabled }}${{ steps.skip-if-redundant.outputs.enabled }}
> @@ -77,6 +78,7 @@ jobs:
>              }
>  
>    windows-build:
> +    name: win build
>      needs: ci-config
>      if: needs.ci-config.outputs.enabled == 'yes'
>      runs-on: windows-latest
> @@ -97,6 +99,7 @@ jobs:
>          name: windows-artifacts
>          path: artifacts
>    windows-test:
> +    name: win test
>      runs-on: windows-latest
>      needs: [windows-build]
>      strategy:
> @@ -127,6 +130,7 @@ jobs:
>          name: failed-tests-windows
>          path: ${{env.FAILED_TEST_ARTIFACTS}}
>    vs-build:
> +    name: win+VS build
>      needs: ci-config
>      if: needs.ci-config.outputs.enabled == 'yes'
>      env:
> @@ -178,6 +182,7 @@ jobs:
>          name: vs-artifacts
>          path: artifacts
>    vs-test:
> +    name: win+VS test
>      runs-on: windows-latest
>      needs: vs-build
>      strategy:
> @@ -210,6 +215,7 @@ jobs:
>          name: failed-tests-windows
>          path: ${{env.FAILED_TEST_ARTIFACTS}}
>    regular:
> +    name: ${{matrix.vector.jobname}} (${{matrix.vector.pool}})
>      needs: ci-config
>      if: needs.ci-config.outputs.enabled == 'yes'
>      strategy:
> @@ -251,6 +257,7 @@ jobs:
>          name: failed-tests-${{matrix.vector.jobname}}
>          path: ${{env.FAILED_TEST_ARTIFACTS}}
>    dockerized:
> +    name: ${{matrix.vector.jobname}} (${{matrix.vector.image}})
>      needs: ci-config
>      if: needs.ci-config.outputs.enabled == 'yes'
>      strategy:
> @@ -310,6 +317,7 @@ jobs:
>        run: ci/install-dependencies.sh
>      - run: make sparse
>    documentation:
> +    name: documentation
>      needs: ci-config
>      if: needs.ci-config.outputs.enabled == 'yes'
>      env:
> diff --git a/README.md b/README.md
> index eb8115e6b04..f6f43e78deb 100644
> --- a/README.md
> +++ b/README.md
> @@ -1,4 +1,4 @@
> -[![Build status](https://github.com/git/git/workflows/CI/PR/badge.svg)](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush)
> +[![Build status](https://github.com/git/git/workflows/CI/badge.svg)](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush)
>  
>  Git - fast, scalable, distributed revision control system
>  =========================================================
> 

This addresses all of my earlier comments - looks good to me!

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3 4/5] CI: use "$runs_on_pool", not "$jobname" to select packages & config
  2021-11-20 12:10     ` [PATCH v3 4/5] CI: use "$runs_on_pool", not "$jobname" to select packages & config Ævar Arnfjörð Bjarmason
@ 2021-11-21  7:21       ` Junio C Hamano
  0 siblings, 0 replies; 38+ messages in thread
From: Junio C Hamano @ 2021-11-21  7:21 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: git, Johannes Schindelin, SZEDER Gábor, Victoria Dye

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

> -case "$jobname" in
> -linux-clang|linux-gcc|linux-leaks)
> +case "$runs_on_pool" in
> +ubuntu-latest)
>  	sudo apt-get -q update
> ...
>  	;;
> -osx-clang|osx-gcc)
> +macos-latest)
>  	export HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_INSTALL_CLEANUP=1

Indeed this is nice improvement in maintainability.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH v4 0/5] CI: Remove Travis CI, shorten names for GH tooltips, split jobs
  2021-11-20 12:10   ` [PATCH v3 0/5] " Ævar Arnfjörð Bjarmason
                       ` (4 preceding siblings ...)
  2021-11-20 12:10     ` [PATCH v3 5/5] CI: don't run "make test" twice in one job Ævar Arnfjörð Bjarmason
@ 2021-11-23 16:29     ` Ævar Arnfjörð Bjarmason
  2021-11-23 16:29       ` [PATCH v4 1/5] CI: remove Travis CI support Ævar Arnfjörð Bjarmason
                         ` (4 more replies)
  5 siblings, 5 replies; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-23 16:29 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, SZEDER Gábor,
	Victoria Dye, Ævar Arnfjörð Bjarmason

A small update to the v3[1] of this series of CI UX improvements. A CI
run can be seen at:

    https://github.com/avar/git/runs/4299892497

And the improvement to tooltips by shortening them at (click on the CI
status symbol next to the commit subject):

    https://github.com/avar/git/tree/avar/ci-rm-travis-cleanup-ci-names-4

For comparison, the same for "master":

    https://github.com/git/git/runs/4289544907
    https://github.com/git/git/tree/master

Changes:

 * In removing Travis support I omitted the removal of now-dead
   linux-gcc-4.8 supporting code. Remove it too.

 * Clarify with a comment where new test targets in
   ci/run-build-and-tests.sh need to be added, to avoid a repeat of
   running new tests in the compilation-only "pedantic" job.

1. https://lore.kernel.org/git/cover-v3-0.5-00000000000-20211120T115414Z-avarab@gmail.com/

Ævar Arnfjörð Bjarmason (5):
  CI: remove Travis CI support
  CI: use shorter names that fit in UX tooltips
  CI: rename the "Linux32" job to lower-case "linux32"
  CI: use "$runs_on_pool", not "$jobname" to select packages & config
  CI: don't run "make test" twice in one job

 .github/workflows/main.yml        | 26 ++++++++++++--
 .travis.yml                       | 60 -------------------------------
 README.md                         |  2 +-
 ci/install-dependencies.sh        | 35 ++++++++----------
 ci/install-docker-dependencies.sh |  2 +-
 ci/lib.sh                         | 60 ++++++++-----------------------
 ci/print-test-failures.sh         | 10 ------
 ci/run-build-and-tests.sh         | 27 +++++++-------
 ci/run-docker-build.sh            | 11 +-----
 ci/run-docker.sh                  |  4 +--
 10 files changed, 72 insertions(+), 165 deletions(-)
 delete mode 100644 .travis.yml

Range-diff against v3:
1:  96433bcc02f ! 1:  6a4f1961cd2 CI: remove Travis CI support
    @@ Commit message
         into more general code. See 0f0c51181df (travis-ci: install packages
         in 'ci/install-dependencies.sh', 2018-11-01).
     
    +    Remove the "linux-gcc-4.8" job added in fb9d7431cf4 (travis-ci: build
    +    with GCC 4.8 as well, 2019-07-18), it only ran in Travis CI.
    +
         1. https://travis-ci.org/github/git/git/builds
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
    @@ ci/install-dependencies.sh: UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl
      	sudo apt-get -q update
      	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
      		$UBUNTU_COMMON_PKGS
    +@@ ci/install-dependencies.sh: Documentation)
    + 	test -n "$ALREADY_HAVE_ASCIIDOCTOR" ||
    + 	sudo gem install --version 1.5.8 asciidoctor
    + 	;;
    +-linux-gcc-default|linux-gcc-4.8)
    ++linux-gcc-default)
    + 	sudo apt-get -q update
    + 	sudo apt-get -q -y install $UBUNTU_COMMON_PKGS
    + 	;;
     
      ## ci/lib.sh ##
     @@ ci/lib.sh: save_good_tree () {
    @@ ci/print-test-failures.sh: do
     -	echo "  curl https://api.travis-ci.org/v3/job/$TRAVIS_JOB_ID/log.txt |./ci/util/extract-trash-dirs.sh"
     -fi
     
    + ## ci/run-build-and-tests.sh ##
    +@@ ci/run-build-and-tests.sh: linux-clang)
    + 	export GIT_TEST_DEFAULT_HASH=sha256
    + 	make test
    + 	;;
    +-linux-gcc-4.8|pedantic)
    ++pedantic)
    + 	# Don't run the tests; we only care about whether Git can be
    +-	# built with GCC 4.8 or with pedantic
    ++	# built.
    + 	;;
    + *)
    + 	make test
    +
      ## ci/run-docker-build.sh ##
     @@ ci/run-docker-build.sh: else
      	else
2:  b09cd076aeb = 2:  5d53b79347f CI: use shorter names that fit in UX tooltips
3:  fb1b0ecbadd = 3:  37b97fc6c3a CI: rename the "Linux32" job to lower-case "linux32"
4:  54913e775c1 = 4:  614a99f7b64 CI: use "$runs_on_pool", not "$jobname" to select packages & config
5:  877f27d847c ! 5:  ee2f9254fc7 CI: don't run "make test" twice in one job
    @@ Commit message
         the variables, and then override it to just "all" for the compile-only
         tests.
     
    +    Add a comment to clarify that new "test" targets should adjust
    +    $MAKE_TARGETS rather than being added after the "case/esac". This
    +    should avoid future confusion where e.g. the compilation-only
    +    "pedantic" target will unexpectedly start running tests. See [1] and
    +    [2].
    +
    +    1. https://lore.kernel.org/git/211122.86ee78yxts.gmgdl@evledraar.gmail.com/
    +    2. https://lore.kernel.org/git/211123.86ilwjujmd.gmgdl@evledraar.gmail.com/
    +
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## .github/workflows/main.yml ##
    @@ ci/run-build-and-tests.sh: linux-gcc)
      	export GIT_TEST_DEFAULT_HASH=sha256
     -	make test
      	;;
    --linux-gcc-4.8|pedantic)
    --	# Don't run the tests; we only care about whether Git can be
    --	# built with GCC 4.8 or with pedantic
    -+pedantic)
    -+	export DEVOPTS=pedantic
    -+	export MAKE_TARGETS=all
    - 	;;
    + pedantic)
    + 	# Don't run the tests; we only care about whether Git can be
    + 	# built.
    +-	;;
     -*)
     -	make test
    -+linux-gcc-4.8)
    ++	export DEVOPTS=pedantic
     +	export MAKE_TARGETS=all
      	;;
      esac
      
    ++# Any new "test" targets should not go after this "make", but should
    ++# adjust $MAKE_TARGETS. Otherwise compilation-only targets above will
    ++# start running tests.
     +make $MAKE_TARGETS
    -+
      check_unignored_build_artifacts
      
      save_good_tree
-- 
2.34.0.830.gb9cdc59c8af


^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH v4 1/5] CI: remove Travis CI support
  2021-11-23 16:29     ` [PATCH v4 0/5] CI: Remove Travis CI, shorten names for GH tooltips, split jobs Ævar Arnfjörð Bjarmason
@ 2021-11-23 16:29       ` Ævar Arnfjörð Bjarmason
  2021-11-23 16:29       ` [PATCH v4 2/5] CI: use shorter names that fit in UX tooltips Ævar Arnfjörð Bjarmason
                         ` (3 subsequent siblings)
  4 siblings, 0 replies; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-23 16:29 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, SZEDER Gábor,
	Victoria Dye, Ævar Arnfjörð Bjarmason

Remove support for running the CI in travis. The last builds in it are
from 5 months ago[1] (as of 2021-11-19), and our documentation has
referred to GitHub CI instead since f003a91f5c5 (SubmittingPatches:
replace discussion of Travis with GitHub Actions, 2021-07-22).

We'll now run the "t9810 t9816" and tests on OSX. We didn't before, as
we'd carried the Travis exclusion of them forward from
522354d70f4 (Add Travis CI support, 2015-11-27). Let's hope whatever
issue there was with them was either Travis specific, or fixed since
then (I'm not sure).

The "apt-add-repository" invocation (which we were doing in GitHub CI)
isn't needed, it was another Travis-only case that was carried forward
into more general code. See 0f0c51181df (travis-ci: install packages
in 'ci/install-dependencies.sh', 2018-11-01).

Remove the "linux-gcc-4.8" job added in fb9d7431cf4 (travis-ci: build
with GCC 4.8 as well, 2019-07-18), it only ran in Travis CI.

1. https://travis-ci.org/github/git/git/builds

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 .travis.yml                | 60 --------------------------------------
 ci/install-dependencies.sh |  3 +-
 ci/lib.sh                  | 37 ++---------------------
 ci/print-test-failures.sh  | 10 -------
 ci/run-build-and-tests.sh  |  4 +--
 ci/run-docker-build.sh     |  9 ------
 ci/run-docker.sh           |  2 +-
 7 files changed, 7 insertions(+), 118 deletions(-)
 delete mode 100644 .travis.yml

diff --git a/.travis.yml b/.travis.yml
deleted file mode 100644
index 908330a0a3d..00000000000
--- a/.travis.yml
+++ /dev/null
@@ -1,60 +0,0 @@
-language: c
-
-cache:
-  directories:
-    - $HOME/travis-cache
-
-os:
-  - linux
-  - osx
-
-osx_image: xcode10.1
-
-compiler:
-  - clang
-  - gcc
-
-matrix:
-  include:
-    - env: jobname=linux-gcc-default
-      os: linux
-      compiler:
-      addons:
-      before_install:
-    - env: jobname=linux-gcc-4.8
-      os: linux
-      dist: trusty
-      compiler:
-    - env: jobname=Linux32
-      os: linux
-      compiler:
-      addons:
-      services:
-        - docker
-      before_install:
-      script: ci/run-docker.sh
-    - env: jobname=linux-musl
-      os: linux
-      compiler:
-      addons:
-      services:
-        - docker
-      before_install:
-      script: ci/run-docker.sh
-    - env: jobname=StaticAnalysis
-      os: linux
-      compiler:
-      script: ci/run-static-analysis.sh
-      after_failure:
-    - env: jobname=Documentation
-      os: linux
-      compiler:
-      script: ci/test-documentation.sh
-      after_failure:
-
-before_install: ci/install-dependencies.sh
-script: ci/run-build-and-tests.sh
-after_failure: ci/print-test-failures.sh
-
-notifications:
-  email: false
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 1d0e48f4515..49a4ae7a988 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -13,7 +13,6 @@ UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
 
 case "$jobname" in
 linux-clang|linux-gcc|linux-leaks)
-	sudo apt-add-repository -y "ppa:ubuntu-toolchain-r/test"
 	sudo apt-get -q update
 	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
 		$UBUNTU_COMMON_PKGS
@@ -77,7 +76,7 @@ Documentation)
 	test -n "$ALREADY_HAVE_ASCIIDOCTOR" ||
 	sudo gem install --version 1.5.8 asciidoctor
 	;;
-linux-gcc-default|linux-gcc-4.8)
+linux-gcc-default)
 	sudo apt-get -q update
 	sudo apt-get -q -y install $UBUNTU_COMMON_PKGS
 	;;
diff --git a/ci/lib.sh b/ci/lib.sh
index 82cb17f8eea..73d959e87f7 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -34,7 +34,7 @@ save_good_tree () {
 # successfully before (e.g. because the branch got rebased, changing only
 # the commit messages).
 skip_good_tree () {
-	if test "$TRAVIS_DEBUG_MODE" = true || test true = "$GITHUB_ACTIONS"
+	if test true = "$GITHUB_ACTIONS"
 	then
 		return
 	fi
@@ -60,7 +60,7 @@ skip_good_tree () {
 			cat <<-EOF
 			$(tput setaf 2)Skipping build job for commit $CI_COMMIT.$(tput sgr0)
 			This commit's tree has already been built and tested successfully in build job $prev_good_job_number for commit $prev_good_commit.
-			The log of that build job is available at $(url_for_job_id $prev_good_job_id)
+			The log of that build job is available at $SYSTEM_TASKDEFINITIONSURI$SYSTEM_TEAMPROJECT/_build/results?buildId=$prev_good_job_id
 			To force a re-build delete the branch's cache and then hit 'Restart job'.
 			EOF
 		fi
@@ -91,29 +91,7 @@ export MAKEFLAGS=
 # and installing dependencies.
 set -ex
 
-if test true = "$TRAVIS"
-then
-	CI_TYPE=travis
-	# When building a PR, TRAVIS_BRANCH refers to the *target* branch. Not
-	# what we want here. We want the source branch instead.
-	CI_BRANCH="${TRAVIS_PULL_REQUEST_BRANCH:-$TRAVIS_BRANCH}"
-	CI_COMMIT="$TRAVIS_COMMIT"
-	CI_JOB_ID="$TRAVIS_JOB_ID"
-	CI_JOB_NUMBER="$TRAVIS_JOB_NUMBER"
-	CI_OS_NAME="$TRAVIS_OS_NAME"
-	CI_REPO_SLUG="$TRAVIS_REPO_SLUG"
-
-	cache_dir="$HOME/travis-cache"
-
-	url_for_job_id () {
-		echo "https://travis-ci.org/$CI_REPO_SLUG/jobs/$1"
-	}
-
-	BREW_INSTALL_PACKAGES="git-lfs gettext"
-	export GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save"
-	export GIT_TEST_OPTS="--verbose-log -x --immediate"
-	MAKEFLAGS="$MAKEFLAGS --jobs=2"
-elif test -n "$SYSTEM_COLLECTIONURI" || test -n "$SYSTEM_TASKDEFINITIONSURI"
+if test -n "$SYSTEM_COLLECTIONURI" || test -n "$SYSTEM_TASKDEFINITIONSURI"
 then
 	CI_TYPE=azure-pipelines
 	# We are running in Azure Pipelines
@@ -130,10 +108,6 @@ then
 	# among *all* phases)
 	cache_dir="$HOME/test-cache/$SYSTEM_PHASENAME"
 
-	url_for_job_id () {
-		echo "$SYSTEM_TASKDEFINITIONSURI$SYSTEM_TEAMPROJECT/_build/results?buildId=$1"
-	}
-
 	export GIT_PROVE_OPTS="--timer --jobs 10 --state=failed,slow,save"
 	export GIT_TEST_OPTS="--verbose-log -x --write-junit-xml"
 	MAKEFLAGS="$MAKEFLAGS --jobs=10"
@@ -214,11 +188,6 @@ osx-clang|osx-gcc)
 	else
 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python2)"
 	fi
-
-	# t9810 occasionally fails on Travis CI OS X
-	# t9816 occasionally fails with "TAP out of sequence errors" on
-	# Travis CI OS X
-	export GIT_SKIP_TESTS="t9810 t9816"
 	;;
 linux-gcc-default)
 	;;
diff --git a/ci/print-test-failures.sh b/ci/print-test-failures.sh
index c70d6cdbf24..57277eefcd0 100755
--- a/ci/print-test-failures.sh
+++ b/ci/print-test-failures.sh
@@ -39,8 +39,6 @@ do
 		test_name="${test_name##*/}"
 		trash_dir="trash directory.$test_name"
 		case "$CI_TYPE" in
-		travis)
-			;;
 		azure-pipelines)
 			mkdir -p failed-test-artifacts
 			mv "$trash_dir" failed-test-artifacts
@@ -88,11 +86,3 @@ do
 		fi
 	fi
 done
-
-if [ $combined_trash_size -gt 0 ]
-then
-	echo "------------------------------------------------------------------------"
-	echo "Trash directories embedded in this log can be extracted by running:"
-	echo
-	echo "  curl https://api.travis-ci.org/v3/job/$TRAVIS_JOB_ID/log.txt |./ci/util/extract-trash-dirs.sh"
-fi
diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
index cc62616d806..18056501ec2 100755
--- a/ci/run-build-and-tests.sh
+++ b/ci/run-build-and-tests.sh
@@ -41,9 +41,9 @@ linux-clang)
 	export GIT_TEST_DEFAULT_HASH=sha256
 	make test
 	;;
-linux-gcc-4.8|pedantic)
+pedantic)
 	# Don't run the tests; we only care about whether Git can be
-	# built with GCC 4.8 or with pedantic
+	# built.
 	;;
 *)
 	make test
diff --git a/ci/run-docker-build.sh b/ci/run-docker-build.sh
index 8d47a5fda3b..5d2764ad3a3 100755
--- a/ci/run-docker-build.sh
+++ b/ci/run-docker-build.sh
@@ -47,15 +47,6 @@ else
 	else
 		useradd -u $HOST_UID $CI_USER
 	fi
-
-	# Due to a bug the test suite was run as root in the past, so
-	# a prove state file created back then is only accessible by
-	# root.  Now that bug is fixed, the test suite is run as a
-	# regular user, but the prove state file coming from Travis
-	# CI's cache might still be owned by root.
-	# Make sure that this user has rights to any cached files,
-	# including an existing prove state file.
-	test -n "$cache_dir" && chown -R $HOST_UID:$HOST_UID "$cache_dir"
 fi
 
 # Build and test
diff --git a/ci/run-docker.sh b/ci/run-docker.sh
index 37fa372052d..b610dd4db84 100755
--- a/ci/run-docker.sh
+++ b/ci/run-docker.sh
@@ -25,7 +25,7 @@ docker pull "$CI_CONTAINER"
 # root@container:/# export jobname=<jobname>
 # root@container:/# /usr/src/git/ci/run-docker-build.sh <host-user-id>
 
-container_cache_dir=/tmp/travis-cache
+container_cache_dir=/tmp/container-cache
 
 docker run \
 	--interactive \
-- 
2.34.0.830.gb9cdc59c8af


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v4 2/5] CI: use shorter names that fit in UX tooltips
  2021-11-23 16:29     ` [PATCH v4 0/5] CI: Remove Travis CI, shorten names for GH tooltips, split jobs Ævar Arnfjörð Bjarmason
  2021-11-23 16:29       ` [PATCH v4 1/5] CI: remove Travis CI support Ævar Arnfjörð Bjarmason
@ 2021-11-23 16:29       ` Ævar Arnfjörð Bjarmason
  2021-11-23 16:29       ` [PATCH v4 3/5] CI: rename the "Linux32" job to lower-case "linux32" Ævar Arnfjörð Bjarmason
                         ` (2 subsequent siblings)
  4 siblings, 0 replies; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-23 16:29 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, SZEDER Gábor,
	Victoria Dye, Ævar Arnfjörð Bjarmason

Change the names used for the GitHub CI workflows to be short enough
to (mostly) fit in the pop-up tool-tips that GitHub shows in the
commit view. I.e. when mouse-clicking on the passing or failing
check-mark next to the commit subject.

These names are seemingly truncated to 17-20 characters followed by
three dots ("..."). Since a "CI/PR / " prefix is added to them the job
names looked like this before (windows-test and vs-test jobs omitted):

    CI/PR / ci-config (p...
    CI/PR / windows-buil...
    CI/PR / vs-build (pu...
    CI/PR / regular (lin...
    CI/PR / regular (lin...
    CI/PR / regular (os...
    CI/PR / regular (os...
    CI/PR / regular (lin...
    CI/PR / regular (lin...
    CI/PR / dockerized (...
    CI/PR / dockerized (...
    CI/PR / dockerized (...
    CI/PR / static-anal...
    CI/PR / sparse (pu...
    CI/PR / documenta...

By omitting the "/PR" from the top-level name, and pushing the
$jobname to the front we'll now instead get:

    CI / config (push)
    CI / win build (push...
    CI / win+VS build (...
    CI / linux-clang (ub...
    CI / linux-gcc (ubun...
    CI / osx-clang (osx)...
    CI / osx-gcc (osx) (...
    CI / linux-gcc-defau...
    CI / linux-leaks (ub...
    CI / linux-musl (alp...
    CI / Linux32 (daald/...
    CI / pedantic (fedor...
    CI / static-analysis...
    CI / sparse (push)...
    CI / documentation

We then have no truncation in the expanded view. See [1] for how it
looked before, [2] for a currently visible CI run using this commit,
and [3] for the GitHub workflow syntax involved being changed here.

Let's also use the existing "pool" field as before. It's occasionally
useful to know we're running on say ubuntu v.s. fedora. The "-latest"
suffix is useful to some[4], and since it's now at the end it doesn't
hurt readability in the short view compared to saying "ubuntu" or
"macos".

1. https://github.com/git/git/tree/master/
2. https://github.com/avar/git/tree/avar/ci-rm-travis-cleanup-ci-names-3
3. https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions
3. https://lore.kernel.org/git/d9b07ca5-b58d-a535-d25b-85d7f12e6295@github.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 .github/workflows/main.yml | 10 +++++++++-
 README.md                  |  2 +-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 6ed6a9e8076..c7c10456572 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -1,4 +1,4 @@
-name: CI/PR
+name: CI
 
 on: [push, pull_request]
 
@@ -7,6 +7,7 @@ env:
 
 jobs:
   ci-config:
+    name: config
     runs-on: ubuntu-latest
     outputs:
       enabled: ${{ steps.check-ref.outputs.enabled }}${{ steps.skip-if-redundant.outputs.enabled }}
@@ -77,6 +78,7 @@ jobs:
             }
 
   windows-build:
+    name: win build
     needs: ci-config
     if: needs.ci-config.outputs.enabled == 'yes'
     runs-on: windows-latest
@@ -97,6 +99,7 @@ jobs:
         name: windows-artifacts
         path: artifacts
   windows-test:
+    name: win test
     runs-on: windows-latest
     needs: [windows-build]
     strategy:
@@ -127,6 +130,7 @@ jobs:
         name: failed-tests-windows
         path: ${{env.FAILED_TEST_ARTIFACTS}}
   vs-build:
+    name: win+VS build
     needs: ci-config
     if: needs.ci-config.outputs.enabled == 'yes'
     env:
@@ -178,6 +182,7 @@ jobs:
         name: vs-artifacts
         path: artifacts
   vs-test:
+    name: win+VS test
     runs-on: windows-latest
     needs: vs-build
     strategy:
@@ -210,6 +215,7 @@ jobs:
         name: failed-tests-windows
         path: ${{env.FAILED_TEST_ARTIFACTS}}
   regular:
+    name: ${{matrix.vector.jobname}} (${{matrix.vector.pool}})
     needs: ci-config
     if: needs.ci-config.outputs.enabled == 'yes'
     strategy:
@@ -251,6 +257,7 @@ jobs:
         name: failed-tests-${{matrix.vector.jobname}}
         path: ${{env.FAILED_TEST_ARTIFACTS}}
   dockerized:
+    name: ${{matrix.vector.jobname}} (${{matrix.vector.image}})
     needs: ci-config
     if: needs.ci-config.outputs.enabled == 'yes'
     strategy:
@@ -310,6 +317,7 @@ jobs:
       run: ci/install-dependencies.sh
     - run: make sparse
   documentation:
+    name: documentation
     needs: ci-config
     if: needs.ci-config.outputs.enabled == 'yes'
     env:
diff --git a/README.md b/README.md
index eb8115e6b04..f6f43e78deb 100644
--- a/README.md
+++ b/README.md
@@ -1,4 +1,4 @@
-[![Build status](https://github.com/git/git/workflows/CI/PR/badge.svg)](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush)
+[![Build status](https://github.com/git/git/workflows/CI/badge.svg)](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush)
 
 Git - fast, scalable, distributed revision control system
 =========================================================
-- 
2.34.0.830.gb9cdc59c8af


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v4 3/5] CI: rename the "Linux32" job to lower-case "linux32"
  2021-11-23 16:29     ` [PATCH v4 0/5] CI: Remove Travis CI, shorten names for GH tooltips, split jobs Ævar Arnfjörð Bjarmason
  2021-11-23 16:29       ` [PATCH v4 1/5] CI: remove Travis CI support Ævar Arnfjörð Bjarmason
  2021-11-23 16:29       ` [PATCH v4 2/5] CI: use shorter names that fit in UX tooltips Ævar Arnfjörð Bjarmason
@ 2021-11-23 16:29       ` Ævar Arnfjörð Bjarmason
  2021-11-23 16:29       ` [PATCH v4 4/5] CI: use "$runs_on_pool", not "$jobname" to select packages & config Ævar Arnfjörð Bjarmason
  2021-11-23 16:29       ` [PATCH v4 5/5] CI: don't run "make test" twice in one job Ævar Arnfjörð Bjarmason
  4 siblings, 0 replies; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-23 16:29 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, SZEDER Gábor,
	Victoria Dye, Ævar Arnfjörð Bjarmason

As a follow-up to the preceding commit's shortening of CI job names,
rename the only job that starts with an upper-case letter to be
consistent with the rest. It was added in 88dedd5e72c (Travis: also
test on 32-bit Linux, 2017-03-05).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 .github/workflows/main.yml        | 3 ++-
 ci/install-docker-dependencies.sh | 2 +-
 ci/lib.sh                         | 2 +-
 ci/run-docker-build.sh            | 2 +-
 ci/run-docker.sh                  | 2 +-
 5 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index c7c10456572..91b565f75bb 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -266,7 +266,8 @@ jobs:
         vector:
         - jobname: linux-musl
           image: alpine
-        - jobname: Linux32
+        - jobname: linux32
+          os: ubuntu32
           image: daald/ubuntu32:xenial
         - jobname: pedantic
           image: fedora
diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
index 07a8c6b199d..78b7e326da6 100755
--- a/ci/install-docker-dependencies.sh
+++ b/ci/install-docker-dependencies.sh
@@ -4,7 +4,7 @@
 #
 
 case "$jobname" in
-Linux32)
+linux32)
 	linux32 --32bit i386 sh -c '
 		apt update >/dev/null &&
 		apt install -y build-essential libcurl4-openssl-dev \
diff --git a/ci/lib.sh b/ci/lib.sh
index 73d959e87f7..0b3b0144882 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -191,7 +191,7 @@ osx-clang|osx-gcc)
 	;;
 linux-gcc-default)
 	;;
-Linux32)
+linux32)
 	CC=gcc
 	;;
 linux-musl)
diff --git a/ci/run-docker-build.sh b/ci/run-docker-build.sh
index 5d2764ad3a3..6cd832efb9c 100755
--- a/ci/run-docker-build.sh
+++ b/ci/run-docker-build.sh
@@ -15,7 +15,7 @@ then
 fi
 
 case "$jobname" in
-Linux32)
+linux32)
 	switch_cmd="linux32 --32bit i386"
 	;;
 linux-musl)
diff --git a/ci/run-docker.sh b/ci/run-docker.sh
index b610dd4db84..af89d1624a4 100755
--- a/ci/run-docker.sh
+++ b/ci/run-docker.sh
@@ -6,7 +6,7 @@
 . ${0%/*}/lib.sh
 
 case "$jobname" in
-Linux32)
+linux32)
 	CI_CONTAINER="daald/ubuntu32:xenial"
 	;;
 linux-musl)
-- 
2.34.0.830.gb9cdc59c8af


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v4 4/5] CI: use "$runs_on_pool", not "$jobname" to select packages & config
  2021-11-23 16:29     ` [PATCH v4 0/5] CI: Remove Travis CI, shorten names for GH tooltips, split jobs Ævar Arnfjörð Bjarmason
                         ` (2 preceding siblings ...)
  2021-11-23 16:29       ` [PATCH v4 3/5] CI: rename the "Linux32" job to lower-case "linux32" Ævar Arnfjörð Bjarmason
@ 2021-11-23 16:29       ` Ævar Arnfjörð Bjarmason
  2021-11-23 16:29       ` [PATCH v4 5/5] CI: don't run "make test" twice in one job Ævar Arnfjörð Bjarmason
  4 siblings, 0 replies; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-23 16:29 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, SZEDER Gábor,
	Victoria Dye, Ævar Arnfjörð Bjarmason

Change the setup hooks for the CI to use "$runs_on_pool" for the
"$regular" job. Now we won't need as much boilerplate when adding new
jobs to the "regular" matrix, see 956d2e4639b (tests: add a test mode
for SANITIZE=leak, run it in CI, 2021-09-23) for the last such commit.

I.e. now instead of needing to enumerate each jobname when we select
packages we can install things depending on the pool we're running
in.

That we didn't do this dates back to the now gone dependency on Travis
CI, but even if we add a new CI target in the future this'll be easier
to port over, since we can probably treat "ubuntu-latest" as a
stand-in for some recent Linux that can run "apt" commands.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 .github/workflows/main.yml |  4 ++++
 ci/install-dependencies.sh | 32 ++++++++++++++------------------
 ci/lib.sh                  | 21 +++++++++++----------
 3 files changed, 29 insertions(+), 28 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 91b565f75bb..d402402a18b 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -227,12 +227,14 @@ jobs:
             pool: ubuntu-latest
           - jobname: linux-gcc
             cc: gcc
+            cc_package: gcc-8
             pool: ubuntu-latest
           - jobname: osx-clang
             cc: clang
             pool: macos-latest
           - jobname: osx-gcc
             cc: gcc
+            cc_package: gcc-9
             pool: macos-latest
           - jobname: linux-gcc-default
             cc: gcc
@@ -242,7 +244,9 @@ jobs:
             pool: ubuntu-latest
     env:
       CC: ${{matrix.vector.cc}}
+      CC_PACKAGE: ${{matrix.vector.cc_package}}
       jobname: ${{matrix.vector.jobname}}
+      runs_on_pool: ${{matrix.vector.pool}}
     runs-on: ${{matrix.vector.pool}}
     steps:
     - uses: actions/checkout@v2
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 49a4ae7a988..dbcebad2fb2 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -11,17 +11,11 @@ UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
  tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl
  libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl"
 
-case "$jobname" in
-linux-clang|linux-gcc|linux-leaks)
+case "$runs_on_pool" in
+ubuntu-latest)
 	sudo apt-get -q update
 	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
-		$UBUNTU_COMMON_PKGS
-	case "$jobname" in
-	linux-gcc)
-		sudo apt-get -q -y install gcc-8
-		;;
-	esac
-
+		$UBUNTU_COMMON_PKGS $CC_PACKAGE
 	mkdir --parents "$P4_PATH"
 	pushd "$P4_PATH"
 		wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d"
@@ -36,7 +30,7 @@ linux-clang|linux-gcc|linux-leaks)
 		cp git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs .
 	popd
 	;;
-osx-clang|osx-gcc)
+macos-latest)
 	export HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_INSTALL_CLEANUP=1
 	# Uncomment this if you want to run perf tests:
 	# brew install gnu-time
@@ -50,15 +44,17 @@ osx-clang|osx-gcc)
 		brew install --cask --no-quarantine perforce
 	} ||
 	brew install homebrew/cask/perforce
-	case "$jobname" in
-	osx-gcc)
-		brew install gcc@9
-		# Just in case the image is updated to contain gcc@9
-		# pre-installed but not linked.
-		brew link gcc@9
-		;;
-	esac
+
+	if test -n "$CC_PACKAGE"
+	then
+		BREW_PACKAGE=${CC_PACKAGE/-/@}
+		brew install "$BREW_PACKAGE"
+		brew link "$BREW_PACKAGE"
+	fi
 	;;
+esac
+
+case "$jobname" in
 StaticAnalysis)
 	sudo apt-get -q update
 	sudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \
diff --git a/ci/lib.sh b/ci/lib.sh
index 0b3b0144882..cbc2f8f1caa 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -156,11 +156,15 @@ export DEFAULT_TEST_TARGET=prove
 export GIT_TEST_CLONE_2GB=true
 export SKIP_DASHED_BUILT_INS=YesPlease
 
-case "$jobname" in
-linux-clang|linux-gcc|linux-leaks)
+case "$runs_on_pool" in
+ubuntu-latest)
+	if test "$jobname" = "linux-gcc-default"
+	then
+		break
+	fi
+
 	if [ "$jobname" = linux-gcc ]
 	then
-		export CC=gcc-8
 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python3"
 	else
 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python2"
@@ -180,17 +184,17 @@ linux-clang|linux-gcc|linux-leaks)
 	GIT_LFS_PATH="$HOME/custom/git-lfs"
 	export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH"
 	;;
-osx-clang|osx-gcc)
+macos-latest)
 	if [ "$jobname" = osx-gcc ]
 	then
-		export CC=gcc-9
 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python3)"
 	else
 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python2)"
 	fi
 	;;
-linux-gcc-default)
-	;;
+esac
+
+case "$jobname" in
 linux32)
 	CC=gcc
 	;;
@@ -200,9 +204,6 @@ linux-musl)
 	MAKEFLAGS="$MAKEFLAGS NO_REGEX=Yes ICONV_OMITS_BOM=Yes"
 	MAKEFLAGS="$MAKEFLAGS GIT_TEST_UTF8_LOCALE=C.UTF-8"
 	;;
-esac
-
-case "$jobname" in
 linux-leaks)
 	export SANITIZE=leak
 	export GIT_TEST_PASSING_SANITIZE_LEAK=true
-- 
2.34.0.830.gb9cdc59c8af


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v4 5/5] CI: don't run "make test" twice in one job
  2021-11-23 16:29     ` [PATCH v4 0/5] CI: Remove Travis CI, shorten names for GH tooltips, split jobs Ævar Arnfjörð Bjarmason
                         ` (3 preceding siblings ...)
  2021-11-23 16:29       ` [PATCH v4 4/5] CI: use "$runs_on_pool", not "$jobname" to select packages & config Ævar Arnfjörð Bjarmason
@ 2021-11-23 16:29       ` Ævar Arnfjörð Bjarmason
  4 siblings, 0 replies; 38+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-23 16:29 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, SZEDER Gábor,
	Victoria Dye, Ævar Arnfjörð Bjarmason

The "linux-clang" and "linux-gcc" jobs both run "make test" twice, but
with different environment variables. Running these in sequence seems
to have been done to work around some constraint on Travis, see
ae59a4e44f3 (travis: run tests with GIT_TEST_SPLIT_INDEX, 2018-01-07).

By having these run in parallel we'll get jobs that finish much sooner
than they otherwise would have.

We can also simplify the control flow in "ci/run-build-and-tests.sh"
as a result, since we won't run "make test" twice we don't need to run
"make" twice at all, let's default to "make all test" after setting
the variables, and then override it to just "all" for the compile-only
tests.

Add a comment to clarify that new "test" targets should adjust
$MAKE_TARGETS rather than being added after the "case/esac". This
should avoid future confusion where e.g. the compilation-only
"pedantic" target will unexpectedly start running tests. See [1] and
[2].

1. https://lore.kernel.org/git/211122.86ee78yxts.gmgdl@evledraar.gmail.com/
2. https://lore.kernel.org/git/211123.86ilwjujmd.gmgdl@evledraar.gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 .github/workflows/main.yml |  9 +++++++++
 ci/run-build-and-tests.sh  | 23 +++++++++++------------
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index d402402a18b..628bcbf495e 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -225,10 +225,19 @@ jobs:
           - jobname: linux-clang
             cc: clang
             pool: ubuntu-latest
+          - jobname: linux-sha256
+            cc: clang
+            os: ubuntu
+            pool: ubuntu-latest
           - jobname: linux-gcc
             cc: gcc
             cc_package: gcc-8
             pool: ubuntu-latest
+          - jobname: linux-TEST-vars
+            cc: gcc
+            os: ubuntu
+            cc_package: gcc-8
+            pool: ubuntu-latest
           - jobname: osx-clang
             cc: clang
             pool: macos-latest
diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
index 18056501ec2..280dda7d285 100755
--- a/ci/run-build-and-tests.sh
+++ b/ci/run-build-and-tests.sh
@@ -10,16 +10,13 @@ windows*) cmd //c mklink //j t\\.prove "$(cygpath -aw "$cache_dir/.prove")";;
 *) ln -s "$cache_dir/.prove" t/.prove;;
 esac
 
-if test "$jobname" = "pedantic"
-then
-	export DEVOPTS=pedantic
-fi
+export MAKE_TARGETS="all test"
 
-make
 case "$jobname" in
 linux-gcc)
 	export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
-	make test
+	;;
+linux-TEST-vars)
 	export GIT_TEST_SPLIT_INDEX=yes
 	export GIT_TEST_MERGE_ALGORITHM=recursive
 	export GIT_TEST_FULL_IN_PACK_ARRAY=true
@@ -33,23 +30,25 @@ linux-gcc)
 	export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
 	export GIT_TEST_WRITE_REV_INDEX=1
 	export GIT_TEST_CHECKOUT_WORKERS=2
-	make test
 	;;
 linux-clang)
 	export GIT_TEST_DEFAULT_HASH=sha1
-	make test
+	;;
+linux-sha256)
 	export GIT_TEST_DEFAULT_HASH=sha256
-	make test
 	;;
 pedantic)
 	# Don't run the tests; we only care about whether Git can be
 	# built.
-	;;
-*)
-	make test
+	export DEVOPTS=pedantic
+	export MAKE_TARGETS=all
 	;;
 esac
 
+# Any new "test" targets should not go after this "make", but should
+# adjust $MAKE_TARGETS. Otherwise compilation-only targets above will
+# start running tests.
+make $MAKE_TARGETS
 check_unignored_build_artifacts
 
 save_good_tree
-- 
2.34.0.830.gb9cdc59c8af


^ permalink raw reply related	[flat|nested] 38+ messages in thread

end of thread, other threads:[~2021-11-23 16:29 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-19 13:56 [PATCH 0/2] CI: use shorter names for CI jobs, less truncation Ævar Arnfjörð Bjarmason
2021-11-19 13:56 ` [PATCH 1/2] CI: use shorter names that fit in UX tooltips Ævar Arnfjörð Bjarmason
2021-11-19 14:58   ` Johannes Schindelin
2021-11-19 20:39     ` Ævar Arnfjörð Bjarmason
2021-11-19 16:02   ` Victoria Dye
2021-11-19 20:33     ` Ævar Arnfjörð Bjarmason
2021-11-19 21:55       ` Victoria Dye
2021-11-20  2:51         ` Ævar Arnfjörð Bjarmason
2021-11-19 22:14     ` Junio C Hamano
2021-11-19 13:56 ` [PATCH 2/2] CI: rename the "Linux32" job to lower-case "linux32" Ævar Arnfjörð Bjarmason
2021-11-19 14:57   ` Jeff King
2021-11-19 15:03 ` [PATCH 0/2] CI: use shorter names for CI jobs, less truncation Jeff King
2021-11-19 19:57 ` Junio C Hamano
2021-11-19 20:48   ` Ævar Arnfjörð Bjarmason
2021-11-20  3:28 ` [PATCH v2 0/6] CI: Remove Travis CI, shorten names for GH tooltips, split jobs Ævar Arnfjörð Bjarmason
2021-11-20  3:28   ` [PATCH v2 1/6] CI: remove Travis CI support Ævar Arnfjörð Bjarmason
2021-11-20  3:28   ` [PATCH v2 2/6] CI: use shorter names that fit in UX tooltips Ævar Arnfjörð Bjarmason
2021-11-20  7:01     ` Victoria Dye
2021-11-20  3:28   ` [PATCH v2 3/6] CI: rename the "Linux32" job to lower-case "linux32" Ævar Arnfjörð Bjarmason
2021-11-20  3:28   ` [PATCH v2 4/6] CI: use "$runs_on_pool", not "$jobname" to select packages & config Ævar Arnfjörð Bjarmason
2021-11-20  3:28   ` [PATCH v2 5/6] CI: don't run "make test" twice in one job Ævar Arnfjörð Bjarmason
2021-11-20  3:28   ` [PATCH v2 6/6] CI: run "documentation" via run-build-and-test.sh Ævar Arnfjörð Bjarmason
2021-11-20  8:05   ` [PATCH v2 0/6] CI: Remove Travis CI, shorten names for GH tooltips, split jobs Johannes Schindelin
2021-11-20 12:14     ` Ævar Arnfjörð Bjarmason
2021-11-20 12:10   ` [PATCH v3 0/5] " Ævar Arnfjörð Bjarmason
2021-11-20 12:10     ` [PATCH v3 1/5] CI: remove Travis CI support Ævar Arnfjörð Bjarmason
2021-11-20 12:10     ` [PATCH v3 2/5] CI: use shorter names that fit in UX tooltips Ævar Arnfjörð Bjarmason
2021-11-20 19:06       ` Victoria Dye
2021-11-20 12:10     ` [PATCH v3 3/5] CI: rename the "Linux32" job to lower-case "linux32" Ævar Arnfjörð Bjarmason
2021-11-20 12:10     ` [PATCH v3 4/5] CI: use "$runs_on_pool", not "$jobname" to select packages & config Ævar Arnfjörð Bjarmason
2021-11-21  7:21       ` Junio C Hamano
2021-11-20 12:10     ` [PATCH v3 5/5] CI: don't run "make test" twice in one job Ævar Arnfjörð Bjarmason
2021-11-23 16:29     ` [PATCH v4 0/5] CI: Remove Travis CI, shorten names for GH tooltips, split jobs Ævar Arnfjörð Bjarmason
2021-11-23 16:29       ` [PATCH v4 1/5] CI: remove Travis CI support Ævar Arnfjörð Bjarmason
2021-11-23 16:29       ` [PATCH v4 2/5] CI: use shorter names that fit in UX tooltips Ævar Arnfjörð Bjarmason
2021-11-23 16:29       ` [PATCH v4 3/5] CI: rename the "Linux32" job to lower-case "linux32" Ævar Arnfjörð Bjarmason
2021-11-23 16:29       ` [PATCH v4 4/5] CI: use "$runs_on_pool", not "$jobname" to select packages & config Ævar Arnfjörð Bjarmason
2021-11-23 16:29       ` [PATCH v4 5/5] CI: don't run "make test" twice in one job Ævar Arnfjörð Bjarmason

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).