ci: replace our Azure Pipeline by GitHub Actions
cover ci: replace our Azure Pipeline by GitHub Actions
Our Azure Pipeline has served us well over the course of the past year or so, steadily catching issues before the respective patches hit the next branch.
There is a GitHub-native CI system now, though, called "GitHub Actions"
[https://github.com/features/actions] which is essentially on par with Azure Pipelines as far as our needs are concerned, and it brings a couple of advantages:
* It is substantially easier to set up than Azure Pipelines: all you need is to add the YAML-based build definition, push to your fork on GitHub, and that's it.
* The syntax is a bit easier to read than Azure Pipelines'.
* We get more concurrent jobs (Azure Pipelines is limited to 10 concurrent jobs).
With this change, users also no longer need to open a PR at
https://github.com/git/git or at https://github.com/gitgitgadget/git just to get the benefit of a CI build. They just push to their fork on GitHub and monitor the build. Easier than making apple pie.
The only caveat is that this will only work once the patch series makes it to master. In the meantime, you can adore the CI build here:
https://github.com/dscho/git/actions/runs/67349068
Johannes Schindelin (5):
ci/lib: if CI type is unknown, show the environment variables ci/lib: allow running in GitHub Actions
ci: configure GitHub Actions for CI/PR
README: add a build badge for the GitHub Actions runs
ci: retire the Azure Pipelines definition
.github/workflows/main.yml | 271 ++++++++++++++++++ README.md | 2 +- azure-pipelines.yml | 558 ------------------------------------- ci/lib.sh | 21 +- 4 files changed, 292 insertions(+), 560 deletions(-) create mode 100644 .github/workflows/main.yml delete mode 100644 azure-pipelines.yml
base-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-743%2Fdscho%2Fgithub-actions-git.git-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-743/dscho/github-actions-git.git-v1
Pull-Request: https://github.com/git/git/pull/743
Signature · 1 line
gitgitgadget
PATCH 1/5 ci/lib: if CI type is unknown, show the environment variables
From: Johannes Schindelin <[email protected]>
This should help with adding new CI-specific if-else arms.
--- ci/lib.sh | 1 + 1 file changed, 1 insertion(+)
ci/lib.sh +1 -0
@@ -138,6 +138,7 @@ then GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS" else echo "Could not identify CI type" >&2 + env >&2 exit 1 fi
Signature · 1 line
gitgitgadget
PATCH 4/5 README: add a build badge for the GitHub Actions runs
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
--- README.md | 1 + 1 file changed, 1 insertion(+)
README.md +1 -0
@@ -1,3 +1,4 @@ +[](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush) [](https://dev.azure.com/git/git/_build/latest?definitionId=11) Git - fast, scalable, distributed revision control system
Signature · 1 line
gitgitgadget
PATCH 2/5 ci/lib: allow running in GitHub Actions
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
For each CI system we support, we need a specific arm in that if/else construct in ci/lib.sh. Let's add one for GitHub Actions.
--- ci/lib.sh | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)
ci/lib.sh +19 -1
@@ -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 + if test "$TRAVIS_DEBUG_MODE" = true || test true = "$GITHUB_ACTIONS" then return fi
@@ -136,6 +136,24 @@ then MAKEFLAGS="$MAKEFLAGS --jobs=10" test windows_nt != "$CI_OS_NAME" || GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS" +elif test true = "$GITHUB_ACTIONS" +then + CI_TYPE=github-actions + CI_BRANCH="$GITHUB_REF" + CI_COMMIT="$GITHUB_SHA" + CI_OS_NAME="$(echo "$RUNNER_OS" | tr A-Z a-z)" + test macos != "$CI_OS_NAME" || CI_OS_NAME=osx + CI_REPO_SLUG="$GITHUB_REPOSITORY" + CI_JOB_ID="$GITHUB_RUN_ID" + CC="${CC:-gcc}" + + cache_dir="$HOME/none" + + export GIT_PROVE_OPTS="--timer --jobs 10" + export GIT_TEST_OPTS="--verbose-log -x" + MAKEFLAGS="$MAKEFLAGS --jobs=10" + test windows != "$CI_OS_NAME" || + GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS" else echo "Could not identify CI type" >&2 env >&2
Signature · 1 line
gitgitgadget
PATCH 5/5 ci: retire the Azure Pipelines definition
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
We have GitHub Actions now. Running the same builds and tests in Azure Pipelines would be redundant, and a waste of energy.
--- README.md | 1 - azure-pipelines.yml | 558 -------------------------------------------- 2 files changed, 559 deletions(-) delete mode 100644 azure-pipelines.yml
README.md +0 -1
@@ -1,5 +1,4 @@ [](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush) -[](https://dev.azure.com/git/git/_build/latest?definitionId=11) Git - fast, scalable, distributed revision control system =========================================================
azure-pipelines.yml deleted +0 -558
558 lines not on this page · raw message
Signature · 1 line
gitgitgadget
PATCH 3/5 ci: configure GitHub Actions for CI/PR
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
This patch adds CI builds via GitHub Actions. While the underlying technology is at least _very_ similar to that of Azure Pipelines, GitHub Actions are much easier to set up than Azure Pipelines: no need to install a GitHub App, no need to set up an Azure DevOps account, all you need to do is push to your fork on GitHub.
Therefore, it makes a lot of sense for us to have a working GitHub Actions setup.
While transmogrifying azure-pipelines.yml into .github/workflows/main.yml, we also use the opportunity to accelerate the step that sets up a minimal subset of Git for Windows' SDK in the Windows-build job: we now download a .tar.xz stored in Azure Blobs and extract it simultaneously (by calling curl and piping the result to tar, decompressing via xz, all three utilities being available by grace of using Git for Windows' Bash that is installed on the build agents). This accelerates that step from ~1m50s to ~7s.
Also, we do away with the parts that try to mount a file share on which prove can store data between runs. It is just too complicated to set up, so it's little return on investment there.
--- .github/workflows/main.yml | 271 +++++++++++++++++++++++++++++++++++++ 1 file changed, 271 insertions(+) create mode 100644 .github/workflows/main.yml
.github/workflows/main.yml new file +271 -0
@@ -0,0 +1,271 @@ +name: CI/PR + +on: [push, pull_request] + +jobs: + windows-build: + runs-on: windows-latest + steps: + - uses: actions/checkout@v1 + - name: Download git-sdk-64-minimal + shell: bash + run: a=git-sdk-64-minimal && mkdir -p $a && curl -# https://wingit.blob.core.windows.net/ci-artifacts/$a.tar.xz | tar -C $a -xJf - + - name: Build + shell: powershell + env: + HOME: ${{runner.workspace}} + MSYSTEM: MINGW64 + DEVELOPER: 1 + NO_PERL: 1 + run: | + & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @" + printf '%s\n' /git-sdk-64-minimal/ >>.git/info/exclude + + ci/make-test-artifacts.sh artifacts + "@ + if (!$?) { exit(1) } + - name: Upload build artifacts + uses: actions/upload-artifact@v1 + with: + name: windows-artifacts + path: artifacts + windows-test: + runs-on: windows-latest + needs: [windows-build] + strategy: + matrix: + nr: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9] + steps: + - uses: actions/checkout@v1 + - name: Download git-sdk-64-minimal + shell: bash + run: a=git-sdk-64-minimal && mkdir -p $a && curl -# https://wingit.blob.core.windows.net/ci-artifacts/$a.tar.xz | tar -C $a -xJf - + - name: Download build artifacts + uses: actions/download-artifact@v1 + with: + name: windows-artifacts + path: ${{github.workspace}} + - name: Test + shell: powershell + run: | + & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @" + test -f artifacts.tar.gz || { + echo No test artifacts found\; skipping >&2 + exit 0 + } + tar xf artifacts.tar.gz || exit 1 + + # Let Git ignore the SDK + printf '%s\n' /git-sdk-64-minimal/ >>.git/info/exclude + + ci/run-test-slice.sh ${{matrix.nr}} 10 || { + ci/print-test-failures.sh + exit 1 + } + "@ + if (!$?) { exit(1) } + vs-build: + runs-on: windows-latest + steps: + - uses: actions/checkout@v1 + - name: Download git-sdk-64-minimal + shell: bash + run: a=git-sdk-64-minimal && mkdir -p $a && curl -# https://wingit.blob.core.windows.net/ci-artifacts/$a.tar.xz | tar -C $a -xJf - + - name: Generate Visual Studio Solution + shell: powershell + env: + MSYSTEM: MINGW64 + DEVELOPER: 1 + NO_PERL: 1 + GIT_CONFIG_PARAMETERS: "'user.name=CI' 'user.email=ci@git'" + run: | + & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @" + make NDEBUG=1 DEVELOPER=1 vcxproj + "@ + if (!$?) { exit(1) } + - name: Download vcpkg artifacts + shell: powershell + run: | + $urlbase = "https://dev.azure.com/git/git/_apis/build/builds" + $id = ((Invoke-WebRequest -UseBasicParsing "${urlbase}?definitions=9&statusFilter=completed&resultFilter=succeeded&`$top=1").content | ConvertFrom-JSON).value[0].id + $downloadUrl = ((Invoke-WebRequest -UseBasicParsing "${urlbase}/$id/artifacts").content | ConvertFrom-JSON).value[0].resource.downloadUrl + (New-Object Net.WebClient).DownloadFile($downloadUrl, "compat.zip") + Expand-Archive compat.zip -DestinationPath . -Force + Remove-Item compat.zip + - name: Add msbuild to PATH + uses: microsoft/[email protected] + - name: MSBuild + run: msbuild git.sln -property:Configuration=Release -property:Platform=x64 -maxCpuCount:4 -property:PlatformToolset=v142 + - name: Bundle artifact tar + shell: powershell + env: + MSYSTEM: MINGW64 + DEVELOPER: 1 + NO_PERL: 1 + MSVC: 1 + VCPKG_ROOT: ${{github.workspace}}\compat\vcbuild\vcpkg + run: | + & compat\vcbuild\vcpkg_copy_dlls.bat release + if (!$?) { exit(1) } + & git-sdk-64-minimal\usr\bin\bash.exe -lc @" + mkdir -p artifacts && + eval \"`$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts 2>&1 | grep ^tar)\" + "@ + if (!$?) { exit(1) } + - name: Upload build artifacts + uses: actions/upload-artifact@v1 + with: + name: vs-artifacts + path: artifacts + vs-test: + runs-on: windows-latest + needs: [vs-build] + strategy: + matrix: + nr: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9] + steps: + - uses: actions/checkout@v1 + - name: Download git-64-portable + shell: bash + run: a=git-64-portable && mkdir -p $a && curl -# https://wingit.blob.core.windows.net/ci-artifacts/$a.tar.xz | tar -C $a -xJf - + - name: Download build artifacts + uses: actions/download-artifact@v1 + with: + name: vs-artifacts + path: ${{github.workspace}} + - name: Test (parallel) + shell: powershell + env: + MSYSTEM: MINGW64 + NO_SVN_TESTS: 1 + GIT_TEST_SKIP_REBASE_P: 1 + run: | + & git-64-portable\git-cmd.exe --command=usr\bin\bash.exe -lc @" + test -f artifacts.tar.gz || { + echo No test artifacts found\; skipping >&2 + exit 0 + } + tar xf artifacts.tar.gz || exit 1 + + # Let Git ignore the SDK and the test-cache + printf '%s\n' /git-64-portable/ /test-cache/ >>.git/info/exclude + + cd t && + PATH=\"`$PWD/helper:`$PATH\" && + test-tool.exe run-command testsuite --jobs=10 -V -x --write-junit-xml \ + `$(test-tool.exe path-utils slice-tests \ + ${{matrix.nr}} 10 t[0-9]*.sh) + "@ + if (!$?) { exit(1) } + linux-clang: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + env: + CC: clang + run: | + sudo apt-get update && + sudo apt-get -y install git gcc make libssl-dev libcurl4-openssl-dev libexpat-dev tcl tk gettext git-email zlib1g-dev apache2-bin && + ci/install-dependencies.sh + - name: ci/run-build-and-test.sh + env: + CC: clang + run: | + ci/run-build-and-tests.sh || { + ci/print-test-failures.sh + exit 1 + } + linux-gcc: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + run: | + sudo add-apt-repository ppa:ubuntu-toolchain-r/test && + sudo apt-get update && + sudo apt-get -y install git gcc make libssl-dev libcurl4-openssl-dev libexpat-dev tcl tk gettext git-email zlib1g-dev apache2 language-pack-is git-svn gcc-8 && + ci/install-dependencies.sh + - name: ci/run-build-and-tests.sh + run: | + ci/run-build-and-tests.sh || { + ci/print-test-failures.sh + exit 1 + } + osx-clang: + runs-on: macos-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + env: + CC: clang + run: ci/install-dependencies.sh + - name: ci/run-build-and-tests.sh + env: + CC: clang + run: | + ci/run-build-and-tests.sh || { + ci/print-test-failures.sh + exit 1 + } + osx-gcc: + runs-on: macos-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + run: ci/install-dependencies.sh + - name: ci/run-build-and-tests.sh + run: | + ci/run-build-and-tests.sh || { + ci/print-test-failures.sh + exit 1 + } + GETTEXT_POISON: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + run: | + sudo apt-get update && + sudo apt-get -y install git gcc make libssl-dev libcurl4-openssl-dev libexpat-dev tcl tk gettext git-email zlib1g-dev + - name: ci/run-build-and-tests.sh + env: + jobname: GETTEXT_POISON + run: | + ci/run-build-and-tests.sh || { + ci/print-test-failures.sh + exit 1 + } + linux32: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: ci/run-linux32-docker.sh + run: | + res=0 + sudo GITHUB_ACTIONS="$GITHUB_ACTIONS" RUNNER_OS="$RUNNER_OS" GITHUB_REF="$GITHUB_REF" GITHUB_SHA="$GITHUB_SHA" GITHUB_REPOSITORY="$GITHUB_REPOSITORY" GITHUB_RUN_ID="$GITHUB_RUN_ID" CC=$CC MAKEFLAGS="$MAKEFLAGS" bash -lxc ci/run-linux32-docker.sh || res=1 + static-analysis: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + run: | + sudo apt-get update && + sudo apt-get install -y coccinelle libcurl4-openssl-dev libssl-dev libexpat-dev gettext + - name: ci/run-static-analysis.sh + env: + jobname: StaticAnalysis + run: ci/run-static-analysis.sh + documentation: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + run: | + sudo apt-get update && + sudo apt-get install -y asciidoc xmlto asciidoctor docbook-xsl-ns + - name: ci/test-documentation.sh + env: + ALREADY_HAVE_ASCIIDOCTOR: yes. + jobname: Documentation + run: ci/test-documentation.sh
Signature · 1 line
gitgitgadget
Quoted 16 lines · Johannes Schindelin via GitGitGadget
Our Azure Pipeline has served us well over the course of the past year or so, steadily catching issues before the respective patches hit the next branch.
There is a GitHub-native CI system now, though, called "GitHub Actions"
[https://github.com/features/actions] which is essentially on par with Azure Pipelines as far as our needs are concerned, and it brings a couple of advantages:* It is substantially easier to set up than Azure Pipelines: all you need is to add the YAML-based build definition, push to your fork on GitHub, and that's it.
* The syntax is a bit easier to read than Azure Pipelines'.
* We get more concurrent jobs (Azure Pipelines is limited to 10 concurrent jobs).
Quoted 7 lines
With this change, users also no longer need to open a PR at
https://github.com/git/git or at https://github.com/gitgitgadget/git just to get the benefit of a CI build. They just push to their fork on GitHub and monitor the build. Easier than making apple pie.The only caveat is that this will only work once the patch series makes it to master.
Github Actions also works in other branches, at least in pu:
https://github.com/sgn/git/runs/548975243?check_suite_focus=true
Anyway, this series will conflicts with my series for linux-musl CI. And, Github Actions' Documentation job in pu failed because of missing "curl-config".
Attached patches can be used to merge this series into pu.
Signature · 1 line
Danh
From dfc8e913c9878a1b5a9e58d1c69884a1b7bd7864 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?=
<[email protected]>
Date: Tue, 31 Mar 2020 21:21:23 +0700
Subject: [PATCH 1/2] github-actions: fixup to dd/musl-libc-travis-ci
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
--- .github/workflows/main.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
.github/workflows/main.yml +2 -2
@@ -240,10 +240,10 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v1 - - name: ci/run-linux32-docker.sh + - name: jobname=Linux32 ci/run-docker.sh run: | res=0 - sudo GITHUB_ACTIONS="$GITHUB_ACTIONS" RUNNER_OS="$RUNNER_OS" GITHUB_REF="$GITHUB_REF" GITHUB_SHA="$GITHUB_SHA" GITHUB_REPOSITORY="$GITHUB_REPOSITORY" GITHUB_RUN_ID="$GITHUB_RUN_ID" CC=$CC MAKEFLAGS="$MAKEFLAGS" bash -lxc ci/run-linux32-docker.sh || res=1 + sudo GITHUB_ACTIONS="$GITHUB_ACTIONS" RUNNER_OS="$RUNNER_OS" GITHUB_REF="$GITHUB_REF" GITHUB_SHA="$GITHUB_SHA" GITHUB_REPOSITORY="$GITHUB_REPOSITORY" GITHUB_RUN_ID="$GITHUB_RUN_ID" CC=$CC MAKEFLAGS="$MAKEFLAGS" jobname=Linux32 bash -lxc ci/run-docker.sh || res=1 static-analysis: runs-on: ubuntu-latest steps:
Signature · 1 line
2.26.0.334.g6536db25bb
From 48d4247a8c70a6b99171a863a43fc40c925351b4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?=
<[email protected]>
Date: Tue, 31 Mar 2020 21:23:16 +0700
Subject: [PATCH 2/2] github-actions: enable linux-musl check
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
--- .github/workflows/main.yml | 8 ++++++++ 1 file changed, 8 insertions(+)
.github/workflows/main.yml +8 -0
@@ -236,6 +236,14 @@ jobs: ci/print-test-failures.sh exit 1 } + linux-musl: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: jobname=linux-musl ci/run-docker.sh + run: | + res=0 + sudo GITHUB_ACTIONS="$GITHUB_ACTIONS" RUNNER_OS="$RUNNER_OS" GITHUB_REF="$GITHUB_REF" GITHUB_SHA="$GITHUB_SHA" GITHUB_REPOSITORY="$GITHUB_REPOSITORY" GITHUB_RUN_ID="$GITHUB_RUN_ID" CC=$CC MAKEFLAGS="$MAKEFLAGS" jobname=linux-musl bash -lxc ci/run-docker.sh || res=1 linux32: runs-on: ubuntu-latest steps:
Signature · 1 line
2.26.0.334.g6536db25bb
Quoted 11 lines · Danh Doan
Quoted 2 lines
The only caveat is that this will only work once the patch series makes it to master.
Github Actions also works in other branches, at least in pu:
https://github.com/sgn/git/runs/548975243?check_suite_focus=trueAnyway, this series will conflicts with my series for linux-musl CI. And, Github Actions' Documentation job in pu failed because of missing "curl-config".
Attached patches can be used to merge this series into pu.
Please work together to have either one of them build on the other. Thanks.
v2 cover ci: replace our Azure Pipeline by GitHub Actions
In reply to Johannes Schindelin via GitGitGadgetThis series is based on work started by Dscho,
I started to work with this series since there're merge conflicts with my series at dd/ci-musl-lib, and Dscho said he was busy for a while.
Changes frome Dscho's version:
* Based on dd/ci-musl-libc
* Move artifact extraction out of Test phase of windows-test
* Move ci/print-test-failures.sh out of build-and-test phase
* set TERM environment variable if not exist
* add linux-musl job
* v1 doesn't report failure on linux32, fixed
* run linux32 directly inside container
* install development file of curl in documentation job because "curl-config" will be called in pu's Makefile
+ Other approach could be call make CURL_CONFIG=: in test-documentation.sh
Sample run:
* of this series: https://github.com/sgn/git/actions/runs/68291472
* pretend compile failure: https://github.com/sgn/git/actions/runs/68292112
* pretend test failure: https://github.com/sgn/git/actions/runs/68293056
* merged to pu: https://github.com/sgn/git/actions/runs/68301122
Johannes Schindelin (4):
ci/lib: if CI type is unknown, show the environment variables ci/lib: allow running in GitHub Actions
README: add a build badge for the GitHub Actions runs
ci: retire the Azure Pipelines definition
Đoàn Trần Công Danh (2):
ci/lib: set TERM environment variable if not exist
ci: configure GitHub Actions for CI/PR
.github/workflows/main.yml | 314 ++++++++++++++++++++ README.md | 2 +- azure-pipelines.yml | 593 ------------------------------------- ci/lib.sh | 24 +- 4 files changed, 338 insertions(+), 595 deletions(-) create mode 100644 .github/workflows/main.yml delete mode 100644 azure-pipelines.yml
Range-diff against v1:
1: 64d61c3e38 ! 1: 27cb77c257 ci/lib: if CI type is unknown, show the environment variables
@@ Commit message
This should help with adding new CI-specific if-else arms.
- Signed-off-by: Đoàn Trần Công Danh <[email protected]>
## ci/lib.sh ##
@@ ci/lib.sh: then
2: 4a0ec072cc ! 2: 079038c4a7 ci/lib: allow running in GitHub Actions
@@ Commit message
construct in ci/lib.sh. Let's add one for GitHub Actions.
- Signed-off-by: Đoàn Trần Công Danh <[email protected]>
## ci/lib.sh ##
@@ ci/lib.sh: save_good_tree () {
-: ---------- > 3: f9d0b6b0d5 ci/lib: set TERM environment variable if not exist 3: e33653fdf0 ! 4: fb714f5889 ci: configure GitHub Actions for CI/PR
@@
## Metadata ##
-Author: Johannes Schindelin <[email protected]> +Author: Đoàn Trần Công Danh <[email protected]>
## Commit message ##
ci: configure GitHub Actions for CI/PR
This patch adds CI builds via GitHub Actions. While the underlying technology is at least _very_ similar to that of Azure Pipelines, GitHub
- Actions are much easier to set up than Azure Pipelines: no need to
- install a GitHub App, no need to set up an Azure DevOps account, all you
- need to do is push to your fork on GitHub.
+ Actions are much easier to set up than Azure Pipelines:
+ - no need to install a GitHub App,
+ - no need to set up an Azure DevOps account,
+ - all you need to do is push to your fork on GitHub.
Therefore, it makes a lot of sense for us to have a working GitHub Actions setup.
- While transmogrifying azure-pipelines.yml into
+ While translate-and-modify azure-pipelines.yml into
.github/workflows/main.yml, we also use the opportunity to accelerate the step that sets up a minimal subset of Git for Windows' SDK in the
- Windows-build job: we now download a .tar.xz stored in Azure Blobs and
- extract it simultaneously (by calling curl and piping the result to
- tar, decompressing via xz, all three utilities being available by
- grace of using Git for Windows' Bash that is installed on the build
- agents). This accelerates that step from ~1m50s to ~7s.
+ Windows-build job:
+ - we now download a .tar.xz stored in Azure Blobs and extract it
+ simultaneously (by calling curl and piping the result to tar,
+ - decompressing via xz,
+ - all three utilities are installed together with Git for Windows
Also, we do away with the parts that try to mount a file share on which prove can store data between runs. It is just too complicated to set up, so it's little return on investment there.
+ Based-on-patch-by: Johannes Schindelin <[email protected]>
@@ .github/workflows/main.yml (new)
+ with:
+ name: windows-artifacts
+ path: ${{github.workspace}}
-+ - name: Test
++ - name: extract artifact
+ shell: powershell
+ run: |
+ & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @"
@@ .github/workflows/main.yml (new)
+ echo No test artifacts found\; skipping >&2
+ exit 0
+ }
-+ tar xf artifacts.tar.gz || exit 1
-+
++ tar xf artifacts.tar.gz
++ "@
++ if (!$?) { exit(1) }
++ - name: Test
++ shell: powershell
++ run: |
++ & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @"
+ # Let Git ignore the SDK
+ printf '%s\n' /git-sdk-64-minimal/ >>.git/info/exclude
+
-+ ci/run-test-slice.sh ${{matrix.nr}} 10 || {
-+ ci/print-test-failures.sh
-+ exit 1
-+ }
++ ci/run-test-slice.sh ${{matrix.nr}} 10
+ "@
+ if (!$?) { exit(1) }
++ - name: ci/print-test-failures.sh
++ if: failure()
++ shell: powershell
++ run: |
++ & .\git-sdk-64-minimal\usr\bin\bash.exe -lc ci/print-test-failures.sh
++ exit(1)
+ vs-build:
+ runs-on: windows-latest
+ steps:
@@ .github/workflows/main.yml (new)
+ - name: ci/run-build-and-test.sh
+ env:
+ CC: clang
++ run: ci/run-build-and-tests.sh
++ - name: ci/print-test-failures.sh
++ if: failure()
++ env:
++ CC: clang
+ run: |
-+ ci/run-build-and-tests.sh || {
-+ ci/print-test-failures.sh
-+ exit 1
-+ }
++ ci/print-test-failures.sh
++ exit 1
+ linux-gcc:
+ runs-on: ubuntu-latest
+ steps:
@@ .github/workflows/main.yml (new)
+ sudo apt-get -y install git gcc make libssl-dev libcurl4-openssl-dev libexpat-dev tcl tk gettext git-email zlib1g-dev apache2 language-pack-is git-svn gcc-8 &&
+ ci/install-dependencies.sh
+ - name: ci/run-build-and-tests.sh
++ run: ci/run-build-and-tests.sh
++ - name: ci/print-test-failures.sh
++ if: failure()
+ run: |
-+ ci/run-build-and-tests.sh || {
-+ ci/print-test-failures.sh
-+ exit 1
-+ }
++ ci/print-test-failures.sh
++ exit 1
+ osx-clang:
+ runs-on: macos-latest
+ steps:
@@ .github/workflows/main.yml (new)
+ env:
+ CC: clang
+ run: ci/install-dependencies.sh
-+ - name: ci/run-build-and-tests.sh
++ - name: ci/run-build-and-test.sh
++ env:
++ CC: clang
++ run: ci/run-build-and-tests.sh
++ - name: ci/print-test-failures.sh
++ if: failure()
+ env:
+ CC: clang
+ run: |
-+ ci/run-build-and-tests.sh || {
-+ ci/print-test-failures.sh
-+ exit 1
-+ }
++ ci/print-test-failures.sh
++ exit 1
+ osx-gcc:
+ runs-on: macos-latest
+ steps:
@@ .github/workflows/main.yml (new)
+ - name: install dependencies
+ run: ci/install-dependencies.sh
+ - name: ci/run-build-and-tests.sh
++ run: ci/run-build-and-tests.sh
++ - name: ci/print-test-failures.sh
++ if: failure()
+ run: |
-+ ci/run-build-and-tests.sh || {
-+ ci/print-test-failures.sh
-+ exit 1
-+ }
++ ci/print-test-failures.sh
++ exit 1
+ GETTEXT_POISON:
+ runs-on: ubuntu-latest
+ steps:
@@ .github/workflows/main.yml (new)
+ - name: ci/run-build-and-tests.sh
+ env:
+ jobname: GETTEXT_POISON
++ run: ci/run-build-and-tests.sh
++ - name: ci/print-test-failures.sh
++ if: failure()
++ env:
++ jobname: GETTEXT_POISON
+ run: |
-+ ci/run-build-and-tests.sh || {
-+ ci/print-test-failures.sh
-+ exit 1
-+ }
-+ linux32:
++ ci/print-test-failures.sh
++ exit 1
++ linux-musl:
+ runs-on: ubuntu-latest
++ container: alpine
+ steps:
+ - uses: actions/checkout@v1
-+ - name: ci/run-linux32-docker.sh
++ - name: install additional dependencies
++ run: apk add --update ncurses perl-utils
++ - name: prepare /usr/src/git
+ run: |
-+ res=0
-+ sudo GITHUB_ACTIONS="$GITHUB_ACTIONS" RUNNER_OS="$RUNNER_OS" GITHUB_REF="$GITHUB_REF" GITHUB_SHA="$GITHUB_SHA" GITHUB_REPOSITORY="$GITHUB_REPOSITORY" GITHUB_RUN_ID="$GITHUB_RUN_ID" CC=$CC MAKEFLAGS="$MAKEFLAGS" bash -lxc ci/run-linux32-docker.sh || res=1
++ mkdir -p /usr/src &&
++ rm -rf /usr/src/git &&
++ ln -sf $(pwd) /usr/src/git
++ - name: ci/run-alpine-build.sh
++ run: ci/run-alpine-build.sh $(id -u)
++ - name: ci/print-test-failures.sh
++ if: failure()
++ run: ci/print-test-failures.sh
++ linux32:
++ runs-on: ubuntu-latest
++ container: daald/ubuntu32:xenial
++ steps:
++ - uses: actions/checkout@v1
++ - name: prepare /usr/src/git
++ run:
++ mkdir -p /usr/src &&
++ rm -rf /usr/src/git &&
++ ln -sf $(pwd) /usr/src/git
++ - name: ci/run-linux32-build.sh
++ run: ci/run-linux32-build.sh $(id -u)
++ - name: ci/print-test-failures.sh
++ if: failure()
++ run: ci/print-test-failures.sh
+ static-analysis:
+ runs-on: ubuntu-latest
+ steps:
@@ .github/workflows/main.yml (new)
+ - name: install dependencies
+ run: |
+ sudo apt-get update &&
-+ sudo apt-get install -y asciidoc xmlto asciidoctor docbook-xsl-ns
++ sudo apt-get install -y asciidoc xmlto asciidoctor docbook-xsl-ns libcurl4-openssl-dev
+ - name: ci/test-documentation.sh
+ env:
+ ALREADY_HAVE_ASCIIDOCTOR: yes.
4: c4f7338184 ! 5: 4310a7c9d6 README: add a build badge for the GitHub Actions runs
@@ Commit message
README: add a build badge for the GitHub Actions runs
- Signed-off-by: Đoàn Trần Công Danh <[email protected]>
## README.md ##
@@
5: 9598f73435 ! 6: 2451d4991f ci: retire the Azure Pipelines definition
@@ Commit message
Pipelines would be redundant, and a waste of energy.
+ [Danh: fix apply conflicts]
## README.md ##
@@ azure-pipelines.yml (deleted)
- test "$GITFILESHAREPWD" = '$(gitfileshare.pwd)' || ci/mount-fileshare.sh //gitfileshare.file.core.windows.net/test-cache gitfileshare "$GITFILESHAREPWD" "$HOME/test-cache" || exit 1
-
- res=0
-- sudo AGENT_OS="$AGENT_OS" BUILD_BUILDNUMBER="$BUILD_BUILDNUMBER" BUILD_REPOSITORY_URI="$BUILD_REPOSITORY_URI" BUILD_SOURCEBRANCH="$BUILD_SOURCEBRANCH" BUILD_SOURCEVERSION="$BUILD_SOURCEVERSION" SYSTEM_PHASENAME="$SYSTEM_PHASENAME" SYSTEM_TASKDEFINITIONSURI="$SYSTEM_TASKDEFINITIONSURI" SYSTEM_TEAMPROJECT="$SYSTEM_TEAMPROJECT" CC=$CC MAKEFLAGS="$MAKEFLAGS" bash -lxc ci/run-linux32-docker.sh || res=1
+- sudo AGENT_OS="$AGENT_OS" BUILD_BUILDNUMBER="$BUILD_BUILDNUMBER" BUILD_REPOSITORY_URI="$BUILD_REPOSITORY_URI" BUILD_SOURCEBRANCH="$BUILD_SOURCEBRANCH" BUILD_SOURCEVERSION="$BUILD_SOURCEVERSION" SYSTEM_PHASENAME="$SYSTEM_PHASENAME" SYSTEM_TASKDEFINITIONSURI="$SYSTEM_TASKDEFINITIONSURI" SYSTEM_TEAMPROJECT="$SYSTEM_TEAMPROJECT" CC=$CC MAKEFLAGS="$MAKEFLAGS" jobname=Linux32 bash -lxc ci/run-docker.sh || res=1
-
- sudo chmod a+r t/out/TEST-*.xml
- test ! -d t/failed-test-artifacts || sudo chmod a+r t/failed-test-artifacts
-
- test "$GITFILESHAREPWD" = '$(gitfileshare.pwd)' || sudo umount "$HOME/test-cache" || res=1
- exit $res
-- displayName: 'ci/run-linux32-docker.sh'
+- displayName: 'jobname=Linux32 ci/run-docker.sh'
- env:
- GITFILESHAREPWD: $(gitfileshare.pwd)
- - task: PublishTestResults@2
@@ azure-pipelines.yml (deleted)
- PathtoPublish: t/failed-test-artifacts
- ArtifactName: failed-test-artifacts
-
+-- job: linux_musl
+- displayName: linux-musl
+- condition: succeeded()
+- pool:
+- vmImage: ubuntu-latest
+- steps:
+- - bash: |
+- test "$GITFILESHAREPWD" = '$(gitfileshare.pwd)' || ci/mount-fileshare.sh //gitfileshare.file.core.windows.net/test-cache gitfileshare "$GITFILESHAREPWD" "$HOME/test-cache" || exit 1
+-
+- res=0
+- sudo AGENT_OS="$AGENT_OS" BUILD_BUILDNUMBER="$BUILD_BUILDNUMBER" BUILD_REPOSITORY_URI="$BUILD_REPOSITORY_URI" BUILD_SOURCEBRANCH="$BUILD_SOURCEBRANCH" BUILD_SOURCEVERSION="$BUILD_SOURCEVERSION" SYSTEM_PHASENAME="$SYSTEM_PHASENAME" SYSTEM_TASKDEFINITIONSURI="$SYSTEM_TASKDEFINITIONSURI" SYSTEM_TEAMPROJECT="$SYSTEM_TEAMPROJECT" CC=$CC MAKEFLAGS="$MAKEFLAGS" jobname=linux-musl bash -lxc ci/run-docker.sh || res=1
+-
+- sudo chmod a+r t/out/TEST-*.xml
+- test ! -d t/failed-test-artifacts || sudo chmod a+r t/failed-test-artifacts
+-
+- test "$GITFILESHAREPWD" = '$(gitfileshare.pwd)' || sudo umount "$HOME/test-cache" || res=1
+- exit $res
+- displayName: 'jobname=linux-musl ci/run-docker.sh'
+- env:
+- GITFILESHAREPWD: $(gitfileshare.pwd)
+- - task: PublishTestResults@2
+- displayName: 'Publish Test Results **/TEST-*.xml'
+- inputs:
+- mergeTestResults: true
+- testRunTitle: 'musl'
+- platform: Linux
+- publishRunAttachments: false
+- condition: succeededOrFailed()
+- - task: PublishBuildArtifacts@1
+- displayName: 'Publish trash directories of failed tests'
+- condition: failed()
+- inputs:
+- PathtoPublish: t/failed-test-artifacts
+- ArtifactName: failed-test-artifacts
+-
-- job: static_analysis
- displayName: StaticAnalysis
- condition: succeeded()
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v2 1/6 ci/lib: if CI type is unknown, show the environment variables
From: Johannes Schindelin <[email protected]>
This should help with adding new CI-specific if-else arms.
--- ci/lib.sh | 1 + 1 file changed, 1 insertion(+)
ci/lib.sh +1 -0
@@ -138,6 +138,7 @@ then GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS" else echo "Could not identify CI type" >&2 + env >&2 exit 1 fi
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v2 2/6 ci/lib: allow running in GitHub Actions
In reply to Đoàn Trần Công DanhFrom: Johannes Schindelin <[email protected]>
For each CI system we support, we need a specific arm in that if/else construct in ci/lib.sh. Let's add one for GitHub Actions.
--- ci/lib.sh | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)
ci/lib.sh +19 -1
@@ -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 + if test "$TRAVIS_DEBUG_MODE" = true || test true = "$GITHUB_ACTIONS" then return fi
@@ -136,6 +136,24 @@ then MAKEFLAGS="$MAKEFLAGS --jobs=10" test windows_nt != "$CI_OS_NAME" || GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS" +elif test true = "$GITHUB_ACTIONS" +then + CI_TYPE=github-actions + CI_BRANCH="$GITHUB_REF" + CI_COMMIT="$GITHUB_SHA" + CI_OS_NAME="$(echo "$RUNNER_OS" | tr A-Z a-z)" + test macos != "$CI_OS_NAME" || CI_OS_NAME=osx + CI_REPO_SLUG="$GITHUB_REPOSITORY" + CI_JOB_ID="$GITHUB_RUN_ID" + CC="${CC:-gcc}" + + cache_dir="$HOME/none" + + export GIT_PROVE_OPTS="--timer --jobs 10" + export GIT_TEST_OPTS="--verbose-log -x" + MAKEFLAGS="$MAKEFLAGS --jobs=10" + test windows != "$CI_OS_NAME" || + GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS" else echo "Could not identify CI type" >&2 env >&2
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v2 3/6 ci/lib: set TERM environment variable if not exist
In reply to Đoàn Trần Công DanhGitHub Action doesn't set TERM environment variable, which is required by "tput".
Fallback to dumb if it's not set.
--- ci/lib.sh | 3 +++ 1 file changed, 3 insertions(+)
ci/lib.sh +3 -0
@@ -79,6 +79,9 @@ check_unignored_build_artifacts () } } +# GitHub Action doesn't set TERM, which is required by tput +export TERM=${TERM:-dumb} + # Clear MAKEFLAGS that may come from the outside world. export MAKEFLAGS=
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v2 5/6 README: add a build badge for the GitHub Actions runs
In reply to Đoàn Trần Công DanhFrom: Johannes Schindelin <[email protected]>
--- README.md | 1 + 1 file changed, 1 insertion(+)
README.md +1 -0
@@ -1,3 +1,4 @@ +[](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush) [](https://dev.azure.com/git/git/_build/latest?definitionId=11) Git - fast, scalable, distributed revision control system
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v2 4/6 ci: configure GitHub Actions for CI/PR
In reply to Đoàn Trần Công DanhThis patch adds CI builds via GitHub Actions. While the underlying technology is at least _very_ similar to that of Azure Pipelines, GitHub Actions are much easier to set up than Azure Pipelines:
- no need to install a GitHub App,
- no need to set up an Azure DevOps account,
- all you need to do is push to your fork on GitHub.
Therefore, it makes a lot of sense for us to have a working GitHub Actions setup.
While translate-and-modify azure-pipelines.yml into .github/workflows/main.yml, we also use the opportunity to accelerate the step that sets up a minimal subset of Git for Windows' SDK in the Windows-build job:
- we now download a .tar.xz stored in Azure Blobs and extract it simultaneously (by calling curl and piping the result to tar,
- decompressing via xz,
- all three utilities are installed together with Git for Windows
Also, we do away with the parts that try to mount a file share on which prove can store data between runs. It is just too complicated to set up, so it's little return on investment there.
--- .github/workflows/main.yml | 314 +++++++++++++++++++++++++++++++++++++ 1 file changed, 314 insertions(+) create mode 100644 .github/workflows/main.yml
.github/workflows/main.yml new file +314 -0
@@ -0,0 +1,314 @@ +name: CI/PR + +on: [push, pull_request] + +jobs: + windows-build: + runs-on: windows-latest + steps: + - uses: actions/checkout@v1 + - name: Download git-sdk-64-minimal + shell: bash + run: a=git-sdk-64-minimal && mkdir -p $a && curl -# https://wingit.blob.core.windows.net/ci-artifacts/$a.tar.xz | tar -C $a -xJf - + - name: Build + shell: powershell + env: + HOME: ${{runner.workspace}} + MSYSTEM: MINGW64 + DEVELOPER: 1 + NO_PERL: 1 + run: | + & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @" + printf '%s\n' /git-sdk-64-minimal/ >>.git/info/exclude + + ci/make-test-artifacts.sh artifacts + "@ + if (!$?) { exit(1) } + - name: Upload build artifacts + uses: actions/upload-artifact@v1 + with: + name: windows-artifacts + path: artifacts + windows-test: + runs-on: windows-latest + needs: [windows-build] + strategy: + matrix: + nr: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9] + steps: + - uses: actions/checkout@v1 + - name: Download git-sdk-64-minimal + shell: bash + run: a=git-sdk-64-minimal && mkdir -p $a && curl -# https://wingit.blob.core.windows.net/ci-artifacts/$a.tar.xz | tar -C $a -xJf - + - name: Download build artifacts + uses: actions/download-artifact@v1 + with: + name: windows-artifacts + path: ${{github.workspace}} + - name: extract artifact + shell: powershell + run: | + & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @" + test -f artifacts.tar.gz || { + echo No test artifacts found\; skipping >&2 + exit 0 + } + tar xf artifacts.tar.gz + "@ + if (!$?) { exit(1) } + - name: Test + shell: powershell + run: | + & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @" + # Let Git ignore the SDK + printf '%s\n' /git-sdk-64-minimal/ >>.git/info/exclude + + ci/run-test-slice.sh ${{matrix.nr}} 10 + "@ + if (!$?) { exit(1) } + - name: ci/print-test-failures.sh + if: failure() + shell: powershell + run: | + & .\git-sdk-64-minimal\usr\bin\bash.exe -lc ci/print-test-failures.sh + exit(1) + vs-build: + runs-on: windows-latest + steps: + - uses: actions/checkout@v1 + - name: Download git-sdk-64-minimal + shell: bash + run: a=git-sdk-64-minimal && mkdir -p $a && curl -# https://wingit.blob.core.windows.net/ci-artifacts/$a.tar.xz | tar -C $a -xJf - + - name: Generate Visual Studio Solution + shell: powershell + env: + MSYSTEM: MINGW64 + DEVELOPER: 1 + NO_PERL: 1 + GIT_CONFIG_PARAMETERS: "'user.name=CI' 'user.email=ci@git'" + run: | + & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @" + make NDEBUG=1 DEVELOPER=1 vcxproj + "@ + if (!$?) { exit(1) } + - name: Download vcpkg artifacts + shell: powershell + run: | + $urlbase = "https://dev.azure.com/git/git/_apis/build/builds" + $id = ((Invoke-WebRequest -UseBasicParsing "${urlbase}?definitions=9&statusFilter=completed&resultFilter=succeeded&`$top=1").content | ConvertFrom-JSON).value[0].id + $downloadUrl = ((Invoke-WebRequest -UseBasicParsing "${urlbase}/$id/artifacts").content | ConvertFrom-JSON).value[0].resource.downloadUrl + (New-Object Net.WebClient).DownloadFile($downloadUrl, "compat.zip") + Expand-Archive compat.zip -DestinationPath . -Force + Remove-Item compat.zip + - name: Add msbuild to PATH + uses: microsoft/[email protected] + - name: MSBuild + run: msbuild git.sln -property:Configuration=Release -property:Platform=x64 -maxCpuCount:4 -property:PlatformToolset=v142 + - name: Bundle artifact tar + shell: powershell + env: + MSYSTEM: MINGW64 + DEVELOPER: 1 + NO_PERL: 1 + MSVC: 1 + VCPKG_ROOT: ${{github.workspace}}\compat\vcbuild\vcpkg + run: | + & compat\vcbuild\vcpkg_copy_dlls.bat release + if (!$?) { exit(1) } + & git-sdk-64-minimal\usr\bin\bash.exe -lc @" + mkdir -p artifacts && + eval \"`$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts 2>&1 | grep ^tar)\" + "@ + if (!$?) { exit(1) } + - name: Upload build artifacts + uses: actions/upload-artifact@v1 + with: + name: vs-artifacts + path: artifacts + vs-test: + runs-on: windows-latest + needs: [vs-build] + strategy: + matrix: + nr: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9] + steps: + - uses: actions/checkout@v1 + - name: Download git-64-portable + shell: bash + run: a=git-64-portable && mkdir -p $a && curl -# https://wingit.blob.core.windows.net/ci-artifacts/$a.tar.xz | tar -C $a -xJf - + - name: Download build artifacts + uses: actions/download-artifact@v1 + with: + name: vs-artifacts + path: ${{github.workspace}} + - name: Test (parallel) + shell: powershell + env: + MSYSTEM: MINGW64 + NO_SVN_TESTS: 1 + GIT_TEST_SKIP_REBASE_P: 1 + run: | + & git-64-portable\git-cmd.exe --command=usr\bin\bash.exe -lc @" + test -f artifacts.tar.gz || { + echo No test artifacts found\; skipping >&2 + exit 0 + } + tar xf artifacts.tar.gz || exit 1 + + # Let Git ignore the SDK and the test-cache + printf '%s\n' /git-64-portable/ /test-cache/ >>.git/info/exclude + + cd t && + PATH=\"`$PWD/helper:`$PATH\" && + test-tool.exe run-command testsuite --jobs=10 -V -x --write-junit-xml \ + `$(test-tool.exe path-utils slice-tests \ + ${{matrix.nr}} 10 t[0-9]*.sh) + "@ + if (!$?) { exit(1) } + linux-clang: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + env: + CC: clang + run: | + sudo apt-get update && + sudo apt-get -y install git gcc make libssl-dev libcurl4-openssl-dev libexpat-dev tcl tk gettext git-email zlib1g-dev apache2-bin && + ci/install-dependencies.sh + - name: ci/run-build-and-test.sh + env: + CC: clang + run: ci/run-build-and-tests.sh + - name: ci/print-test-failures.sh + if: failure() + env: + CC: clang + run: | + ci/print-test-failures.sh + exit 1 + linux-gcc: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + run: | + sudo add-apt-repository ppa:ubuntu-toolchain-r/test && + sudo apt-get update && + sudo apt-get -y install git gcc make libssl-dev libcurl4-openssl-dev libexpat-dev tcl tk gettext git-email zlib1g-dev apache2 language-pack-is git-svn gcc-8 && + ci/install-dependencies.sh + - name: ci/run-build-and-tests.sh + run: ci/run-build-and-tests.sh + - name: ci/print-test-failures.sh + if: failure() + run: | + ci/print-test-failures.sh + exit 1 + osx-clang: + runs-on: macos-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + env: + CC: clang + run: ci/install-dependencies.sh + - name: ci/run-build-and-test.sh + env: + CC: clang + run: ci/run-build-and-tests.sh + - name: ci/print-test-failures.sh + if: failure() + env: + CC: clang + run: | + ci/print-test-failures.sh + exit 1 + osx-gcc: + runs-on: macos-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + run: ci/install-dependencies.sh + - name: ci/run-build-and-tests.sh + run: ci/run-build-and-tests.sh + - name: ci/print-test-failures.sh + if: failure() + run: | + ci/print-test-failures.sh + exit 1 + GETTEXT_POISON: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + run: | + sudo apt-get update && + sudo apt-get -y install git gcc make libssl-dev libcurl4-openssl-dev libexpat-dev tcl tk gettext git-email zlib1g-dev + - name: ci/run-build-and-tests.sh + env: + jobname: GETTEXT_POISON + run: ci/run-build-and-tests.sh + - name: ci/print-test-failures.sh + if: failure() + env: + jobname: GETTEXT_POISON + run: | + ci/print-test-failures.sh + exit 1 + linux-musl: + runs-on: ubuntu-latest + container: alpine + steps: + - uses: actions/checkout@v1 + - name: install additional dependencies + run: apk add --update ncurses perl-utils + - name: prepare /usr/src/git + run: | + mkdir -p /usr/src && + rm -rf /usr/src/git && + ln -sf $(pwd) /usr/src/git + - name: ci/run-alpine-build.sh + run: ci/run-alpine-build.sh $(id -u) + - name: ci/print-test-failures.sh + if: failure() + run: ci/print-test-failures.sh + linux32: + runs-on: ubuntu-latest + container: daald/ubuntu32:xenial + steps: + - uses: actions/checkout@v1 + - name: prepare /usr/src/git + run: + mkdir -p /usr/src && + rm -rf /usr/src/git && + ln -sf $(pwd) /usr/src/git + - name: ci/run-linux32-build.sh + run: ci/run-linux32-build.sh $(id -u) + - name: ci/print-test-failures.sh + if: failure() + run: ci/print-test-failures.sh + static-analysis: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + run: | + sudo apt-get update && + sudo apt-get install -y coccinelle libcurl4-openssl-dev libssl-dev libexpat-dev gettext + - name: ci/run-static-analysis.sh + env: + jobname: StaticAnalysis + run: ci/run-static-analysis.sh + documentation: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + run: | + sudo apt-get update && + sudo apt-get install -y asciidoc xmlto asciidoctor docbook-xsl-ns libcurl4-openssl-dev + - name: ci/test-documentation.sh + env: + ALREADY_HAVE_ASCIIDOCTOR: yes. + jobname: Documentation + run: ci/test-documentation.sh
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v2 6/6 ci: retire the Azure Pipelines definition
In reply to Đoàn Trần Công DanhFrom: Johannes Schindelin <[email protected]>
We have GitHub Actions now. Running the same builds and tests in Azure Pipelines would be redundant, and a waste of energy.
--- README.md | 1 - azure-pipelines.yml | 593 -------------------------------------------- 2 files changed, 594 deletions(-) delete mode 100644 azure-pipelines.yml
README.md +0 -1
@@ -1,5 +1,4 @@ [](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush) -[](https://dev.azure.com/git/git/_build/latest?definitionId=11) Git - fast, scalable, distributed revision control system =========================================================
azure-pipelines.yml deleted +0 -593
593 lines not on this page · raw message
Signature · 1 line
2.26.0.334.g6536db25bb
Quoted 16 lines · Đoàn Trần Công Danh
This series is based on work started by Dscho,
I started to work with this series since there're merge conflicts with my series at dd/ci-musl-lib, and Dscho said he was busy for a while.Changes frome Dscho's version:
* Based on dd/ci-musl-libc
* Move artifact extraction out of Test phase of windows-test
* Moveci/print-test-failures.shout of build-and-test phase
* set TERM environment variable if not exist
* add linux-musl job
* v1 doesn't report failure on linux32, fixed
* run linux32 directly inside container
* install development file of curl in documentation job because "curl-config" will be called in pu'sMakefile
+ Other approach could be call make CURL_CONFIG=: intest-documentation.sh
Quoted 5 lines
Sample run:
* of this series: https://github.com/sgn/git/actions/runs/68291472
* pretend compile failure: https://github.com/sgn/git/actions/runs/68292112
* pretend test failure: https://github.com/sgn/git/actions/runs/68293056
* merged to pu: https://github.com/sgn/git/actions/runs/68301122
When I look at
https://github.com/git/git/actions
it appears that every time I push[*1*], we are running _two_ sets of the same tests, one says "Pu nu bu?" and the other says "CI/PR".
In addition, the former refers to "Pull request #nnn synchronize by gitster", with #nnn part made clickable, but clicking on it results in 404 page, as there is no such pull request.
Does this series need some help from me (i.e. changing my workflow) to make the CI tests run more smoothly?
Thanks.
[Footnote]
*1* I always push to github.com/git/git and github.com/gitster/git almost at the same time---the latter has all the topics that gets merged to 'pu' separated out. But the latter push does not say anything about anybody asking to anybody else to pull anything.
Quoted 31 lines · Junio C Hamano
Quoted 16 lines · Đoàn Trần Công Danh
This series is based on work started by Dscho,
I started to work with this series since there're merge conflicts with my series at dd/ci-musl-lib, and Dscho said he was busy for a while.Changes frome Dscho's version:
* Based on dd/ci-musl-libc
* Move artifact extraction out of Test phase of windows-test
* Moveci/print-test-failures.shout of build-and-test phase
* set TERM environment variable if not exist
* add linux-musl job
* v1 doesn't report failure on linux32, fixed
* run linux32 directly inside container
* install development file of curl in documentation job because "curl-config" will be called in pu'sMakefile
+ Other approach could be call make CURL_CONFIG=: intest-documentation.shQuoted 5 lines
Sample run:
* of this series: https://github.com/sgn/git/actions/runs/68291472
* pretend compile failure: https://github.com/sgn/git/actions/runs/68292112
* pretend test failure: https://github.com/sgn/git/actions/runs/68293056
* merged to pu: https://github.com/sgn/git/actions/runs/68301122When I look at
https://github.com/git/git/actions
it appears that every time I push[*1*], we are running _two_ sets of the same tests, one says "Pu nu bu?" and the other says "CI/PR".
Quoted 6 lines
In addition, the former refers to "Pull request #nnn synchronize by gitster", with #nnn part made clickable, but clicking on it results in 404 page, as there is no such pull request.
Does this series need some help from me (i.e. changing my workflow) to make the CI tests run more smoothly?
I suspect there is a github Pull Request #738 (merging pu to master) in private mode because it's what reported by Github actions.
It seems like the last 2 pushes to pu doesn't trigger it anymore?
If the problem still persists,
I think either you or Dscho can login to github and see what is there.
Signature · 1 line
Danh
Đoàn Trần Công Danh
Appliedv3 cover ci: replace our Azure Pipeline by GitHub Actions
In reply to Đoàn Trần Công DanhThis series is based on work started by Dscho,
I started to work with this series since there're merge conflicts with my series at dd/ci-musl-lib, and Dscho said he was busy for a while.
Changes from v3:
* Base on v3 of dd/ci-musl-libc (submited)
* linux32 and linux-musl jobs' structure is the same with linux-gcc and linux-clang, they will be run inside container instead of vm:
+ they will install dependencies in separate step by common script
+ they will run ci/run-build-and-tests
* StaticAnalysis job will use ci/install-dependencies to install
Changes in v2 frome Dscho's version:
* Based on dd/ci-musl-libc
* Move artifact extraction out of Test phase of windows-test
* Move ci/print-test-failures.sh out of build-and-test phase
* set TERM environment variable if not exist
* add linux-musl job
* v1 doesn't report failure on linux32, fixed
* run linux32 directly inside container
* install development file of curl in documentation job because "curl-config" will be called in pu's Makefile
+ Other approach could be call make CURL_CONFIG=: in test-documentation.sh
Sample run without busybox fix:
https://github.com/sgn/git/actions/runs/69030441
Merged into old pu:
https://github.com/sgn/git/actions/runs/69048780
Johannes Schindelin (4):
ci/lib: if CI type is unknown, show the environment variables ci/lib: allow running in GitHub Actions
README: add a build badge for the GitHub Actions runs
ci: retire the Azure Pipelines definition
Đoàn Trần Công Danh (2):
ci/lib: set TERM environment variable if not exist
ci: configure GitHub Actions for CI/PR
.github/workflows/main.yml | 314 ++++++++++++++++++++ README.md | 2 +- azure-pipelines.yml | 593 ------------------------------------- ci/lib.sh | 24 +- 4 files changed, 338 insertions(+), 595 deletions(-) create mode 100644 .github/workflows/main.yml delete mode 100644 azure-pipelines.yml
Range-diff against v2:
1: 4cc17cfe43 = 1: 3f9f1c6335 ci/lib: if CI type is unknown, show the environment variables
2: e7e52d5d4c = 2: 7a4f646bc1 ci/lib: allow running in GitHub Actions
3: 043f6be3ee = 3: 9a03c0844c ci/lib: set TERM environment variable if not exist
4: 834b0f73b0 ! 4: 7308199e24 ci: configure GitHub Actions for CI/PR
@@ .github/workflows/main.yml (new)
+ container: alpine
+ steps:
+ - uses: actions/checkout@v1
-+ - name: install additional dependencies
-+ run: apk add --update ncurses perl-utils
-+ - name: prepare /usr/src/git
-+ run: |
-+ mkdir -p /usr/src &&
-+ rm -rf /usr/src/git &&
-+ ln -sf $(pwd) /usr/src/git
-+ - name: ci/run-alpine-build.sh
-+ run: ci/run-alpine-build.sh $(id -u)
++ - name: install dependencies
++ env:
++ jobname: linux-musl
++ run: ci/install-docker-dependencies.sh
++ - name: ci/run-build-and-tests.sh
++ env:
++ jobname: linux-musl
++ run: ci/run-build-and-tests.sh
+ - name: ci/print-test-failures.sh
+ if: failure()
+ run: ci/print-test-failures.sh
@@ .github/workflows/main.yml (new)
+ container: daald/ubuntu32:xenial
+ steps:
+ - uses: actions/checkout@v1
-+ - name: prepare /usr/src/git
-+ run:
-+ mkdir -p /usr/src &&
-+ rm -rf /usr/src/git &&
-+ ln -sf $(pwd) /usr/src/git
-+ - name: ci/run-linux32-build.sh
-+ run: ci/run-linux32-build.sh $(id -u)
++ - name: install dependencies
++ env:
++ jobname: Linux32
++ run: ci/install-docker-dependencies.sh
++ - name: ci/run-build-and-tests.sh
++ env:
++ jobname: Linux32
++ run: ci/run-build-and-tests.sh
+ - name: ci/print-test-failures.sh
+ if: failure()
+ run: ci/print-test-failures.sh
@@ .github/workflows/main.yml (new)
+ steps:
+ - uses: actions/checkout@v1
+ - name: install dependencies
-+ run: |
-+ sudo apt-get update &&
-+ sudo apt-get install -y coccinelle libcurl4-openssl-dev libssl-dev libexpat-dev gettext
++ env:
++ jobname: StaticAnalysis
++ run: ci/install-dependencies.sh
+ - name: ci/run-static-analysis.sh
+ env:
+ jobname: StaticAnalysis
5: 0d14645c32 = 5: 365ba5e831 README: add a build badge for the GitHub Actions runs
6: 87abc123b0 = 6: 53094612d3 ci: retire the Azure Pipelines definitionSignature · 1 line
2.26.0.334.g6536db25bb
PATCH v3 1/6 ci/lib: if CI type is unknown, show the environment variables
From: Johannes Schindelin <[email protected]>
This should help with adding new CI-specific if-else arms.
--- ci/lib.sh | 1 + 1 file changed, 1 insertion(+)
ci/lib.sh +1 -0
@@ -138,6 +138,7 @@ then GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS" else echo "Could not identify CI type" >&2 + env >&2 exit 1 fi
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v3 3/6 ci/lib: set TERM environment variable if not exist
In reply to Đoàn Trần Công DanhGitHub Action doesn't set TERM environment variable, which is required by "tput".
Fallback to dumb if it's not set.
--- ci/lib.sh | 3 +++ 1 file changed, 3 insertions(+)
ci/lib.sh +3 -0
@@ -79,6 +79,9 @@ check_unignored_build_artifacts () } } +# GitHub Action doesn't set TERM, which is required by tput +export TERM=${TERM:-dumb} + # Clear MAKEFLAGS that may come from the outside world. export MAKEFLAGS=
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v3 2/6 ci/lib: allow running in GitHub Actions
In reply to Đoàn Trần Công DanhFrom: Johannes Schindelin <[email protected]>
For each CI system we support, we need a specific arm in that if/else construct in ci/lib.sh. Let's add one for GitHub Actions.
--- ci/lib.sh | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)
ci/lib.sh +19 -1
@@ -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 + if test "$TRAVIS_DEBUG_MODE" = true || test true = "$GITHUB_ACTIONS" then return fi
@@ -136,6 +136,24 @@ then MAKEFLAGS="$MAKEFLAGS --jobs=10" test windows_nt != "$CI_OS_NAME" || GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS" +elif test true = "$GITHUB_ACTIONS" +then + CI_TYPE=github-actions + CI_BRANCH="$GITHUB_REF" + CI_COMMIT="$GITHUB_SHA" + CI_OS_NAME="$(echo "$RUNNER_OS" | tr A-Z a-z)" + test macos != "$CI_OS_NAME" || CI_OS_NAME=osx + CI_REPO_SLUG="$GITHUB_REPOSITORY" + CI_JOB_ID="$GITHUB_RUN_ID" + CC="${CC:-gcc}" + + cache_dir="$HOME/none" + + export GIT_PROVE_OPTS="--timer --jobs 10" + export GIT_TEST_OPTS="--verbose-log -x" + MAKEFLAGS="$MAKEFLAGS --jobs=10" + test windows != "$CI_OS_NAME" || + GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS" else echo "Could not identify CI type" >&2 env >&2
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v3 4/6 ci: configure GitHub Actions for CI/PR
In reply to Đoàn Trần Công DanhThis patch adds CI builds via GitHub Actions. While the underlying technology is at least _very_ similar to that of Azure Pipelines, GitHub Actions are much easier to set up than Azure Pipelines:
- no need to install a GitHub App,
- no need to set up an Azure DevOps account,
- all you need to do is push to your fork on GitHub.
Therefore, it makes a lot of sense for us to have a working GitHub Actions setup.
While translate-and-modify azure-pipelines.yml into .github/workflows/main.yml, we also use the opportunity to accelerate the step that sets up a minimal subset of Git for Windows' SDK in the Windows-build job:
- we now download a .tar.xz stored in Azure Blobs and extract it simultaneously (by calling curl and piping the result to tar,
- decompressing via xz,
- all three utilities are installed together with Git for Windows
Also, we do away with the parts that try to mount a file share on which prove can store data between runs. It is just too complicated to set up, so it's little return on investment there.
--- .github/workflows/main.yml | 314 +++++++++++++++++++++++++++++++++++++ 1 file changed, 314 insertions(+) create mode 100644 .github/workflows/main.yml
.github/workflows/main.yml new file +314 -0
@@ -0,0 +1,314 @@ +name: CI/PR + +on: [push, pull_request] + +jobs: + windows-build: + runs-on: windows-latest + steps: + - uses: actions/checkout@v1 + - name: Download git-sdk-64-minimal + shell: bash + run: a=git-sdk-64-minimal && mkdir -p $a && curl -# https://wingit.blob.core.windows.net/ci-artifacts/$a.tar.xz | tar -C $a -xJf - + - name: Build + shell: powershell + env: + HOME: ${{runner.workspace}} + MSYSTEM: MINGW64 + DEVELOPER: 1 + NO_PERL: 1 + run: | + & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @" + printf '%s\n' /git-sdk-64-minimal/ >>.git/info/exclude + + ci/make-test-artifacts.sh artifacts + "@ + if (!$?) { exit(1) } + - name: Upload build artifacts + uses: actions/upload-artifact@v1 + with: + name: windows-artifacts + path: artifacts + windows-test: + runs-on: windows-latest + needs: [windows-build] + strategy: + matrix: + nr: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9] + steps: + - uses: actions/checkout@v1 + - name: Download git-sdk-64-minimal + shell: bash + run: a=git-sdk-64-minimal && mkdir -p $a && curl -# https://wingit.blob.core.windows.net/ci-artifacts/$a.tar.xz | tar -C $a -xJf - + - name: Download build artifacts + uses: actions/download-artifact@v1 + with: + name: windows-artifacts + path: ${{github.workspace}} + - name: extract artifact + shell: powershell + run: | + & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @" + test -f artifacts.tar.gz || { + echo No test artifacts found\; skipping >&2 + exit 0 + } + tar xf artifacts.tar.gz + "@ + if (!$?) { exit(1) } + - name: Test + shell: powershell + run: | + & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @" + # Let Git ignore the SDK + printf '%s\n' /git-sdk-64-minimal/ >>.git/info/exclude + + ci/run-test-slice.sh ${{matrix.nr}} 10 + "@ + if (!$?) { exit(1) } + - name: ci/print-test-failures.sh + if: failure() + shell: powershell + run: | + & .\git-sdk-64-minimal\usr\bin\bash.exe -lc ci/print-test-failures.sh + exit(1) + vs-build: + runs-on: windows-latest + steps: + - uses: actions/checkout@v1 + - name: Download git-sdk-64-minimal + shell: bash + run: a=git-sdk-64-minimal && mkdir -p $a && curl -# https://wingit.blob.core.windows.net/ci-artifacts/$a.tar.xz | tar -C $a -xJf - + - name: Generate Visual Studio Solution + shell: powershell + env: + MSYSTEM: MINGW64 + DEVELOPER: 1 + NO_PERL: 1 + GIT_CONFIG_PARAMETERS: "'user.name=CI' 'user.email=ci@git'" + run: | + & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @" + make NDEBUG=1 DEVELOPER=1 vcxproj + "@ + if (!$?) { exit(1) } + - name: Download vcpkg artifacts + shell: powershell + run: | + $urlbase = "https://dev.azure.com/git/git/_apis/build/builds" + $id = ((Invoke-WebRequest -UseBasicParsing "${urlbase}?definitions=9&statusFilter=completed&resultFilter=succeeded&`$top=1").content | ConvertFrom-JSON).value[0].id + $downloadUrl = ((Invoke-WebRequest -UseBasicParsing "${urlbase}/$id/artifacts").content | ConvertFrom-JSON).value[0].resource.downloadUrl + (New-Object Net.WebClient).DownloadFile($downloadUrl, "compat.zip") + Expand-Archive compat.zip -DestinationPath . -Force + Remove-Item compat.zip + - name: Add msbuild to PATH + uses: microsoft/[email protected] + - name: MSBuild + run: msbuild git.sln -property:Configuration=Release -property:Platform=x64 -maxCpuCount:4 -property:PlatformToolset=v142 + - name: Bundle artifact tar + shell: powershell + env: + MSYSTEM: MINGW64 + DEVELOPER: 1 + NO_PERL: 1 + MSVC: 1 + VCPKG_ROOT: ${{github.workspace}}\compat\vcbuild\vcpkg + run: | + & compat\vcbuild\vcpkg_copy_dlls.bat release + if (!$?) { exit(1) } + & git-sdk-64-minimal\usr\bin\bash.exe -lc @" + mkdir -p artifacts && + eval \"`$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts 2>&1 | grep ^tar)\" + "@ + if (!$?) { exit(1) } + - name: Upload build artifacts + uses: actions/upload-artifact@v1 + with: + name: vs-artifacts + path: artifacts + vs-test: + runs-on: windows-latest + needs: [vs-build] + strategy: + matrix: + nr: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9] + steps: + - uses: actions/checkout@v1 + - name: Download git-64-portable + shell: bash + run: a=git-64-portable && mkdir -p $a && curl -# https://wingit.blob.core.windows.net/ci-artifacts/$a.tar.xz | tar -C $a -xJf - + - name: Download build artifacts + uses: actions/download-artifact@v1 + with: + name: vs-artifacts + path: ${{github.workspace}} + - name: Test (parallel) + shell: powershell + env: + MSYSTEM: MINGW64 + NO_SVN_TESTS: 1 + GIT_TEST_SKIP_REBASE_P: 1 + run: | + & git-64-portable\git-cmd.exe --command=usr\bin\bash.exe -lc @" + test -f artifacts.tar.gz || { + echo No test artifacts found\; skipping >&2 + exit 0 + } + tar xf artifacts.tar.gz || exit 1 + + # Let Git ignore the SDK and the test-cache + printf '%s\n' /git-64-portable/ /test-cache/ >>.git/info/exclude + + cd t && + PATH=\"`$PWD/helper:`$PATH\" && + test-tool.exe run-command testsuite --jobs=10 -V -x --write-junit-xml \ + `$(test-tool.exe path-utils slice-tests \ + ${{matrix.nr}} 10 t[0-9]*.sh) + "@ + if (!$?) { exit(1) } + linux-clang: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + env: + CC: clang + run: | + sudo apt-get update && + sudo apt-get -y install git gcc make libssl-dev libcurl4-openssl-dev libexpat-dev tcl tk gettext git-email zlib1g-dev apache2-bin && + ci/install-dependencies.sh + - name: ci/run-build-and-test.sh + env: + CC: clang + run: ci/run-build-and-tests.sh + - name: ci/print-test-failures.sh + if: failure() + env: + CC: clang + run: | + ci/print-test-failures.sh + exit 1 + linux-gcc: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + run: | + sudo add-apt-repository ppa:ubuntu-toolchain-r/test && + sudo apt-get update && + sudo apt-get -y install git gcc make libssl-dev libcurl4-openssl-dev libexpat-dev tcl tk gettext git-email zlib1g-dev apache2 language-pack-is git-svn gcc-8 && + ci/install-dependencies.sh + - name: ci/run-build-and-tests.sh + run: ci/run-build-and-tests.sh + - name: ci/print-test-failures.sh + if: failure() + run: | + ci/print-test-failures.sh + exit 1 + osx-clang: + runs-on: macos-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + env: + CC: clang + run: ci/install-dependencies.sh + - name: ci/run-build-and-test.sh + env: + CC: clang + run: ci/run-build-and-tests.sh + - name: ci/print-test-failures.sh + if: failure() + env: + CC: clang + run: | + ci/print-test-failures.sh + exit 1 + osx-gcc: + runs-on: macos-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + run: ci/install-dependencies.sh + - name: ci/run-build-and-tests.sh + run: ci/run-build-and-tests.sh + - name: ci/print-test-failures.sh + if: failure() + run: | + ci/print-test-failures.sh + exit 1 + GETTEXT_POISON: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + run: | + sudo apt-get update && + sudo apt-get -y install git gcc make libssl-dev libcurl4-openssl-dev libexpat-dev tcl tk gettext git-email zlib1g-dev + - name: ci/run-build-and-tests.sh + env: + jobname: GETTEXT_POISON + run: ci/run-build-and-tests.sh + - name: ci/print-test-failures.sh + if: failure() + env: + jobname: GETTEXT_POISON + run: | + ci/print-test-failures.sh + exit 1 + linux-musl: + runs-on: ubuntu-latest + container: alpine + steps: + - uses: actions/checkout@v1 + - name: install dependencies + env: + jobname: linux-musl + run: ci/install-docker-dependencies.sh + - name: ci/run-build-and-tests.sh + env: + jobname: linux-musl + run: ci/run-build-and-tests.sh + - name: ci/print-test-failures.sh + if: failure() + run: ci/print-test-failures.sh + linux32: + runs-on: ubuntu-latest + container: daald/ubuntu32:xenial + steps: + - uses: actions/checkout@v1 + - name: install dependencies + env: + jobname: Linux32 + run: ci/install-docker-dependencies.sh + - name: ci/run-build-and-tests.sh + env: + jobname: Linux32 + run: ci/run-build-and-tests.sh + - name: ci/print-test-failures.sh + if: failure() + run: ci/print-test-failures.sh + static-analysis: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + env: + jobname: StaticAnalysis + run: ci/install-dependencies.sh + - name: ci/run-static-analysis.sh + env: + jobname: StaticAnalysis + run: ci/run-static-analysis.sh + documentation: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + run: | + sudo apt-get update && + sudo apt-get install -y asciidoc xmlto asciidoctor docbook-xsl-ns libcurl4-openssl-dev + - name: ci/test-documentation.sh + env: + ALREADY_HAVE_ASCIIDOCTOR: yes. + jobname: Documentation + run: ci/test-documentation.sh
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v3 5/6 README: add a build badge for the GitHub Actions runs
In reply to Đoàn Trần Công DanhFrom: Johannes Schindelin <[email protected]>
--- README.md | 1 + 1 file changed, 1 insertion(+)
README.md +1 -0
@@ -1,3 +1,4 @@ +[](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush) [](https://dev.azure.com/git/git/_build/latest?definitionId=11) Git - fast, scalable, distributed revision control system
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v3 6/6 ci: retire the Azure Pipelines definition
In reply to Đoàn Trần Công DanhFrom: Johannes Schindelin <[email protected]>
We have GitHub Actions now. Running the same builds and tests in Azure Pipelines would be redundant, and a waste of energy.
--- README.md | 1 - azure-pipelines.yml | 593 -------------------------------------------- 2 files changed, 594 deletions(-) delete mode 100644 azure-pipelines.yml
README.md +0 -1
@@ -1,5 +1,4 @@ [](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush) -[](https://dev.azure.com/git/git/_build/latest?definitionId=11) Git - fast, scalable, distributed revision control system =========================================================
azure-pipelines.yml deleted +0 -593
593 lines not on this page · raw message
Signature · 1 line
2.26.0.334.g6536db25bb
Re: [PATCH 2/5] ci/lib: allow running in GitHub Actions
In reply to Johannes Schindelin via GitGitGadgetQuoted 24 lines · Johannes Schindelin via GitGitGadget
From: Johannes Schindelin <[email protected]>
For each CI system we support, we need a specific arm in that if/else construct in
ci/lib.sh. Let's add one for GitHub Actions.Signed-off-by Johannes Schindelin <[email protected]>--- ci/lib.sh | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)ci/lib.sh +1
-1@@ -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 + if test "$TRAVIS_DEBUG_MODE" = true || test true = "$GITHUB_ACTIONS" then return fi
ci/lib.sh then · line 145
Quoted 10 lines
ci/lib.sh +7
-0@@ -136,6 +136,24 @@ then MAKEFLAGS="$MAKEFLAGS --jobs=10" test windows_nt != "$CI_OS_NAME" || GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS" +elif test true = "$GITHUB_ACTIONS" +then + CI_TYPE=github-actions + CI_BRANCH="$GITHUB_REF" + CI_COMMIT="$GITHUB_SHA" + CI_OS_NAME="$(echo "$RUNNER_OS" | tr A-Z a-z)" + test macos != "$CI_OS_NAME" || CI_OS_NAME=osx
Hmm, if "macos" isn't not equal to $CI_OS_NAME, then set CI_OS_NAME=osx. This is head-scratchingly backwards, and I think
test "$CI_OS_NAME" = macos && CI_OS_NAME=osx
would read better.
ci/lib.sh then · line 156
Quoted 11 lines
ci/lib.sh +11
-0+ CI_REPO_SLUG="$GITHUB_REPOSITORY" + CI_JOB_ID="$GITHUB_RUN_ID" + CC="${CC:-gcc}" + + cache_dir="$HOME/none" + + export GIT_PROVE_OPTS="--timer --jobs 10" + export GIT_TEST_OPTS="--verbose-log -x" + MAKEFLAGS="$MAKEFLAGS --jobs=10" + test windows != "$CI_OS_NAME" || + GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS"
Likewise.
Quoted 6 lines
ci/lib.sh +0
-0else echo "Could not identify CI type" >&2 env >&2Signature · 1 line
gitgitgadget
Re: [PATCH 3/5] ci: configure GitHub Actions for CI/PR
In reply to Johannes Schindelin via GitGitGadgetQuoted 7 lines · Johannes Schindelin via GitGitGadget
From: Johannes Schindelin <[email protected]>
This patch adds CI builds via GitHub Actions. While the underlying technology is at least _very_ similar to that of Azure Pipelines, GitHub Actions are much easier to set up than Azure Pipelines: no need to install a GitHub App, no need to set up an Azure DevOps account, all you need to do is push to your fork on GitHub.
And then receive emails from GitHub if a build fails, even though you never asked for it? Well, I've always preferred opt-in instead of opt-out.
Btw, does it offer direct shell access for debugging?
Quoted 12 lines
Therefore, it makes a lot of sense for us to have a working GitHub Actions setup.
While transmogrifying
azure-pipelines.ymlinto.github/workflows/main.yml, we also use the opportunity to accelerate the step that sets up a minimal subset of Git for Windows' SDK in the Windows-build job: we now download a.tar.xzstored in Azure Blobs and extract it simultaneously (by callingcurland piping the result totar, decompressing viaxz, all three utilities being available by grace of using Git for Windows' Bash that is installed on the build agents). This accelerates that step from ~1m50s to ~7s.
Quoted 2 lines
Also, we do away with the parts that try to mount a file share on which
provecan store data between runs.
... and on which we store the list of already successfully tested trees, so when a branch is rebased changing e.g. only the commit messages but leaving the code intact, then the CI won't run unnecessary builds.
Quoted 6 lines
It is just too complicated to set
up, so it's little return on investment there.Signed-off-by Johannes Schindelin <[email protected]>---
.github/workflows/main.yml line 169
Quoted 10 lines
.github/workflows/main.yml new file +10
-0+ linux-clang: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + env: + CC: clang + run: | + sudo apt-get update && + sudo apt-get -y install git gcc make libssl-dev libcurl4-openssl-dev libexpat-dev tcl tk gettext git-email zlib1g-dev apache2-bin &&
Why install all these dependencies here, when the very next command is 'ci/install-dependencies.sh', whose purpose is to install the dependencies?
Quoted 8 lines
.github/workflows/main.yml new file +8
-0+ ci/install-dependencies.sh + - name: ci/run-build-and-test.sh + env: + CC: clang + run: | + ci/run-build-and-tests.sh || { + ci/print-test-failures.sh + exit 1
.github/workflows/main.yml line 187
Quoted 10 lines
.github/workflows/main.yml new file +10
-0+ } + linux-gcc: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + run: | + sudo add-apt-repository ppa:ubuntu-toolchain-r/test && + sudo apt-get update && + sudo apt-get -y install git gcc make libssl-dev libcurl4-openssl-dev libexpat-dev tcl tk gettext git-email zlib1g-dev apache2 language-pack-is git-svn gcc-8 &&
Likewise, and here you install some additional packages compared to the clang job as well.
Quoted 57 lines
.github/workflows/main.yml new file +57
-0+ ci/install-dependencies.sh + - name: ci/run-build-and-tests.sh + run: | + ci/run-build-and-tests.sh || { + ci/print-test-failures.sh + exit 1 + } + osx-clang: + runs-on: macos-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + env: + CC: clang + run: ci/install-dependencies.sh + - name: ci/run-build-and-tests.sh + env: + CC: clang + run: | + ci/run-build-and-tests.sh || { + ci/print-test-failures.sh + exit 1 + } + osx-gcc: + runs-on: macos-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + run: ci/install-dependencies.sh + - name: ci/run-build-and-tests.sh + run: | + ci/run-build-and-tests.sh || { + ci/print-test-failures.sh + exit 1 + } + GETTEXT_POISON: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + run: | + sudo apt-get update && + sudo apt-get -y install git gcc make libssl-dev libcurl4-openssl-dev libexpat-dev tcl tk gettext git-email zlib1g-dev + - name: ci/run-build-and-tests.sh + env: + jobname: GETTEXT_POISON + run: | + ci/run-build-and-tests.sh || { + ci/print-test-failures.sh + exit 1 + } + linux32: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: ci/run-linux32-docker.sh + run: |
.github/workflows/main.yml line 254
Quoted 10 lines
.github/workflows/main.yml new file +10
-0+ res=0 + sudo GITHUB_ACTIONS="$GITHUB_ACTIONS" RUNNER_OS="$RUNNER_OS" GITHUB_REF="$GITHUB_REF" GITHUB_SHA="$GITHUB_SHA" GITHUB_REPOSITORY="$GITHUB_REPOSITORY" GITHUB_RUN_ID="$GITHUB_RUN_ID" CC=$CC MAKEFLAGS="$MAKEFLAGS" bash -lxc ci/run-linux32-docker.sh || res=1 + static-analysis: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + run: | + sudo apt-get update && + sudo apt-get install -y coccinelle libcurl4-openssl-dev libssl-dev libexpat-dev gettext
Likewise, except here you don't even run 'ci/install-dependencies.sh' (which would install the same packages, btw.)
Quoted 20 lines
.github/workflows/main.yml new file +17
-0+ - name: ci/run-static-analysis.sh + env: + jobname: StaticAnalysis + run: ci/run-static-analysis.sh + documentation: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + run: | + sudo apt-get update && + sudo apt-get install -y asciidoc xmlto asciidoctor docbook-xsl-ns + - name: ci/test-documentation.sh + env: + ALREADY_HAVE_ASCIIDOCTOR: yes. + jobname: Documentation + run: ci/test-documentation.shSignature · 1 line
gitgitgadget
Re: [PATCH 3/5] ci: configure GitHub Actions for CI/PR
Hi Gábor,
Quoted 9 lines · SZEDER Gábor
Quoted 7 lines · Johannes Schindelin via GitGitGadget
From: Johannes Schindelin <[email protected]>
This patch adds CI builds via GitHub Actions. While the underlying technology is at least _very_ similar to that of Azure Pipelines, GitHub Actions are much easier to set up than Azure Pipelines: no need to install a GitHub App, no need to set up an Azure DevOps account, all you need to do is push to your fork on GitHub.
Quoted 3 lines
And then receive emails from GitHub if a build fails, even though you never asked for it? Well, I've always preferred opt-in instead of opt-out.
This complaint is misdirected.
You can easily disable it if you want to
(https://help.github.com/en/github/administering-a-repository/disabling-or-limiting-github-actions-for-a-repository)
and for the vast majority of Git contributors, the default to run this workflow is totally the best idea.
Quoted 1 line
Btw, does it offer direct shell access for debugging?
There is https://github.com/marketplace/actions/debugging-with-tmate
Quoted 12 lines
Quoted 12 lines
Therefore, it makes a lot of sense for us to have a working GitHub Actions setup.
While transmogrifying
azure-pipelines.ymlinto.github/workflows/main.yml, we also use the opportunity to accelerate the step that sets up a minimal subset of Git for Windows' SDK in the Windows-build job: we now download a.tar.xzstored in Azure Blobs and extract it simultaneously (by callingcurland piping the result totar, decompressing viaxz, all three utilities being available by grace of using Git for Windows' Bash that is installed on the build agents). This accelerates that step from ~1m50s to ~7s.
Quoted 7 lines
Quoted 2 lines
Also, we do away with the parts that try to mount a file share on which
provecan store data between runs.... and on which we store the list of already successfully tested trees, so when a branch is rebased changing e.g. only the commit messages but leaving the code intact, then the CI won't run unnecessary builds.
In practice, I saw this Pipeline being run time and time again, as the tree _did_ change most of the time.
A pipe dream of mine is to use actual Test Impact Analysis so that we truly can run only the affected tests, but that seems so, so far away.
Quoted 17 lines
Quoted 6 lines
It is just too complicated to set
up, so it's little return on investment there.Signed-off-by Johannes Schindelin <[email protected]>---
.github/workflows/main.yml line 169
Quoted 10 lines
.github/workflows/main.yml new file +10
-0+ linux-clang: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + env: + CC: clang + run: | + sudo apt-get update && + sudo apt-get -y install git gcc make libssl-dev libcurl4-openssl-dev libexpat-dev tcl tk gettext git-email zlib1g-dev apache2-bin &&
Quoted 3 lines
Why install all these dependencies here, when the very next command is '
ci/install-dependencies.sh', whose purpose is to install the dependencies?
I copy/pasted these from azure-pipelines.yml, and agree that they should be handled via ci/install-dependencies.sh instead.
Danh (is this the correct way to address you?), if you want to have a look at that, go for it. Otherwise I'll try to find some time to do that myself.
Thanks,
Dscho
Quoted 114 lines
Quoted 8 lines
.github/workflows/main.yml new file +8
-0+ ci/install-dependencies.sh + - name: ci/run-build-and-test.sh + env: + CC: clang + run: | + ci/run-build-and-tests.sh || { + ci/print-test-failures.sh + exit 1.github/workflows/main.yml line 187
Quoted 10 lines
.github/workflows/main.yml new file +10
-0+ } + linux-gcc: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + run: | + sudo add-apt-repository ppa:ubuntu-toolchain-r/test && + sudo apt-get update && + sudo apt-get -y install git gcc make libssl-dev libcurl4-openssl-dev libexpat-dev tcl tk gettext git-email zlib1g-dev apache2 language-pack-is git-svn gcc-8 &&Likewise, and here you install some additional packages compared to the clang job as well.
Quoted 57 lines
.github/workflows/main.yml new file +57
-0+ ci/install-dependencies.sh + - name: ci/run-build-and-tests.sh + run: | + ci/run-build-and-tests.sh || { + ci/print-test-failures.sh + exit 1 + } + osx-clang: + runs-on: macos-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + env: + CC: clang + run: ci/install-dependencies.sh + - name: ci/run-build-and-tests.sh + env: + CC: clang + run: | + ci/run-build-and-tests.sh || { + ci/print-test-failures.sh + exit 1 + } + osx-gcc: + runs-on: macos-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + run: ci/install-dependencies.sh + - name: ci/run-build-and-tests.sh + run: | + ci/run-build-and-tests.sh || { + ci/print-test-failures.sh + exit 1 + } + GETTEXT_POISON: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + run: | + sudo apt-get update && + sudo apt-get -y install git gcc make libssl-dev libcurl4-openssl-dev libexpat-dev tcl tk gettext git-email zlib1g-dev + - name: ci/run-build-and-tests.sh + env: + jobname: GETTEXT_POISON + run: | + ci/run-build-and-tests.sh || { + ci/print-test-failures.sh + exit 1 + } + linux32: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: ci/run-linux32-docker.sh + run: |.github/workflows/main.yml line 254
Quoted 10 lines
.github/workflows/main.yml new file +10
-0+ res=0 + sudo GITHUB_ACTIONS="$GITHUB_ACTIONS" RUNNER_OS="$RUNNER_OS" GITHUB_REF="$GITHUB_REF" GITHUB_SHA="$GITHUB_SHA" GITHUB_REPOSITORY="$GITHUB_REPOSITORY" GITHUB_RUN_ID="$GITHUB_RUN_ID" CC=$CC MAKEFLAGS="$MAKEFLAGS" bash -lxc ci/run-linux32-docker.sh || res=1 + static-analysis: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + run: | + sudo apt-get update && + sudo apt-get install -y coccinelle libcurl4-openssl-dev libssl-dev libexpat-dev gettextLikewise, except here you don't even run '
ci/install-dependencies.sh' (which would install the same packages, btw.)Quoted 20 lines
.github/workflows/main.yml new file +17
-1+ - name: ci/run-static-analysis.sh + env: + jobname: StaticAnalysis + run: ci/run-static-analysis.sh + documentation: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + run: | + sudo apt-get update && + sudo apt-get install -y asciidoc xmlto asciidoctor docbook-xsl-ns + - name: ci/test-documentation.sh + env: + ALREADY_HAVE_ASCIIDOCTOR: yes. + jobname: Documentation + run: ci/test-documentation.sh --gitgitgadget
Re: [PATCH 2/5] ci/lib: allow running in GitHub Actions
In reply to SZEDER GáborHi Gábor,
Quoted 39 lines · SZEDER Gábor
Quoted 24 lines · Johannes Schindelin via GitGitGadget
From: Johannes Schindelin <[email protected]>
For each CI system we support, we need a specific arm in that if/else construct in
ci/lib.sh. Let's add one for GitHub Actions.Signed-off-by Johannes Schindelin <[email protected]>--- ci/lib.sh | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)ci/lib.sh +1
-1@@ -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 + if test "$TRAVIS_DEBUG_MODE" = true || test true = "$GITHUB_ACTIONS" then return fici/lib.sh then · line 145
Quoted 10 lines
ci/lib.sh +7
-0@@ -136,6 +136,24 @@ then MAKEFLAGS="$MAKEFLAGS --jobs=10" test windows_nt != "$CI_OS_NAME" || GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS" +elif test true = "$GITHUB_ACTIONS" +then + CI_TYPE=github-actions + CI_BRANCH="$GITHUB_REF" + CI_COMMIT="$GITHUB_SHA" + CI_OS_NAME="$(echo "$RUNNER_OS" | tr A-Z a-z)" + test macos != "$CI_OS_NAME" || CI_OS_NAME=osxHmm, if "macos" isn't not equal to $CI_OS_NAME, then set CI_OS_NAME=osx. This is head-scratchingly backwards, and I think
Quoted 3 lines
test "$CI_OS_NAME" = macos && CI_OS_NAME=osxwould read better.
I can understand where you come from, but your code is not set -e safe, which is the reason why I wrote the code this way (compare to the already existing code in the previous clause, which was copy-edited here).
Ciao,
Dscho
Quoted 22 lines
ci/lib.sh then · line 156
Quoted 12 lines
ci/lib.sh +11
-0+ CI_REPO_SLUG="$GITHUB_REPOSITORY" + CI_JOB_ID="$GITHUB_RUN_ID" + CC="${CC:-gcc}" + + cache_dir="$HOME/none" + + export GIT_PROVE_OPTS="--timer --jobs 10" + export GIT_TEST_OPTS="--verbose-log -x" + MAKEFLAGS="$MAKEFLAGS --jobs=10" + test windows != "$CI_OS_NAME" || + GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS"Likewise.
Quoted 6 lines
ci/lib.sh +0
-1else echo "Could not identify CI type" >&2 env >&2 --gitgitgadget
Re: [PATCH 3/5] ci: configure GitHub Actions for CI/PR
In reply to Johannes SchindelinQuoted 15 lines · Johannes Schindelin
Quoted 11 lines
.github/workflows/main.yml line 169
Quoted 10 lines
.github/workflows/main.yml new file +10
-0+ linux-clang: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: install dependencies + env: + CC: clang + run: | + sudo apt-get update && + sudo apt-get -y install git gcc make libssl-dev libcurl4-openssl-dev libexpat-dev tcl tk gettext git-email zlib1g-dev apache2-bin &&Quoted 3 lines
Why install all these dependencies here, when the very next command is '
ci/install-dependencies.sh', whose purpose is to install the dependencies?
Quoted 6 lines
I copy/pasted these from
azure-pipelines.yml, and agree that they should be handled viaci/install-dependencies.shinstead.Danh (is this the correct way to address you?), if you want to have a look at that, go for it. Otherwise I'll try to find some time to do that myself.
I've managed to fix this part in the v4 series (only send privately to Dscho, because I'm waiting for other conversations to settle).
Signature · 1 line
Danh
Re: [PATCH 2/5] ci/lib: allow running in GitHub Actions
In reply to Johannes SchindelinQuoted 13 lines · Johannes Schindelin
Hi Gábor,
Quoted 9 lines · SZEDER Gábor
ci/lib.sh then · line 145
Quoted 5 lines
ci/lib.sh +5
-0+ CI_TYPE=github-actions + CI_BRANCH="$GITHUB_REF" + CI_COMMIT="$GITHUB_SHA" + CI_OS_NAME="$(echo "$RUNNER_OS" | tr A-Z a-z)" + test macos != "$CI_OS_NAME" || CI_OS_NAME=osxHmm, if "macos" isn't not equal to $CI_OS_NAME, then set CI_OS_NAME=osx. This is head-scratchingly backwards, and I think
Quoted 7 lines
Quoted 3 lines
test "$CI_OS_NAME" = macos && CI_OS_NAME=osxwould read better.
I can understand where you come from, but your code is not
set -esafe, which is the reason why I wrote the code this way (compare to the already existing code in the previous clause, which was copy-edited here).
I certainly saw a shell that broke on
set -e
test false && ..
I couldn't recall it, though.
Would it be OK if we change it this way:
if "$CI_OS_NAME" = macos; then CI_OS_NAME=osx; fi
Signature · 1 line
Danh
Re: [PATCH 2/5] ci/lib: allow running in GitHub Actions
Hi Danh,
Quoted 24 lines · Danh Doan
Quoted 13 lines · Johannes Schindelin
Hi Gábor,
Quoted 9 lines · SZEDER Gábor
ci/lib.sh then · line 145
Quoted 5 lines
ci/lib.sh +5
-0+ CI_TYPE=github-actions + CI_BRANCH="$GITHUB_REF" + CI_COMMIT="$GITHUB_SHA" + CI_OS_NAME="$(echo "$RUNNER_OS" | tr A-Z a-z)" + test macos != "$CI_OS_NAME" || CI_OS_NAME=osxHmm, if "macos" isn't not equal to $CI_OS_NAME, then set CI_OS_NAME=osx. This is head-scratchingly backwards, and I think
Quoted 7 lines
Quoted 3 lines
test "$CI_OS_NAME" = macos && CI_OS_NAME=osxwould read better.
I can understand where you come from, but your code is not
set -esafe, which is the reason why I wrote the code this way (compare to the already existing code in the previous clause, which was copy-edited here).I certainly saw a shell that broke on
Quoted 8 lines
set -e
test false && ..I couldn't recall it, though.
Would it be OK if we change it this way:
if "$CI_OS_NAME" = macos; then CI_OS_NAME=osx; fi
We would have to fix quite a bit of code if we wanted to avoid the <opposite> || <action> pattern.
If we _would_ need to (which I don't think we do), I'd prefer:
case "$CI_OS_NAME" in macos) CI_OS_NAME=osx;; esac
But again, we would have to clean up quite a few instances of this, I am sure, and it might not be worth the effort, given the short-and-sweet nature of the originally-proposed solution.
Ciao,
Dscho
v4 cover ci: replace our Azure Pipeline by GitHub Actions
In reply to Johannes Schindelin via GitGitGadgetOur Azure Pipeline has served us well over the course of the past year or so, steadily catching issues before the respective patches hit the next branch.
There is a GitHub-native CI system now, though, called "GitHub Actions"
[https://github.com/features/actions] which is essentially on par with Azure Pipelines as far as our needs are concerned, and it brings a couple of advantages:
* It is substantially easier to set up than Azure Pipelines: all you need is to add the YAML-based build definition, push to your fork on GitHub, and that's it.
* The syntax is a bit easier to read than Azure Pipelines'.
* We get more concurrent jobs (Azure Pipelines is limited to 10 concurrent jobs).
With this change, users also no longer need to open a PR at
https://github.com/git/git or at https://github.com/gitgitgadget/git just to get the benefit of a CI build.
Sample run on top of dd/ci-musl-libc with dd/test-with-busybox merged:
https://github.com/sgn/git/actions/runs/73179413
Sample run when this series applied into git-for-windows
https://github.com/git-for-windows/git/runs/568625109
Change from v3:
- Use build matrix
- All dependencies are install by scripts
- stop repeating environment variables
- build failure's artifacts will be uploaded
Johannes Schindelin (9):
ci/lib: if CI type is unknown, show the environment variables ci/lib: allow running in GitHub Actions
ci: fix the jobname of the GETTEXT_POISON job
ci: run gem with sudo to install asciidoctor
README: add a build badge for the GitHub Actions runs
ci: retire the Azure Pipelines definition
tests: when run in Bash, annotate test failures with file name/line
number
ci: add a problem matcher for GitHub Actions
ci: let GitHub Actions upload failed tests' directories
Đoàn Trần Công Danh (3):
ci/lib: set TERM environment variable if not exist
ci: explicit install all required packages
ci: configure GitHub Actions for CI/PR
.github/workflows/main.yml | 230 +++++++++++++++ .travis.yml | 2 +- README.md | 2 +- azure-pipelines.yml | 558 ------------------------------------ ci/git-problem-matcher.json | 16 ++ ci/install-dependencies.sh | 16 +- ci/lib.sh | 31 +- ci/print-test-failures.sh | 7 + t/test-lib.sh | 14 +- 9 files changed, 310 insertions(+), 566 deletions(-) create mode 100644 .github/workflows/main.yml delete mode 100644 azure-pipelines.yml create mode 100644 ci/git-problem-matcher.json
Range-diff against v3:
1: 3f9f1c6335 = 1: 2219bf3db9 ci/lib: if CI type is unknown, show the environment variables 2: 7a4f646bc1 = 2: 2818799a4b ci/lib: allow running in GitHub Actions 3: 9a03c0844c = 3: b88586c2c5 ci/lib: set TERM environment variable if not exist 4: 7308199e24 < -: ---------- ci: configure GitHub Actions for CI/PR -: ---------- > 4: 1df60e677c ci: fix the `jobname` of the `GETTEXT_POISON` job -: ---------- > 5: 4f80724641 ci: explicit install all required packages -: ---------- > 6: 795ec656c6 ci: run gem with sudo to install asciidoctor -: ---------- > 7: ec0aa20119 ci: configure GitHub Actions for CI/PR 5: 365ba5e831 = 8: 46f2b6bce6 README: add a build badge for the GitHub Actions runs 6: 53094612d3 ! 9: 92f2623dc7 ci: retire the Azure Pipelines definition
@@ Commit message
Pipelines would be redundant, and a waste of energy.
- [Danh: fix apply conflicts]
## README.md ##
@@ azure-pipelines.yml (deleted)
- PathtoPublish: t/failed-test-artifacts
- ArtifactName: failed-test-artifacts
-
--- job: linux_musl
-- displayName: linux-musl
-- condition: succeeded()
-- pool:
-- vmImage: ubuntu-latest
-- steps:
-- - bash: |
-- test "$GITFILESHAREPWD" = '$(gitfileshare.pwd)' || ci/mount-fileshare.sh //gitfileshare.file.core.windows.net/test-cache gitfileshare "$GITFILESHAREPWD" "$HOME/test-cache" || exit 1
--
-- res=0
-- sudo AGENT_OS="$AGENT_OS" BUILD_BUILDNUMBER="$BUILD_BUILDNUMBER" BUILD_REPOSITORY_URI="$BUILD_REPOSITORY_URI" BUILD_SOURCEBRANCH="$BUILD_SOURCEBRANCH" BUILD_SOURCEVERSION="$BUILD_SOURCEVERSION" SYSTEM_PHASENAME="$SYSTEM_PHASENAME" SYSTEM_TASKDEFINITIONSURI="$SYSTEM_TASKDEFINITIONSURI" SYSTEM_TEAMPROJECT="$SYSTEM_TEAMPROJECT" CC=$CC MAKEFLAGS="$MAKEFLAGS" jobname=linux-musl bash -lxc ci/run-docker.sh || res=1
--
-- sudo chmod a+r t/out/TEST-*.xml
-- test ! -d t/failed-test-artifacts || sudo chmod a+r t/failed-test-artifacts
--
-- test "$GITFILESHAREPWD" = '$(gitfileshare.pwd)' || sudo umount "$HOME/test-cache" || res=1
-- exit $res
-- displayName: 'jobname=linux-musl ci/run-docker.sh'
-- env:
-- GITFILESHAREPWD: $(gitfileshare.pwd)
-- - task: PublishTestResults@2
-- displayName: 'Publish Test Results **/TEST-*.xml'
-- inputs:
-- mergeTestResults: true
-- testRunTitle: 'musl'
-- platform: Linux
-- publishRunAttachments: false
-- condition: succeededOrFailed()
-- - task: PublishBuildArtifacts@1
-- displayName: 'Publish trash directories of failed tests'
-- condition: failed()
-- inputs:
-- PathtoPublish: t/failed-test-artifacts
-- ArtifactName: failed-test-artifacts
--
-- job: static_analysis
- displayName: StaticAnalysis
- condition: succeeded()
-: ---------- > 10: f688fa50d3 tests: when run in Bash, annotate test failures with file name/line number -: ---------- > 11: 715d1f732f ci: add a problem matcher for GitHub Actions -: ---------- > 12: 0908d5ab9b ci: let GitHub Actions upload failed tests' directories
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v4 1/12 ci/lib: if CI type is unknown, show the environment variables
From: Johannes Schindelin <[email protected]>
This should help with adding new CI-specific if-else arms.
--- ci/lib.sh | 1 + 1 file changed, 1 insertion(+)
ci/lib.sh +1 -0
@@ -138,6 +138,7 @@ then GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS" else echo "Could not identify CI type" >&2 + env >&2 exit 1 fi
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v4 2/12 ci/lib: allow running in GitHub Actions
In reply to Đoàn Trần Công DanhFrom: Johannes Schindelin <[email protected]>
For each CI system we support, we need a specific arm in that if/else construct in ci/lib.sh. Let's add one for GitHub Actions.
--- ci/lib.sh | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)
ci/lib.sh +19 -1
@@ -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 + if test "$TRAVIS_DEBUG_MODE" = true || test true = "$GITHUB_ACTIONS" then return fi
@@ -136,6 +136,24 @@ then MAKEFLAGS="$MAKEFLAGS --jobs=10" test windows_nt != "$CI_OS_NAME" || GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS" +elif test true = "$GITHUB_ACTIONS" +then + CI_TYPE=github-actions + CI_BRANCH="$GITHUB_REF" + CI_COMMIT="$GITHUB_SHA" + CI_OS_NAME="$(echo "$RUNNER_OS" | tr A-Z a-z)" + test macos != "$CI_OS_NAME" || CI_OS_NAME=osx + CI_REPO_SLUG="$GITHUB_REPOSITORY" + CI_JOB_ID="$GITHUB_RUN_ID" + CC="${CC:-gcc}" + + cache_dir="$HOME/none" + + export GIT_PROVE_OPTS="--timer --jobs 10" + export GIT_TEST_OPTS="--verbose-log -x" + MAKEFLAGS="$MAKEFLAGS --jobs=10" + test windows != "$CI_OS_NAME" || + GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS" else echo "Could not identify CI type" >&2 env >&2
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v4 3/12 ci/lib: set TERM environment variable if not exist
In reply to Đoàn Trần Công DanhGitHub Action doesn't set TERM environment variable, which is required by "tput".
Fallback to dumb if it's not set.
--- ci/lib.sh | 3 +++ 1 file changed, 3 insertions(+)
ci/lib.sh +3 -0
@@ -79,6 +79,9 @@ check_unignored_build_artifacts () } } +# GitHub Action doesn't set TERM, which is required by tput +export TERM=${TERM:-dumb} + # Clear MAKEFLAGS that may come from the outside world. export MAKEFLAGS=
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v4 4/12 ci: fix the `jobname` of the `GETTEXT_POISON` job
In reply to Đoàn Trần Công DanhFrom: Johannes Schindelin <[email protected]>
In 6cdccfce1e0f (i18n: make GETTEXT_POISON a runtime option, 2018-11-08), the jobname was adjusted to have the GIT_TEST_ prefix, but that prefix makes no sense in this context.
--- .travis.yml | 2 +- ci/lib.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
.travis.yml +1 -1
@@ -16,7 +16,7 @@ compiler: matrix: include: - - env: jobname=GIT_TEST_GETTEXT_POISON + - env: jobname=GETTEXT_POISON os: linux compiler: addons:
ci/lib.sh +1 -1
@@ -211,7 +211,7 @@ osx-clang|osx-gcc) # Travis CI OS X export GIT_SKIP_TESTS="t9810 t9816" ;; -GIT_TEST_GETTEXT_POISON) +GETTEXT_POISON) export GIT_TEST_GETTEXT_POISON=true ;; Linux32)
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v4 5/12 ci: explicit install all required packages
In reply to Đoàn Trần Công DanhIn a later patch, we will support GitHub Action.
Explicitly install all of our build dependencies.
Since GitHub Action VM hasn't install our build dependencies, yet. And there're no harm to reinstall them (in Travis)
--- ci/install-dependencies.sh | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)
ci/install-dependencies.sh +12 -2
@@ -7,12 +7,17 @@ P4WHENCE=http://filehost.perforce.com/perforce/r$LINUX_P4_VERSION LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION +UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev + perl-modules liberror-perl tcl tk gettext zlib1g-dev apache2 + libauthen-sasl-perl libemail-valid-perl libio-socket-ssl-perl + libnet-smtp-ssl-perl" case "$jobname" in linux-clang|linux-gcc) 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 + 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
@@ -63,11 +68,16 @@ StaticAnalysis) ;; Documentation) sudo apt-get -q update - sudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns + sudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns \ + libcurl4-openssl-dev test -n "$ALREADY_HAVE_ASCIIDOCTOR" || gem install --version 1.5.8 asciidoctor ;; +GETTEXT_POISON) + sudo apt-get -q update + sudo apt-get -q -y install $UBUNTU_COMMON_PKGS + ;; esac if type p4d >/dev/null && type p4 >/dev/null
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v4 6/12 ci: run gem with sudo to install asciidoctor
In reply to Đoàn Trần Công DanhFrom: Johannes Schindelin <[email protected]>
In a later patch, we will run Documentation job in GitHub Actions. The job will run without elevated permission.
Run gem with sudo to elevate permission in order to be able to install to system location.
This will also keep this installation in-line with other installation in our Linux system for CI.
--- ci/install-dependencies.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
ci/install-dependencies.sh +1 -1
@@ -72,7 +72,7 @@ Documentation) libcurl4-openssl-dev test -n "$ALREADY_HAVE_ASCIIDOCTOR" || - gem install --version 1.5.8 asciidoctor + sudo gem install --version 1.5.8 asciidoctor ;; GETTEXT_POISON) sudo apt-get -q update
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v4 7/12 ci: configure GitHub Actions for CI/PR
In reply to Đoàn Trần Công DanhThis patch adds CI builds via GitHub Actions. While the underlying technology is at least _very_ similar to that of Azure Pipelines, GitHub Actions are much easier to set up than Azure Pipelines:
- no need to install a GitHub App,
- no need to set up an Azure DevOps account,
- all you need to do is push to your fork on GitHub.
Therefore, it makes a lot of sense for us to have a working GitHub Actions setup.
While copy/editing azure-pipelines.yml into .github/workflows/main.yml, we also use the opportunity to accelerate the step that sets up a minimal subset of Git for Windows' SDK in the Windows-build job:
- we now download a .tar.xz stored in Azure Blobs and extract it simultaneously by calling curl and piping the result to tar,
- decompressing via xz,
- all three utilities are installed together with Git for Windows
At the same time, we also make use of the matrix build feature, which reduces the amount of repeated text by quite a bit.
Also, we do away with the parts that try to mount a file share on which prove can store data between runs. It is just too complicated to set up, and most times the tree changes anyway, so there is little return on investment there.
--- .github/workflows/main.yml | 212 +++++++++++++++++++++++++++++++++++++ 1 file changed, 212 insertions(+) create mode 100644 .github/workflows/main.yml
.github/workflows/main.yml new file +212 -0
@@ -0,0 +1,212 @@ +name: CI/PR + +on: [push, pull_request] + +env: + DEVELOPER: 1 + +jobs: + windows-build: + runs-on: windows-latest + steps: + - uses: actions/checkout@v1 + - name: download git-sdk-64-minimal + shell: bash + run: a=git-sdk-64-minimal && mkdir -p $a && curl -# https://wingit.blob.core.windows.net/ci-artifacts/$a.tar.xz | tar -C $a -xJf - + - name: build + shell: powershell + env: + HOME: ${{runner.workspace}} + MSYSTEM: MINGW64 + NO_PERL: 1 + run: | + & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @" + printf '%s\n' /git-sdk-64-minimal/ >>.git/info/exclude + + ci/make-test-artifacts.sh artifacts + "@ + - name: upload build artifacts + uses: actions/upload-artifact@v1 + with: + name: windows-artifacts + path: artifacts + windows-test: + runs-on: windows-latest + needs: [windows-build] + strategy: + matrix: + nr: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9] + steps: + - uses: actions/checkout@v1 + - name: download git-sdk-64-minimal + shell: bash + run: a=git-sdk-64-minimal && mkdir -p $a && curl -# https://wingit.blob.core.windows.net/ci-artifacts/$a.tar.xz | tar -C $a -xJf - + - name: download build artifacts + uses: actions/download-artifact@v1 + with: + name: windows-artifacts + path: ${{github.workspace}} + - name: extract build artifacts + shell: bash + run: tar xf artifacts.tar.gz + - name: test + shell: powershell + run: | + & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @" + # Let Git ignore the SDK + printf '%s\n' /git-sdk-64-minimal/ >>.git/info/exclude + + ci/run-test-slice.sh ${{matrix.nr}} 10 + "@ + - name: ci/print-test-failures.sh + if: failure() + shell: powershell + run: | + & .\git-sdk-64-minimal\usr\bin\bash.exe -lc ci/print-test-failures.sh + vs-build: + env: + MSYSTEM: MINGW64 + NO_PERL: 1 + GIT_CONFIG_PARAMETERS: "'user.name=CI' 'user.email=ci@git'" + runs-on: windows-latest + steps: + - uses: actions/checkout@v1 + - name: download git-sdk-64-minimal + shell: bash + run: a=git-sdk-64-minimal && mkdir -p $a && curl -# https://wingit.blob.core.windows.net/ci-artifacts/$a.tar.xz | tar -C $a -xJf - + - name: generate Visual Studio solution + shell: powershell + run: | + & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @" + make NDEBUG=1 DEVELOPER=1 vcxproj + "@ + if (!$?) { exit(1) } + - name: download vcpkg artifacts + shell: powershell + run: | + $urlbase = "https://dev.azure.com/git/git/_apis/build/builds" + $id = ((Invoke-WebRequest -UseBasicParsing "${urlbase}?definitions=9&statusFilter=completed&resultFilter=succeeded&`$top=1").content | ConvertFrom-JSON).value[0].id + $downloadUrl = ((Invoke-WebRequest -UseBasicParsing "${urlbase}/$id/artifacts").content | ConvertFrom-JSON).value[0].resource.downloadUrl + (New-Object Net.WebClient).DownloadFile($downloadUrl, "compat.zip") + Expand-Archive compat.zip -DestinationPath . -Force + Remove-Item compat.zip + - name: add msbuild to PATH + uses: microsoft/[email protected] + - name: MSBuild + run: msbuild git.sln -property:Configuration=Release -property:Platform=x64 -maxCpuCount:4 -property:PlatformToolset=v142 + - name: bundle artifact tar + shell: powershell + env: + MSVC: 1 + VCPKG_ROOT: ${{github.workspace}}\compat\vcbuild\vcpkg + run: | + & compat\vcbuild\vcpkg_copy_dlls.bat release + if (!$?) { exit(1) } + & git-sdk-64-minimal\usr\bin\bash.exe -lc @" + mkdir -p artifacts && + eval \"`$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts 2>&1 | grep ^tar)\" + "@ + - name: upload build artifacts + uses: actions/upload-artifact@v1 + with: + name: vs-artifacts + path: artifacts + vs-test: + runs-on: windows-latest + needs: [vs-build] + strategy: + matrix: + nr: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9] + steps: + - uses: actions/checkout@v1 + - name: download git-64-portable + shell: bash + run: a=git-64-portable && mkdir -p $a && curl -# https://wingit.blob.core.windows.net/ci-artifacts/$a.tar.xz | tar -C $a -xJf - + - name: download build artifacts + uses: actions/download-artifact@v1 + with: + name: vs-artifacts + path: ${{github.workspace}} + - name: extract build artifacts + shell: bash + run: tar xf artifacts.tar.gz + - name: test (parallel) + shell: powershell + env: + MSYSTEM: MINGW64 + NO_SVN_TESTS: 1 + GIT_TEST_SKIP_REBASE_P: 1 + run: | + & git-64-portable\git-cmd.exe --command=usr\bin\bash.exe -lc @" + # Let Git ignore the SDK and the test-cache + printf '%s\n' /git-64-portable/ /test-cache/ >>.git/info/exclude + + cd t && + PATH=\"`$PWD/helper:`$PATH\" && + test-tool.exe run-command testsuite --jobs=10 -V -x --write-junit-xml \ + `$(test-tool.exe path-utils slice-tests \ + ${{matrix.nr}} 10 t[0-9]*.sh) + "@ + regular: + strategy: + matrix: + vector: + - jobname: linux-clang + cc: clang + pool: ubuntu-latest + - jobname: linux-gcc + cc: gcc + pool: ubuntu-latest + - jobname: osx-clang + cc: clang + pool: macos-latest + - jobname: osx-gcc + cc: gcc + pool: macos-latest + - jobname: GETTEXT_POISON + cc: gcc + pool: ubuntu-latest + env: + CC: ${{matrix.vector.cc}} + jobname: ${{matrix.vector.jobname}} + runs-on: ${{matrix.vector.pool}} + steps: + - uses: actions/checkout@v1 + - run: ci/install-dependencies.sh + - run: ci/run-build-and-tests.sh + - run: ci/print-test-failures.sh + if: failure() + dockerized: + strategy: + matrix: + vector: + - jobname: linux-musl + image: alpine + - jobname: Linux32 + image: daald/ubuntu32:xenial + env: + jobname: ${{matrix.vector.jobname}} + runs-on: ubuntu-latest + container: ${{matrix.vector.image}} + steps: + - uses: actions/checkout@v1 + - run: ci/install-docker-dependencies.sh + - run: ci/run-build-and-tests.sh + - run: ci/print-test-failures.sh + if: failure() + static-analysis: + env: + jobname: StaticAnalysis + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - run: ci/install-dependencies.sh + - run: ci/run-static-analysis.sh + documentation: + env: + jobname: Documentation + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - run: ci/install-dependencies.sh + - run: ci/test-documentation.sh
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v4 8/12 README: add a build badge for the GitHub Actions runs
In reply to Đoàn Trần Công DanhFrom: Johannes Schindelin <[email protected]>
--- README.md | 1 + 1 file changed, 1 insertion(+)
README.md +1 -0
@@ -1,3 +1,4 @@ +[](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush) [](https://dev.azure.com/git/git/_build/latest?definitionId=11) Git - fast, scalable, distributed revision control system
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v4 9/12 ci: retire the Azure Pipelines definition
In reply to Đoàn Trần Công DanhFrom: Johannes Schindelin <[email protected]>
We have GitHub Actions now. Running the same builds and tests in Azure Pipelines would be redundant, and a waste of energy.
--- README.md | 1 - azure-pipelines.yml | 558 -------------------------------------------- 2 files changed, 559 deletions(-) delete mode 100644 azure-pipelines.yml
README.md +0 -1
@@ -1,5 +1,4 @@ [](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush) -[](https://dev.azure.com/git/git/_build/latest?definitionId=11) Git - fast, scalable, distributed revision control system =========================================================
azure-pipelines.yml deleted +0 -558
558 lines not on this page · raw message
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v4 10/12 tests: when run in Bash, annotate test failures with file name/line number
In reply to Đoàn Trần Công DanhFrom: Johannes Schindelin <[email protected]>
When a test fails, it is nice to see where the corresponding code lives in the worktree. Sadly, it seems that only Bash allows us to infer this information. Let's do it when we detect that we're running in a Bash.
This will come in handy in the next commit, where we teach the GitHub Actions workflow to annotate failed test runs with this information.
--- t/test-lib.sh | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
t/test-lib.sh +13 -1
@@ -657,6 +657,18 @@ die () { fi } +file_lineno () { + test -z "$GIT_TEST_FRAMEWORK_SELFTEST" && test -n "$BASH" || return 0 + local i + for i in ${!BASH_SOURCE[*]} + do + case $i,"${BASH_SOURCE[$i]##*/}" in + 0,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:$LINENO: ${1+$1: }"; return;; + *,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:${BASH_LINENO[$(($i-1))]}: ${1+$1: }"; return;; + esac + done +} + GIT_EXIT_OK= trap 'die' EXIT # Disable '-x' tracing, because with some shells, notably dash, it
@@ -702,7 +714,7 @@ test_failure_ () { write_junit_xml_testcase "$1" " $junit_insert" fi test_failure=$(($test_failure + 1)) - say_color error "not ok $test_count - $1" + say_color error "$(file_lineno error)not ok $test_count - $1" shift printf '%s\n' "$*" | sed -e 's/^/# /' test "$immediate" = "" || { finalize_junit_xml; GIT_EXIT_OK=t; exit 1; }
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v4 12/12 ci: let GitHub Actions upload failed tests' directories
In reply to Đoàn Trần Công DanhFrom: Johannes Schindelin <[email protected]>
Arguably, CI builds' most important task is to not only identify regressions, but to make it as easy as possible to investigate what went wrong.
In that light, we will want to provide users with a way to inspect the tests' output as well as the corresponding directories.
This commit adds build steps that are only executed when tests failed, uploading the relevant information as build artifacts. These artifacts can then be downloaded by interested parties to diagnose the failures more efficiently.
--- .github/workflows/main.yml | 18 ++++++++++++++++++ ci/print-test-failures.sh | 7 +++++++ 2 files changed, 25 insertions(+)
.github/workflows/main.yml +18 -0
@@ -63,6 +63,12 @@ jobs: shell: powershell run: | & .\git-sdk-64-minimal\usr\bin\bash.exe -lc ci/print-test-failures.sh + - name: Upload failed tests' directories + if: failure() && env.FAILED_TEST_ARTIFACTS != '' + uses: actions/upload-artifact@v1 + with: + name: failed-tests-windows + path: ${{env.FAILED_TEST_ARTIFACTS}} vs-build: env: MSYSTEM: MINGW64
@@ -176,6 +182,12 @@ jobs: - run: ci/run-build-and-tests.sh - run: ci/print-test-failures.sh if: failure() + - name: Upload failed tests' directories + if: failure() && env.FAILED_TEST_ARTIFACTS != '' + uses: actions/upload-artifact@v1 + with: + name: failed-tests-${{matrix.vector.jobname}} + path: ${{env.FAILED_TEST_ARTIFACTS}} dockerized: strategy: matrix:
@@ -194,6 +206,12 @@ jobs: - run: ci/run-build-and-tests.sh - run: ci/print-test-failures.sh if: failure() + - name: Upload failed tests' directories + if: failure() && env.FAILED_TEST_ARTIFACTS != '' + uses: actions/upload-artifact@v1 + with: + name: failed-tests-${{matrix.vector.jobname}} + path: ${{env.FAILED_TEST_ARTIFACTS}} static-analysis: env: jobname: StaticAnalysis
ci/print-test-failures.sh +7 -0
@@ -46,6 +46,13 @@ do mv "$trash_dir" failed-test-artifacts continue ;; + github-actions) + mkdir -p failed-test-artifacts + echo "::set-env name=FAILED_TEST_ARTIFACTS::t/failed-test-artifacts" + cp "${TEST_EXIT%.exit}.out" failed-test-artifacts/ + tar czf failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir" + continue + ;; *) echo "Unhandled CI type: $CI_TYPE" >&2 exit 1
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v4 11/12 ci: add a problem matcher for GitHub Actions
In reply to Đoàn Trần Công DanhFrom: Johannes Schindelin <[email protected]>
With this patch, test failures will be annotated with a helpful, clickable message in GitHub Actions. For details, see
https://github.com/actions/toolkit/blob/master/docs/problem-matchers.md
Note: we need to set TEST_SHELL_PATH to Bash so that the problem matcher is fed a file and line number for each test failure.
--- ci/git-problem-matcher.json | 16 ++++++++++++++++ ci/lib.sh | 5 +++++ 2 files changed, 21 insertions(+) create mode 100644 ci/git-problem-matcher.json
ci/git-problem-matcher.json new file +16 -0
@@ -0,0 +1,16 @@ +{ + "problemMatcher": [ + { + "owner": "git-test-suite", + "pattern": [ + { + "regexp": "^([^ :]+\\.sh):(\\d+): (error|warning|info):\\s+(.*)$", + "file": 1, + "line": 2, + "severity": 3, + "message": 4 + } + ] + } + ] +}
ci/lib.sh +5 -0
@@ -157,6 +157,11 @@ then MAKEFLAGS="$MAKEFLAGS --jobs=10" test windows != "$CI_OS_NAME" || GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS" + + # https://github.com/actions/toolkit/blob/master/docs/commands.md#problem-matchers + echo "::add-matcher::ci/git-problem-matcher.json" + test linux-musl = "$jobname" || + MAKEFLAGS="$MAKEFLAGS TEST_SHELL_PATH=/bin/sh" else echo "Could not identify CI type" >&2 env >&2
Signature · 1 line
2.26.0.334.g6536db25bb
Quoted 23 lines · Đoàn Trần Công Danh
Our Azure Pipeline has served us well over the course of the past year or so, steadily catching issues before the respective patches hit the next branch.
There is a GitHub-native CI system now, though, called "GitHub Actions"
[https://github.com/features/actions] which is essentially on par with Azure Pipelines as far as our needs are concerned, and it brings a couple of advantages:* It is substantially easier to set up than Azure Pipelines: all you need is to add the YAML-based build definition, push to your fork on GitHub, and that's it.
* The syntax is a bit easier to read than Azure Pipelines'.
* We get more concurrent jobs (Azure Pipelines is limited to 10 concurrent jobs).With this change, users also no longer need to open a PR at
https://github.com/git/git or at https://github.com/gitgitgadget/git just to get the benefit of a CI build.Sample run on top of dd/ci-musl-libc with dd/test-with-busybox merged:
https://github.com/sgn/git/actions/runs/73179413
Quoted 8 lines
Sample run when this series applied into git-for-windows
https://github.com/git-for-windows/git/runs/568625109Change from v3:
- Use build matrix
- All dependencies are install by scripts
- stop repeating environment variables
- build failure's artifacts will be uploaded
I did not see any particular thing that is bad in any of the three series involved; do people have further comments?
I am not exactly happy that these had to be queued on top of a merge of two topics in flight, which makes it cumbersome to deal with a breakage in these two other topics, though, but that would be a pain only until these two topics prove to be stable enough to build on.
Judging from two CI runs for 'pu' ([*1*] and [*2*]), among the topics that are cooking, there are only a few topics that these tests are unhappy about. Perhaps those on Windows can help these topics to pass these tests?
[References]
*1* https://github.com/git/git/actions/runs/74687673 is 'pu' with
all cooking topics.
*2* https://github.com/git/git/actions/runs/74741625 is 'pu' with
some topics excluded.
Hi Junio,
Quoted 28 lines · Junio C Hamano
Quoted 26 lines · Đoàn Trần Công Danh
Our Azure Pipeline has served us well over the course of the past year or so, steadily catching issues before the respective patches hit the next branch.
There is a GitHub-native CI system now, though, called "GitHub Actions"
[https://github.com/features/actions] which is essentially on par with Azure Pipelines as far as our needs are concerned, and it brings a couple of advantages:* It is substantially easier to set up than Azure Pipelines: all you need is to add the YAML-based build definition, push to your fork on GitHub, and that's it.
* The syntax is a bit easier to read than Azure Pipelines'.
* We get more concurrent jobs (Azure Pipelines is limited to 10 concurrent jobs).With this change, users also no longer need to open a PR at
https://github.com/git/git or at https://github.com/gitgitgadget/git just to get the benefit of a CI build.Sample run on top of dd/ci-musl-libc with dd/test-with-busybox merged:
https://github.com/sgn/git/actions/runs/73179413Sample run when this series applied into git-for-windows
https://github.com/git-for-windows/git/runs/568625109
Quoted 8 lines
Quoted 5 lines
Change from v3:
- Use build matrix
- All dependencies are install by scripts
- stop repeating environment variables
- build failure's artifacts will be uploadedI did not see any particular thing that is bad in any of the three series involved; do people have further comments?
FWIW I consider this work good enough that I already merged it into Git for Windows. It should make it easier for contributors to test their branches "privately", in their own forks, before opening a PR (most people do not like to have relatively trivial issues pointed out by fellow human beings, but are much more okay with machines telling them what needs to be improved).
Please mark this up as a vote of confidence from my side.
Quoted 4 lines
I am not exactly happy that these had to be queued on top of a merge of two topics in flight, which makes it cumbersome to deal with a breakage in these two other topics, though, but that would be a pain only until these two topics prove to be stable enough to build on.
Yes, and the fact that ci-musl-libc was _not_ based on top of test-with-busybox makes it a bit more awkward. I, for one, had totally missed that the latter patch series is _required_ in order to make the former work correctly. Hunting for the cause for almost an hour until Danh kindly pointed out that he had fixed all the issues in test-with-busybox already.
Quoted 4 lines
Judging from two CI runs for 'pu' ([*1*] and [*2*]), among the topics that are cooking, there are only a few topics that these tests are unhappy about. Perhaps those on Windows can help these topics to pass these tests?
I would like to point out that there is only one single topic that is cause for sorrow here, and it is the reftable one.
Before going further, let me point out that the pu branch has been broken for quite a long time now, primarily because of bugreport and... of course because of reftable. Whenever pu included reftable, the CI builds failed. So these reftable problems are not a good reason, in my mind, to hold up the GitHub workflow patches from advancing.
Seeing the short stat 35 files changed, 6719 insertions(+) of even a single patch in the reftable patch series _really_ does not entice me to spend time even looking at it, certainly not at a time when I am short on time, let alone to try to find time to fix it.
However, since you asked so nicely, I did start to look into it. First, let me present you the less controversial of two patches I want to show you:
-- snip --
From 5f42a3f6ef9cf7d90bd274e55539b145cae40e28 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <[email protected]>
Date: Fri, 10 Apr 2020 14:23:40 +0200
Subject: [PATCH] fixup??? Reftable support for git-core
As I had already pointed out over a month ago in
https://github.com/gitgitgadget/git/pull/539#issuecomment-589157008 this C code violates the C standard, and MS Visual C is not as lenient as GCC/clang on it: structs cannot be initialized with = {}.
Compile errors aside, while this code conforms to the C syntax, it feels more like Java when it initializes e.g. struct object_id only to _immediately_ overwrite the contents.
--- refs/reftable-backend.c | 52 ++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 26 deletions(-)
refs/reftable-backend.c +26 -26
@@ -31,7 +31,7 @@ static void clear_reftable_log_record(struct reftable_log_record *log)static void fill_reftable_log_record (struct reftable_log_record *log) {const char *info =git_committer_info (0 ); -struct ident_split split = {}; +struct ident_split split = {NULL };int result =split_ident_line (&split, info,strlen (info));int sign =1 ;assert (0 == result);
@@ -230,7 +230,7 @@ static int reftable_transaction_abort(struct ref_store *ref_store,static int reftable_check_old_oid (struct ref_store *refs,const char *refname,struct object_id *want_oid) { -struct object_id out_oid = {}; +struct object_id out_oid;int out_flags =0 ;const char *resolved =refs_resolve_ref_unsafe ( refs, refname,RESOLVE_REF_READING , &out_oid, &out_flags);
@@ -287,14 +287,14 @@ static int write_transaction_table(struct reftable_writer *writer, void *arg) log->message = u->msg;if (u->flags &REF_HAVE_NEW ) { -struct object_id out_oid = {}; +struct object_id out_oid;int out_flags =0 ;/* Memory owned by refs_resolve_ref_unsafe, no need to * free(). */ const char *resolved =refs_resolve_ref_unsafe ( transaction->ref_store, u->refname,0 , &out_oid, &out_flags); -struct reftable_ref_record ref = {}; +struct reftable_ref_record ref = {NULL }; ref.ref_name = (char *)(resolved ? resolved : u->refname); log->ref_name = ref.ref_name;
@@ -376,8 +376,8 @@ static int write_delete_refs_table(struct reftable_writer *writer, void *argv) }for (int i =0 ; i < arg->refnames->nr; i++) { -struct reftable_log_record log = {}; -struct reftable_ref_record current = {}; +struct reftable_log_record log = {NULL }; +struct reftable_ref_record current = {NULL };fill_reftable_log_record (&log); log.message =xstrdup (arg->logmsg); log.new_hash =NULL ;
@@ -455,10 +455,10 @@ static int write_create_symref_table(struct reftable_writer *writer, void *arg) } { -struct reftable_log_record log = {}; -struct object_id new_oid = {}; -struct object_id old_oid = {}; -struct reftable_ref_record current = {}; +struct reftable_log_record log = {NULL }; +struct object_id new_oid; +struct object_id old_oid; +struct reftable_ref_record current = {NULL };reftable_stack_read_ref (create->refs->stack, create->refname, ¤t);fill_reftable_log_record (&log);
@@ -513,7 +513,7 @@ static int write_rename_table(struct reftable_writer *writer, void *argv) {struct write_rename_arg *arg = (struct write_rename_arg *)argv;uint64_t ts =reftable_stack_next_update_index (arg->stack); -struct reftable_ref_record ref = {}; +struct reftable_ref_record ref = {NULL };int err =reftable_stack_read_ref (arg->stack, arg->oldname, &ref);if (err) {
@@ -531,7 +531,7 @@ static int write_rename_table(struct reftable_writer *writer, void *argv) ref.update_index = ts; { -struct reftable_ref_record todo[2 ] = {}; +struct reftable_ref_record todo[2 ] = { {NULL } }; todo[0 ].ref_name = (char *)arg->oldname; todo[0 ].update_index = ts;/* leave todo[0] empty */
@@ -545,7 +545,7 @@ static int write_rename_table(struct reftable_writer *writer, void *argv) }if (ref.value !=NULL ) { -struct reftable_log_record todo[2 ] = {}; +struct reftable_log_record todo[2 ] = { {NULL } };fill_reftable_log_record (&todo[0 ]);fill_reftable_log_record (&todo[1 ]);
@@ -688,12 +688,12 @@ reftable_for_each_reflog_ent_newest_first(struct ref_store *ref_store,const char *refname, each_reflog_ent_fn fn, void *cb_data) { - struct reftable_iterator it = {}; + struct reftable_iterator it = { NULL };struct git_reftable_ref_store *refs = (struct git_reftable_ref_store *)ref_store;struct reftable_merged_table *mt =NULL ;int err =0 ; -struct reftable_log_record log = {}; +struct reftable_log_record log = {NULL };if (refs->err <0 ) {return refs->err;
@@ -712,8 +712,8 @@ reftable_for_each_reflog_ent_newest_first(struct ref_store *ref_store, } { -struct object_id old_oid = {}; -struct object_id new_oid = {}; +struct object_id old_oid; +struct object_id new_oid;const char *full_committer ="" ;hashcpy (old_oid.hash, log.old_hash);
@@ -744,7 +744,7 @@ reftable_for_each_reflog_ent_oldest_first(struct ref_store *ref_store,const char *refname, each_reflog_ent_fn fn, void *cb_data) { - struct reftable_iterator it = {}; + struct reftable_iterator it = { NULL };struct git_reftable_ref_store *refs = (struct git_reftable_ref_store *)ref_store;struct reftable_merged_table *mt =NULL ;
@@ -760,7 +760,7 @@ reftable_for_each_reflog_ent_oldest_first(struct ref_store *ref_store, err =reftable_merged_table_seek_log (mt, &it, refname);while (err ==0 ) { -struct reftable_log_record log = {}; +struct reftable_log_record log = {NULL }; err =reftable_iterator_next_log (it, &log);if (err !=0 ) {break ;
@@ -780,8 +780,8 @@ reftable_for_each_reflog_ent_oldest_first(struct ref_store *ref_store,for (int i = len; i--;) {struct reftable_log_record *log = &logs[i]; -struct object_id old_oid = {}; -struct object_id new_oid = {}; +struct object_id old_oid; +struct object_id new_oid;const char *full_committer ="" ;hashcpy (old_oid.hash, log->old_hash);
@@ -903,8 +903,8 @@ static int reftable_reflog_expire(struct ref_store *ref_store,struct reflog_expiry_arg arg = { .refs = refs, }; -struct reftable_log_record log = {}; -struct reftable_iterator it = {}; +struct reftable_log_record log = {NULL }; +struct reftable_iterator it = {NULL };int err =0 ;if (refs->err <0 ) {return refs->err;
@@ -917,8 +917,8 @@ static int reftable_reflog_expire(struct ref_store *ref_store, }while (1 ) { -struct object_id ooid = {}; -struct object_id noid = {}; +struct object_id ooid; +struct object_id noid;int err =reftable_iterator_next_log (it, &log);if (err <0 ) {
@@ -950,7 +950,7 @@ static int reftable_read_raw_ref(struct ref_store *ref_store, {struct git_reftable_ref_store *refs = (struct git_reftable_ref_store *)ref_store; -struct reftable_ref_record ref = {}; +struct reftable_ref_record ref = {NULL };int err =0 ;if (refs->err <0 ) {return refs->err;
-- snap --
This patch should fix the vs-build job in the Azure Pipeline as well as in the GitHub workflow.
However, it does _not_ fix the test failure on Windows. When I tried to investigate this, I wanted to compare the results between Windows and Linux (WSL, of course, it is a major time saver for me these days because I don't have to power up a VM, and I can access WSL files from Windows and vice versa), and it turns out that the 000000000002-000000000002.ref file is different, it even has different sizes (242 bytes on Windows instead of 268 bytes on Linux), and notably it contains the string HEAD on Windows and refs/heads/master on Linux, but not vice versa.
So I dug a bit deeper and was stopped rudely by the fact that the t0031-reftable.sh script produces different output every time it runs. Because it does not even use test_commit.
Therefore, let me present you with this patch (whose commit message conveys a rather alarming indication that this endeavor of fixing reftable could become a major time sink):
-- snip -
From 6ba47e70a2eb8efe2116c12eb950ddb90c473d11 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <[email protected]>
Date: Fri, 10 Apr 2020 16:10:53 +0200
Subject: [PATCH] fixup??? Reftable support for git-core
The test for the reftable functionality should use the convenience functions we provide for test scripts. Using test_commit in particular does help with reproducible output (otherwise the SHA-1s will change together with the time the tests were run).
Currently, this here seemingly innocuous change causes quite a few warnings throughout the test, though, e.g. this slur of warnings when committing the last commit in the test script:
warning: bad replace ref name: e
warning: bad replace ref name: ber-1
warning: bad replace ref name: ber-2
warning: bad replace ref name: ber-3
warning: bad replace ref name: ber-4
warning: bad replace ref name: ber-5
warning: bad replace ref name: ber-6
warning: bad replace ref name: ber-7
warning: bad replace ref name: ber-8
warning: bad replace ref name: ber-9
This is notably _not_ a problem that was introduced by this here patch, of course, but a very real concern about the reftable patches, most likely the big one that introduces the reftable library in one fell swoop.
--- t/t0031-reftable.sh | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-)
t/t0031-reftable.sh +9 -11
@@ -8,28 +8,26 @@ test_description='reftable basics' . ./test-lib.sh test_expect_success 'basic operation of reftable storage' ' - git init --ref-storage=reftable repo && ( - cd repo && - echo "hello" >world.txt && - git add world.txt && - git commit -m "first post" && - test_write_lines HEAD refs/heads/master >expect && + rm -rf .git && + git init --ref-storage=reftable && + mv .git/hooks .git/hooks-disabled && + test_commit file && + test_write_lines HEAD refs/heads/master refs/tags/file >expect && git show-ref && git show-ref | cut -f2 -d" " > actual && test_cmp actual expect && for count in $(test_seq 1 10) do - echo "hello" >>world.txt - git commit -m "number ${count}" world.txt || + test_commit "number $count" file.t $count number-$count || return 1 done && git gc && - nfiles=$(ls -1 .git/reftable | wc -l ) && - test ${nfiles} = "2" && + ls -1 .git/reftable >table-files && + test_line_count = 2 table-files && git reflog refs/heads/master >output && test_line_count = 11 output && grep "commit (initial): first post" output && - grep "commit: number 10" output ) + grep "commit: number 10" output ' test_done
-- snap --
While I am very happy with the post-image of this diff, I am super unhappy about the output of it. It makes me believe that this reftable patch series is in serious need of being "incrementalized" _after the fact_. Otherwise it will be simply impossible to build enough confidence in the correctness of it, especially given the fact that it obviously does some incorrect things right now (see the "bad replace ref name" warning mentioned above).
I'll take a break from this now, but I would like to encourage you to apply both patches as SQUASH??? on top of hn/reftable for the time being.
Ciao,
Dscho
Quoted 11 lines
[References]
*1* https://github.com/git/git/actions/runs/74687673 is 'pu' with
all cooking topics.*2* https://github.com/git/git/actions/runs/74741625 is 'pu' with
some topics excluded.
Hi Junio,
me again, just quickly, because the t0031-reftable.sh --valgrind run just came back with this:
-- snip --
[...]
+ git gc
==28394== error calling PR_SET_PTRACER, vgdb might block ==28399== error calling PR_SET_PTRACER, vgdb might block ==28399== error calling PR_SET_PTRACER, vgdb might block ==28404== error calling PR_SET_PTRACER, vgdb might block ==28404== Invalid read of size 1
==28404== at 0x4C32CF2: strlen (in/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==28404== by 0x551D9AD: strdup (strdup.c:41) ==28404== by 0x39D15A: xstrdup (wrapper.c:29) ==28404== by 0x3DA9CA: reftable_log_record_copy_from (record.c:605) ==28404== by 0x3DB844: record_copy_from (record.c:968) ==28404== by 0x3D64B3: merged_iter_next (merged.c:117) ==28404== by 0x3D656B: merged_iter_next_void (merged.c:131) ==28404== by 0x3D597D: iterator_next (iter.c:45) ==28404== by 0x3D5AD2: reftable_iterator_next_log (iter.c:71) ==28404== by 0x3DE037: stack_write_compact (stack.c:718) ==28404== by 0x3DDBEA: stack_compact_locked (stack.c:632) ==28404== by 0x3DE5AD: stack_compact_range (stack.c:847)
==28404== Address 0x0 is not stack'd, malloc'd or (recently) free'd ==28404==
{
<insert_a_suppression_name_here>
Memcheck:Addr1
fun:strlen
fun:strdup
fun:xstrdup
fun:reftable_log_record_copy_from
fun:record_copy_from
fun:merged_iter_next
fun:merged_iter_next_void
fun:iterator_next
fun:reftable_iterator_next_log
fun:stack_write_compact
fun:stack_compact_locked
fun:stack_compact_range
}
==28404==
==28404== Process terminating with default action of signal 11 (SIGSEGV) ==28404== Access not within mapped region at address 0x0
==28404== at 0x4C32CF2: strlen (in/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==28404== by 0x551D9AD: strdup (strdup.c:41) ==28404== by 0x39D15A: xstrdup (wrapper.c:29) ==28404== by 0x3DA9CA: reftable_log_record_copy_from (record.c:605) ==28404== by 0x3DB844: record_copy_from (record.c:968) ==28404== by 0x3D64B3: merged_iter_next (merged.c:117) ==28404== by 0x3D656B: merged_iter_next_void (merged.c:131) ==28404== by 0x3D597D: iterator_next (iter.c:45) ==28404== by 0x3D5AD2: reftable_iterator_next_log (iter.c:71) ==28404== by 0x3DE037: stack_write_compact (stack.c:718) ==28404== by 0x3DDBEA: stack_compact_locked (stack.c:632) ==28404== by 0x3DE5AD: stack_compact_range (stack.c:847) ==28404== If you believe this happened as a result of a stack ==28404== overflow in your program's main thread (unlikely but ==28404== possible), you can try to increase the size of the ==28404== main thread stack using the --main-stacksize= flag. ==28404== The main thread stack size used in this run was 8388608.
error: reflog died of signal 11
fatal: failed to run reflog
error: last command exited with $?=128
-- snap --
But now _really_ want to take a break from this,
Dscho
Quoted 411 lines · Johannes Schindelin
Hi Junio,
Quoted 28 lines · Junio C Hamano
Quoted 26 lines · Đoàn Trần Công Danh
Our Azure Pipeline has served us well over the course of the past year or so, steadily catching issues before the respective patches hit the next branch.
There is a GitHub-native CI system now, though, called "GitHub Actions"
[https://github.com/features/actions] which is essentially on par with Azure Pipelines as far as our needs are concerned, and it brings a couple of advantages:* It is substantially easier to set up than Azure Pipelines: all you need is to add the YAML-based build definition, push to your fork on GitHub, and that's it.
* The syntax is a bit easier to read than Azure Pipelines'.
* We get more concurrent jobs (Azure Pipelines is limited to 10 concurrent jobs).With this change, users also no longer need to open a PR at
https://github.com/git/git or at https://github.com/gitgitgadget/git just to get the benefit of a CI build.Sample run on top of dd/ci-musl-libc with dd/test-with-busybox merged:
https://github.com/sgn/git/actions/runs/73179413Sample run when this series applied into git-for-windows
https://github.com/git-for-windows/git/runs/568625109Quoted 8 lines
Quoted 5 lines
Change from v3:
- Use build matrix
- All dependencies are install by scripts
- stop repeating environment variables
- build failure's artifacts will be uploadedI did not see any particular thing that is bad in any of the three series involved; do people have further comments?
FWIW I consider this work good enough that I already merged it into Git for Windows. It should make it easier for contributors to test their branches "privately", in their own forks, before opening a PR (most people do not like to have relatively trivial issues pointed out by fellow human beings, but are much more okay with machines telling them what needs to be improved).
Please mark this up as a vote of confidence from my side.
Quoted 4 lines
I am not exactly happy that these had to be queued on top of a merge of two topics in flight, which makes it cumbersome to deal with a breakage in these two other topics, though, but that would be a pain only until these two topics prove to be stable enough to build on.
Yes, and the fact that
ci-musl-libcwas _not_ based on top oftest-with-busyboxmakes it a bit more awkward. I, for one, had totally missed that the latter patch series is _required_ in order to make the former work correctly. Hunting for the cause for almost an hour until Danh kindly pointed out that he had fixed all the issues intest-with-busyboxalready.Quoted 4 lines
Judging from two CI runs for 'pu' ([*1*] and [*2*]), among the topics that are cooking, there are only a few topics that these tests are unhappy about. Perhaps those on Windows can help these topics to pass these tests?
I would like to point out that there is only one single topic that is cause for sorrow here, and it is the reftable one.
Before going further, let me point out that the
pubranch has been broken for quite a long time now, primarily because ofbugreportand... of course because ofreftable. Wheneverpuincludedreftable, the CI builds failed. So thesereftableproblems are not a good reason, in my mind, to hold up the GitHub workflow patches from advancing.Seeing the short stat
35 files changed, 6719 insertions(+)of even a single patch in thereftablepatch series _really_ does not entice me to spend time even looking at it, certainly not at a time when I am short on time, let alone to try to find time to fix it.However, since you asked so nicely, I did start to look into it. First, let me present you the less controversial of two patches I want to show you:
-- snip --
From5f42a3f6ef9cf7d90bd274e55539b145cae40e28Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <[email protected]>
Date: Fri, 10 Apr 2020 14:23:40 +0200
Subject: [PATCH] fixup??? Reftable support for git-coreAs I had already pointed out over a month ago in
https://github.com/gitgitgadget/git/pull/539#issuecomment-589157008 this C code violates the C standard, and MS Visual C is not as lenient as GCC/clang on it:structs cannot be initialized with= {}.Compile errors aside, while this code conforms to the C syntax, it feels more like Java when it initializes e.g.
struct object_idonly to _immediately_ overwrite the contents.Signed-off-by Johannes Schindelin <[email protected]>--- refs/reftable-backend.c | 52 ++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 26 deletions(-)refs/reftable-backend.c +26
-27@@ -31,7 +31,7 @@ static void clear_reftable_log_record(struct reftable_log_record *log)static void fill_reftable_log_record (struct reftable_log_record *log) {const char *info =git_committer_info (0 ); -struct ident_split split = {}; +struct ident_split split = {NULL };int result =split_ident_line (&split, info,strlen (info));int sign =1 ;assert (0 == result);@@ -230,7 +230,7 @@ static int reftable_transaction_abort(struct ref_store *ref_store,static int reftable_check_old_oid (struct ref_store *refs,const char *refname,struct object_id *want_oid) { -struct object_id out_oid = {}; +struct object_id out_oid;int out_flags =0 ;const char *resolved =refs_resolve_ref_unsafe ( refs, refname,RESOLVE_REF_READING , &out_oid, &out_flags);@@ -287,14 +287,14 @@ static int write_transaction_table(struct reftable_writer *writer, void *arg) log->message = u->msg;if (u->flags &REF_HAVE_NEW ) { -struct object_id out_oid = {}; +struct object_id out_oid;int out_flags =0 ;/* Memory owned by refs_resolve_ref_unsafe, no need to * free(). */ const char *resolved =refs_resolve_ref_unsafe ( transaction->ref_store, u->refname,0 , &out_oid, &out_flags); -struct reftable_ref_record ref = {}; +struct reftable_ref_record ref = {NULL }; ref.ref_name = (char *)(resolved ? resolved : u->refname); log->ref_name = ref.ref_name;@@ -376,8 +376,8 @@ static int write_delete_refs_table(struct reftable_writer *writer, void *argv) }for (int i =0 ; i < arg->refnames->nr; i++) { -struct reftable_log_record log = {}; -struct reftable_ref_record current = {}; +struct reftable_log_record log = {NULL }; +struct reftable_ref_record current = {NULL };fill_reftable_log_record (&log); log.message =xstrdup (arg->logmsg); log.new_hash =NULL ;@@ -455,10 +455,10 @@ static int write_create_symref_table(struct reftable_writer *writer, void *arg) } { -struct reftable_log_record log = {}; -struct object_id new_oid = {}; -struct object_id old_oid = {}; -struct reftable_ref_record current = {}; +struct reftable_log_record log = {NULL }; +struct object_id new_oid; +struct object_id old_oid; +struct reftable_ref_record current = {NULL };reftable_stack_read_ref (create->refs->stack, create->refname, ¤t);fill_reftable_log_record (&log);@@ -513,7 +513,7 @@ static int write_rename_table(struct reftable_writer *writer, void *argv) {struct write_rename_arg *arg = (struct write_rename_arg *)argv;uint64_t ts =reftable_stack_next_update_index (arg->stack); -struct reftable_ref_record ref = {}; +struct reftable_ref_record ref = {NULL };int err =reftable_stack_read_ref (arg->stack, arg->oldname, &ref);if (err) {@@ -531,7 +531,7 @@ static int write_rename_table(struct reftable_writer *writer, void *argv) ref.update_index = ts; { -struct reftable_ref_record todo[2 ] = {}; +struct reftable_ref_record todo[2 ] = { {NULL } }; todo[0 ].ref_name = (char *)arg->oldname; todo[0 ].update_index = ts;/* leave todo[0] empty */ @@ -545,7 +545,7 @@ static int write_rename_table(struct reftable_writer *writer, void *argv) }if (ref.value !=NULL ) { -struct reftable_log_record todo[2 ] = {}; +struct reftable_log_record todo[2 ] = { {NULL } };fill_reftable_log_record (&todo[0 ]);fill_reftable_log_record (&todo[1 ]);@@ -688,12 +688,12 @@ reftable_for_each_reflog_ent_newest_first(struct ref_store *ref_store,const char *refname, each_reflog_ent_fn fn, void *cb_data) { - struct reftable_iterator it = {}; + struct reftable_iterator it = { NULL };struct git_reftable_ref_store *refs = (struct git_reftable_ref_store *)ref_store;struct reftable_merged_table *mt =NULL ;int err =0 ; -struct reftable_log_record log = {}; +struct reftable_log_record log = {NULL };if (refs->err <0 ) {return refs->err;@@ -712,8 +712,8 @@ reftable_for_each_reflog_ent_newest_first(struct ref_store *ref_store, } { -struct object_id old_oid = {}; -struct object_id new_oid = {}; +struct object_id old_oid; +struct object_id new_oid;const char *full_committer ="" ;hashcpy (old_oid.hash, log.old_hash);@@ -744,7 +744,7 @@ reftable_for_each_reflog_ent_oldest_first(struct ref_store *ref_store,const char *refname, each_reflog_ent_fn fn, void *cb_data) { - struct reftable_iterator it = {}; + struct reftable_iterator it = { NULL };struct git_reftable_ref_store *refs = (struct git_reftable_ref_store *)ref_store;struct reftable_merged_table *mt =NULL ;@@ -760,7 +760,7 @@ reftable_for_each_reflog_ent_oldest_first(struct ref_store *ref_store, err =reftable_merged_table_seek_log (mt, &it, refname);while (err ==0 ) { -struct reftable_log_record log = {}; +struct reftable_log_record log = {NULL }; err =reftable_iterator_next_log (it, &log);if (err !=0 ) {break ;@@ -780,8 +780,8 @@ reftable_for_each_reflog_ent_oldest_first(struct ref_store *ref_store,for (int i = len; i--;) {struct reftable_log_record *log = &logs[i]; -struct object_id old_oid = {}; -struct object_id new_oid = {}; +struct object_id old_oid; +struct object_id new_oid;const char *full_committer ="" ;hashcpy (old_oid.hash, log->old_hash);@@ -903,8 +903,8 @@ static int reftable_reflog_expire(struct ref_store *ref_store,struct reflog_expiry_arg arg = { .refs = refs, }; -struct reftable_log_record log = {}; -struct reftable_iterator it = {}; +struct reftable_log_record log = {NULL }; +struct reftable_iterator it = {NULL };int err =0 ;if (refs->err <0 ) {return refs->err;@@ -917,8 +917,8 @@ static int reftable_reflog_expire(struct ref_store *ref_store, }while (1 ) { -struct object_id ooid = {}; -struct object_id noid = {}; +struct object_id ooid; +struct object_id noid;int err =reftable_iterator_next_log (it, &log);if (err <0 ) {@@ -950,7 +950,7 @@ static int reftable_read_raw_ref(struct ref_store *ref_store, {struct git_reftable_ref_store *refs = (struct git_reftable_ref_store *)ref_store; -struct reftable_ref_record ref = {}; +struct reftable_ref_record ref = {NULL };int err =0 ;if (refs->err <0 ) {return refs->err; -- snap --This patch should fix the
vs-buildjob in the Azure Pipeline as well as in the GitHub workflow.However, it does _not_ fix the test failure on Windows. When I tried to investigate this, I wanted to compare the results between Windows and Linux (WSL, of course, it is a major time saver for me these days because I don't have to power up a VM, and I can access WSL files from Windows and vice versa), and it turns out that the
000000000002-000000000002.reffile is different, it even has different sizes (242 bytes on Windows instead of 268 bytes on Linux), and notably it contains the stringHEADon Windows andrefs/heads/masteron Linux, but not vice versa.So I dug a bit deeper and was stopped rudely by the fact that the
t0031-reftable.shscript produces different output every time it runs. Because it does not even usetest_commit.Therefore, let me present you with this patch (whose commit message conveys a rather alarming indication that this endeavor of fixing
reftablecould become a major time sink):-- snip -
From6ba47e70a2eb8efe2116c12eb950ddb90c473d11Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <[email protected]>
Date: Fri, 10 Apr 2020 16:10:53 +0200
Subject: [PATCH] fixup??? Reftable support for git-coreThe test for the reftable functionality should use the convenience functions we provide for test scripts. Using
test_commitin particular does help with reproducible output (otherwise the SHA-1s will change together with the time the tests were run).Currently, this here seemingly innocuous change causes quite a few warnings throughout the test, though, e.g. this slur of warnings when committing the last commit in the test script:
warning: bad replace ref name: e
warning: bad replace ref name: ber-1
warning: bad replace ref name: ber-2
warning: bad replace ref name: ber-3
warning: bad replace ref name: ber-4
warning: bad replace ref name: ber-5
warning: bad replace ref name: ber-6
warning: bad replace ref name: ber-7
warning: bad replace ref name: ber-8
warning: bad replace ref name: ber-9This is notably _not_ a problem that was introduced by this here patch, of course, but a very real concern about the reftable patches, most likely the big one that introduces the reftable library in one fell swoop.
Signed-off-by Johannes Schindelin <[email protected]>--- t/t0031-reftable.sh | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-)t/t0031-reftable.sh +9
-12@@ -8,28 +8,26 @@ test_description='reftable basics' . ./test-lib.sh test_expect_success 'basic operation of reftable storage' ' - git init --ref-storage=reftable repo && ( - cd repo && - echo "hello" >world.txt && - git add world.txt && - git commit -m "first post" && - test_write_lines HEAD refs/heads/master >expect && + rm -rf .git && + git init --ref-storage=reftable && + mv .git/hooks .git/hooks-disabled && + test_commit file && + test_write_lines HEAD refs/heads/master refs/tags/file >expect && git show-ref && git show-ref | cut -f2 -d" " > actual && test_cmp actual expect && for count in $(test_seq 1 10) do - echo "hello" >>world.txt - git commit -m "number ${count}" world.txt || + test_commit "number $count" file.t $count number-$count || return 1 done && git gc && - nfiles=$(ls -1 .git/reftable | wc -l ) && - test ${nfiles} = "2" && + ls -1 .git/reftable >table-files && + test_line_count = 2 table-files && git reflog refs/heads/master >output && test_line_count = 11 output && grep "commit (initial): first post" output && - grep "commit: number 10" output ) + grep "commit: number 10" output ' test_done -- snap --While I am very happy with the post-image of this diff, I am super unhappy about the output of it. It makes me believe that this
reftablepatch series is in serious need of being "incrementalized"_afterthefact_. Otherwise it will be simply impossible to build enough confidence in the correctness of it, especially given the fact that it obviously does some incorrect things right now (see the "bad replace ref name" warning mentioned above).I'll take a break from this now, but I would like to encourage you to apply both patches as
SQUASH???on top ofhn/reftablefor the time being.Ciao,
DschoQuoted 11 lines
[References]
*1* https://github.com/git/git/actions/runs/74687673 is 'pu' with
all cooking topics.*2* https://github.com/git/git/actions/runs/74741625 is 'pu' with
some topics excluded.
Re: [PATCH 2/5] ci/lib: allow running in GitHub Actions
In reply to Johannes SchindelinQuoted 10 lines · Johannes Schindelin
Quoted 8 lines
ci/lib.sh then · line 145
Quoted 1 line
ci/lib.sh +1
-0+ test macos != "$CI_OS_NAME" || CI_OS_NAME=osxHmm, if "macos" isn't not equal to $CI_OS_NAME, then set CI_OS_NAME=osx. This is head-scratchingly backwards, and I think
test "$CI_OS_NAME" = macos && CI_OS_NAME=osxwould read better.
I can understand where you come from, but your code is not
set -esafe,
It works as expected in at least in dash, Bash, BusyBox sh, ksh, ksh93, mksh/lksh, yash, posh, FreeBSD /bin/sh, NetBSD /bin/sh. But it doesn't really matter.
This is not in the part of Git that should run on a wide variety of platforms, but only in our CI systems, where we do know which system and which shell we use, and we can rely on that it just works.
Quoted 2 lines
which is the reason why I wrote the code this way (compare to the already existing code in the previous clause, which was copy-edited here).
Didn't like it there, either.
Quoted 7 lines · Johannes Schindelin
Quoted 4 lines
Judging from two CI runs for 'pu' ([*1*] and [*2*]), among the topics that are cooking, there are only a few topics that these tests are unhappy about. Perhaps those on Windows can help these topics to pass these tests?
I would like to point out that there is only one single topic that is cause for sorrow here, and it is the reftable one.
I first thought so, too, but vsbuild failures like the one in
https://github.com/git/git/runs/575116793 do not appear to be caused by that topic (6a8c1d17b8 excludes reftable), so there must be somebody else that is broken.
An all green build https://github.com/git/git/actions/runs/74741625
was my attempt to see how ready these tests are (not 'how ready other topics are to be tested by this topic) by moving the swap-azure-pipelines-with-github-actions topic early in 'pu', temporarily discarding many other topics, and pushing it out, by the way.
Re: [PATCH v4 05/12] ci: explicit install all required packages
In reply to Đoàn Trần Công DanhQuoted 3 lines · Đoàn Trần Công Danh
In a later patch, we will support GitHub Action.
Explicitly install all of our build dependencies.
... on Linux. This patch doesn't touch the parts installing dependencies in the osx jobs, but we do rely on some packages being installed by default in the osx images we use. This is worth clarifying in the commit message, and in its subject line.
Quoted 1 line
Since GitHub Action VM hasn't install our build dependencies, yet.
s/install/installed/
I'm not sure what you mean with "yet".
Quoted 12 lines
And there're no harm to reinstall them (in Travis)
Signed-off-by Đoàn Trần Công Danh <[email protected]>--- ci/install-dependencies.sh | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)ci/install-dependencies.sh +0
-0
ci/install-dependencies.sh line 13
Quoted 7 lines
ci/install-dependencies.sh +4
-0@@ -7,12 +7,17 @@ P4WHENCE=http://filehost.perforce.com/perforce/r$LINUX_P4_VERSION LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION +UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev + perl-modules liberror-perl tcl tk gettext zlib1g-dev apache2 + libauthen-sasl-perl libemail-valid-perl libio-socket-ssl-perl + libnet-smtp-ssl-perl"
I note that this list includes 'make' and 'apache2'.
ci/install-dependencies.sh line 19
Quoted 6 lines
ci/install-dependencies.sh +1
-1case "$jobname" in linux-clang|linux-gcc) 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 + sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
'apache2' is listed here again.
ci/install-dependencies.sh StaticAnalysis) · line 72
Quoted 11 lines
ci/install-dependencies.sh +3
-1+ $UBUNTU_COMMON_PKGS case "$jobname" in linux-gcc) sudo apt-get -q -y install gcc-8@@ -63,11 +68,16 @@ StaticAnalysis) ;; Documentation) sudo apt-get -q update - sudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns + sudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns \ + libcurl4-openssl-dev
Does the Documentation job really need the 'libcurl4-openssl-dev' package? FWIW, I just removed this package from my system, and 'make doc' still succeeded.
Furthermore, this doesn't install 'make', though in other jobs it is installed explicitly. Note that the StaticAnalysis job requires 'make' as well.
Also note that we have a 'linux-gcc-4.8' job as well...
ci/install-dependencies.sh StaticAnalysis) · line 79
Quoted 7 lines
ci/install-dependencies.sh +3
-0test -n "$ALREADY_HAVE_ASCIIDOCTOR" || gem install --version 1.5.8 asciidoctor ;; +GETTEXT_POISON) + sudo apt-get -q update + sudo apt-get -q -y install $UBUNTU_COMMON_PKGS
The GETTEXT_POISON job currently doesn't install 'apache2', but with this change it will. If this change is intentional, then please justify it in the commit message. But I think that we shouldn't include 'apache2' in $UBUNTU_COMMON_PKGS.
Quoted 7 lines
ci/install-dependencies.sh +1
-0+ ;; esac if type p4d >/dev/null && type p4 >/dev/nullSignature · 1 line
2.26.0.334.g6536db25bb
Re: [PATCH v4 05/12] ci: explicit install all required packages
Quoted 9 lines · SZEDER Gábor
Quoted 3 lines · Đoàn Trần Công Danh
In a later patch, we will support GitHub Action.
Explicitly install all of our build dependencies.
... on Linux. This patch doesn't touch the parts installing dependencies in the osx jobs, but we do rely on some packages being installed by default in the osx images we use. This is worth clarifying in the commit message, and in its subject line.
Fair enough.
Quoted 10 lines
Quoted 1 line
Since GitHub Action VM hasn't install our build dependencies, yet.
s/install/installed/
Quoted 4 lines
+UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
+ perl-modules liberror-perl tcl tk gettext zlib1g-dev apache2
+ libauthen-sasl-perl libemail-valid-perl libio-socket-ssl-perl
+ libnet-smtp-ssl-perl"I note that this list includes 'make' and 'apache2'.
I'll remove apache2.
Quoted 9 lines
ci/install-dependencies.sh line 19
Quoted 6 lines
case "$jobname" in
linux-clang|linux-gcc)ci/install-dependencies.sh +1
-1sudo 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 + sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \'apache2' is listed here again.
Quoted 15 lines
ci/install-dependencies.sh StaticAnalysis) · line 72
Quoted 11 lines
ci/install-dependencies.sh +3
-1+ $UBUNTU_COMMON_PKGS case "$jobname" in linux-gcc) sudo apt-get -q -y install gcc-8@@ -63,11 +68,16 @@ StaticAnalysis) ;; Documentation) sudo apt-get -q update - sudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns + sudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns \ + libcurl4-openssl-devDoes the Documentation job really need the 'libcurl4-openssl-dev' package? FWIW, I just removed this package from my system, and 'make doc' still succeeded.
At the time of writing this series.
pu requires curl-config for Documentation jobs.
Skimming over the mail archive, Peff has sent a patch to fix it. I haven't checked again, though.
Quoted 3 lines
Furthermore, this doesn't install 'make', though in other jobs it is installed explicitly. Note that the StaticAnalysis job requires 'make' as well.
I copied them from Azure Pipelines declaration. I think it's better to list make explicitly in all jobs.
Quoted 1 line
Also note that we have a 'linux-gcc-4.8' job as well...
Will do in the re-roll.
Quoted 8 lines
ci/install-dependencies.sh StaticAnalysis) · line 79
Quoted 3 lines
ci/install-dependencies.sh +3
-0+GETTEXT_POISON) + sudo apt-get -q update + sudo apt-get -q -y install $UBUNTU_COMMON_PKGSThe
GETTEXT_POISONjob currently doesn't install 'apache2', but with this change it will. If this change is intentional, then please justify it in the commit message. But I think that we shouldn't include 'apache2' in $UBUNTU_COMMON_PKGS.
No, this patch shouldn't change it.
I will remove apache2 from $UBUNTU_COMMON_PKGS
Signature · 1 line
Danh
Re: [PATCH v4 05/12] ci: explicit install all required packages
Quoted 9 lines · Danh Doan
On 2020-04-10 17:53:22+0200, SZEDER Gábor <[email protected]> wrote:
...Quoted 3 lines
Furthermore, this doesn't install 'make', though in other jobs it is installed explicitly. Note that the StaticAnalysis job requires 'make' as well.
I copied them from Azure Pipelines declaration. I think it's better to list make explicitly in all jobs.
Quoted 6 lines
Quoted 1 line
Also note that we have a 'linux-gcc-4.8' job as well...
Will do in the re-roll.
...
No, this patch shouldn't change it.
I will remove apache2 from $UBUNTU_COMMON_PKGS
Thanks both for working together to trim excess bits and add back missing bits to polish it.
v5 cover ci: replace our Azure Pipeline by GitHub Actions
In reply to Johannes Schindelin via GitGitGadgetOur Azure Pipeline has served us well over the course of the past year or so, steadily catching issues before the respective patches hit the next branch.
There is a GitHub-native CI system now, though, called "GitHub Actions"
[https://github.com/features/actions] which is essentially on par with Azure Pipelines as far as our needs are concerned, and it brings a couple of advantages:
* It is substantially easier to set up than Azure Pipelines: all you need is to add the YAML-based build definition, push to your fork on GitHub, and that's it.
* The syntax is a bit easier to read than Azure Pipelines'.
* We get more concurrent jobs (Azure Pipelines is limited to 10 concurrent jobs).
With this change, users also no longer need to open a PR at
https://github.com/git/git or at https://github.com/gitgitgadget/git just to get the benefit of a CI build.
Change in v5 from v4:
- cleanup on dependencies installation.
Change in v4 from v3:
- Use build matrix
- All dependencies are install by scripts
- stop repeating environment variables
- build failure's artifacts will be uploaded
*** BLURB HERE ***
Johannes Schindelin (9):
ci/lib: if CI type is unknown, show the environment variables ci/lib: allow running in GitHub Actions
ci: fix the jobname of the GETTEXT_POISON job
ci: run gem with sudo to install asciidoctor
README: add a build badge for the GitHub Actions runs
ci: retire the Azure Pipelines definition
tests: when run in Bash, annotate test failures with file name/line
number
ci: add a problem matcher for GitHub Actions
ci: let GitHub Actions upload failed tests' directories
Đoàn Trần Công Danh (3):
ci/lib: set TERM environment variable if not exist
ci: explicit install all required packages
ci: configure GitHub Actions for CI/PR
.github/workflows/main.yml | 230 +++++++++++++++ .travis.yml | 2 +- README.md | 2 +- azure-pipelines.yml | 558 ------------------------------------ ci/git-problem-matcher.json | 16 ++ ci/install-dependencies.sh | 16 +- ci/lib.sh | 31 +- ci/print-test-failures.sh | 7 + t/test-lib.sh | 14 +- 9 files changed, 309 insertions(+), 567 deletions(-) create mode 100644 .github/workflows/main.yml delete mode 100644 azure-pipelines.yml create mode 100644 ci/git-problem-matcher.json
Range-diff against v4:
1: 2219bf3db9 = 1: 2219bf3db9 ci/lib: if CI type is unknown, show the environment variables 2: 2818799a4b = 2: 2818799a4b ci/lib: allow running in GitHub Actions 3: b88586c2c5 = 3: b88586c2c5 ci/lib: set TERM environment variable if not exist 4: 1df60e677c = 4: 1df60e677c ci: fix the `jobname` of the `GETTEXT_POISON` job 5: 4f80724641 ! 5: ac7e247bb3 ci: explicit install all required packages
@@ Commit message
In a later patch, we will support GitHub Action.
- Explicitly install all of our build dependencies.
- Since GitHub Action VM hasn't install our build dependencies, yet.
+ Explicitly install all of our build dependencies on Linux.
+ Since GitHub Action's Linux VM hasn't installed our build dependencies.
And there're no harm to reinstall them (in Travis)
@@ ci/install-dependencies.sh
P4WHENCE=http://filehost.perforce.com/perforce/r$LINUX_P4_VERSION LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION
+UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
-+ perl-modules liberror-perl tcl tk gettext zlib1g-dev apache2
-+ libauthen-sasl-perl libemail-valid-perl libio-socket-ssl-perl
-+ libnet-smtp-ssl-perl"
++ 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)
@@ ci/install-dependencies.sh
case "$jobname" in
linux-gcc)
sudo apt-get -q -y install gcc-8
-@@ ci/install-dependencies.sh: StaticAnalysis) +@@ ci/install-dependencies.sh: osx-clang|osx-gcc)
+ StaticAnalysis)
+ sudo apt-get -q update
+ sudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \
+- libexpat-dev gettext
++ libexpat-dev gettext make
;;
Documentation)
sudo apt-get -q update
- sudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns
-+ sudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns \
-+ libcurl4-openssl-dev
++ sudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns make
test -n "$ALREADY_HAVE_ASCIIDOCTOR" ||
gem install --version 1.5.8 asciidoctor
;;
-+GETTEXT_POISON)
++linux-gcc-4.8|GETTEXT_POISON)
+ sudo apt-get -q update
+ sudo apt-get -q -y install $UBUNTU_COMMON_PKGS
+ ;;
6: 795ec656c6 < -: ---------- ci: run gem with sudo to install asciidoctor -: ---------- > 6: 2fb9f2e2f2 ci: run gem with sudo to install asciidoctor 7: ec0aa20119 = 7: 40fe4f7e2c ci: configure GitHub Actions for CI/PR 8: 46f2b6bce6 = 8: 09735fb1de README: add a build badge for the GitHub Actions runs 9: 92f2623dc7 = 9: 575b3afd3c ci: retire the Azure Pipelines definition 10: f688fa50d3 = 10: ce00385987 tests: when run in Bash, annotate test failures with file name/line number 11: 715d1f732f = 11: 3caeb0b7f4 ci: add a problem matcher for GitHub Actions 12: 0908d5ab9b = 12: 8de46ee1c6 ci: let GitHub Actions upload failed tests' directories
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v5 1/12 ci/lib: if CI type is unknown, show the environment variables
From: Johannes Schindelin <[email protected]>
This should help with adding new CI-specific if-else arms.
--- ci/lib.sh | 1 + 1 file changed, 1 insertion(+)
ci/lib.sh +1 -0
@@ -138,6 +138,7 @@ then GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS" else echo "Could not identify CI type" >&2 + env >&2 exit 1 fi
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v5 3/12 ci/lib: set TERM environment variable if not exist
In reply to Đoàn Trần Công DanhGitHub Action doesn't set TERM environment variable, which is required by "tput".
Fallback to dumb if it's not set.
--- ci/lib.sh | 3 +++ 1 file changed, 3 insertions(+)
ci/lib.sh +3 -0
@@ -79,6 +79,9 @@ check_unignored_build_artifacts () } } +# GitHub Action doesn't set TERM, which is required by tput +export TERM=${TERM:-dumb} + # Clear MAKEFLAGS that may come from the outside world. export MAKEFLAGS=
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v5 2/12 ci/lib: allow running in GitHub Actions
In reply to Đoàn Trần Công DanhFrom: Johannes Schindelin <[email protected]>
For each CI system we support, we need a specific arm in that if/else construct in ci/lib.sh. Let's add one for GitHub Actions.
--- ci/lib.sh | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)
ci/lib.sh +19 -1
@@ -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 + if test "$TRAVIS_DEBUG_MODE" = true || test true = "$GITHUB_ACTIONS" then return fi
@@ -136,6 +136,24 @@ then MAKEFLAGS="$MAKEFLAGS --jobs=10" test windows_nt != "$CI_OS_NAME" || GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS" +elif test true = "$GITHUB_ACTIONS" +then + CI_TYPE=github-actions + CI_BRANCH="$GITHUB_REF" + CI_COMMIT="$GITHUB_SHA" + CI_OS_NAME="$(echo "$RUNNER_OS" | tr A-Z a-z)" + test macos != "$CI_OS_NAME" || CI_OS_NAME=osx + CI_REPO_SLUG="$GITHUB_REPOSITORY" + CI_JOB_ID="$GITHUB_RUN_ID" + CC="${CC:-gcc}" + + cache_dir="$HOME/none" + + export GIT_PROVE_OPTS="--timer --jobs 10" + export GIT_TEST_OPTS="--verbose-log -x" + MAKEFLAGS="$MAKEFLAGS --jobs=10" + test windows != "$CI_OS_NAME" || + GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS" else echo "Could not identify CI type" >&2 env >&2
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v5 4/12 ci: fix the `jobname` of the `GETTEXT_POISON` job
In reply to Đoàn Trần Công DanhFrom: Johannes Schindelin <[email protected]>
In 6cdccfce1e0f (i18n: make GETTEXT_POISON a runtime option, 2018-11-08), the jobname was adjusted to have the GIT_TEST_ prefix, but that prefix makes no sense in this context.
--- .travis.yml | 2 +- ci/lib.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
.travis.yml +1 -1
@@ -16,7 +16,7 @@ compiler: matrix: include: - - env: jobname=GIT_TEST_GETTEXT_POISON + - env: jobname=GETTEXT_POISON os: linux compiler: addons:
ci/lib.sh +1 -1
@@ -211,7 +211,7 @@ osx-clang|osx-gcc) # Travis CI OS X export GIT_SKIP_TESTS="t9810 t9816" ;; -GIT_TEST_GETTEXT_POISON) +GETTEXT_POISON) export GIT_TEST_GETTEXT_POISON=true ;; Linux32)
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v5 5/12 ci: explicit install all required packages
In reply to Đoàn Trần Công DanhIn a later patch, we will support GitHub Action.
Explicitly install all of our build dependencies on Linux. Since GitHub Action's Linux VM hasn't installed our build dependencies. And there're no harm to reinstall them (in Travis)
--- ci/install-dependencies.sh | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)
ci/install-dependencies.sh +11 -3
@@ -7,12 +7,16 @@ P4WHENCE=http://filehost.perforce.com/perforce/r$LINUX_P4_VERSION LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION +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) 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 + 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
@@ -59,15 +63,19 @@ osx-clang|osx-gcc) StaticAnalysis) sudo apt-get -q update sudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \ - libexpat-dev gettext + libexpat-dev gettext make ;; Documentation) sudo apt-get -q update - sudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns + sudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns make test -n "$ALREADY_HAVE_ASCIIDOCTOR" || gem install --version 1.5.8 asciidoctor ;; +linux-gcc-4.8|GETTEXT_POISON) + sudo apt-get -q update + sudo apt-get -q -y install $UBUNTU_COMMON_PKGS + ;; esac if type p4d >/dev/null && type p4 >/dev/null
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v5 6/12 ci: run gem with sudo to install asciidoctor
In reply to Đoàn Trần Công DanhFrom: Johannes Schindelin <[email protected]>
In a later patch, we will run Documentation job in GitHub Actions. The job will run without elevated permission.
Run gem with sudo to elevate permission in order to be able to install to system location.
This will also keep this installation in-line with other installation in our Linux system for CI.
--- ci/install-dependencies.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
ci/install-dependencies.sh +1 -1
@@ -70,7 +70,7 @@ Documentation) sudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns make test -n "$ALREADY_HAVE_ASCIIDOCTOR" || - gem install --version 1.5.8 asciidoctor + sudo gem install --version 1.5.8 asciidoctor ;; linux-gcc-4.8|GETTEXT_POISON) sudo apt-get -q update
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v5 7/12 ci: configure GitHub Actions for CI/PR
In reply to Đoàn Trần Công DanhThis patch adds CI builds via GitHub Actions. While the underlying technology is at least _very_ similar to that of Azure Pipelines, GitHub Actions are much easier to set up than Azure Pipelines:
- no need to install a GitHub App,
- no need to set up an Azure DevOps account,
- all you need to do is push to your fork on GitHub.
Therefore, it makes a lot of sense for us to have a working GitHub Actions setup.
While copy/editing azure-pipelines.yml into .github/workflows/main.yml, we also use the opportunity to accelerate the step that sets up a minimal subset of Git for Windows' SDK in the Windows-build job:
- we now download a .tar.xz stored in Azure Blobs and extract it simultaneously by calling curl and piping the result to tar,
- decompressing via xz,
- all three utilities are installed together with Git for Windows
At the same time, we also make use of the matrix build feature, which reduces the amount of repeated text by quite a bit.
Also, we do away with the parts that try to mount a file share on which prove can store data between runs. It is just too complicated to set up, and most times the tree changes anyway, so there is little return on investment there.
--- .github/workflows/main.yml | 212 +++++++++++++++++++++++++++++++++++++ 1 file changed, 212 insertions(+) create mode 100644 .github/workflows/main.yml
.github/workflows/main.yml new file +212 -0
@@ -0,0 +1,212 @@ +name: CI/PR + +on: [push, pull_request] + +env: + DEVELOPER: 1 + +jobs: + windows-build: + runs-on: windows-latest + steps: + - uses: actions/checkout@v1 + - name: download git-sdk-64-minimal + shell: bash + run: a=git-sdk-64-minimal && mkdir -p $a && curl -# https://wingit.blob.core.windows.net/ci-artifacts/$a.tar.xz | tar -C $a -xJf - + - name: build + shell: powershell + env: + HOME: ${{runner.workspace}} + MSYSTEM: MINGW64 + NO_PERL: 1 + run: | + & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @" + printf '%s\n' /git-sdk-64-minimal/ >>.git/info/exclude + + ci/make-test-artifacts.sh artifacts + "@ + - name: upload build artifacts + uses: actions/upload-artifact@v1 + with: + name: windows-artifacts + path: artifacts + windows-test: + runs-on: windows-latest + needs: [windows-build] + strategy: + matrix: + nr: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9] + steps: + - uses: actions/checkout@v1 + - name: download git-sdk-64-minimal + shell: bash + run: a=git-sdk-64-minimal && mkdir -p $a && curl -# https://wingit.blob.core.windows.net/ci-artifacts/$a.tar.xz | tar -C $a -xJf - + - name: download build artifacts + uses: actions/download-artifact@v1 + with: + name: windows-artifacts + path: ${{github.workspace}} + - name: extract build artifacts + shell: bash + run: tar xf artifacts.tar.gz + - name: test + shell: powershell + run: | + & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @" + # Let Git ignore the SDK + printf '%s\n' /git-sdk-64-minimal/ >>.git/info/exclude + + ci/run-test-slice.sh ${{matrix.nr}} 10 + "@ + - name: ci/print-test-failures.sh + if: failure() + shell: powershell + run: | + & .\git-sdk-64-minimal\usr\bin\bash.exe -lc ci/print-test-failures.sh + vs-build: + env: + MSYSTEM: MINGW64 + NO_PERL: 1 + GIT_CONFIG_PARAMETERS: "'user.name=CI' 'user.email=ci@git'" + runs-on: windows-latest + steps: + - uses: actions/checkout@v1 + - name: download git-sdk-64-minimal + shell: bash + run: a=git-sdk-64-minimal && mkdir -p $a && curl -# https://wingit.blob.core.windows.net/ci-artifacts/$a.tar.xz | tar -C $a -xJf - + - name: generate Visual Studio solution + shell: powershell + run: | + & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @" + make NDEBUG=1 DEVELOPER=1 vcxproj + "@ + if (!$?) { exit(1) } + - name: download vcpkg artifacts + shell: powershell + run: | + $urlbase = "https://dev.azure.com/git/git/_apis/build/builds" + $id = ((Invoke-WebRequest -UseBasicParsing "${urlbase}?definitions=9&statusFilter=completed&resultFilter=succeeded&`$top=1").content | ConvertFrom-JSON).value[0].id + $downloadUrl = ((Invoke-WebRequest -UseBasicParsing "${urlbase}/$id/artifacts").content | ConvertFrom-JSON).value[0].resource.downloadUrl + (New-Object Net.WebClient).DownloadFile($downloadUrl, "compat.zip") + Expand-Archive compat.zip -DestinationPath . -Force + Remove-Item compat.zip + - name: add msbuild to PATH + uses: microsoft/[email protected] + - name: MSBuild + run: msbuild git.sln -property:Configuration=Release -property:Platform=x64 -maxCpuCount:4 -property:PlatformToolset=v142 + - name: bundle artifact tar + shell: powershell + env: + MSVC: 1 + VCPKG_ROOT: ${{github.workspace}}\compat\vcbuild\vcpkg + run: | + & compat\vcbuild\vcpkg_copy_dlls.bat release + if (!$?) { exit(1) } + & git-sdk-64-minimal\usr\bin\bash.exe -lc @" + mkdir -p artifacts && + eval \"`$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts 2>&1 | grep ^tar)\" + "@ + - name: upload build artifacts + uses: actions/upload-artifact@v1 + with: + name: vs-artifacts + path: artifacts + vs-test: + runs-on: windows-latest + needs: [vs-build] + strategy: + matrix: + nr: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9] + steps: + - uses: actions/checkout@v1 + - name: download git-64-portable + shell: bash + run: a=git-64-portable && mkdir -p $a && curl -# https://wingit.blob.core.windows.net/ci-artifacts/$a.tar.xz | tar -C $a -xJf - + - name: download build artifacts + uses: actions/download-artifact@v1 + with: + name: vs-artifacts + path: ${{github.workspace}} + - name: extract build artifacts + shell: bash + run: tar xf artifacts.tar.gz + - name: test (parallel) + shell: powershell + env: + MSYSTEM: MINGW64 + NO_SVN_TESTS: 1 + GIT_TEST_SKIP_REBASE_P: 1 + run: | + & git-64-portable\git-cmd.exe --command=usr\bin\bash.exe -lc @" + # Let Git ignore the SDK and the test-cache + printf '%s\n' /git-64-portable/ /test-cache/ >>.git/info/exclude + + cd t && + PATH=\"`$PWD/helper:`$PATH\" && + test-tool.exe run-command testsuite --jobs=10 -V -x --write-junit-xml \ + `$(test-tool.exe path-utils slice-tests \ + ${{matrix.nr}} 10 t[0-9]*.sh) + "@ + regular: + strategy: + matrix: + vector: + - jobname: linux-clang + cc: clang + pool: ubuntu-latest + - jobname: linux-gcc + cc: gcc + pool: ubuntu-latest + - jobname: osx-clang + cc: clang + pool: macos-latest + - jobname: osx-gcc + cc: gcc + pool: macos-latest + - jobname: GETTEXT_POISON + cc: gcc + pool: ubuntu-latest + env: + CC: ${{matrix.vector.cc}} + jobname: ${{matrix.vector.jobname}} + runs-on: ${{matrix.vector.pool}} + steps: + - uses: actions/checkout@v1 + - run: ci/install-dependencies.sh + - run: ci/run-build-and-tests.sh + - run: ci/print-test-failures.sh + if: failure() + dockerized: + strategy: + matrix: + vector: + - jobname: linux-musl + image: alpine + - jobname: Linux32 + image: daald/ubuntu32:xenial + env: + jobname: ${{matrix.vector.jobname}} + runs-on: ubuntu-latest + container: ${{matrix.vector.image}} + steps: + - uses: actions/checkout@v1 + - run: ci/install-docker-dependencies.sh + - run: ci/run-build-and-tests.sh + - run: ci/print-test-failures.sh + if: failure() + static-analysis: + env: + jobname: StaticAnalysis + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - run: ci/install-dependencies.sh + - run: ci/run-static-analysis.sh + documentation: + env: + jobname: Documentation + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - run: ci/install-dependencies.sh + - run: ci/test-documentation.sh
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v5 8/12 README: add a build badge for the GitHub Actions runs
In reply to Đoàn Trần Công DanhFrom: Johannes Schindelin <[email protected]>
--- README.md | 1 + 1 file changed, 1 insertion(+)
README.md +1 -0
@@ -1,3 +1,4 @@ +[](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush) [](https://dev.azure.com/git/git/_build/latest?definitionId=11) Git - fast, scalable, distributed revision control system
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v5 10/12 tests: when run in Bash, annotate test failures with file name/line number
In reply to Đoàn Trần Công DanhFrom: Johannes Schindelin <[email protected]>
When a test fails, it is nice to see where the corresponding code lives in the worktree. Sadly, it seems that only Bash allows us to infer this information. Let's do it when we detect that we're running in a Bash.
This will come in handy in the next commit, where we teach the GitHub Actions workflow to annotate failed test runs with this information.
--- t/test-lib.sh | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
t/test-lib.sh +13 -1
@@ -657,6 +657,18 @@ die () { fi } +file_lineno () { + test -z "$GIT_TEST_FRAMEWORK_SELFTEST" && test -n "$BASH" || return 0 + local i + for i in ${!BASH_SOURCE[*]} + do + case $i,"${BASH_SOURCE[$i]##*/}" in + 0,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:$LINENO: ${1+$1: }"; return;; + *,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:${BASH_LINENO[$(($i-1))]}: ${1+$1: }"; return;; + esac + done +} + GIT_EXIT_OK= trap 'die' EXIT # Disable '-x' tracing, because with some shells, notably dash, it
@@ -702,7 +714,7 @@ test_failure_ () { write_junit_xml_testcase "$1" " $junit_insert" fi test_failure=$(($test_failure + 1)) - say_color error "not ok $test_count - $1" + say_color error "$(file_lineno error)not ok $test_count - $1" shift printf '%s\n' "$*" | sed -e 's/^/# /' test "$immediate" = "" || { finalize_junit_xml; GIT_EXIT_OK=t; exit 1; }
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v5 9/12 ci: retire the Azure Pipelines definition
In reply to Đoàn Trần Công DanhFrom: Johannes Schindelin <[email protected]>
We have GitHub Actions now. Running the same builds and tests in Azure Pipelines would be redundant, and a waste of energy.
--- README.md | 1 - azure-pipelines.yml | 558 -------------------------------------------- 2 files changed, 559 deletions(-) delete mode 100644 azure-pipelines.yml
README.md +0 -1
@@ -1,5 +1,4 @@ [](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush) -[](https://dev.azure.com/git/git/_build/latest?definitionId=11) Git - fast, scalable, distributed revision control system =========================================================
azure-pipelines.yml deleted +0 -558
558 lines not on this page · raw message
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v5 12/12 ci: let GitHub Actions upload failed tests' directories
In reply to Đoàn Trần Công DanhFrom: Johannes Schindelin <[email protected]>
Arguably, CI builds' most important task is to not only identify regressions, but to make it as easy as possible to investigate what went wrong.
In that light, we will want to provide users with a way to inspect the tests' output as well as the corresponding directories.
This commit adds build steps that are only executed when tests failed, uploading the relevant information as build artifacts. These artifacts can then be downloaded by interested parties to diagnose the failures more efficiently.
--- .github/workflows/main.yml | 18 ++++++++++++++++++ ci/print-test-failures.sh | 7 +++++++ 2 files changed, 25 insertions(+)
.github/workflows/main.yml +18 -0
@@ -63,6 +63,12 @@ jobs: shell: powershell run: | & .\git-sdk-64-minimal\usr\bin\bash.exe -lc ci/print-test-failures.sh + - name: Upload failed tests' directories + if: failure() && env.FAILED_TEST_ARTIFACTS != '' + uses: actions/upload-artifact@v1 + with: + name: failed-tests-windows + path: ${{env.FAILED_TEST_ARTIFACTS}} vs-build: env: MSYSTEM: MINGW64
@@ -176,6 +182,12 @@ jobs: - run: ci/run-build-and-tests.sh - run: ci/print-test-failures.sh if: failure() + - name: Upload failed tests' directories + if: failure() && env.FAILED_TEST_ARTIFACTS != '' + uses: actions/upload-artifact@v1 + with: + name: failed-tests-${{matrix.vector.jobname}} + path: ${{env.FAILED_TEST_ARTIFACTS}} dockerized: strategy: matrix:
@@ -194,6 +206,12 @@ jobs: - run: ci/run-build-and-tests.sh - run: ci/print-test-failures.sh if: failure() + - name: Upload failed tests' directories + if: failure() && env.FAILED_TEST_ARTIFACTS != '' + uses: actions/upload-artifact@v1 + with: + name: failed-tests-${{matrix.vector.jobname}} + path: ${{env.FAILED_TEST_ARTIFACTS}} static-analysis: env: jobname: StaticAnalysis
ci/print-test-failures.sh +7 -0
@@ -46,6 +46,13 @@ do mv "$trash_dir" failed-test-artifacts continue ;; + github-actions) + mkdir -p failed-test-artifacts + echo "::set-env name=FAILED_TEST_ARTIFACTS::t/failed-test-artifacts" + cp "${TEST_EXIT%.exit}.out" failed-test-artifacts/ + tar czf failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir" + continue + ;; *) echo "Unhandled CI type: $CI_TYPE" >&2 exit 1
Signature · 1 line
2.26.0.334.g6536db25bb
PATCH v5 11/12 ci: add a problem matcher for GitHub Actions
In reply to Đoàn Trần Công DanhFrom: Johannes Schindelin <[email protected]>
With this patch, test failures will be annotated with a helpful, clickable message in GitHub Actions. For details, see
https://github.com/actions/toolkit/blob/master/docs/problem-matchers.md
Note: we need to set TEST_SHELL_PATH to Bash so that the problem matcher is fed a file and line number for each test failure.
--- ci/git-problem-matcher.json | 16 ++++++++++++++++ ci/lib.sh | 5 +++++ 2 files changed, 21 insertions(+) create mode 100644 ci/git-problem-matcher.json
ci/git-problem-matcher.json new file +16 -0
@@ -0,0 +1,16 @@ +{ + "problemMatcher": [ + { + "owner": "git-test-suite", + "pattern": [ + { + "regexp": "^([^ :]+\\.sh):(\\d+): (error|warning|info):\\s+(.*)$", + "file": 1, + "line": 2, + "severity": 3, + "message": 4 + } + ] + } + ] +}
ci/lib.sh +5 -0
@@ -157,6 +157,11 @@ then MAKEFLAGS="$MAKEFLAGS --jobs=10" test windows != "$CI_OS_NAME" || GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS" + + # https://github.com/actions/toolkit/blob/master/docs/commands.md#problem-matchers + echo "::add-matcher::ci/git-problem-matcher.json" + test linux-musl = "$jobname" || + MAKEFLAGS="$MAKEFLAGS TEST_SHELL_PATH=/bin/sh" else echo "Could not identify CI type" >&2 env >&2
Signature · 1 line
2.26.0.334.g6536db25bb
Quoted 65 lines · Johannes Schindelin
Hi Junio,
me again, just quickly, because the
t0031-reftable.sh --valgrindrun just came back with this:-- snip --
[...]
+ git gc
==28394== error callingPR_SET_PTRACER, vgdb might block ==28399== error callingPR_SET_PTRACER, vgdb might block ==28399== error callingPR_SET_PTRACER, vgdb might block ==28404== error callingPR_SET_PTRACER, vgdb might block ==28404== Invalid read of size 1
==28404== at0x4C32CF2: strlen (in/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)==28404== by 0x551D9AD: strdup (strdup.c:41) ==28404== by 0x39D15A: xstrdup (wrapper.c:29) ==28404== by 0x3DA9CA: reftable_log_record_copy_from (record.c:605) ==28404== by 0x3DB844: record_copy_from (record.c:968) ==28404== by 0x3D64B3: merged_iter_next (merged.c:117) ==28404== by 0x3D656B: merged_iter_next_void (merged.c:131) ==28404== by 0x3D597D: iterator_next (iter.c:45) ==28404== by 0x3D5AD2: reftable_iterator_next_log (iter.c:71) ==28404== by 0x3DE037: stack_write_compact (stack.c:718) ==28404== by 0x3DDBEA: stack_compact_locked (stack.c:632) ==28404== by 0x3DE5AD: stack_compact_range (stack.c:847)==28404== Address
0x0is not stack'd, malloc'd or (recently) free'd ==28404==
{
<insert_a_suppression_name_here>
Memcheck:Addr1
fun:strlen
fun:strdup
fun:xstrdup
fun:reftable_log_record_copy_from
fun:record_copy_from
fun:merged_iter_next
fun:merged_iter_next_void
fun:iterator_next
fun:reftable_iterator_next_log
fun:stack_write_compact
fun:stack_compact_locked
fun:stack_compact_range
}
==28404==
==28404== Process terminating with default action of signal 11 (SIGSEGV) ==28404== Access not within mapped region at address0x0
==28404== at0x4C32CF2: strlen (in/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)==28404== by 0x551D9AD: strdup (strdup.c:41) ==28404== by 0x39D15A: xstrdup (wrapper.c:29) ==28404== by 0x3DA9CA: reftable_log_record_copy_from (record.c:605) ==28404== by 0x3DB844: record_copy_from (record.c:968) ==28404== by 0x3D64B3: merged_iter_next (merged.c:117) ==28404== by 0x3D656B: merged_iter_next_void (merged.c:131) ==28404== by 0x3D597D: iterator_next (iter.c:45) ==28404== by 0x3D5AD2: reftable_iterator_next_log (iter.c:71) ==28404== by 0x3DE037: stack_write_compact (stack.c:718) ==28404== by 0x3DDBEA: stack_compact_locked (stack.c:632) ==28404== by 0x3DE5AD: stack_compact_range (stack.c:847) ==28404== If you believe this happened as a result of a stack ==28404== overflow in your program's main thread (unlikely but ==28404== possible), you can try to increase the size of the ==28404== main thread stack using the --main-stacksize= flag. ==28404== The main thread stack size used in this run was 8388608.
Quoted 4 lines
error: reflog died of signal 11
fatal: failed to run reflog
error: last command exited with $?=128
-- snap --
The second patch from Dscho (which is only test code in sh) trigger segfault with and without the first patch on top of pu.
Git was built with DEVELOPER=1
I merely run "./t0031-reftable.sh -d -v -i"
Signature · 1 line
Danh
Quoted 7 lines · Junio C Hamano
Quoted 5 lines · Johannes Schindelin
Quoted 4 lines
Judging from two CI runs for 'pu' ([*1*] and [*2*]), among the topics that are cooking, there are only a few topics that these tests are unhappy about. Perhaps those on Windows can help these topics to pass these tests?
Quoted 7 lines
Quoted 2 lines
I would like to point out that there is only one single topic that is cause for sorrow here, and it is the reftable one.
I first thought so, too, but vsbuild failures like the one in
https://github.com/git/git/runs/575116793 do not appear to be caused by that topic (6a8c1d17b8excludes reftable), so there must be somebody else that is broken.
Excerpt from build log:
Quoted 1 line
fatal error C1083: Cannot open include file: '
config-list.h'
It's from bugreport topic.
I've seen this failure in the past (when testing with pu), then I saw it disappear.
I thought it was fixed during my testing for v4.
Signature · 1 line
Danh
Re: [PATCH 2/5] ci/lib: allow running in GitHub Actions
In reply to SZEDER GáborQuoted 10 lines · SZEDER Gábor
Quoted 7 lines
Quoted 5 lines
CI_OS_NAME=osx. This is head-scratchingly backwards, and I think
test "$CI_OS_NAME" = macos && CI_OS_NAME=osxwould read better.
I can understand where you come from, but your code is not
set -esafe,It works as expected in at least in dash, Bash, BusyBox sh, ksh, ksh93, mksh/lksh, yash, posh, FreeBSD
/bin/sh, NetBSD/bin/sh.
Thanks for a clarification. I do not use "set -e" myself (it is not a good idea to rely on it in general), and was wondering if what was said up there was true, as it did not sound like a useful behaviour at all. Not complaining about a non-zero exit before && or || makes it usable ;-)
Re: [PATCH 2/5] ci/lib: allow running in GitHub Actions
Hi,
Quoted 10 lines · Junio C Hamano
Quoted 8 lines · SZEDER Gábor
Quoted 7 lines
Quoted 5 lines
CI_OS_NAME=osx. This is head-scratchingly backwards, and I think
test "$CI_OS_NAME" = macos && CI_OS_NAME=osxwould read better.
I can understand where you come from, but your code is not
set -esafe,
Quoted 8 lines
Quoted 2 lines
It works as expected in at least in dash, Bash, BusyBox sh, ksh, ksh93, mksh/lksh, yash, posh, FreeBSD
/bin/sh, NetBSD/bin/sh.Thanks for a clarification. I do not use "set -e" myself (it is not a good idea to rely on it in general), and was wondering if what was said up there was true, as it did not sound like a useful behaviour at all. Not complaining about a non-zero exit before && or || makes it usable ;-)
While it is not in general useful to rely on set -e, there are situations when it comes in handy for debugging. Or in copy-edited code.
As to the "It works as expected", this is in a Bash:
$ test "$CI_OS_NAME" = macos && CI_OS_NAME=osx
$ echo $? 1
Yes, it works, but in the common case, it exits with an error (that we ignore, unless run with sh -e ...).
Just making sure that we're talking about the same expectation here.
Ciao,
Dscho
Re: [PATCH 2/5] ci/lib: allow running in GitHub Actions
Quoted 11 lines · Johannes Schindelin
As to the "It works as expected", this is in a Bash:
$ test "$CI_OS_NAME" = macos && CI_OS_NAME=osx$ echo $? 1Yes, it works, but in the common case, it exits with an error (that we ignore, unless run with
sh -e ...).Just making sure that we're talking about the same expectation here.
My expectation is that "set -e" won't prevent anything that comes after "make sure we set CI_OS_NAME to osx on macos but do not do so on other platforms" we see above from running. In other words, with a script like this:
$ cat >X <<\EOF
#!/bin/sh
set -x -e
CI_OS_NAME=linux echo different
test "$CI_OS_NAME" = macos && CI_OS_NAME=OSX
CI_OS_NAME=macos
echo same
test "$CI_OS_NAME" = macos && CI_OS_NAME=OSX
echo both passed
false
echo at the very end
EOF
I would expect all "echo" would execute, except for the last one that is not reached because of the "false", which is the first "2.9.3 list" whose last "2.9.2 pipeline"[*1*] yields a non-zero status, would stop the execution. Two 'test' before && may yield success or failure and they may control whether the assignment to CI_OS_NAME is done or skipped, but I expect that they do not affect how "set -e" behaves.
$ bash X
+ CI_OS_NAME=linux
+ echo different
different
+ test linux = macos
+ CI_OS_NAME=macos
+ echo same
same
+ test macos = macos
+ CI_OS_NAME=OSX
+ echo both passed
both passed
+ false $ dash X
+ CI_OS_NAME=linux
+ echo different
different
+ test linux = macos
+ CI_OS_NAME=macos
+ echo same
same
+ test macos = macos
+ CI_OS_NAME=OSX
+ echo both passed
both passed
+ false[Footnote]
*1* these two words are from the shell grammar
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09
Re: [PATCH 2/5] ci/lib: allow running in GitHub Actions
Quoted 4 lines · Junio C Hamano
... Two 'test' before && may yield
success or failure and they may control whether the assignment toCI_OS_NAMEis done or skipped, but I expect that they do not affect how "set -e" behaves.
Heh, I should just have quoted from
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09
Look for "set -e" where three bullet points are there. The relevant is the second one.
2. The -e setting shall be ignored when executing the compound list following the while, until, if, or elif reserved word, a pipeline beginning with the ! reserved word, or any command of an AND-OR list other than the last.
The test to see what the current value of CI_OS_NAME is is a command of an AND list that is not the last one (the last one being the assignment), so its failure does not allow "set -e" to abort the shell.
Quoted 13 lines · Danh Doan
Quoted 9 lines · Junio C Hamano
Johannes Schindelin <[email protected]> writes:
...Quoted 2 lines
I would like to point out that there is only one single topic that is cause for sorrow here, and it is the reftable one.
I first thought so, too, but vsbuild failures like the one in
https://github.com/git/git/runs/575116793 do not appear to be caused by that topic (6a8c1d17b8excludes reftable), so there must be somebody else that is broken.Excerpt from build log:
Quoted 7 lines
Quoted 1 line
fatal error C1083: Cannot open include file: '
config-list.h'It's from bugreport topic.
I've seen this failure in the past (when testing with pu), then I saw it disappear.I thought it was fixed during my testing for v4.
Is the issue something similar to 976aaedc (msvc: add a Makefile target to pre-generate the Visual Studio solution, 2019-07-29)?
If that is the case, perhaps something like this would help? I'll tentatively queue it on top of es/bugreport and merge the result to 'pu' to see what happens.
-- >8 --
Subject: msvc: the bugreport topic depends on a generated config-list.h file
For reasons explained in 976aaedc (msvc: add a Makefile target to pre-generate the Visual Studio solution, 2019-07-29), some build artifacts we consider non-source files cannot be generated in the Visual Studio environment, and we already have some Makefile tweaks to help Visual Studio to use generated command-list.h header file.
As this topic starts to depend on another such generated header file, config-list.h, let's do the same to it.
--- compat/vcbuild/README | 4 ++-- config.mak.uname | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-)
compat/vcbuild/README +2 -2
@@ -92,8 +92,8 @@ The Steps of Build Git with VS2008 the git operations. 3. Inside Git's directory run the command: - make command-list.h - to generate the command-list.h file needed to compile git. + make command-list.h config-list.h + to generate the header file needed to compile git. 4. Then either build Git with the GNU Make Makefile in the Git projects root
config.mak.uname +3 -3
@@ -721,9 +721,9 @@ vcxproj: echo '</Project>') >git-remote-http/LinkOrCopyRemoteHttp.targets git add -f git/LinkOrCopyBuiltins.targets git-remote-http/LinkOrCopyRemoteHttp.targets - # Add command-list.h - $(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 command-list.h - git add -f command-list.h + # Add command-list.h and config-list.h + $(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 config-list.h command-list.h + git add -f config-list.h command-list.h # Add scripts rm -f perl/perl.mak
fixing ci failure of 'pu' with the es/bugreport topic
Quoted 10 lines · Junio C Hamano
Quoted 8 lines · Danh Doan
Excerpt from build log:
Quoted 1 line
fatal error C1083: Cannot open include file: '
config-list.h'It's from bugreport topic.
I've seen this failure in the past (when testing with pu), then I saw it disappear.
Quoted 8 lines
Quoted 1 line
I thought it was fixed during my testing for v4.
Is the issue something similar to
976aaedc(msvc: add aMakefiletarget to pre-generate the Visual Studio solution, 2019-07-29)?If that is the case, perhaps something like this would help? I'll tentatively queue it on top of es/bugreport and merge the result to 'pu' to see what happens.
The build just passed: https://github.com/git/git/runs/590781044
Emily, you may need to squash in something along the line of this change to the commit in your series that starts building and using the config-list.h file (was it the first one?). I've queued mine as a follow-up "oops, it was wrong" patch, but that would not be kosher from bisectability's point of view.
But before we see a full reroll of the topic, it would be good to have a quick "looks OK" from somebody who does Windows (Dscho CC'ed).
Thanks.
Quoted 50 lines
-- >8 --
Subject: msvc: the bugreport topic depends on a generatedconfig-list.hfileFor reasons explained in
976aaedc(msvc: add aMakefiletarget to pre-generate the Visual Studio solution, 2019-07-29), some build artifacts we consider non-source files cannot be generated in the Visual Studio environment, and we already have someMakefiletweaks to help Visual Studio to use generatedcommand-list.hheader file.As this topic starts to depend on another such generated header file,
config-list.h, let's do the same to it.Signed-off-by Junio C Hamano <[email protected]>--- compat/vcbuild/README | 4 ++-- config.mak.uname | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-)compat/vcbuild/README +2
-2@@ -92,8 +92,8 @@ The Steps of Build Git with VS2008 the git operations. 3. Inside Git's directory run the command: - make command-list.h - to generate the command-list.h file needed to compile git. + make command-list.h config-list.h + to generate the header file needed to compile git. 4. Then either build Git with the GNU Make Makefile in the Git projects rootconfig.mak.uname +3
-3@@ -721,9 +721,9 @@ vcxproj: echo '</Project>') >git-remote-http/LinkOrCopyRemoteHttp.targets git add -f git/LinkOrCopyBuiltins.targets git-remote-http/LinkOrCopyRemoteHttp.targets - # Add command-list.h - $(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 command-list.h - git add -f command-list.h + # Add command-list.h and config-list.h + $(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 config-list.h command-list.h + git add -f config-list.h command-list.h # Add scripts rm -f perl/perl.mak
Re: fixing ci failure of 'pu' with the es/bugreport topic
Quoted 22 lines · Junio C Hamano
Quoted 10 lines · Junio C Hamano
Quoted 8 lines · Danh Doan
Excerpt from build log:
Quoted 1 line
fatal error C1083: Cannot open include file: '
config-list.h'It's from bugreport topic.
I've seen this failure in the past (when testing with pu), then I saw it disappear.Quoted 8 lines
Quoted 1 line
I thought it was fixed during my testing for v4.
Is the issue something similar to
976aaedc(msvc: add aMakefiletarget to pre-generate the Visual Studio solution, 2019-07-29)?If that is the case, perhaps something like this would help? I'll tentatively queue it on top of es/bugreport and merge the result to 'pu' to see what happens.
Quoted 7 lines
The build just passed: https://github.com/git/git/runs/590781044
Emily, you may need to squash in something along the line of this change to the commit in your series that starts building and using the
config-list.hfile (was it the first one?). I've queued mine as a follow-up "oops, it was wrong" patch, but that would not be kosher from bisectability's point of view.
Hm, ok. I'll send a reroll squashing this in verbatim tomorrow unless I hear otherwise from Dscho? Looks like it's indeed the first one (dd763e).
I'm curious to know how I can check this build method for myself for next time.
(If I misunderstood and I should send a reroll ASAP, please let me know; otherwise I already switched off for the evening.)
- Emily
Re: fixing ci failure of 'pu' with the es/bugreport topic
Quoted 7 lines · Emily Shaffer
Hm, ok. I'll send a reroll squashing this in verbatim tomorrow unless I hear otherwise from Dscho? Looks like it's indeed the first one (dd763e).
I'm curious to know how I can check this build method for myself for next time.(If I misunderstood and I should send a reroll ASAP, please let me know; otherwise I already switched off for the evening.)
Nah, I do not think it is all that urgent. I'd rather wait until we hear positive "yup, that's the right way to do it" (or "no, you'd do it this way instead" guidance) to waste an extra round.
Having said that, the topic won't touch 'next' with a known CI glitch whose fix ought to be straight-forward especially with help/nod from experts, and as far as I recall, there wasn't any other outstanding issues for this round, even though we may already have plans for possible follow-up enhancements, so let's not keep it hanging unnecessarily longer. Perhaps we'd all be happy if we can resolve it before the end of this week or early next week?
Thanks.
Re: fixing ci failure of 'pu' with the es/bugreport topic
In reply to Emily ShafferHi Emily,
Quoted 32 lines · Emily Shaffer
Quoted 22 lines · Junio C Hamano
Quoted 10 lines · Junio C Hamano
Quoted 8 lines · Danh Doan
Excerpt from build log:
Quoted 1 line
fatal error C1083: Cannot open include file: '
config-list.h'It's from bugreport topic.
I've seen this failure in the past (when testing with pu), then I saw it disappear.Quoted 8 lines
Quoted 1 line
I thought it was fixed during my testing for v4.
Is the issue something similar to
976aaedc(msvc: add aMakefiletarget to pre-generate the Visual Studio solution, 2019-07-29)?If that is the case, perhaps something like this would help? I'll tentatively queue it on top of es/bugreport and merge the result to 'pu' to see what happens.
Quoted 7 lines
The build just passed: https://github.com/git/git/runs/590781044
Emily, you may need to squash in something along the line of this change to the commit in your series that starts building and using the
config-list.hfile (was it the first one?). I've queued mine as a follow-up "oops, it was wrong" patch, but that would not be kosher from bisectability's point of view.
Quoted 4 lines
Hm, ok. I'll send a reroll squashing this in verbatim tomorrow unless I hear otherwise from Dscho? Looks like it's indeed the first one (dd763e).
I'm curious to know how I can check this build method for myself for next time.
Create a fork of github.com/git/git and open a pull request against it. (I believe you could also fork github.com/gitgitgadget/git and do a pull request against it, but I switched over to /git/git a while ago.) Immediately upon opening the pull request, a bunch of linux, mac, windows, and freebsd builds will be triggered with various runs of the testsuite. Has been very useful for catching issues for me before I sent them off to the list.
You can also make use of Dscho's gitgitgadget magic at that point to take care of the git send-email step for you too by commenting '/submit' in the PR, though you don't have to do that. It's perfectly fine to just open a PR for the automated testing and then close the PR and do the rest yourself. But if you leave it open and have it submit the patch emails for you, it'll track their status. I kinda like being able to go to https://github.com/git/git/pulls/newren and have it track the state of where all my open pull requests are. (You might even be able to click through some of those to see example build results)
Dscho has done some great work with his gitgitgadget work and azure builds. SZEDER Gábor also has a few builds triggered via Travis that check a few more things off the same PRs (static anlysis and whatnot). I've been very happily using them all by just opening PRs, and have appreciated the heads up of potential issues I would've otherwise caused on various platforms otherwise.
Hope that helps,
Elijah
Re: fixing ci failure of 'pu' with the es/bugreport topic
Quoted 41 lines · Elijah Newren
Hi Emily,
Quoted 32 lines · Emily Shaffer
Quoted 22 lines · Junio C Hamano
Quoted 10 lines · Junio C Hamano
Quoted 8 lines · Danh Doan
Excerpt from build log:
Quoted 1 line
fatal error C1083: Cannot open include file: '
config-list.h'It's from bugreport topic.
I've seen this failure in the past (when testing with pu), then I saw it disappear.Quoted 8 lines
Quoted 1 line
I thought it was fixed during my testing for v4.
Is the issue something similar to
976aaedc(msvc: add aMakefiletarget to pre-generate the Visual Studio solution, 2019-07-29)?If that is the case, perhaps something like this would help? I'll tentatively queue it on top of es/bugreport and merge the result to 'pu' to see what happens.
Quoted 7 lines
The build just passed: https://github.com/git/git/runs/590781044
Emily, you may need to squash in something along the line of this change to the commit in your series that starts building and using the
config-list.hfile (was it the first one?). I've queued mine as a follow-up "oops, it was wrong" patch, but that would not be kosher from bisectability's point of view.Quoted 4 lines
Hm, ok. I'll send a reroll squashing this in verbatim tomorrow unless I hear otherwise from Dscho? Looks like it's indeed the first one (dd763e).
I'm curious to know how I can check this build method for myself for next time.
Quoted 7 lines
Create a fork of github.com/git/git and open a pull request against it. (I believe you could also fork github.com/gitgitgadget/git and do a pull request against it, but I switched over to /git/git a while ago.) Immediately upon opening the pull request, a bunch of linux, mac, windows, and freebsd builds will be triggered with various runs of the testsuite. Has been very useful for catching issues for me before I sent them off to the list.
I did before I sent this iteration, and it passed:
https://github.com/gitgitgadget/git/pull/573
That's why I'm confused :) Did I do something differently? I don't use GGG to send the emails, but I do use it to run CI checks.
- Emily
Re: fixing ci failure of 'pu' with the es/bugreport topic
Quoted 10 lines · Emily Shaffer
Quoted 8 lines · Elijah Newren
Create a fork of github.com/git/git and open a pull request against it. (I believe you could also fork github.com/gitgitgadget/git and do a pull request against it, but I switched over to /git/git a while ago.) Immediately upon opening the pull request, a bunch of linux, mac, windows, and freebsd builds will be triggered with various runs of the testsuite. Has been very useful for catching issues for me before I sent them off to the list.
Quoted 5 lines
I did before I sent this iteration, and it passed:
https://github.com/gitgitgadget/git/pull/573That's why I'm confused :) Did I do something differently? I don't use GGG to send the emails, but I do use it to run CI checks.
Comparing the list of "checks" revealed by clicking "Show all checks" there, with the list of "Actions" with recent tip of 'pu', say, https://github.com/git/git/actions/runs/79416884, I notice that the former does not have vs-build.
Also, the former seems to be on "Azure Pipelines" (e.g. [*1*] which is "Windows build" among the "checks" on the list), while the latter is "Github Actions" (e.g. [*2*], among which exists "VS-build" that seems to be missing from the former).
The latter is coming from having the dd/ci-swap-azure-pipelines-with-github-actions topic and other two topics that it depends on, which are right now only in 'pu'. As we'd like to advance the Github Actions CI support to 'next', I've been looking at the failures to it caused by individual topics (and right now, we know of two topics, one is the bugreport and the other is reftable) to make sure these other topics can enter 'next'.
Thanks.
[References]
*1* https://github.com/gitgitgadget/git/pull/573/checks?check_run_id=565642291
*2* https://github.com/git/git/runs/590781044?check_suite_focus=true
Re: fixing ci failure of 'pu' with the es/bugreport topic
In reply to Elijah NewrenQuoted 40 lines · Elijah Newren
Hi Emily,
Quoted 32 lines · Emily Shaffer
Quoted 22 lines · Junio C Hamano
Quoted 10 lines · Junio C Hamano
Quoted 8 lines · Danh Doan
Excerpt from build log:
Quoted 1 line
fatal error C1083: Cannot open include file: '
config-list.h'It's from bugreport topic.
I've seen this failure in the past (when testing with pu), then I saw it disappear.Quoted 8 lines
Quoted 1 line
I thought it was fixed during my testing for v4.
Is the issue something similar to
976aaedc(msvc: add aMakefiletarget to pre-generate the Visual Studio solution, 2019-07-29)?If that is the case, perhaps something like this would help? I'll tentatively queue it on top of es/bugreport and merge the result to 'pu' to see what happens.
Quoted 7 lines
The build just passed: https://github.com/git/git/runs/590781044
Emily, you may need to squash in something along the line of this change to the commit in your series that starts building and using the
config-list.hfile (was it the first one?). I've queued mine as a follow-up "oops, it was wrong" patch, but that would not be kosher from bisectability's point of view.Quoted 4 lines
Hm, ok. I'll send a reroll squashing this in verbatim tomorrow unless I hear otherwise from Dscho? Looks like it's indeed the first one (dd763e).
I'm curious to know how I can check this build method for myself for next time.
Quoted 7 lines
Create a fork of github.com/git/git and open a pull request against it. (I believe you could also fork github.com/gitgitgadget/git and do a pull request against it, but I switched over to /git/git a while ago.) Immediately upon opening the pull request, a bunch of linux, mac, windows, and freebsd builds will be triggered with various runs of the testsuite. Has been very useful for catching issues for me before I sent them off to the list.
For the time being, open a Github PR will trigger Azure Pipelines to check various things with both Linux, macOS, and Windows. This Azure thing doesn't have that vs-build target, yet.
We're moving to Github Actions. When that topic graduate to master, we can simply branch out from master and push to our fork in GitHub, it will run automatically. No need to create a PR on git.git anymore
To check that vs-build target for the time being by merging dd/ci-swap-azure-pipelines-with-github-actions
and push to your GitHub fork.
Signature · 1 line
Danh
Re: fixing ci failure of 'pu' with the es/bugreport topic
Hi Emily,
Quoted 16 lines · Danh Doan
Quoted 6 lines · Elijah Newren
Quoted 2 lines · Emily Shaffer
I'm curious to know how I can check this build method for myself for next time.
Quoted 7 lines
Create a fork of github.com/git/git and open a pull request against it. (I believe you could also fork github.com/gitgitgadget/git and do a pull request against it, but I switched over to /git/git a while ago.) Immediately upon opening the pull request, a bunch of linux, mac, windows, and freebsd builds will be triggered with various runs of the testsuite. Has been very useful for catching issues for me before I sent them off to the list.
Quoted 1 line
For the time being, open a Github PR will trigger Azure Pipelines to
Please spell it with an upper-case H: there is no th sound in GitHub.
Quoted 2 lines
check various things with both Linux, macOS, and Windows. This Azure thing doesn't have that vs-build target, yet.
More concretely, you will want to open a PR at https://github.com/git/git,
not at https://github.com/gitgitgadget/git.
For reasons (having to do with Junio's practice to base branches on older commits, where azure-pipelines.yml either does not exist, or needs changes to pass), the latter runs an Azure Pipeline on Pull Requests which is based _not_ on azure-pipelines.yml, but is essentially a manual re-implementation of it that does _not_ use YAML (but is revisioned separately), with manual patches for all kinds of issues on top that have made it into core Git's master.
However, in your case I would strongly advise to simply use a throw-away branch, merge in the GitHub workflow patches by Danh and myself (as described in the quoted text below), and push it to your fork on GitHub. That will execute a workflow run that will show up at
https://github.com/nasamuffin/git/actions/new.
Ciao,
Dscho
Quoted 13 lines
We're moving to Github Actions. When that topic graduate to master, we can simply branch out from master and push to our fork in GitHub, it will run automatically. No need to create a PR on git.git anymore
To check that vs-build target for the time being by merging dd/ci-swap-azure-pipelines-with-github-actions
and push to your GitHub fork.--
Danh
Hi Junio,
Quoted 21 lines · Junio C Hamano
Quoted 19 lines · Danh Doan
Quoted 9 lines · Junio C Hamano
Johannes Schindelin <[email protected]> writes:
...Quoted 2 lines
I would like to point out that there is only one single topic that is cause for sorrow here, and it is the reftable one.
I first thought so, too, but vsbuild failures like the one in
https://github.com/git/git/runs/575116793 do not appear to be caused by that topic (6a8c1d17b8excludes reftable), so there must be somebody else that is broken.Excerpt from build log:
Quoted 1 line
fatal error C1083: Cannot open include file: '
config-list.h'It's from bugreport topic.
I've seen this failure in the past (when testing with pu), then I saw it disappear.
Quoted 8 lines
Quoted 1 line
I thought it was fixed during my testing for v4.
Is the issue something similar to
976aaedc(msvc: add aMakefiletarget to pre-generate the Visual Studio solution, 2019-07-29)?If that is the case, perhaps something like this would help? I'll tentatively queue it on top of es/bugreport and merge the result to 'pu' to see what happens.
This patch is morally equivalent to (albeit a bit more complete than) the patch I suggested in my mail to Emily that I sent on February 26th:
https://lore.kernel.org/git/[email protected]/
So yes, I am very much in favor of that patch.
Thanks,
Dscho
Quoted 53 lines
-- >8 --
Subject: msvc: the bugreport topic depends on a generatedconfig-list.hfileFor reasons explained in
976aaedc(msvc: add aMakefiletarget to pre-generate the Visual Studio solution, 2019-07-29), some build artifacts we consider non-source files cannot be generated in the Visual Studio environment, and we already have someMakefiletweaks to help Visual Studio to use generatedcommand-list.hheader file.As this topic starts to depend on another such generated header file,
config-list.h, let's do the same to it.Signed-off-by Junio C Hamano <[email protected]>--- compat/vcbuild/README | 4 ++-- config.mak.uname | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-)compat/vcbuild/README +2
-2@@ -92,8 +92,8 @@ The Steps of Build Git with VS2008 the git operations. 3. Inside Git's directory run the command: - make command-list.h - to generate the command-list.h file needed to compile git. + make command-list.h config-list.h + to generate the header file needed to compile git. 4. Then either build Git with the GNU Make Makefile in the Git projects rootconfig.mak.uname +3
-3@@ -721,9 +721,9 @@ vcxproj: echo '</Project>') >git-remote-http/LinkOrCopyRemoteHttp.targets git add -f git/LinkOrCopyBuiltins.targets git-remote-http/LinkOrCopyRemoteHttp.targets - # Add command-list.h - $(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 command-list.h - git add -f command-list.h + # Add command-list.h and config-list.h + $(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 config-list.h command-list.h + git add -f config-list.h command-list.h # Add scripts rm -f perl/perl.mak
Quoted 3 lines · Johannes Schindelin
This patch is morally equivalent to (albeit a bit more complete than) the patch I suggested in my mail to Emily that I sent on February 26th:
https://lore.kernel.org/git/[email protected]/
Ah, that was why it looked vaguely familiar. Figures.
Quoted 1 line
So yes, I am very much in favor of that patch.
Thanks.
Re: [PATCH v5 10/12] tests: when run in Bash, annotate test failures with file name/line number
In reply to Đoàn Trần Công Danht/test-lib.sh die () { · line 663
Quoted 12 lines · Đoàn Trần Công Danh
t/test-lib.sh +4
-0@@ -657,6 +657,18 @@ die () { fi } +file_lineno () { + test -z "$GIT_TEST_FRAMEWORK_SELFTEST" && test -n "$BASH" || return 0 + local i + for i in ${!BASH_SOURCE[*]}
this line breaks with NetBSD's sh (and probably other POSIX complaint shells)
the Coding Guidelines mention "no shell arrays" and while the tests are more relaxed against that rule, usually workarounds are needed, as it is shown by:5826b7b595 (test-lib: check Bash version for '-x' without using shell arrays, 2019-01-03)
Carlo
Re: [PATCH v5 10/12] tests: when run in Bash, annotate test failures with file name/line number
Quoted 16 lines · Carlo Marcelo Arenas Belón
t/test-lib.sh die () { · line 663
Quoted 12 lines · Đoàn Trần Công Danh
t/test-lib.sh +4
-0@@ -657,6 +657,18 @@ die () { fi } +file_lineno () { + test -z "$GIT_TEST_FRAMEWORK_SELFTEST" && test -n "$BASH" || return 0 + local i + for i in ${!BASH_SOURCE[*]}this line breaks with NetBSD's sh (and probably other POSIX complaint shells)
Quoted 4 lines
the Coding Guidelines mention "no shell arrays" and while the tests are more relaxed against that rule, usually workarounds are needed, as it is shown by:
5826b7b595(test-lib: check Bash version for '-x' without using shell arrays, 2019-01-03)
This function will be called in CI only, and when the the shell used is bash, to annotate the faulty line.
We have a test guarding it already.
So, I think it's fine.
Signature · 1 line
Danh
Re: [PATCH v5 10/12] tests: when run in Bash, annotate test failures with file name/line number
Quoted 11 lines · Danh Doan
Quoted 7 lines
t/test-lib.sh die () { · line 663
Quoted 4 lines
t/test-lib.sh +4
-0+file_lineno () { + test -z "$GIT_TEST_FRAMEWORK_SELFTEST" && test -n "$BASH" || return 0 + local i + for i in ${!BASH_SOURCE[*]}this line breaks with NetBSD's sh (and probably other POSIX complaint shells)
...This function will be called in CI only, and when the the shell used is bash, to annotate the faulty line.
We have a test guarding it already.
Carlo, you said "this line breaks"---implying that you actually saw breakage when running our tests on affected platforms. Is that the case?
It is entirely possible that some shell implementations may try to parse the contents of the shell function and trigger a syntax error on that line, even before evaluationg "are we running tests and do we have BASH set?", so I can believe "we have a test guarding", if the mention refers to the first line of the helper function, is insufficient and does break with some shells.
Re: [PATCH v5 10/12] tests: when run in Bash, annotate test failures with file name/line number
In reply to Danh DoanQuoted 17 lines · Danh Doan
Quoted 16 lines · Carlo Marcelo Arenas Belón
t/test-lib.sh die () { · line 663
Quoted 12 lines · Đoàn Trần Công Danh
t/test-lib.sh +4
-0@@ -657,6 +657,18 @@ die () { fi } +file_lineno () { + test -z "$GIT_TEST_FRAMEWORK_SELFTEST" && test -n "$BASH" || return 0 + local i + for i in ${!BASH_SOURCE[*]}this line breaks with NetBSD's sh (and probably other POSIX complaint shells)
Quoted 7 lines
Quoted 4 lines
the Coding Guidelines mention "no shell arrays" and while the tests are more relaxed against that rule, usually workarounds are needed, as it is shown by:
5826b7b595(test-lib: check Bash version for '-x' without using shell arrays, 2019-01-03)This function will be called in CI only, and when the the shell used is bash, to annotate the faulty line.
that part is correct, but the test is meant to be able to also run outside of CI (specially considering that there are several systems that are not covered by it yet)
Quoted 2 lines
We have a test guarding it already.
So, I think it's fine.
no; because the shell has to parse the whole script (regardless if it executes the line or not), so if you checkout master in a NetBSD machine and try to run the tests you get :
** t0000-basic.sh ***
./test-lib.sh: 681: Syntax error: Bad substitution gmake[2]: *** [Makefile:56: t0000-basic.sh] Error 2
the coding guidelines warns specifically against using arrays for this reason, and therefore it will be ideal that we follow such guidelines by rewriting the code not to use shell arrays (which is not a POSIX sh feature) as it affects not only NetBSD but anyone else not running bash.
but there is an "easy" workaround as shown by the reference I sent, so a naive and very ugly "fix" would be:
--- >8 ---
t/test-lib.sh +2 -0
@@ -677,6 +677,7 @@ die () { file_lineno () { test -z "$GIT_TEST_FRAMEWORK_SELFTEST" && test -n "$BASH" || return 0 + eval ' local i for i in ${!BASH_SOURCE[*]} do
@@ -685,6 +686,7 @@ file_lineno () { *,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:${BASH_LINENO[$(($i-1))]}: ${1+$1: }"; return;; esac done + ' } GIT_EXIT_OK=
Re: [PATCH v5 10/12] tests: when run in Bash, annotate test failures with file name/line number
In reply to Junio C HamanoQuoted 9 lines · Junio C Hamano
Quoted 7 lines · Danh Doan
Quoted 7 lines
Review
Quoted 4 lines
(quoted diff) +4
-0+file_lineno () { + test -z "$GIT_TEST_FRAMEWORK_SELFTEST" && test -n "$BASH" || return 0 + local i + for i in ${!BASH_SOURCE[*]}this line breaks with NetBSD's sh (and probably other POSIX complaint shells)
...
Quoted 8 lines
Quoted 4 lines
This function will be called in CI only, and when the the shell used is bash, to annotate the faulty line.
We have a test guarding it already.
Carlo, you said "this line breaks"---implying that you actually saw breakage when running our tests on affected platforms. Is that the case?
correct. and as described in the followup email[1]
Quoted 6 lines
It is entirely possible that some shell implementations may try to parse the contents of the shell function and trigger a syntax error on that line, even before evaluationg "are we running tests and do we have BASH set?", so I can believe "we have a test guarding", if the mention refers to the first line of the helper function, is insufficient and does break with some shells.
it would seem that a POSIX sh version would be enough for what we need and as shown after the scissors.
got away also with the IMHO unnecesary rewriting of the script path and hardcoding of "t/" as it looks cleaner.
Carlo
--- >8 ---
Subject: [PATCH] tests: make file_lineno() POSIX sh compatible
662f9cf154 (tests: when run in Bash, annotate test failures with file name/line number, 2020-04-11), introduces a way to report the exact location where an error was triggered.
The implementation was using bash specific syntax and was restricted with a guard so it will only be used with bash, but NetBSD sh will still have to parse the function and the use of arrays triggers a syntax error as shown by:
** t0000-basic.sh ***
./test-lib.sh: 681: Syntax error: Bad substitution
Instead of guarding the code, use a simpler version written in POSIX sh as per the Coding Guidelines.
--- t/test-lib.sh | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)
t/test-lib.sh +3 -9
@@ -676,15 +676,9 @@ die () { } file_lineno () { - test -z "$GIT_TEST_FRAMEWORK_SELFTEST" && test -n "$BASH" || return 0 - local i - for i in ${!BASH_SOURCE[*]} - do - case $i,"${BASH_SOURCE[$i]##*/}" in - 0,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:$LINENO: ${1+$1: }"; return;; - *,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:${BASH_LINENO[$(($i-1))]}: ${1+$1: }"; return;; - esac - done + test -z "$GIT_TEST_FRAMEWORK_SELFTEST" || return 0 + + echo "$0:$LINENO: ${1+$1: }" } GIT_EXIT_OK=
Signature · 1 line
2.26.2.686.gfaf46a9ccd
Re: [PATCH v5 10/12] tests: when run in Bash, annotate test failures with file name/line number
Hi Carlo,
Quoted 10 lines · Carlo Marcelo Arenas Belón
t/test-lib.sh +0
-2@@ -676,15 +676,9 @@ die () { } file_lineno () { - test -z "$GIT_TEST_FRAMEWORK_SELFTEST" && test -n "$BASH" || return 0 - local i
t/test-lib.sh die () { · line 681
Quoted 10 lines
t/test-lib.sh +3
-7- for i in ${!BASH_SOURCE[*]} - do - case $i,"${BASH_SOURCE[$i]##*/}" in - 0,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:$LINENO: ${1+$1: }"; return;; - *,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:${BASH_LINENO[$(($i-1))]}: ${1+$1: }"; return;; - esac - done + test -z "$GIT_TEST_FRAMEWORK_SELFTEST" || return 0 + + echo "$0:$LINENO: ${1+$1: }"
That suppresses the error all right.
Unfortunately, it completely breaks the feature. At that point, $LINENO is either unset (e.g. in dash) or it contains the number of the line _containing the echo. That is totally useless information at this point, we want the line number of the caller.
Try this, for example:
```
#!/bin/sh
file_lineno () {
echo "$0:$LINENO: hello"
}
file_lineno
```When you run this, it will print 4. What we want is 7.
Even worse, as $0 does _not_ contain test-lib.sh at this point, the printed information is totally bogus.
So we will have to use that eval unless we find a better solution.
Ciao,
Dscho
Re: [PATCH v5 10/12] tests: when run in Bash, annotate test failures with file name/line number
Quoted 24 lines · Johannes Schindelin
Quoted 10 lines · Carlo Marcelo Arenas Belón
t/test-lib.sh +0
-2@@ -676,15 +676,9 @@ die () { } file_lineno () { - test -z "$GIT_TEST_FRAMEWORK_SELFTEST" && test -n "$BASH" || return 0 - local it/test-lib.sh die () { · line 681
Quoted 10 lines
t/test-lib.sh +3
-7- for i in ${!BASH_SOURCE[*]} - do - case $i,"${BASH_SOURCE[$i]##*/}" in - 0,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:$LINENO: ${1+$1: }"; return;; - *,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:${BASH_LINENO[$(($i-1))]}: ${1+$1: }"; return;; - esac - done + test -z "$GIT_TEST_FRAMEWORK_SELFTEST" || return 0 + + echo "$0:$LINENO: ${1+$1: }"That suppresses the error all right.
Quoted 4 lines
Unfortunately, it completely breaks the feature. At that point,
$LINENOis either unset (e.g. indash) or it contains the number of the line_containingtheecho. That is totally useless information at this point, we want the line number of the caller.
that seems like a bug in dash, which NetBSD sh doesn't have, as LINENO wouldn't be unset.
Quoted 12 lines
Try this, for example:
``` #!/bin/sh file_lineno () { echo "$0:$LINENO: hello" } file_lineno ```
Quoted 1 line
When you run this, it will print
4. What we want is7.
so you need instead :
```
#!/bin/sh
file_lineno () {
echo "$0:$1: hello"
}
file_lineno $LINENO
```Quoted 2 lines
Even worse, as
$0does _not_ containtest-lib.shat this point, the printed information is totally bogus.
not sure I understand what you mean here, at least when runnning with bash the original code shows $0 correctly as t????.sh when I tried to force a failure to test.
Carlo
Re: [PATCH v5 10/12] tests: when run in Bash, annotate test failures with file name/line number
Hi Carlo,
Quoted 25 lines · Carlo Marcelo Arenas Belón
Quoted 24 lines · Johannes Schindelin
Quoted 10 lines · Carlo Marcelo Arenas Belón
t/test-lib.sh +0
-2@@ -676,15 +676,9 @@ die () { } file_lineno () { - test -z "$GIT_TEST_FRAMEWORK_SELFTEST" && test -n "$BASH" || return 0 - local it/test-lib.sh die () { · line 681
Quoted 10 lines
t/test-lib.sh +3
-7- for i in ${!BASH_SOURCE[*]} - do - case $i,"${BASH_SOURCE[$i]##*/}" in - 0,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:$LINENO: ${1+$1: }"; return;; - *,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:${BASH_LINENO[$(($i-1))]}: ${1+$1: }"; return;; - esac - done + test -z "$GIT_TEST_FRAMEWORK_SELFTEST" || return 0 + + echo "$0:$LINENO: ${1+$1: }"That suppresses the error all right.
Quoted 7 lines
Quoted 4 lines
Unfortunately, it completely breaks the feature. At that point,
$LINENOis either unset (e.g. indash) or it contains the number of the line_containingtheecho. That is totally useless information at this point, we want the line number of the caller.that seems like a bug in dash, which NetBSD sh doesn't have, as LINENO wouldn't be unset.
And your patch makes this a real problem as you no longer skip the echo in the non-Bash case.
That's what I wanted to point out: this needs to be fixed.
Quoted 16 lines
Quoted 12 lines
Try this, for example:
``` #!/bin/sh file_lineno () { echo "$0:$LINENO: hello" } file_lineno ```Quoted 1 line
When you run this, it will print
4. What we want is7.so you need instead :
Quoted 8 lines
```
#!/bin/shfile_lineno () { echo "$0:$1: hello" }
file_lineno$LINENO
No.
Please understand what the intention of the current (Bash-specific) code
is: in case that there is a failure, it needs to print out the file and line number of the actual statement that caused that problem.
Take this example:
test_expect_success 'For Carlo' ' false '
Obviously, this will fail, and it will print out an error message. What we want here is that the file that contains that test_expect_success and the actual line number of this call are printed.
Your suggestion would be to clutter each and every such call with $LINENO, like so:
test_expect_success $LINENO 'For Carlo' '
I don't think that is sensible an idea.
Besides, it would _still_ not work, for parameterized functions that call test_expect_success and that are defined in lib-<whatever>.sh.
Example:
# in t/lib-whatever.sh
super_repetitive_test () {
test_expect_success "first $1" '
...
'test_expect_success "second $1" ' ... ' ... test_expect_success "gazillionth $1" ' ... ' } # in t/t1234-actual-caller.sh . lib-whatever.sh super_repetitive_test hello super_repetitive_test world super_repetitive_test good-bye super_repetitive_test dreams
We will not want to print out the line number of the call in t/lib-whatever.sh. That is what your proposal would amount to, unless you want to clutter even the super_repetitive_test calles, which would fly even less.
Quoted 6 lines
Quoted 2 lines
Even worse, as
$0does _not_ containtest-lib.shat this point, the printed information is totally bogus.not sure I understand what you mean here, at least when runnning with bash the original code shows $0 correctly as t????.sh when I tried to force a failure to test.
Yes, $0 shows the correct file. But since the line number that is printed is from a totally different file, the combination <file>:<lineno> is completely and utterly bogus. Misleading. Less than useless.
Ciao,
Dscho
Re: [PATCH v5 10/12] tests: when run in Bash, annotate test failures with file name/line number
In reply to Carlo Marcelo Arenas BelónQuoted 7 lines · Carlo Marcelo Arenas Belón
Quoted 4 lines
Unfortunately, it completely breaks the feature. At that point,
$LINENOis either unset (e.g. indash) or it contains the number of the line_containingtheecho. That is totally useless information at this point, we want the line number of the caller.that seems like a bug in dash, which NetBSD sh doesn't have, as LINENO wouldn't be unset.
I thought you already gave a perfectly well working eval '...' approach. Was it insufficient?
Re: [PATCH v5 10/12] tests: when run in Bash, annotate test failures with file name/line number
In reply to Johannes SchindelinQuoted 31 lines · Johannes Schindelin
Quoted 30 lines · Carlo Marcelo Arenas Belón
Quoted 28 lines · Johannes Schindelin
Quoted 10 lines · Carlo Marcelo Arenas Belón
t/test-lib.sh +0
-2@@ -676,15 +676,9 @@ die () { } file_lineno () { - test -z "$GIT_TEST_FRAMEWORK_SELFTEST" && test -n "$BASH" || return 0 - local it/test-lib.sh die () { · line 681
Quoted 10 lines
t/test-lib.sh +3
-7- for i in ${!BASH_SOURCE[*]} - do - case $i,"${BASH_SOURCE[$i]##*/}" in - 0,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:$LINENO: ${1+$1: }"; return;; - *,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:${BASH_LINENO[$(($i-1))]}: ${1+$1: }"; return;; - esac - done + test -z "$GIT_TEST_FRAMEWORK_SELFTEST" || return 0 + + echo "$0:$LINENO: ${1+$1: }"That suppresses the error all right.
Unfortunately, it completely breaks the feature. At that point,
$LINENOis either unset (e.g. indash) or it contains the number of the line_containingtheecho. That is totally useless information at this point, we want the line number of the caller.
Quoted 5 lines
Quoted 2 lines
that seems like a bug in dash, which NetBSD sh doesn't have, as LINENO wouldn't be unset.
And your patch makes this a real problem as you no longer skip the
echoin the non-Bash case.
FWIW, at least NetBSD's dash (0.5.10.2) always sets LINENO
Quoted 23 lines
Quoted 23 lines
Quoted 12 lines
Try this, for example:
``` #!/bin/sh file_lineno () { echo "$0:$LINENO: hello" } file_lineno ```Quoted 1 line
When you run this, it will print
4. What we want is7.so you need instead :
```
#!/bin/shfile_lineno () { echo "$0:$1: hello" }
Quoted 7 lines
Quoted 1 line
file_lineno$LINENONo.
Please understand what the intention of the current (Bash-specific) code
is: in case that there is a failure, it needs to print out the file and line number of the actual statement that caused that problem.
that is what I got from the commmit message but then I was puzzled not to find the line inside the test case that faile but instead the one where the last test function was located.
eitherway, I was careless to send the previous version without checking more deeply; indeed the use of LINENO in the original code was part of that confusion and is therefore gone in this new one.
Quoted 47 lines
Take this example:
test_expect_success 'For Carlo' ' false 'Obviously, this will fail, and it will print out an error message. What we want here is that the file that contains that
test_expect_successand the actual line number of this call are printed.Your suggestion would be to clutter each and every such call with
$LINENO, like so:
test_expect_success$LINENO 'For Carlo' 'I don't think that is sensible an idea.
Besides, it would _still_ not work, for parameterized functions that call
test_expect_successand that are defined inlib-<whatever>.sh.Example:
# in t/lib-whatever.sh super_repetitive_test () { test_expect_success "first $1" ' ... 'test_expect_success "second $1" ' ... ' ... test_expect_success "gazillionth $1" ' ... ' } # in t/t1234-actual-caller.sh . lib-whatever.sh super_repetitive_test hello super_repetitive_test world super_repetitive_test good-bye super_repetitive_test dreams
Quoted 4 lines
We will not want to print out the line number of the call in
t/lib-whatever.sh. That is what your proposal would amount to, unless you want to clutter even thesuper_repetitive_testcalles, which would fly even less.
that was very useful to understand better the constrain that was referred in the commit message and that forced to build a bash specific solution.
I have to admit too, I was confused by the code, but with the clear guidance you provided I think the new proposed[1] version is also clearer, and goes away with the problem that triggered this whole discussion.
Carlo
Re: [PATCH v5 10/12] tests: when run in Bash, annotate test failures with file name/line number
In reply to Đoàn Trần Công DanhHi,
Le 10/04/2020 à 19:18, Đoàn Trần Công Danh a écrit :
Quoted 40 lines
From: Johannes Schindelin <[email protected]>
When a test fails, it is nice to see where the corresponding code lives in the worktree. Sadly, it seems that only Bash allows us to infer this information. Let's do it when we detect that we're running in a Bash.
This will come in handy in the next commit, where we teach the GitHub Actions workflow to annotate failed test runs with this information.
Signed-off-by Johannes Schindelin <[email protected]>Signed-off-by Đoàn Trần Công Danh <[email protected]>--- t/test-lib.sh | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)t/test-lib.sh +12
-0@@ -657,6 +657,18 @@ die () { fi } +file_lineno () { + test -z "$GIT_TEST_FRAMEWORK_SELFTEST" && test -n "$BASH" || return 0 + local i + for i in ${!BASH_SOURCE[*]} + do + case $i,"${BASH_SOURCE[$i]##*/}" in + 0,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:$LINENO: ${1+$1: }"; return;; + *,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:${BASH_LINENO[$(($i-1))]}: ${1+$1: }"; return;; + esac + done +} + GIT_EXIT_OK= trap 'die' EXIT # Disable '-x' tracing, because with some shells, notably dash, it
t/test-lib.sh test_failure_ () { · line 720
Quoted 8 lines
t/test-lib.sh +1
-1@@ -702,7 +714,7 @@ test_failure_ () { write_junit_xml_testcase "$1" " $junit_insert" fi test_failure=$(($test_failure + 1)) - say_color error "not ok $test_count - $1" + say_color error "$(file_lineno error)not ok $test_count - $1" shift printf '%s\n' "$*" | sed -e 's/^/# /' test "$immediate" = "" || { finalize_junit_xml; GIT_EXIT_OK=t; exit 1; }
This violates the TAP specification, it seems. Without this patch, when a test fails, `prove' shows clearly which test(s) broke, and how many failed:
Quoted 5 lines
Test Summary Report
-------------------t0001-init.sh(Wstat: 256 Tests: 45 Failed: 1)
Failed test: 2
Non-zero exit status: 1
With this patch, it shows this:
Quoted 11 lines
Test Summary Report
-------------------t0001-init.sh(Wstat: 256 Tests: 44 Failed: 0)
Non-zero exit status: 1Parse errors: Tests out of sequence. Found (3) but expected (2) Tests out of sequence. Found (4) but expected (3) Tests out of sequence. Found (5) but expected (4) Tests out of sequence. Found (6) but expected (5) Tests out of sequence. Found (7) but expected (6)Displayed the first 5 of 44 TAP syntax errors.
Re-run prove with the -p option to see them all.
Not nice to see clearly which tests failed: we do not see all failed tests (what if a test failed after the 7th test?), and the counter displays 0.
This happens on my machine, as well as on github.
Cheers,
Alban
RFC PATCH 1/1 t: move metadata into TAP test description
662f9cf154 (tests: when run in Bash, annotate test failures with file name/line number, 2020-04-11) adds metadata to the TAP output to help identify the location of the failed test, but does it in a way that break the TAP format and therefore confuses prove.
Move the metadata to the description to workaround the issue and change the regex from 676eb0c1ce (ci: add a problem matcher for GitHub Actions, 2020-04-11) to match.
--- ci/git-problem-matcher.json | 10 +++++----- t/test-lib.sh | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-)
ci/git-problem-matcher.json +5 -5
@@ -4,11 +4,11 @@ "owner": "git-test-suite", "pattern": [ { - "regexp": "^([^ :]+\\.sh):(\\d+): (error|warning|info):\\s+(.*)$", - "file": 1, - "line": 2, - "severity": 3, - "message": 4 + "regexp": "^(.*)(error|warning|info):\\([^ :]+\\.sh):(\\d+)\\)$", + "file": 3, + "line": 4, + "severity": 2, + "message": 1 } ] }
t/test-lib.sh +3 -3
@@ -682,8 +682,8 @@ file_lineno () { for i in ${!BASH_SOURCE[*]} do case $i,"${BASH_SOURCE[$i]##*/}" in - 0,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:$LINENO: ${1+$1: }"; return;; - *,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:${BASH_LINENO[$(($i-1))]}: ${1+$1: }"; return;; + 0,t[0-9]*.sh) echo "(${1+$1:}t/${BASH_SOURCE[$i]}:$LINENO)"; return;; + *,t[0-9]*.sh) echo "(${1+$1:}t/${BASH_SOURCE[$i]}:${BASH_LINENO[$(($i-1))]})"; return;; esac done '
@@ -734,7 +734,7 @@ test_failure_ () { write_junit_xml_testcase "$1" " $junit_insert" fi test_failure=$(($test_failure + 1)) - say_color error "$(file_lineno error)not ok $test_count - $1" + say_color error "not ok $test_count - $1$(file_lineno error)" shift printf '%s\n' "$*" | sed -e 's/^/# /' test "$immediate" = "" || { finalize_junit_xml; GIT_EXIT_OK=t; exit 1; }
Signature · 1 line
2.26.2.812.g046d49d455
Re: [RFC PATCH] t: move metadata into TAP test description
Quoted 10 lines · Carlo Marcelo Arenas Belón
662f9cf154(tests: when run in Bash, annotate test failures with file name/line number, 2020-04-11) adds metadata to the TAP output to help identify the location of the failed test, but does it in a way that break the TAP format and therefore confuses prove.Move the metadata to the description to workaround the issue and change the regex from
676eb0c1ce(ci: add a problem matcher for GitHub Actions, 2020-04-11) to match.Reported-by Alban Gruin <[email protected]>
Missing sign-off.
Re: [PATCH v5 10/12] tests: when run in Bash, annotate test failures with file name/line number
In reply to Alban GruinQuoted 2 lines · Alban Gruin
This happens on my machine, as well as on github.
could you point me to a failed github report?, also would the patch I posted earlier (better with the following on top) help?
Carlo
-- >8 --
ci/git-problem-matcher.json +1 -1
@@ -4,7 +4,7 @@ "owner": "git-test-suite", "pattern": [ { - "regexp": "^(.*)(error|warning|info):\\([^ :]+\\.sh):(\\d+)\\)$", + "regexp": "^(.*)\\((error|warning|info):([^ :]+\\.sh):(\\d+)\\)$", "file": 3, "line": 4, "severity": 2,
Re: [PATCH v5 10/12] tests: when run in Bash, annotate test failures with file name/line number
Hi Carlo,
Le 15/05/2020 à 17:28, Carlo Marcelo Arenas Belón a écrit :
Quoted 6 lines · Alban Gruin
Quoted 2 lines
This happens on my machine, as well as on github.
could you point me to a failed github report?,
Here you go:
https://github.com/agrn/git/runs/678028019?check_suite_focus=true
I hope you can read it you're logged in.
Quoted 2 lines
also would the patch I posted
earlier (better with the following on top) help?
Yes, it does, thank you! I have a few comments about it, I'll send them in a minute.
Cheers,
Alban
Re: [RFC PATCH] t: move metadata into TAP test description
In reply to Carlo Marcelo Arenas BelónLe 15/05/2020 à 17:00, Carlo Marcelo Arenas Belón a écrit :
Quoted 41 lines
662f9cf154(tests: when run in Bash, annotate test failures with file name/line number, 2020-04-11) adds metadata to the TAP output to help identify the location of the failed test, but does it in a way that break the TAP format and therefore confuses prove.Move the metadata to the description to workaround the issue and change the regex from
676eb0c1ce(ci: add a problem matcher for GitHub Actions, 2020-04-11) to match.Reported-by Alban Gruin <[email protected]>--- ci/git-problem-matcher.json | 10 +++++----- t/test-lib.sh | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-)ci/git-problem-matcher.json +5
-5@@ -4,11 +4,11 @@ "owner": "git-test-suite", "pattern": [ { - "regexp": "^([^ :]+\\.sh):(\\d+): (error|warning|info):\\s+(.*)$", - "file": 1, - "line": 2, - "severity": 3, - "message": 4 + "regexp": "^(.*)(error|warning|info):\\([^ :]+\\.sh):(\\d+)\\)$", + "file": 3, + "line": 4, + "severity": 2, + "message": 1 } ] }t/test-lib.sh +0
-0
t/test-lib.sh file_lineno () { · line 686
Quoted 7 lines
t/test-lib.sh +2
-2@@ -682,8 +682,8 @@ file_lineno () { for i in ${!BASH_SOURCE[*]} do case $i,"${BASH_SOURCE[$i]##*/}" in - 0,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:$LINENO: ${1+$1: }"; return;; - *,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:${BASH_LINENO[$(($i-1))]}: ${1+$1: }"; return;; + 0,t[0-9]*.sh) echo "(${1+$1:}t/${BASH_SOURCE[$i]}:$LINENO)"; return;; + *,t[0-9]*.sh) echo "(${1+$1:}t/${BASH_SOURCE[$i]}:${BASH_LINENO[$(($i-1))]})"; return;;
Could you add a space after the "error:"/"warn:"/"info:"?
t/test-lib.sh test_failure_ () { · line 737
Quoted 9 lines
t/test-lib.sh +1
-1esac done '@@ -734,7 +734,7 @@ test_failure_ () { write_junit_xml_testcase "$1" " $junit_insert" fi test_failure=$(($test_failure + 1)) - say_color error "$(file_lineno error)not ok $test_count - $1" + say_color error "not ok $test_count - $1$(file_lineno error)"
Could you add a space before `$(file_lineno error)'?
t/test-lib.sh test_failure_ () { · line 740
Quoted 4 lines
t/test-lib.sh +0
-0shift printf '%s\n' "$*" | sed -e 's/^/# /' test "$immediate" = "" || { finalize_junit_xml; GIT_EXIT_OK=t; exit 1; }
Thinking about it, I think it would make sense to put this kind of information in a diagnostic line, as we already do for the code of a failed test.
Instead of this:
Quoted 6 lines
not ok 1 - plain(error:t/./t0001-init.sh:33)
## false && # git init plain && # check_config plain/.git false unset#
Something like this:
Quoted 7 lines
not ok 1 - plain
# (error:t/./t0001-init.sh:33)
## false && # git init plain && # check_config plain/.git false unset#
Cheers,
Alban
Re: [RFC PATCH] t: move metadata into TAP test description
Quoted 7 lines · Alban Gruin
Thinking about it, I think it would make sense to put this kind of information in a diagnostic line, as we already do for the code of a failed test.
Instead of this:
Quoted 16 lines
Quoted 6 lines
not ok 1 - plain(error:t/./t0001-init.sh:33)
## false && # git init plain && # check_config plain/.git false unset#
Something like this:
Quoted 7 lines
not ok 1 - plain
# (error:t/./t0001-init.sh:33)
## false && # git init plain && # check_config plain/.git false unset#
indeed, and that is why I mentioned this as a workaround only.
to move to a format like the one you suggest, it might be better to do it by also moving to TAP13[1] (which allows all that metadata)
but of course that would also require us to do more changes to the integration with GitHub's problem matchers and probably a lot more changes I am not even aware of.
Carlo
[1] http://testanything.org/tap-version-13-specification.html
Re: [RFC PATCH] t: move metadata into TAP test description
In reply to Carlo Marcelo Arenas BelónQuoted 20 lines · Carlo Marcelo Arenas Belón
662f9cf154(tests: when run in Bash, annotate test failures with file name/line number, 2020-04-11) adds metadata to the TAP output to help identify the location of the failed test, but does it in a way that break the TAP format and therefore confuses prove.Move the metadata to the description to workaround the issue and change the regex from
676eb0c1ce(ci: add a problem matcher for GitHub Actions, 2020-04-11) to match.Reported-by Alban Gruin <[email protected]>--- ci/git-problem-matcher.json | 10 +++++----- t/test-lib.sh | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-)ci/git-problem-matcher.json +0
-0
ci/git-problem-matcher.json line 7
Quoted 9 lines
ci/git-problem-matcher.json +1
-5@@ -4,11 +4,11 @@ "owner": "git-test-suite", "pattern": [ { - "regexp": "^([^ :]+\\.sh):(\\d+): (error|warning|info):\\s+(.*)$", - "file": 1, - "line": 2, - "severity": 3, - "message": 4 + "regexp": "^(.*)(error|warning|info):\\([^ :]+\\.sh):(\\d+)\\)$",
Do we ever run our tests in ci environments with a shell other than bash? Would we still work fine without the "error:<F>:<L>:" suffix? I guess we just live with degraded output in such a case, and is much better than breaking TAP when tests are run under bash.
The tail part of this pattern (starting at error/warn/info) is tight enough that the overly loose (.*) for the message part may still match the "rest of the message that has arbitrary string" just fine, but it might be less error prone to add an unusual letter (e.g. "|") in the output between "not ok $test_count - $1" and the output from the file_lineno() helper to make sure that we won't end up eating a word "error" etc. at the end of the test title. I dunno.
Quoted 31 lines
ci/git-problem-matcher.json +4
-0+ "file": 3, + "line": 4, + "severity": 2, + "message": 1 } ] }t/test-lib.sh +3
-3@@ -682,8 +682,8 @@ file_lineno () { for i in ${!BASH_SOURCE[*]} do case $i,"${BASH_SOURCE[$i]##*/}" in - 0,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:$LINENO: ${1+$1: }"; return;; - *,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:${BASH_LINENO[$(($i-1))]}: ${1+$1: }"; return;; + 0,t[0-9]*.sh) echo "(${1+$1:}t/${BASH_SOURCE[$i]}:$LINENO)"; return;; + *,t[0-9]*.sh) echo "(${1+$1:}t/${BASH_SOURCE[$i]}:${BASH_LINENO[$(($i-1))]})"; return;; esac done '@@ -734,7 +734,7 @@ test_failure_ () { write_junit_xml_testcase "$1" " $junit_insert" fi test_failure=$(($test_failure + 1)) - say_color error "$(file_lineno error)not ok $test_count - $1" + say_color error "not ok $test_count - $1$(file_lineno error)" shift printf '%s\n' "$*" | sed -e 's/^/# /' test "$immediate" = "" || { finalize_junit_xml; GIT_EXIT_OK=t; exit 1; }
Re: [RFC PATCH] t: move metadata into TAP test description
In reply to Carlo Marcelo Arenas BelónQuoted 25 lines · Carlo Marcelo Arenas Belón
Quoted 7 lines · Alban Gruin
Thinking about it, I think it would make sense to put this kind of information in a diagnostic line, as we already do for the code of a failed test.
Instead of this:
Quoted 16 lines
Quoted 6 lines
not ok 1 - plain(error:t/./t0001-init.sh:33)
## false && # git init plain && # check_config plain/.git false unset#
Something like this:
Quoted 7 lines
not ok 1 - plain
# (error:t/./t0001-init.sh:33)
## false && # git init plain && # check_config plain/.git false unset#
Quoted 8 lines
indeed, and that is why I mentioned this as a workaround only.
to move to a format like the one you suggest, it might be better to do it by also moving to TAP13[1] (which allows all that metadata)
but of course that would also require us to do more changes to the integration with GitHub's problem matchers and probably a lot more changes I am not even aware of.
At this late stage in the cycle, would it be a safer change to revert the whole thing, I wonder, rather than piling fixes on top of fixes to the initial breakage?
303775a2 (t/test_lib: avoid naked bash arrays in file_lineno, 2020-05-07) 662f9cf1 (tests: when run in Bash, annotate test failures with file name/line number, 2020-04-11)
I'll prepare a topic to revert these two directly on top of -rc0 and see how well it goes.
Re: [RFC PATCH] t: move metadata into TAP test description
Quoted 7 lines · Junio C Hamano
At this late stage in the cycle, would it be a safer change to revert the whole thing, I wonder, rather than piling fixes on top of fixes to the initial breakage?
303775a2(t/test_lib: avoid naked bash arrays infile_lineno, 2020-05-07)662f9cf1(tests: when run in Bash, annotate test failures with file name/line number, 2020-04-11)
will also need:
676eb0c1ce (ci: add a problem matcher for GitHub Actions, 2020-04-11)
Carlo
cover Unbreak TAP output under bash
In reply to Junio C HamanoAlban reported and Carlo diagnosed that a recent change breaks TAP output under bash. We know we can work it around but I'd rather not having to worry about the breakage this late in the cycle. Let's revert and re-attempt the enhancement it wanted to do in a way that does not break TAP in the next cycle.
As the same change broke the test under some shells that are not bash, fix for that change is also reverted.
Junio C Hamano (2):
Revert "t/test_lib: avoid naked bash arrays in file_lineno" Revert "tests: when run in Bash, annotate test failures with file
name/line number"
t/test-lib.sh | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-)
Signature · 1 line
2.27.0-rc0
PATCH 1/2 Revert "t/test_lib: avoid naked bash arrays in file_lineno"
This reverts commit 303775a25f0b4ac5d6ad2e96eb4404c24209cad8; instead of trying to salvage the tap-breaking change, let's revert the whole thing for now for the upcoming release.
The enhancement can be re-attempted in the next cycle.
--- t/test-lib.sh | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)
t/test-lib.sh +8 -10
@@ -677,16 +677,14 @@ die () { file_lineno () { test -z "$GIT_TEST_FRAMEWORK_SELFTEST" && test -n "$BASH" || return 0 - eval ' - local i - for i in ${!BASH_SOURCE[*]} - do - case $i,"${BASH_SOURCE[$i]##*/}" in - 0,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:$LINENO: ${1+$1: }"; return;; - *,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:${BASH_LINENO[$(($i-1))]}: ${1+$1: }"; return;; - esac - done - ' + local i + for i in ${!BASH_SOURCE[*]} + do + case $i,"${BASH_SOURCE[$i]##*/}" in + 0,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:$LINENO: ${1+$1: }"; return;; + *,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:${BASH_LINENO[$(($i-1))]}: ${1+$1: }"; return;; + esac + done } GIT_EXIT_OK=
Signature · 1 line
2.27.0-rc0
PATCH 2/2 Revert "tests: when run in Bash, annotate test failures with file name/line number"
In reply to Junio C HamanoThis reverts commit 662f9cf1548cf069cb819e9e95f224657015fcf9, which broke TAP output under bash.
It can be re-attempted in a way that does not break TAP in the next cycle, but we do not want to worry about it at this late in the current cycle.
--- t/test-lib.sh | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-)
t/test-lib.sh +1 -13
@@ -675,18 +675,6 @@ die () { fi } -file_lineno () { - test -z "$GIT_TEST_FRAMEWORK_SELFTEST" && test -n "$BASH" || return 0 - local i - for i in ${!BASH_SOURCE[*]} - do - case $i,"${BASH_SOURCE[$i]##*/}" in - 0,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:$LINENO: ${1+$1: }"; return;; - *,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:${BASH_LINENO[$(($i-1))]}: ${1+$1: }"; return;; - esac - done -} - GIT_EXIT_OK= trap 'die' EXIT # Disable '-x' tracing, because with some shells, notably dash, it
@@ -732,7 +720,7 @@ test_failure_ () { write_junit_xml_testcase "$1" " $junit_insert" fi test_failure=$(($test_failure + 1)) - say_color error "$(file_lineno error)not ok $test_count - $1" + say_color error "not ok $test_count - $1" shift printf '%s\n' "$*" | sed -e 's/^/# /' test "$immediate" = "" || { finalize_junit_xml; GIT_EXIT_OK=t; exit 1; }
Signature · 1 line
2.27.0-rc0
Re: [RFC PATCH] t: move metadata into TAP test description
In reply to Junio C HamanoQuoted 3 lines · Junio C Hamano
Do we ever run our tests in ci environments with a shell other than bash?
the alpine test uses busybox, and the cirrus-ci job uses FreeBSD's sh but most of the other tests (including all windows jobs) use bash and therefore can benefit from this as shown by :
https://github.com/carenas/git/commit/5517179c37904e9b8ac6408fa22643759e91538d#annotation_205089161
Carlo
Re: [RFC PATCH] t: move metadata into TAP test description
In reply to Carlo Marcelo Arenas BelónQuoted 6 lines · Carlo Marcelo Arenas Belón
Quoted 5 lines · Junio C Hamano
At this late stage in the cycle, would it be a safer change to revert the whole thing, I wonder, rather than piling fixes on top of fixes to the initial breakage?
Quoted 6 lines
Quoted 2 lines
303775a2(t/test_lib: avoid naked bash arrays infile_lineno, 2020-05-07)662f9cf1(tests: when run in Bash, annotate test failures with file name/line number, 2020-04-11)will also need:
676eb0c1ce(ci: add a problem matcher for GitHub Actions, 2020-04-11)
Yeah, I think that is a good idea. I suspect that leaving it there won't cause problems, though---it would be just nothing is found to be clicked and that's the end of it, no?
Will add a revert to the series anyway.
Thanks.
Re: [RFC PATCH] t: move metadata into TAP test description
In reply to Carlo Marcelo Arenas BelónLe 15/05/2020 à 17:00, Carlo Marcelo Arenas Belón a écrit :
Quoted 20 lines
662f9cf154(tests: when run in Bash, annotate test failures with file name/line number, 2020-04-11) adds metadata to the TAP output to help identify the location of the failed test, but does it in a way that break the TAP format and therefore confuses prove.Move the metadata to the description to workaround the issue and change the regex from
676eb0c1ce(ci: add a problem matcher for GitHub Actions, 2020-04-11) to match.Reported-by Alban Gruin <[email protected]>--- ci/git-problem-matcher.json | 10 +++++----- t/test-lib.sh | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-)ci/git-problem-matcher.json +0
-0
ci/git-problem-matcher.json line 7
Quoted 9 lines
ci/git-problem-matcher.json +1
-5@@ -4,11 +4,11 @@ "owner": "git-test-suite", "pattern": [ { - "regexp": "^([^ :]+\\.sh):(\\d+): (error|warning|info):\\s+(.*)$", - "file": 1, - "line": 2, - "severity": 3, - "message": 4 + "regexp": "^(.*)(error|warning|info):\\([^ :]+\\.sh):(\\d+)\\)$",
I missed this earlier but this regex is invalid; the last parenthesis in unmatched.
Alban
Quoted 32 lines
ci/git-problem-matcher.json +4
-0+ "file": 3, + "line": 4, + "severity": 2, + "message": 1 } ] }t/test-lib.sh +3
-3@@ -682,8 +682,8 @@ file_lineno () { for i in ${!BASH_SOURCE[*]} do case $i,"${BASH_SOURCE[$i]##*/}" in - 0,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:$LINENO: ${1+$1: }"; return;; - *,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:${BASH_LINENO[$(($i-1))]}: ${1+$1: }"; return;; + 0,t[0-9]*.sh) echo "(${1+$1:}t/${BASH_SOURCE[$i]}:$LINENO)"; return;; + *,t[0-9]*.sh) echo "(${1+$1:}t/${BASH_SOURCE[$i]}:${BASH_LINENO[$(($i-1))]})"; return;; esac done '@@ -734,7 +734,7 @@ test_failure_ () { write_junit_xml_testcase "$1" " $junit_insert" fi test_failure=$(($test_failure + 1)) - say_color error "$(file_lineno error)not ok $test_count - $1" + say_color error "not ok $test_count - $1$(file_lineno error)" shift printf '%s\n' "$*" | sed -e 's/^/# /' test "$immediate" = "" || { finalize_junit_xml; GIT_EXIT_OK=t; exit 1; }
Re: [RFC PATCH] t: move metadata into TAP test description
In reply to Junio C HamanoHi,
Quoted 13 lines · Junio C Hamano
Quoted 11 lines · Carlo Marcelo Arenas Belón
Quoted 7 lines · Junio C Hamano
At this late stage in the cycle, would it be a safer change to revert the whole thing, I wonder, rather than piling fixes on top of fixes to the initial breakage?
303775a2(t/test_lib: avoid naked bash arrays infile_lineno, 2020-05-07)662f9cf1(tests: when run in Bash, annotate test failures with file name/line number, 2020-04-11)will also need:
Quoted 7 lines
Quoted 1 line
676eb0c1ce(ci: add a problem matcher for GitHub Actions, 2020-04-11)Yeah, I think that is a good idea. I suspect that leaving it there won't cause problems, though---it would be just nothing is found to be clicked and that's the end of it, no?
Will add a revert to the series anyway.
Yes, I'm fine with reverting this, as breaking TAP is a rather bad side effect.
Ciao,
Dscho
Re: [RFC PATCH] t: move metadata into TAP test description
Quoted 7 lines · Johannes Schindelin
Quoted 7 lines
Quoted 6 lines
Quoted 2 lines
303775a2(t/test_lib: avoid naked bash arrays infile_lineno, 2020-05-07)662f9cf1(tests: when run in Bash, annotate test failures with file name/line number, 2020-04-11)will also need:
676eb0c1ce(ci: add a problem matcher for GitHub Actions, 2020-04-11)
Quoted 8 lines
Quoted 5 lines
Yeah, I think that is a good idea. I suspect that leaving it there won't cause problems, though---it would be just nothing is found to be clicked and that's the end of it, no?
Will add a revert to the series anyway.
Yes, I'm fine with reverting this, as breaking TAP is a rather bad side effect.
Yup. We can revisit it in the next cycle to find a solution that does not break TAP, either the "a separate comment line", "machine readable cruft at the end of the same line", or some other approach.