* Re: please add link / url to remote - when - git push
From: Taylor Blau @ 2023-10-30 15:55 UTC (permalink / raw)
To: Alexander Mills; +Cc: git
In-Reply-To: <CA+KyZp5mwGJ6YOvjKtfnDMDb9ci3vSq5KNUep6-8EfkHNaxREg@mail.gmail.com>
Hi Alexander,
On Sun, Oct 29, 2023 at 06:15:35PM -0500, Alexander Mills wrote:
> Having the link in the console saves me tremendous time and is
> extremely effective/efficient. Can we get links in the console plz?
That link is generated at the remote end (in your case, this is on
GitHub) and then sent over the wire before being printed out with the
"remote:" prefix at your terminal.
The Git project does not itself generate these messages, so you may want
to relay your request to GitHub's support address at <support@github.com>.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH 0/1] Object ID support for git merge-file
From: Taylor Blau @ 2023-10-30 15:54 UTC (permalink / raw)
To: Elijah Newren; +Cc: brian m. carlson, git, Junio C Hamano, Phillip Wood
In-Reply-To: <CABPp-BFJn5NsSaaxEMpicqND_-8CBw370kyBQuKHGOX-3PRyMw@mail.gmail.com>
On Sat, Oct 28, 2023 at 11:24:06PM -0700, Elijah Newren wrote:
> On Tue, Oct 24, 2023 at 12:58 PM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> >
> > This series introduces an --object-id option to git merge-file such
> > that, instead of reading and writing from files on the system, it reads
> > from and writes to the object store using blobs.
>
> This seems like a reasonable capability to want from such a plumbing command.
Agreed.
> > This is in use at
> > GitHub to produce conflict diffs when a merge fails, and it seems
> > generally useful, so I'm sending it here.
>
> But...wouldn't you already have the conflicts generated when doing the
> merge and learning that it fails? Why would you need to generate them
> again?
brian would know better than I do, but I believe the reason is because
the "attempt this merge" RPC is handled separately from the "show me the
merge conflict(s) at xyz path". Those probably could be combined
(obviating the need for this patch), but doing so is probably rather
complicated.
Since this feature is generally useful for callers that haven't already
completed a tree-level merge and really just care about the result of
merging a single path, I don't have any objections here.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH 0/5] ci: add GitLab CI definition
From: Taylor Blau @ 2023-10-30 15:46 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <cover.1698305961.git.ps@pks.im>
On Thu, Oct 26, 2023 at 09:59:59AM +0200, Patrick Steinhardt wrote:
> And this is exactly what this patch series does: it adds GitLab-specific
> knowledge to our CI scripts and adds a CI definition that builds on top
> of those scripts. This is rather straight forward, as the scripts
> already know to discern Azure Pipelines and GitHub Actions, and adding
> a third item to this list feels quite natural. And by building on top of
> the preexisting infra, the actual ".gitlab-ci.yml" is really quite
> small.
>
> I acknowledge that the Git project may not be willing to fully support
> GitLab CI, and that's fine with me. If we want to further stress that
> point then I'd also be perfectly happy to move the definitions into the
> "contrib/" directory -- it would still be a huge win for our workflow.
> In any case, I'm happy to keep on maintaining the intgeration with
> GitLab CI, and if things break I'll do my best to fix them fast.
I don't have any strong opinions here, but my preference would probably
be to keep any GitLab-specific CI configuration limited to "contrib", if
it lands in the tree at all.
We already have a rather complicated CI setup on GitHub, which I think
we generally consider authoritative in terms of determining whether "CI"
is green. I know we have some Azure remnants in "ci", but I'm not aware
of any of the details there.
So I have some hesitation about trying to mirror this rather complicated
set of build rules in another CI environment. My primary concern would
be that the two might fall out of sync and a series that is green on
GitHub would be red on GitLab, or vice-versa. Importantly, this can
happen even without changes to the build definitions, since (AFAICT)
both forges distribute new images automatically, so the set of packages
installed in GitHub may not exactly match what's in GitLab (and
vice-versa).
My other concern is that we're doubling the cost of any new changes to
our CI definition. Perhaps this is more of an academic concern, but I
think my fear would be that one or the other would fall behind on in
implementation leading to further divergence between the two.
I think having the new CI definition live in "contrib" somewhat
addresses the "which CI is authoritative?" problem, but that it doesn't
address the "we have two of these" problem.
So my preference would probably to have this live out of Junio's tree,
but I'm curious to hear what others think.
Thanks,
Taylor
^ permalink raw reply
* [PATCH] tests: handle "funny" exit code 127 produced by MSVC-compiled exes
From: Johannes Schindelin via GitGitGadget @ 2023-10-30 15:45 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The exit code 127 is well-documented to mean: command not found.
Unfortunately, it is also used as fall-back in Cygwin's
`pinfo::status_exit()` method (which maps things like Windows'
`STATUS_ACCESS_VIOLATION` to `128 | SIGSEGV`).
This is particularly unfortunate because there is no explicit mapping
for `STATUS_STACK_OVERFLOW`. Meaning: when MSVC-compiled executables
produce a stack overflow the exit code in the Cygwin Bash will be 127.
Consequently, the same will be true for the MSYS2 Bash that is used by
Git for Windows.
Now, `jk/tree-name-and-depth-limit` introduces a pair of test cases that
expect a command that produces a stack overflow to fail, which it
typically does with exit code 139 (which means SIGSEGV).
But since MSVC-compiled `git.exe` exits with `STATUS_STACK_OVERFLOW`
which the MSYS2 runtime maps to 127, and since 127 is taken to mean
"command not found" by `test_must_fail`, even though everything works as
planned the two new test cases fail when run in `win+VS test`.
Let's work around this by:
1) recording which C compiler was used, and
2) adding an MSVC-only exception to `test_must_fail` to treat 127 as a
regular failure.
There is a slight downside of this approach in that a real missing
command could be mistaken for a failure. However, this would be caught
on other platforms, and besides, we use `test_must_fail` only for `git`
and `scalar` anymore, and we can be pretty certain that both are there.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Fix t6700.[45] in win+VS test
These two test cases have been failing for a while in Git for Windows'
shears/* branches. Took a good while to figure out, too.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1604%2Fdscho%2Ffix-vs-win-test-with-new-depth-limit-test-cases-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1604/dscho/fix-vs-win-test-with-new-depth-limit-test-cases-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1604
contrib/buildsystems/CMakeLists.txt | 3 ++-
t/test-lib-functions.sh | 3 +++
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index 6b819e2fbdf..e164484be98 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -1057,7 +1057,8 @@ if(NOT PYTHON_TESTS)
set(NO_PYTHON 1)
endif()
-file(WRITE ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "SHELL_PATH='${SHELL_PATH}'\n")
+file(WRITE ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "CMAKE_C_COMPILER='${CMAKE_C_COMPILER}'\n")
+file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "SHELL_PATH='${SHELL_PATH}'\n")
file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "TEST_SHELL_PATH='${TEST_SHELL_PATH}'\n")
file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "PERL_PATH='${PERL_PATH}'\n")
file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "DIFF='${DIFF}'\n")
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 2f8868caa17..ee19c748973 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1112,6 +1112,9 @@ test_must_fail () {
return 1
elif test $exit_code -eq 127
then
+ # Work-around for MSVC-compiled executables
+ case "$CMAKE_C_COMPILER" in *MSVC*) return 0;; esac
+
echo >&4 "test_must_fail: command not found: $*"
return 1
elif test $exit_code -eq 126
base-commit: 3130c155df9a65ebccf128b4af5a19af49532580
--
gitgitgadget
^ permalink raw reply related
* Re: Repository cloned using SSH does not respect bare repository initial branch
From: Sheik @ 2023-10-30 15:33 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20231030093605.GE84866@coredump.intra.peff.net>
On 30/10/23 20:36, Jeff King wrote:
> On Thu, Oct 26, 2023 at 07:36:36AM +1100, Sheik wrote:
>
>> Repository cloned using SSH does not use the branch configured in the bare
>> repository however repository cloned using filesystem does as expected.
>> Shouldn't they both behave the same?
> What version of Git is running on the ssh server?
>
> Your example seems to show that the parent repository has an unborn
> branch (i.e., HEAD points to "refs/heads/test", but there are no commits
> yet). I think the server-side bits you need for that to work showed up
> in 59e1205d16 (ls-refs: report unborn targets of symrefs, 2021-02-05),
> which is in v2.31.
>
> So even though your client seems to be v2.42 (from the output you gave),
> if the server is older it may not be sending sufficient information.
> There were also some other fixes on top of that, but I _think_ they were
> all client-side (so your v2.42 clone command should be doing the right
> thing).
>
> -Peff
Confirming your observation above, yes this is a purely new bare
repository which has no commits yet before cloning.
Thanks
Sheik
^ permalink raw reply
* Re: Repository cloned using SSH does not respect bare repository initial branch
From: Sheik @ 2023-10-30 15:24 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20231030093605.GE84866@coredump.intra.peff.net>
Server version is same as client (v2.42.0) as I ran these commands all
on the same machine.
Test on ssh server:
$ git -v --build-options
#Output
git version 2.42.0
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
Thanks
Sheik
On 30/10/23 20:36, Jeff King wrote:
> On Thu, Oct 26, 2023 at 07:36:36AM +1100, Sheik wrote:
>
>> Repository cloned using SSH does not use the branch configured in the bare
>> repository however repository cloned using filesystem does as expected.
>> Shouldn't they both behave the same?
> What version of Git is running on the ssh server?
>
> Your example seems to show that the parent repository has an unborn
> branch (i.e., HEAD points to "refs/heads/test", but there are no commits
> yet). I think the server-side bits you need for that to work showed up
> in 59e1205d16 (ls-refs: report unborn targets of symrefs, 2021-02-05),
> which is in v2.31.
>
> So even though your client seems to be v2.42 (from the output you gave),
> if the server is older it may not be sending sufficient information.
> There were also some other fixes on top of that, but I _think_ they were
> all client-side (so your v2.42 clone command should be doing the right
> thing).
>
> -Peff
^ permalink raw reply
* Re: [PATCH v3 7/8] ci: install test dependencies for linux-musl
From: Patrick Steinhardt @ 2023-10-30 15:23 UTC (permalink / raw)
To: phillip.wood; +Cc: git, Junio C Hamano, Oswald Buddenhagen
In-Reply-To: <d675b51b-c50b-43e4-b127-650c0e24d3d0@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2737 bytes --]
On Mon, Oct 30, 2023 at 03:13:35PM +0000, Phillip Wood wrote:
> Hi Patrick
>
> On 30/10/2023 12:47, Patrick Steinhardt wrote:
> > On Mon, Oct 30, 2023 at 01:15:10PM +0100, Patrick Steinhardt wrote:
> > But once fixed, tests do indeed start to fail:
> >
> > t5540-http-push-webdav.sh (Wstat: 256 (exited 1) Tests: 20 Failed: 11)
> > Failed tests: 5-11, 13, 15-16, 18
> > Non-zero exit status: 1
> >
> > Seems like another thing to fix in a separate patch series.
>
> Yes, or we could just leave it - I had not realized before that it was only
> the musl job that was not running the httpd tests (I thought
> install-docker-dependencies.sh was missing the packages for ubuntu as well).
> Given that, the status quo does not seem so bad.
I of course couldn't let go. The following would fix this:
diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
index 5e28adf55b6..48cb2e735b5 100755
--- a/ci/install-docker-dependencies.sh
+++ b/ci/install-docker-dependencies.sh
@@ -21,7 +21,8 @@ linux32)
linux-musl)
apk add --update shadow sudo build-base curl-dev openssl-dev expat-dev gettext \
pcre2-dev python3 musl-libintl perl-utils ncurses \
- apache2 bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
+ apache2 apache2-http2 apache2-proxy apache2-ssl apache2-webdav apr-util-dbd_sqlite3 \
+ bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
;;
linux-*)
apt update -q &&
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 9791f94b16f..9ea74927c40 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -128,6 +128,20 @@ else
"Could not identify web server at '$LIB_HTTPD_PATH'"
fi
+if test -n "$LIB_HTTPD_DAV" && test -f /etc/os-release
+then
+ case "$(grep "^ID=" /etc/os-release | cut -d= -f2-)" in
+ alpine)
+ # The WebDAV module in Alpine Linux is broken at least up to
+ # Alpine v3.16 as the default DBM driver is missing.
+ #
+ # https://gitlab.alpinelinux.org/alpine/aports/-/issues/13112
+ test_skip_or_die GIT_TEST_HTTPD \
+ "Apache WebDAV module does not have default DBM backend driver"
+ ;;
+ esac
+fi
+
install_script () {
write_script "$HTTPD_ROOT_PATH/$1" <"$TEST_PATH/$1"
}
I might as well roll it into this patch series now. It increases test
coverage on musl libc and doesn't have any significant downsides. In
fact, it uncovers that tests on Alpine Linux don't work right now, so it
fixes real issues to have the test coverage in our pipelines.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* Re: [PATCH v3 5/8] ci: unify setup of some environment variables
From: Patrick Steinhardt @ 2023-10-30 15:19 UTC (permalink / raw)
To: phillip.wood; +Cc: git, Junio C Hamano, Oswald Buddenhagen
In-Reply-To: <87430c6c-91c0-4be1-b89d-bf442b3f018b@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4391 bytes --]
On Mon, Oct 30, 2023 at 03:09:01PM +0000, Phillip Wood wrote:
> Hi Patrick
>
> On 30/10/2023 12:15, Patrick Steinhardt wrote:
> > Both GitHub Actions and Azue Pipelines set up the environment variables
> > GIT_TEST_OPTS, GIT_PROVE_OPTS and MAKEFLAGS. And while most values are
> > actually the same, the setup is completely duplicate. With the upcoming
> > support for GitLab CI this duplication would only extend even further.
> >
> > Unify the setup of those environment variables so that only the uncommon
> > parts are separated. While at it, we also perform some additional small
> > improvements:
> >
> > - We use nproc instead of a hardcoded count of jobs for make and
> > prove. This ensures that the number of concurrent processes adapts
> > to the host automatically.
>
> Sadly this makes the Windows and MacOS jobs fail on GitHub Actions as nproc
> is not installed[1]. Perhaps we could do
>
> --jobs="$(nproc || echo 2)"
>
> instead. (Maybe 2 is a bit low but the current value of 10 seems pretty high
> for the number of cores on the runners that we use)
Ugh, thanks. I'll update it to keep the hardcoded 10 jobs in place for
now for the other pipelines. Trying to do too many things at once is
only going to make it harder to get this landed, doubly so when you have
no easy way to verify what you're doing :)
> > - We now always pass `--state=failed,slow,save` via GIT_PROVE_OPTS.
> > It doesn't hurt on platforms where we don't persist the state, so
> > this further reduces boilerplate.
> >
> > - When running on Windows systems we set `--no-chain-lint` and
> > `--no-bin-wrappers`. Interestingly though, we did so _after_
> > already having exported the respective environment variables.
> > > - We stop using `export VAR=value` syntax, which is a Bashism. It's
> > not quite worth it as we still use this syntax all over the place,
> > but it doesn't hurt readability either.
>
> I don't mind this change, but the 'export VAR=value' syntax is in POSIX[2]
I don't quite mind it, either. The reason why I chose to use it is that
there was indeed a bug already with the order of exports and
modification of the vars after the export. So with that in mind it made
sense to me to adapt it accordingly.
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> > ci/lib.sh | 24 ++++++++++++++----------
> > 1 file changed, 14 insertions(+), 10 deletions(-)
> >
> > diff --git a/ci/lib.sh b/ci/lib.sh
> > index 9ffdf743903..c7a716a6e3f 100755
> > --- a/ci/lib.sh
> > +++ b/ci/lib.sh
> > @@ -175,11 +175,7 @@ then
> > # among *all* phases)
> > cache_dir="$HOME/test-cache/$SYSTEM_PHASENAME"
> > - export GIT_PROVE_OPTS="--timer --jobs 10 --state=failed,slow,save"
> > - export GIT_TEST_OPTS="--verbose-log -x --write-junit-xml"
> > - MAKEFLAGS="$MAKEFLAGS --jobs=10"
> > - test windows_nt != "$CI_OS_NAME" ||
> > - GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS"
> > + GIT_TEST_OPTS="--write-junit-xml"
> > elif test true = "$GITHUB_ACTIONS"
> > then
> > CI_TYPE=github-actions
> > @@ -198,17 +194,25 @@ then
> > cache_dir="$HOME/none"
> > - export GIT_PROVE_OPTS="--timer --jobs 10"
> > - export GIT_TEST_OPTS="--verbose-log -x --github-workflow-markup"
> > - MAKEFLAGS="$MAKEFLAGS --jobs=10"
> > - test windows != "$CI_OS_NAME" ||
> > - GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS"
> > + GIT_TEST_OPTS="--github-workflow-markup"
> > else
> > echo "Could not identify CI type" >&2
> > env >&2
> > exit 1
> > fi
> > +MAKEFLAGS="$MAKEFLAGS --jobs=$(nproc)"
> > +GIT_PROVE_OPTS="--timer --jobs $(nproc) --state=failed,slow,save"
> > +
> > +GIT_TEST_OPTS="$GIT_TEST_OPTS --verbose-log -x"
> > +if test windows = "$CI_OS_NAME"
> > +then
> > + GIT_TEST_OPTS="$GIT_TEST_OPTS --no-chain-lint --no-bin-wrappers"
> > +fi
> >
> > +export GIT_TEST_OPTS
> > +export GIT_PROVE_OPTS
>
> I was wondering why we don't export MAKEFLAGS here but it is exported
> earlier on before we set it. Apart from the nproc issue this looks like a
> nice improvement
In fact, I also noticed later today that we adapt MAKEFLAGS at a later
point again without reexporting it, so it's got the same issue. I'll
adapt it in the same way.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v3 7/8] ci: install test dependencies for linux-musl
From: Phillip Wood @ 2023-10-30 15:13 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Junio C Hamano, Oswald Buddenhagen
In-Reply-To: <ZT-l4qDDNloizjxl@tanuki>
Hi Patrick
On 30/10/2023 12:47, Patrick Steinhardt wrote:
> On Mon, Oct 30, 2023 at 01:15:10PM +0100, Patrick Steinhardt wrote:
> But once fixed, tests do indeed start to fail:
>
> t5540-http-push-webdav.sh (Wstat: 256 (exited 1) Tests: 20 Failed: 11)
> Failed tests: 5-11, 13, 15-16, 18
> Non-zero exit status: 1
>
> Seems like another thing to fix in a separate patch series.
Yes, or we could just leave it - I had not realized before that it was
only the musl job that was not running the httpd tests (I thought
install-docker-dependencies.sh was missing the packages for ubuntu as
well). Given that, the status quo does not seem so bad.
Best Wishes
Phillip
^ permalink raw reply
* Re: [PATCH v3 5/8] ci: unify setup of some environment variables
From: Phillip Wood @ 2023-10-30 15:09 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Junio C Hamano, Oswald Buddenhagen
In-Reply-To: <6af0075fd875f176e7fdf6c219e7117dac5cd71c.1698667545.git.ps@pks.im>
Hi Patrick
On 30/10/2023 12:15, Patrick Steinhardt wrote:
> Both GitHub Actions and Azue Pipelines set up the environment variables
> GIT_TEST_OPTS, GIT_PROVE_OPTS and MAKEFLAGS. And while most values are
> actually the same, the setup is completely duplicate. With the upcoming
> support for GitLab CI this duplication would only extend even further.
>
> Unify the setup of those environment variables so that only the uncommon
> parts are separated. While at it, we also perform some additional small
> improvements:
>
> - We use nproc instead of a hardcoded count of jobs for make and
> prove. This ensures that the number of concurrent processes adapts
> to the host automatically.
Sadly this makes the Windows and MacOS jobs fail on GitHub Actions as
nproc is not installed[1]. Perhaps we could do
--jobs="$(nproc || echo 2)"
instead. (Maybe 2 is a bit low but the current value of 10 seems pretty
high for the number of cores on the runners that we use)
> - We now always pass `--state=failed,slow,save` via GIT_PROVE_OPTS.
> It doesn't hurt on platforms where we don't persist the state, so
> this further reduces boilerplate.
>
> - When running on Windows systems we set `--no-chain-lint` and
> `--no-bin-wrappers`. Interestingly though, we did so _after_
> already having exported the respective environment variables.
> > - We stop using `export VAR=value` syntax, which is a Bashism.
It's
> not quite worth it as we still use this syntax all over the place,
> but it doesn't hurt readability either.
I don't mind this change, but the 'export VAR=value' syntax is in POSIX[2]
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> ci/lib.sh | 24 ++++++++++++++----------
> 1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/ci/lib.sh b/ci/lib.sh
> index 9ffdf743903..c7a716a6e3f 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -175,11 +175,7 @@ then
> # among *all* phases)
> cache_dir="$HOME/test-cache/$SYSTEM_PHASENAME"
>
> - export GIT_PROVE_OPTS="--timer --jobs 10 --state=failed,slow,save"
> - export GIT_TEST_OPTS="--verbose-log -x --write-junit-xml"
> - MAKEFLAGS="$MAKEFLAGS --jobs=10"
> - test windows_nt != "$CI_OS_NAME" ||
> - GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS"
> + GIT_TEST_OPTS="--write-junit-xml"
> elif test true = "$GITHUB_ACTIONS"
> then
> CI_TYPE=github-actions
> @@ -198,17 +194,25 @@ then
>
> cache_dir="$HOME/none"
>
> - export GIT_PROVE_OPTS="--timer --jobs 10"
> - export GIT_TEST_OPTS="--verbose-log -x --github-workflow-markup"
> - MAKEFLAGS="$MAKEFLAGS --jobs=10"
> - test windows != "$CI_OS_NAME" ||
> - GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS"
> + GIT_TEST_OPTS="--github-workflow-markup"
> else
> echo "Could not identify CI type" >&2
> env >&2
> exit 1
> fi
>
> +MAKEFLAGS="$MAKEFLAGS --jobs=$(nproc)"
> +GIT_PROVE_OPTS="--timer --jobs $(nproc) --state=failed,slow,save"
> +
> +GIT_TEST_OPTS="$GIT_TEST_OPTS --verbose-log -x"
> +if test windows = "$CI_OS_NAME"
> +then
> + GIT_TEST_OPTS="$GIT_TEST_OPTS --no-chain-lint --no-bin-wrappers"
> +fi
>
> +export GIT_TEST_OPTS
> +export GIT_PROVE_OPTS
I was wondering why we don't export MAKEFLAGS here but it is exported
earlier on before we set it. Apart from the nproc issue this looks like
a nice improvement
Best Wishes
Phillip
[1] https://github.com/phillipwood/git/actions/runs/6694263874
[2]
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#export
^ permalink raw reply
* Re: [PATCH v3] bugreport: reject positional arguments
From: Phillip Wood @ 2023-10-30 14:11 UTC (permalink / raw)
To: Junio C Hamano
Cc: Eric Sunshine, emilyshaffer, git, Emily Shaffer, Sheik,
Dragan Simic
In-Reply-To: <xmqqcywwg9am.fsf@gitster.g>
On 30/10/2023 01:59, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Phillip Wood <phillip.wood123@gmail.com> writes:
>>>
>>>> It is rather unfortunate that test_i18ngrep was deprecated without
>>>> providing an alternative that offers the same debugging
>>>> experience.
>>> ...
>>> We could rename test_i18ngrep to test_grep (and make test_i18ngrep
>>> into a thin wrapper with warnings).
>>>
>>> test_grep -e must-exist file &&
>>> test_grep ! -e must-not-exist file
>>
>> ... as the only remaining part in test_18ngrep has no hack to work
>> around the tainted localization tests, so "was deprecated without"
>> is a bit too strong. There is nothing we have lost yet.
>
> Having said all that, when re-reading the test_i18ngrep with a fresh
> pair of eyes, I somehow doubt there was much upside in "debugging
> experience" with test_i18ngrep in the first place, and I doubt if
> retaining it with a new name test_grep has much value.
>
> Given that test_i18ngrep (hence test_grep) requires you to have the
> haystack in a file, between
>
> test_i18ngrep must-exist file &&
> test_i18ngrep ! must-not-exist file
>
> and
>
> grep must-exist file &&
> ! grep must-not-exist file
>
> I do not see any difference in "debugging experience" when you run
> the test with "-i [-v] -d". The two cases you care about are
>
> (1) the test expects the string "must-exist" in the file "file" but
> the string is not there.
>
> (2) the test expects the string "must-not-exist" missing from the
> file "file", but the string is there.
>
> The latter can clearly be seen in output from "-i -v -d" (the "grep"
> outputs a line with "must-not-exist" on it). The former will show
> silence but since you are debugging with "-d", and your haystack is
> in a file, after such a step fails, the test stops, and without
> removing the "file" even if the test piece had test_when_finished
> to remove it (i.e. running tests in debugging mode "-d" and
> immediately stopping upon failure "-i" behaves this way exactly to
> help you debugging), so you can go there to the TRASH_DIRECTORY
> yourself and inspect "file" to see what is going on anyway.
>
> So, I dunno. Surely with a long &&-chain of steps, where a grep
> that expects lack of something is in the middle, it is hard to see
> if the lack of hit is because an earlier step failed (and the
> control did not reach "grep must-exist file") or because the
> haystack lacked the "must-exist" needle, so from that point of view,
> it may be nicer that "did not find an expected match" is explicitly
> stated.
It is this latter point that I had in mind. I find it much easier to
debug a test that says "This command failed" rather than looking at the
output to try and figure out which was the last successful command. I
take your point above that one can go and inspect the file when the test
is run with "-i -d" but it is determining that grep failed in the first
place that I find annoying. I've also found the output from
test_i18ngrep is helpful when debugging CI test failures.
Best Wishes
Phillip
^ permalink raw reply
* Re: [PATCH 5/5] ci: add support for GitLab CI
From: Dragan Simic @ 2023-10-30 14:04 UTC (permalink / raw)
To: phillip.wood; +Cc: Oswald Buddenhagen, Patrick Steinhardt, git
In-Reply-To: <68c4684b-0ab0-427a-90e5-fca602c9d993@gmail.com>
On 2023-10-30 10:49, Phillip Wood wrote:
> On 27/10/2023 18:47, Oswald Buddenhagen wrote:
>> On Fri, Oct 27, 2023 at 03:32:48PM +0100, Phillip Wood wrote:
>>> On 27/10/2023 11:43, Oswald Buddenhagen wrote:
>>>> On Fri, Oct 27, 2023 at 11:22:35AM +0100, Phillip Wood wrote:
>>>>>>>> + CI_BRANCH="$CI_COMMIT_REF_NAME"
>>>>>>>> + CI_COMMIT="$CI_COMMIT_SHA"
>>>>>>>>
>>>>>>> assignments need no quoting to prevent word splitting.
>>>>>>> repeats below.
>>>>>>>
>>>>> I think it is quite common for us to quote variables when it isn't
>>>>> strictly necessary as it makes it clear to anyone reading the
>>>>> script that there is no word splitting going on
>>>>
>>>>> and ensures that we don't start splitting the variable if the
>>>>> contents changes in the future.
>>>>>
>>>> the point was that it *isn't* content-dependent; it's simply the
>>>> shell rules.
>>>
>>> Oh, I'd misunderstood what you were saying which was that assignment
>>> and case statements are not subject to field splitting.
>>>
>>>> of course, many people (apparently you included) don't know these
>>>> subtleties
>>>
>>> I find this comment to be condescending, needlessly antagonistic and
>>> completely at odds with the norms of constructive discussion on this
>>> list.
>>>
>> the observation was necessary for the point i subsequently made (which
>> was basically agreeing with the first part of your response).
>
> It was not necessary to phrase it as you did though. Before replying
> on Friday I showed your comment to someone else and their reaction was
> "That's rude". You could have made your point by saying something like
>
> It is hard to remember all the shell's word splitting rules so
> quoting everywhere is not a bad idea.
>
> This is not the first time I've found your comments unnecessarily
> adversarial and at odds with the norms of constructive discussion and
> respectful disagreement on this list. I don't think I'm the only one
> either - in [1] Junio points out an ad-hominem remark and in [2] Marc
> comments on the unreceptive tone of you review responses.
>
> I would urge you to try and strike a more conciliatory tone in your
> messages - it is perfectly possible to correct or disagree with
> someone without alienating them in the process.
Yeah, I've also noticed a not so great tone is some of Oswald's
messages. It's perfectly fine to disagree on something, but it isn't
that great to put the other party down while doing that, even to the
point of insulting them.
We're all humans, and we should treat each other with respect.
Furthermore, disagreeing in a friendly and polite way can many times
lead to finding a better solution together.
> Best Wishes
>
> Phillip
>
> [1] https://lore.kernel.org/git/xmqqleeihok5.fsf@gitster.g/
> [2]
> https://lore.kernel.org/git/e33f919d-1b6a-4944-ab5d-93ad0d323b68@xiplink.com/
^ permalink raw reply
* Re: [PATCH v3 7/8] ci: install test dependencies for linux-musl
From: Patrick Steinhardt @ 2023-10-30 13:22 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Phillip Wood, Oswald Buddenhagen
In-Reply-To: <ZT-l4qDDNloizjxl@tanuki>
[-- Attachment #1: Type: text/plain, Size: 5019 bytes --]
On Mon, Oct 30, 2023 at 01:47:30PM +0100, Patrick Steinhardt wrote:
> On Mon, Oct 30, 2023 at 01:15:10PM +0100, Patrick Steinhardt wrote:
> > The linux-musl CI job executes tests on Alpine Linux, which is based on
> > musl libc instead of glibc. We're missing some test dependencies though,
> > which causes us to skip a subset of tests.
> >
> > Install these test dependencies to increase our test coverage on this
> > platform. There are still some missing test dependecies, but these do
> > not have a corresponding package in the Alpine repositories:
> >
> > - p4 and p4d, both parts of the Perforce version control system.
> >
> > - cvsps, which generates patch sets for CVS.
> >
> > - Subversion and the SVN::Core Perl library, the latter of which is
> > not available in the Alpine repositories. While the tool itself is
> > available, all Subversion-related tests are skipped without the
> > SVN::Core Perl library anyway.
> >
> > Furthermore, in order to make the Apache-based tests work, this commit
> > also adds the Alpine-specific modules path of it to the list of known
> > paths.
> >
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> > ci/install-docker-dependencies.sh | 3 ++-
> > t/lib-httpd.sh | 3 ++-
> > 2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
> > index d0bc19d3bb3..05dde5c5d40 100755
> > --- a/ci/install-docker-dependencies.sh
> > +++ b/ci/install-docker-dependencies.sh
> > @@ -17,7 +17,8 @@ linux32)
> > ;;
> > linux-musl)
> > apk add --update build-base curl-dev openssl-dev expat-dev gettext \
> > - pcre2-dev python3 musl-libintl perl-utils ncurses >/dev/null
> > + pcre2-dev python3 musl-libintl perl-utils ncurses \
> > + apache2 bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
>
> Meh. I just noticed that I missed a few other dependencies to make
> Apache2 work:
>
> diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
> index 5e28adf55b..ce910e3f3c 100755
> --- a/ci/install-docker-dependencies.sh
> +++ b/ci/install-docker-dependencies.sh
> @@ -21,7 +21,8 @@ linux32)
> linux-musl)
> apk add --update shadow sudo build-base curl-dev openssl-dev expat-dev gettext \
> pcre2-dev python3 musl-libintl perl-utils ncurses \
> - apache2 bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
> + apache2 apache2-http2 apache2-proxy apache2-ssl apache2-webdav \
> + bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
> ;;
> linux-*)
> apt update -q &&
>
> But once fixed, tests do indeed start to fail:
>
> t5540-http-push-webdav.sh (Wstat: 256 (exited 1) Tests: 20 Failed: 11)
> Failed tests: 5-11, 13, 15-16, 18
> Non-zero exit status: 1
>
> Seems like another thing to fix in a separate patch series.
>
> Patrick
I've been digging a bit, and the issue comes from the the DAV module
indeed:
```
[Mon Oct 30 12:36:19.776149 2023] [dav_fs:crit] [pid 275752] (20019)DSO load failed: AH00576: The DBM driver could not be loaded
[Mon Oct 30 12:36:19.776168 2023] [dav:error] [pid 275752] [client 127.0.0.1:51388] Could not LOCK /dumb/test_repo.git/info/refs due to a failed precondition (e.g. other locks). [500, #0]
[Mon Oct 30 12:36:19.776174 2023] [dav:error] [pid 275752] [client 127.0.0.1:51388] The locks could not be queried for verification against a possible "If:" header. [500, #0]
[Mon Oct 30 12:36:19.776177 2023] [dav:error] [pid 275752] [client 127.0.0.1:51388] Could not open the lock database. [500, #400]
[Mon Oct 30 12:36:19.776181 2023] [dav:error] [pid 275752] (20019)DSO load failed: [client 127.0.0.1:51388] Could not open property database. [500, #1]
```
This seems to be a known limitation in Alpine Linux as they do not
package apr-util-dbm_db anymore due to license incompatibilities with
with Berkely DB [1], and the WebDAV module does rely on it to provide
locking.
In the best case we'd be able to detect this limitation and skip those
tests automatically so that we can at least execute all the other Apache
tests. But again, this rather feels like something we should do as a
follow up rather than as part of this series.
Patrick
[1]: https://gitlab.alpinelinux.org/alpine/aports/-/issues/12534
> > ;;
> > pedantic)
> > dnf -yq update >/dev/null &&
> > diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
> > index 2fb1b2ae561..9791f94b16f 100644
> > --- a/t/lib-httpd.sh
> > +++ b/t/lib-httpd.sh
> > @@ -67,7 +67,8 @@ for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \
> > '/usr/lib/apache2/modules' \
> > '/usr/lib64/httpd/modules' \
> > '/usr/lib/httpd/modules' \
> > - '/usr/libexec/httpd'
> > + '/usr/libexec/httpd' \
> > + '/usr/lib/apache2'
> > do
> > if test -d "$DEFAULT_HTTPD_MODULE_PATH"
> > then
> > --
> > 2.42.0
> >
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v3 7/8] ci: install test dependencies for linux-musl
From: Patrick Steinhardt @ 2023-10-30 12:47 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Phillip Wood, Oswald Buddenhagen
In-Reply-To: <f150d61a1ceec5dd6cf370c3f4421a3e2d9f11a3.1698667545.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 3326 bytes --]
On Mon, Oct 30, 2023 at 01:15:10PM +0100, Patrick Steinhardt wrote:
> The linux-musl CI job executes tests on Alpine Linux, which is based on
> musl libc instead of glibc. We're missing some test dependencies though,
> which causes us to skip a subset of tests.
>
> Install these test dependencies to increase our test coverage on this
> platform. There are still some missing test dependecies, but these do
> not have a corresponding package in the Alpine repositories:
>
> - p4 and p4d, both parts of the Perforce version control system.
>
> - cvsps, which generates patch sets for CVS.
>
> - Subversion and the SVN::Core Perl library, the latter of which is
> not available in the Alpine repositories. While the tool itself is
> available, all Subversion-related tests are skipped without the
> SVN::Core Perl library anyway.
>
> Furthermore, in order to make the Apache-based tests work, this commit
> also adds the Alpine-specific modules path of it to the list of known
> paths.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> ci/install-docker-dependencies.sh | 3 ++-
> t/lib-httpd.sh | 3 ++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
> index d0bc19d3bb3..05dde5c5d40 100755
> --- a/ci/install-docker-dependencies.sh
> +++ b/ci/install-docker-dependencies.sh
> @@ -17,7 +17,8 @@ linux32)
> ;;
> linux-musl)
> apk add --update build-base curl-dev openssl-dev expat-dev gettext \
> - pcre2-dev python3 musl-libintl perl-utils ncurses >/dev/null
> + pcre2-dev python3 musl-libintl perl-utils ncurses \
> + apache2 bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
Meh. I just noticed that I missed a few other dependencies to make
Apache2 work:
diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
index 5e28adf55b..ce910e3f3c 100755
--- a/ci/install-docker-dependencies.sh
+++ b/ci/install-docker-dependencies.sh
@@ -21,7 +21,8 @@ linux32)
linux-musl)
apk add --update shadow sudo build-base curl-dev openssl-dev expat-dev gettext \
pcre2-dev python3 musl-libintl perl-utils ncurses \
- apache2 bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
+ apache2 apache2-http2 apache2-proxy apache2-ssl apache2-webdav \
+ bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
;;
linux-*)
apt update -q &&
But once fixed, tests do indeed start to fail:
t5540-http-push-webdav.sh (Wstat: 256 (exited 1) Tests: 20 Failed: 11)
Failed tests: 5-11, 13, 15-16, 18
Non-zero exit status: 1
Seems like another thing to fix in a separate patch series.
Patrick
> ;;
> pedantic)
> dnf -yq update >/dev/null &&
> diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
> index 2fb1b2ae561..9791f94b16f 100644
> --- a/t/lib-httpd.sh
> +++ b/t/lib-httpd.sh
> @@ -67,7 +67,8 @@ for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \
> '/usr/lib/apache2/modules' \
> '/usr/lib64/httpd/modules' \
> '/usr/lib/httpd/modules' \
> - '/usr/libexec/httpd'
> + '/usr/libexec/httpd' \
> + '/usr/lib/apache2'
> do
> if test -d "$DEFAULT_HTTPD_MODULE_PATH"
> then
> --
> 2.42.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v3 8/8] ci: add support for GitLab CI
From: Patrick Steinhardt @ 2023-10-30 12:15 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Phillip Wood, Oswald Buddenhagen
In-Reply-To: <cover.1698667545.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 6995 bytes --]
We already support Azure Pipelines and GitHub Workflows in the Git
project, but until now we do not have support for GitLab CI. While it is
arguably not in the interest of the Git project to maintain a ton of
different CI platforms, GitLab has recently ramped up its efforts and
tries to contribute to the Git project more regularly.
Part of a problem we hit at GitLab rather frequently is that our own,
custom CI setup we have is so different to the setup that the Git
project has. More esoteric jobs like "linux-TEST-vars" that also set a
couple of environment variables do not exist in GitLab's custom CI
setup, and maintaining them to keep up with what Git does feels like
wasted time. The result is that we regularly send patch series upstream
that fail to compile or pass tests in GitHub Workflows. We would thus
like to integrate the GitLab CI configuration into the Git project to
help us send better patch series upstream and thus reduce overhead for
the maintainer.
The integration does not necessarily have to be a first-class citizen,
which would in practice only add to the fallout that pipeline failures
have for the maintainer. That being said, we are happy to maintain this
alternative CI setup for the Git project and will make test results
available as part of our own mirror of the Git project at [1].
This commit introduces the integration into our regular CI scripts so
that most of the setup continues to be shared across all of the CI
solutions. Note that as the builds on GitLab CI run as unprivileged
user, we need to pull in both sudo and shadow packages to our Alpine
based job to set this up.
[1]: https://gitlab.com/gitlab-org/git
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
.gitlab-ci.yml | 53 +++++++++++++++++++++++++++++++
ci/install-docker-dependencies.sh | 13 +++++++-
ci/lib.sh | 43 +++++++++++++++++++++++++
ci/print-test-failures.sh | 6 ++++
4 files changed, 114 insertions(+), 1 deletion(-)
create mode 100644 .gitlab-ci.yml
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
new file mode 100644
index 00000000000..cd98bcb18aa
--- /dev/null
+++ b/.gitlab-ci.yml
@@ -0,0 +1,53 @@
+default:
+ timeout: 2h
+
+workflow:
+ rules:
+ - if: $CI_PIPELINE_SOURCE == "merge_request_event"
+ - if: $CI_COMMIT_TAG
+ - if: $CI_COMMIT_REF_PROTECTED == "true"
+
+test:
+ image: $image
+ before_script:
+ - ./ci/install-docker-dependencies.sh
+ script:
+ - useradd builder --create-home
+ - chown -R builder "${CI_PROJECT_DIR}"
+ - sudo --preserve-env --set-home --user=builder ./ci/run-build-and-tests.sh
+ after_script:
+ - |
+ if test "$CI_JOB_STATUS" != 'success'
+ then
+ sudo --preserve-env --set-home --user=builder ./ci/print-test-failures.sh
+ fi
+ parallel:
+ matrix:
+ - jobname: linux-sha256
+ image: ubuntu:latest
+ CC: clang
+ - jobname: linux-gcc
+ image: ubuntu:20.04
+ CC: gcc
+ CC_PACKAGE: gcc-8
+ - jobname: linux-TEST-vars
+ image: ubuntu:20.04
+ CC: gcc
+ CC_PACKAGE: gcc-8
+ - jobname: linux-gcc-default
+ image: ubuntu:latest
+ CC: gcc
+ - jobname: linux-leaks
+ image: ubuntu:latest
+ CC: gcc
+ - jobname: linux-asan-ubsan
+ image: ubuntu:latest
+ CC: clang
+ - jobname: pedantic
+ image: fedora:latest
+ - jobname: linux-musl
+ image: alpine:latest
+ artifacts:
+ paths:
+ - t/failed-test-artifacts
+ when: on_failure
diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
index 05dde5c5d40..5e28adf55b6 100755
--- a/ci/install-docker-dependencies.sh
+++ b/ci/install-docker-dependencies.sh
@@ -7,6 +7,9 @@
begin_group "Install dependencies"
+# Required so that apt doesn't wait for user input on certain packages.
+export DEBIAN_FRONTEND=noninteractive
+
case "$jobname" in
linux32)
linux32 --32bit i386 sh -c '
@@ -16,10 +19,18 @@ linux32)
'
;;
linux-musl)
- apk add --update build-base curl-dev openssl-dev expat-dev gettext \
+ apk add --update shadow sudo build-base curl-dev openssl-dev expat-dev gettext \
pcre2-dev python3 musl-libintl perl-utils ncurses \
apache2 bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
;;
+linux-*)
+ apt update -q &&
+ apt install -q -y sudo git make language-pack-is libsvn-perl apache2 libssl-dev \
+ libcurl4-openssl-dev libexpat-dev tcl tk gettext zlib1g-dev \
+ perl-modules liberror-perl libauthen-sasl-perl libemail-valid-perl \
+ libdbd-sqlite3-perl libio-socket-ssl-perl libnet-smtp-ssl-perl ${CC_PACKAGE:-${CC:-gcc}} \
+ apache2 cvs cvsps gnupg libcgi-pm-perl subversion
+ ;;
pedantic)
dnf -yq update >/dev/null &&
dnf -yq install make gcc findutils diffutils perl python3 gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel >/dev/null
diff --git a/ci/lib.sh b/ci/lib.sh
index c13b6527cac..d836fad7dd6 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -14,6 +14,22 @@ then
need_to_end_group=
echo '::endgroup::' >&2
}
+elif test true = "$GITLAB_CI"
+then
+ begin_group () {
+ need_to_end_group=t
+ printf "\e[0Ksection_start:$(date +%s):$(echo "$1" | tr ' ' _)\r\e[0K$1\n"
+ trap "end_group '$1'" EXIT
+ set -x
+ }
+
+ end_group () {
+ test -n "$need_to_end_group" || return 0
+ set +x
+ need_to_end_group=
+ printf "\e[0Ksection_end:$(date +%s):$(echo "$1" | tr ' ' _)\r\e[0K\n"
+ trap - EXIT
+ }
else
begin_group () { :; }
end_group () { :; }
@@ -227,6 +243,33 @@ then
cache_dir="$HOME/none"
GIT_TEST_OPTS="--github-workflow-markup"
+elif test true = "$GITLAB_CI"
+then
+ CI_TYPE=gitlab-ci
+ CI_BRANCH="$CI_COMMIT_REF_NAME"
+ CI_COMMIT="$CI_COMMIT_SHA"
+ case "$CI_JOB_IMAGE" in
+ macos-*)
+ CI_OS_NAME=osx;;
+ alpine:*|fedora:*|ubuntu:*)
+ CI_OS_NAME=linux;;
+ *)
+ echo "Could not identify OS image" >&2
+ env >&2
+ exit 1
+ ;;
+ esac
+ CI_REPO_SLUG="$CI_PROJECT_PATH"
+ CI_JOB_ID="$CI_JOB_ID"
+ CC="${CC_PACKAGE:-${CC:-gcc}}"
+ DONT_SKIP_TAGS=t
+ handle_failed_tests () {
+ create_failed_test_artifacts
+ }
+
+ cache_dir="$HOME/none"
+
+ runs_on_pool=$(echo "$CI_JOB_IMAGE" | tr : -)
else
echo "Could not identify CI type" >&2
env >&2
diff --git a/ci/print-test-failures.sh b/ci/print-test-failures.sh
index 57277eefcd0..c33ad4e3a22 100755
--- a/ci/print-test-failures.sh
+++ b/ci/print-test-failures.sh
@@ -51,6 +51,12 @@ do
tar czf failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
continue
;;
+ gitlab-ci)
+ mkdir -p 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
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v3 7/8] ci: install test dependencies for linux-musl
From: Patrick Steinhardt @ 2023-10-30 12:15 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Phillip Wood, Oswald Buddenhagen
In-Reply-To: <cover.1698667545.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2153 bytes --]
The linux-musl CI job executes tests on Alpine Linux, which is based on
musl libc instead of glibc. We're missing some test dependencies though,
which causes us to skip a subset of tests.
Install these test dependencies to increase our test coverage on this
platform. There are still some missing test dependecies, but these do
not have a corresponding package in the Alpine repositories:
- p4 and p4d, both parts of the Perforce version control system.
- cvsps, which generates patch sets for CVS.
- Subversion and the SVN::Core Perl library, the latter of which is
not available in the Alpine repositories. While the tool itself is
available, all Subversion-related tests are skipped without the
SVN::Core Perl library anyway.
Furthermore, in order to make the Apache-based tests work, this commit
also adds the Alpine-specific modules path of it to the list of known
paths.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
ci/install-docker-dependencies.sh | 3 ++-
t/lib-httpd.sh | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
index d0bc19d3bb3..05dde5c5d40 100755
--- a/ci/install-docker-dependencies.sh
+++ b/ci/install-docker-dependencies.sh
@@ -17,7 +17,8 @@ linux32)
;;
linux-musl)
apk add --update build-base curl-dev openssl-dev expat-dev gettext \
- pcre2-dev python3 musl-libintl perl-utils ncurses >/dev/null
+ pcre2-dev python3 musl-libintl perl-utils ncurses \
+ apache2 bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
;;
pedantic)
dnf -yq update >/dev/null &&
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 2fb1b2ae561..9791f94b16f 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -67,7 +67,8 @@ for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \
'/usr/lib/apache2/modules' \
'/usr/lib64/httpd/modules' \
'/usr/lib/httpd/modules' \
- '/usr/libexec/httpd'
+ '/usr/libexec/httpd' \
+ '/usr/lib/apache2'
do
if test -d "$DEFAULT_HTTPD_MODULE_PATH"
then
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v3 6/8] ci: squelch warnings when testing with unusable Git repo
From: Patrick Steinhardt @ 2023-10-30 12:15 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Phillip Wood, Oswald Buddenhagen
In-Reply-To: <cover.1698667545.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2232 bytes --]
Our CI jobs that run on Docker also use mostly the same architecture to
build and test Git via the "ci/run-build-and-tests.sh" script. These
scripts also provide some functionality to massage the Git repository
we're supposedly operating in.
In our Docker-based infrastructure we may not even have a Git repository
available though, which leads to warnings when those functions execute.
Make the helpers exit gracefully in case either there is no Git in our
PATH, or when not running in a Git repository.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
ci/lib.sh | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/ci/lib.sh b/ci/lib.sh
index c7a716a6e3f..c13b6527cac 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -69,10 +69,32 @@ skip_branch_tip_with_tag () {
fi
}
+# Check whether we can use the path passed via the first argument as Git
+# repository.
+is_usable_git_repository () {
+ # We require Git in our PATH, otherwise we cannot access repositories
+ # at all.
+ if ! command -v git >/dev/null
+ then
+ return 1
+ fi
+
+ # And the target directory needs to be a proper Git repository.
+ if ! git -C "$1" rev-parse 2>/dev/null
+ then
+ return 1
+ fi
+}
+
# Save some info about the current commit's tree, so we can skip the build
# job if we encounter the same tree again and can provide a useful info
# message.
save_good_tree () {
+ if ! is_usable_git_repository .
+ then
+ return
+ fi
+
echo "$(git rev-parse $CI_COMMIT^{tree}) $CI_COMMIT $CI_JOB_NUMBER $CI_JOB_ID" >>"$good_trees_file"
# limit the file size
tail -1000 "$good_trees_file" >"$good_trees_file".tmp
@@ -88,6 +110,11 @@ skip_good_tree () {
return
fi
+ if ! is_usable_git_repository .
+ then
+ return
+ fi
+
if ! good_tree_info="$(grep "^$(git rev-parse $CI_COMMIT^{tree}) " "$good_trees_file")"
then
# Haven't seen this tree yet, or no cached good trees file yet.
@@ -119,6 +146,11 @@ skip_good_tree () {
}
check_unignored_build_artifacts () {
+ if ! is_usable_git_repository .
+ then
+ return
+ fi
+
! git ls-files --other --exclude-standard --error-unmatch \
-- ':/*' 2>/dev/null ||
{
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v3 5/8] ci: unify setup of some environment variables
From: Patrick Steinhardt @ 2023-10-30 12:15 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Phillip Wood, Oswald Buddenhagen
In-Reply-To: <cover.1698667545.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2818 bytes --]
Both GitHub Actions and Azue Pipelines set up the environment variables
GIT_TEST_OPTS, GIT_PROVE_OPTS and MAKEFLAGS. And while most values are
actually the same, the setup is completely duplicate. With the upcoming
support for GitLab CI this duplication would only extend even further.
Unify the setup of those environment variables so that only the uncommon
parts are separated. While at it, we also perform some additional small
improvements:
- We use nproc instead of a hardcoded count of jobs for make and
prove. This ensures that the number of concurrent processes adapts
to the host automatically.
- We now always pass `--state=failed,slow,save` via GIT_PROVE_OPTS.
It doesn't hurt on platforms where we don't persist the state, so
this further reduces boilerplate.
- When running on Windows systems we set `--no-chain-lint` and
`--no-bin-wrappers`. Interestingly though, we did so _after_
already having exported the respective environment variables.
- We stop using `export VAR=value` syntax, which is a Bashism. It's
not quite worth it as we still use this syntax all over the place,
but it doesn't hurt readability either.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
ci/lib.sh | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/ci/lib.sh b/ci/lib.sh
index 9ffdf743903..c7a716a6e3f 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -175,11 +175,7 @@ then
# among *all* phases)
cache_dir="$HOME/test-cache/$SYSTEM_PHASENAME"
- export GIT_PROVE_OPTS="--timer --jobs 10 --state=failed,slow,save"
- export GIT_TEST_OPTS="--verbose-log -x --write-junit-xml"
- MAKEFLAGS="$MAKEFLAGS --jobs=10"
- test windows_nt != "$CI_OS_NAME" ||
- GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS"
+ GIT_TEST_OPTS="--write-junit-xml"
elif test true = "$GITHUB_ACTIONS"
then
CI_TYPE=github-actions
@@ -198,17 +194,25 @@ then
cache_dir="$HOME/none"
- export GIT_PROVE_OPTS="--timer --jobs 10"
- export GIT_TEST_OPTS="--verbose-log -x --github-workflow-markup"
- MAKEFLAGS="$MAKEFLAGS --jobs=10"
- test windows != "$CI_OS_NAME" ||
- GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS"
+ GIT_TEST_OPTS="--github-workflow-markup"
else
echo "Could not identify CI type" >&2
env >&2
exit 1
fi
+MAKEFLAGS="$MAKEFLAGS --jobs=$(nproc)"
+GIT_PROVE_OPTS="--timer --jobs $(nproc) --state=failed,slow,save"
+
+GIT_TEST_OPTS="$GIT_TEST_OPTS --verbose-log -x"
+if test windows = "$CI_OS_NAME"
+then
+ GIT_TEST_OPTS="$GIT_TEST_OPTS --no-chain-lint --no-bin-wrappers"
+fi
+
+export GIT_TEST_OPTS
+export GIT_PROVE_OPTS
+
good_trees_file="$cache_dir/good-trees"
mkdir -p "$cache_dir"
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v3 4/8] ci: split out logic to set up failed test artifacts
From: Patrick Steinhardt @ 2023-10-30 12:14 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Phillip Wood, Oswald Buddenhagen
In-Reply-To: <cover.1698667545.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2381 bytes --]
We have some logic in place to create a directory with the output from
failed tests, which will then subsequently be uploaded as CI artifacts.
We're about to add support for GitLab CI, which will want to reuse the
logic.
Split the logic into a separate function so that it is reusable.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
ci/lib.sh | 40 ++++++++++++++++++++++------------------
1 file changed, 22 insertions(+), 18 deletions(-)
diff --git a/ci/lib.sh b/ci/lib.sh
index b3411afae8e..9ffdf743903 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -131,6 +131,27 @@ handle_failed_tests () {
return 1
}
+create_failed_test_artifacts () {
+ mkdir -p t/failed-test-artifacts
+
+ for test_exit in t/test-results/*.exit
+ do
+ test 0 != "$(cat "$test_exit")" || continue
+
+ test_name="${test_exit%.exit}"
+ test_name="${test_name##*/}"
+ printf "\\e[33m\\e[1m=== Failed test: ${test_name} ===\\e[m\\n"
+ echo "The full logs are in the 'print test failures' step below."
+ echo "See also the 'failed-tests-*' artifacts attached to this run."
+ cat "t/test-results/$test_name.markup"
+
+ trash_dir="t/trash directory.$test_name"
+ cp "t/test-results/$test_name.out" t/failed-test-artifacts/
+ tar czf t/failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
+ done
+ return 1
+}
+
# GitHub Action doesn't set TERM, which is required by tput
export TERM=${TERM:-dumb}
@@ -171,25 +192,8 @@ then
CC="${CC_PACKAGE:-${CC:-gcc}}"
DONT_SKIP_TAGS=t
handle_failed_tests () {
- mkdir -p t/failed-test-artifacts
echo "FAILED_TEST_ARTIFACTS=t/failed-test-artifacts" >>$GITHUB_ENV
-
- for test_exit in t/test-results/*.exit
- do
- test 0 != "$(cat "$test_exit")" || continue
-
- test_name="${test_exit%.exit}"
- test_name="${test_name##*/}"
- printf "\\e[33m\\e[1m=== Failed test: ${test_name} ===\\e[m\\n"
- echo "The full logs are in the 'print test failures' step below."
- echo "See also the 'failed-tests-*' artifacts attached to this run."
- cat "t/test-results/$test_name.markup"
-
- trash_dir="t/trash directory.$test_name"
- cp "t/test-results/$test_name.out" t/failed-test-artifacts/
- tar czf t/failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
- done
- return 1
+ create_failed_test_artifacts
}
cache_dir="$HOME/none"
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v3 3/8] ci: group installation of Docker dependencies
From: Patrick Steinhardt @ 2023-10-30 12:14 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Phillip Wood, Oswald Buddenhagen
In-Reply-To: <cover.1698667545.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1168 bytes --]
The output of CI jobs tends to be quite long-winded and hard to digest.
To help with this, many CI systems provide the ability to group output
into collapsible sections, and we're also doing this in some of our
scripts.
One notable omission is the script to install Docker dependencies.
Address it to bring more structure to the output for Docker-based jobs.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
ci/install-docker-dependencies.sh | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
index 78b7e326da6..d0bc19d3bb3 100755
--- a/ci/install-docker-dependencies.sh
+++ b/ci/install-docker-dependencies.sh
@@ -3,6 +3,10 @@
# Install dependencies required to build and test Git inside container
#
+. ${0%/*}/lib.sh
+
+begin_group "Install dependencies"
+
case "$jobname" in
linux32)
linux32 --32bit i386 sh -c '
@@ -20,3 +24,5 @@ pedantic)
dnf -yq install make gcc findutils diffutils perl python3 gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel >/dev/null
;;
esac
+
+end_group "Install dependencies"
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v3 2/8] ci: make grouping setup more generic
From: Patrick Steinhardt @ 2023-10-30 12:14 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Phillip Wood, Oswald Buddenhagen
In-Reply-To: <cover.1698667545.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2645 bytes --]
Make the grouping setup more generic by always calling `begin_group ()`
and `end_group ()` regardless of whether we have stubbed those functions
or not. This ensures we can more readily add support for additional CI
platforms.
Furthermore, the `group ()` function is made generic so that it is the
same for both GitHub Actions and for other platforms. There is a
semantic conflict here though: GitHub Actions used to call `set +x` in
`group ()` whereas the non-GitHub case unconditionally uses `set -x`.
The latter would get overriden if we kept the `set +x` in the generic
version of `group ()`. To resolve this conflict, we simply drop the `set
+x` in the generic variant of this function. As `begin_group ()` calls
`set -x` anyway this is not much of a change though, as the only
commands that aren't printed anymore now are the ones between the
beginning of `group ()` and the end of `begin_group ()`.
Last, this commit changes `end_group ()` to also accept a parameter that
indicates _which_ group should end. This will be required by a later
commit that introduces support for GitLab CI.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
ci/lib.sh | 46 ++++++++++++++++++++++------------------------
1 file changed, 22 insertions(+), 24 deletions(-)
diff --git a/ci/lib.sh b/ci/lib.sh
index eb384f4e952..b3411afae8e 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -14,36 +14,34 @@ then
need_to_end_group=
echo '::endgroup::' >&2
}
- trap end_group EXIT
-
- group () {
- set +x
- begin_group "$1"
- shift
- # work around `dash` not supporting `set -o pipefail`
- (
- "$@" 2>&1
- echo $? >exit.status
- ) |
- sed 's/^\(\([^ ]*\):\([0-9]*\):\([0-9]*:\) \)\(error\|warning\): /::\5 file=\2,line=\3::\1/'
- res=$(cat exit.status)
- rm exit.status
- end_group
- return $res
- }
-
- begin_group "CI setup"
else
begin_group () { :; }
end_group () { :; }
- group () {
- shift
- "$@"
- }
set -x
fi
+group () {
+ group="$1"
+ shift
+ begin_group "$group"
+
+ # work around `dash` not supporting `set -o pipefail`
+ (
+ "$@" 2>&1
+ echo $? >exit.status
+ ) |
+ sed 's/^\(\([^ ]*\):\([0-9]*\):\([0-9]*:\) \)\(error\|warning\): /::\5 file=\2,line=\3::\1/'
+ res=$(cat exit.status)
+ rm exit.status
+
+ end_group "$group"
+ return $res
+}
+
+begin_group "CI setup"
+trap "end_group 'CI setup'" EXIT
+
# Set 'exit on error' for all CI scripts to let the caller know that
# something went wrong.
#
@@ -287,5 +285,5 @@ esac
MAKEFLAGS="$MAKEFLAGS CC=${CC:-cc}"
-end_group
+end_group "CI setup"
set -x
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v3 1/8] ci: reorder definitions for grouping functions
From: Patrick Steinhardt @ 2023-10-30 12:14 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Phillip Wood, Oswald Buddenhagen
In-Reply-To: <cover.1698667545.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1301 bytes --]
We define a set of grouping functions that are used to group together
output in our CI, where these groups then end up as collapsible sections
in the respective pipeline platform. The way these functions are defined
is not easily extensible though as we have an up front check for the CI
_not_ being GitHub Actions, where we define the non-stub logic in the
else branch.
Reorder the conditional branches such that we explicitly handle GitHub
Actions.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
ci/lib.sh | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/ci/lib.sh b/ci/lib.sh
index 6fbb5bade12..eb384f4e952 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -1,16 +1,7 @@
# Library of functions shared by all CI scripts
-if test true != "$GITHUB_ACTIONS"
+if test true = "$GITHUB_ACTIONS"
then
- begin_group () { :; }
- end_group () { :; }
-
- group () {
- shift
- "$@"
- }
- set -x
-else
begin_group () {
need_to_end_group=t
echo "::group::$1" >&2
@@ -42,6 +33,15 @@ else
}
begin_group "CI setup"
+else
+ begin_group () { :; }
+ end_group () { :; }
+
+ group () {
+ shift
+ "$@"
+ }
+ set -x
fi
# Set 'exit on error' for all CI scripts to let the caller know that
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v3 0/8] ci: add GitLab CI definition
From: Patrick Steinhardt @ 2023-10-30 12:14 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Phillip Wood, Oswald Buddenhagen
In-Reply-To: <cover.1698305961.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 7779 bytes --]
Hi,
this is the third version of this patch series that adds GitLab CI
definitions to the Git project. Please refer to the cover letter for v1
of this series [1] for the motivation and intent -- I won't repeat it
here as it's a bit on the longer side.
Changes compared to v2:
- Patch 5: This is a new preparatory step to unify the setup of some
environment variables. It also fixes some smallish issues, like
e.g. the fact that some envvars were set _after_ the export.
- Patch 6: Another new preparatory step. It makes our infra around
certain helper functions that have the intent to interact with the
project repository, e.g. to cache good trees. We now detect the
case when there is no Git or when the project is not a Git repo
and bail out gracefully.
- Patch 7: The last new preparatory patch. Installs a bunch of
dependencies which are required for the test runtime in the Alpine
based job. This increases test coverage.
- Patch 8: Several smaller improvements:
- Added a note why we install sudo and shadow in linux-musl now.
- Fixed an issue where the HOME directory was part of the
project directory, and thus Git complained about newly added
untracked files.
- Added the "pedantic" job that does a pednatic compilation on
Fedora.
- Added more test time dependencies.
- Made the "echo -e" invocation portable by using printf
instead.
What I didn't address yet is a suggestion by Phillip, namely to unify
the logic that sets up unprivileged builds. I don't have the infra
available to test any such change that would ultimately also impact
GitHub Workflows and Azure Pipelines and thus do not feel comfortable
to refactor this. I agree with the suggestion though, so I propose to
rather handle it at a later point in time.
A test run of this patch series can be found at [2].
Thanks!
Patrick
[1]: <cover.1698305961.git.ps@pks.im>
[2]: https://gitlab.com/gitlab-org/git/-/pipelines/1054750795
Patrick Steinhardt (8):
ci: reorder definitions for grouping functions
ci: make grouping setup more generic
ci: group installation of Docker dependencies
ci: split out logic to set up failed test artifacts
ci: unify setup of some environment variables
ci: squelch warnings when testing with unusable Git repo
ci: install test dependencies for linux-musl
ci: add support for GitLab CI
.gitlab-ci.yml | 53 +++++++++
ci/install-docker-dependencies.sh | 22 +++-
ci/lib.sh | 187 +++++++++++++++++++++---------
ci/print-test-failures.sh | 6 +
t/lib-httpd.sh | 3 +-
5 files changed, 215 insertions(+), 56 deletions(-)
create mode 100644 .gitlab-ci.yml
Range-diff against v2:
1: 4eb9cfc816b = 1: ef44ed5c3b1 ci: reorder definitions for grouping functions
2: 85617ef8577 = 2: 77798fa7a7a ci: make grouping setup more generic
3: 57bbc50e3dc = 3: 4542bd38dc2 ci: group installation of Docker dependencies
4: 5ab11d5236d = 4: 5fdda7fd83f ci: split out logic to set up failed test artifacts
-: ----------- > 5: 6af0075fd87 ci: unify setup of some environment variables
-: ----------- > 6: 78d863bf24e ci: squelch warnings when testing with unusable Git repo
-: ----------- > 7: f150d61a1ce ci: install test dependencies for linux-musl
5: 37a507e9b25 ! 8: 5272d66d9f1 ci: add support for GitLab CI
@@ Commit message
This commit introduces the integration into our regular CI scripts so
that most of the setup continues to be shared across all of the CI
- solutions.
+ solutions. Note that as the builds on GitLab CI run as unprivileged
+ user, we need to pull in both sudo and shadow packages to our Alpine
+ based job to set this up.
[1]: https://gitlab.com/gitlab-org/git
@@ .gitlab-ci.yml (new)
+ before_script:
+ - ./ci/install-docker-dependencies.sh
+ script:
-+ - useradd builder --home-dir "${CI_PROJECT_DIR}"
++ - useradd builder --create-home
+ - chown -R builder "${CI_PROJECT_DIR}"
+ - sudo --preserve-env --set-home --user=builder ./ci/run-build-and-tests.sh
+ after_script:
@@ .gitlab-ci.yml (new)
+ - jobname: linux-asan-ubsan
+ image: ubuntu:latest
+ CC: clang
++ - jobname: pedantic
++ image: fedora:latest
+ - jobname: linux-musl
+ image: alpine:latest
+ artifacts:
@@ ci/install-docker-dependencies.sh: linux32)
;;
linux-musl)
- apk add --update build-base curl-dev openssl-dev expat-dev gettext \
-+ apk add --update git shadow sudo build-base curl-dev openssl-dev expat-dev gettext \
- pcre2-dev python3 musl-libintl perl-utils ncurses >/dev/null
++ apk add --update shadow sudo build-base curl-dev openssl-dev expat-dev gettext \
+ pcre2-dev python3 musl-libintl perl-utils ncurses \
+ apache2 bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
;;
+linux-*)
+ apt update -q &&
-+ apt install -q -y sudo git make language-pack-is libsvn-perl apache2 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 ${CC_PACKAGE:-${CC:-gcc}}
++ apt install -q -y sudo git make language-pack-is libsvn-perl apache2 libssl-dev \
++ libcurl4-openssl-dev libexpat-dev tcl tk gettext zlib1g-dev \
++ perl-modules liberror-perl libauthen-sasl-perl libemail-valid-perl \
++ libdbd-sqlite3-perl libio-socket-ssl-perl libnet-smtp-ssl-perl ${CC_PACKAGE:-${CC:-gcc}} \
++ apache2 cvs cvsps gnupg libcgi-pm-perl subversion
+ ;;
pedantic)
dnf -yq update >/dev/null &&
@@ ci/lib.sh: then
+then
+ begin_group () {
+ need_to_end_group=t
-+ echo -e "\e[0Ksection_start:$(date +%s):$(echo "$1" | tr ' ' _)\r\e[0K$1"
++ printf "\e[0Ksection_start:$(date +%s):$(echo "$1" | tr ' ' _)\r\e[0K$1\n"
+ trap "end_group '$1'" EXIT
+ set -x
+ }
@@ ci/lib.sh: then
+ test -n "$need_to_end_group" || return 0
+ set +x
+ need_to_end_group=
-+ echo -e "\e[0Ksection_end:$(date +%s):$(echo "$1" | tr ' ' _)\r\e[0K"
++ printf "\e[0Ksection_end:$(date +%s):$(echo "$1" | tr ' ' _)\r\e[0K\n"
+ trap - EXIT
+ }
else
begin_group () { :; }
end_group () { :; }
@@ ci/lib.sh: then
- MAKEFLAGS="$MAKEFLAGS --jobs=10"
- test windows != "$CI_OS_NAME" ||
- GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS"
+ cache_dir="$HOME/none"
+
+ GIT_TEST_OPTS="--github-workflow-markup"
+elif test true = "$GITLAB_CI"
+then
+ CI_TYPE=gitlab-ci
@@ ci/lib.sh: then
+ case "$CI_JOB_IMAGE" in
+ macos-*)
+ CI_OS_NAME=osx;;
-+ alpine:*|ubuntu:*)
++ alpine:*|fedora:*|ubuntu:*)
+ CI_OS_NAME=linux;;
+ *)
+ echo "Could not identify OS image" >&2
@@ ci/lib.sh: then
+ cache_dir="$HOME/none"
+
+ runs_on_pool=$(echo "$CI_JOB_IMAGE" | tr : -)
-+
-+ export GIT_PROVE_OPTS="--timer --jobs $(nproc)"
-+ export GIT_TEST_OPTS="--verbose-log -x"
-+ MAKEFLAGS="$MAKEFLAGS --jobs=$(nproc)"
-+ 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
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 5/5] ci: add support for GitLab CI
From: Patrick Steinhardt @ 2023-10-30 10:46 UTC (permalink / raw)
To: phillip.wood; +Cc: git, Oswald Buddenhagen
In-Reply-To: <8db10489-546d-428d-9b3e-7ecea87e1313@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1472 bytes --]
On Sun, Oct 29, 2023 at 04:13:39PM +0000, Phillip Wood wrote:
> Hi Patrick
>
> On 27/10/2023 14:02, Phillip Wood wrote:
> > On 27/10/2023 12:57, Patrick Steinhardt wrote:
> > > Hum. After having a look at `ci/run-docker-build.sh` I don't feel like
> > > it's sensible to update it. It's not even used anymore by our CI but
> > > only by `ci/run-docker.sh`, which seems to be more of a developer-facing
> > > script?
> > >
> > > As you said, this smells like rotting bits that might rather be removed.
> > > But in any case, as they don't relate to our current CI infrastructure
> > > except for being in "ci/" I'll leave them be for now.
> >
> > I was trying to suggest that we start using these scripts again.
>
> Having taken a closer look I think we'd be better off adding something like
>
> # Ensure the build and tests run as an unprivileged user
> if test "$(id -u)" -eq 0
> then
> useradd --home-dir "$(pwd)" builder
> chown -R builder .
> exec sudo --preserve-env --set-home --user=builder "$0"
> fi
>
> To the beginning of ci/run-build-and-tests.sh.
That indeed looks like a nice way to handle this, agreed. As mentioned
though, I don't really have an easy way to test this with GitHub
Workflows or Azure Pipelines. So I'd propose to defer this change to a
follow-up patch series -- and in the best case somebody who is familiar
with these CI solutions would pick it up rather than me.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 5/5] ci: add support for GitLab CI
From: Patrick Steinhardt @ 2023-10-30 10:45 UTC (permalink / raw)
To: phillip.wood; +Cc: git, Oswald Buddenhagen
In-Reply-To: <a165fd76-a44b-45c7-b02c-e9b90fda3db3@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3972 bytes --]
On Sun, Oct 29, 2023 at 04:27:44PM +0000, Phillip Wood wrote:
> Hi Patrick
>
> On 27/10/2023 12:19, Patrick Steinhardt wrote:
> > On Fri, Oct 27, 2023 at 11:19:04AM +0100, Phillip Wood wrote:
> > > On 27/10/2023 10:25, Patrick Steinhardt wrote:
> > > > + parallel:
> > > > + matrix:
> > > > + - jobname: linux-sha256
> > > > + image: ubuntu:latest
> > > > + CC: clang
> > > > + - jobname: linux-gcc
> > > > + image: ubuntu:20.04
> > > > + CC: gcc
> > > > + CC_PACKAGE: gcc-8
> > > > + - jobname: linux-TEST-vars
> > > > + image: ubuntu:20.04
> > > > + CC: gcc
> > > > + CC_PACKAGE: gcc-8
> > > > + - jobname: linux-gcc-default
> > > > + image: ubuntu:latest
> > > > + CC: gcc
> > > > + - jobname: linux-leaks
> > > > + image: ubuntu:latest
> > > > + CC: gcc
> > > > + - jobname: linux-asan-ubsan
> > > > + image: ubuntu:latest
> > > > + CC: clang
> > > > + - jobname: linux-musl
> > > > + image: alpine:latest
>
> I assume you've chosen the configurations from the existing GitHub Actions
> that give the best coverage of the various options. One thing I noticed is
> that the is no equivalent of the "pedantic" job that builds git with
> "DEVELOPER=1 DEVOPTS=pedantic"
Yeah, indeed. The list for sure isn't complete, and I also want to
iterate in the future to add support for macOS and Windows. But adding
the pedantic job is easy enough, so I'll include it.
> > > > diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
> > > > index d0bc19d3bb3..1cd92db1876 100755
> > > > --- a/ci/install-docker-dependencies.sh
> > > > +++ b/ci/install-docker-dependencies.sh
> > > > @@ -16,9 +19,13 @@ linux32)
> > > > '
> > > > ;;
> > > > linux-musl)
> > > > - apk add --update build-base curl-dev openssl-dev expat-dev gettext \
> > > > + apk add --update git shadow sudo build-base curl-dev openssl-dev expat-dev gettext \
> > > > pcre2-dev python3 musl-libintl perl-utils ncurses >/dev/null
> > >
> > > It would be helpful to explain the new dependencies in the commit message. I
> > > can see why you're adding sudo, but how were we getting away without
> > > installing the other packages for GitHub Actions?
I've dropped the "git" dependency for now. It doesn't do anything useful
anyway for our Docker-based jobs. Instead, I've added a preparatory
commit that makes the relevant CI library functions more robust by
detecting the case where we either don't have Git available or don't use
a repository.
> > True, that part is missing.
> >
> > - Both sudo and shadow are now required because of `useradd` that we use
> > to set up the unprivileged build.
> >
> > - Git has been required all along, I think. `save_good_tree ()` is used
> > in our CI scripts, and Toon (fellow GitLabber from my team) has
> > noticed that the CI job warned about missing Git. The warning was
> > mostly benign as it seems, but still, doesn't hurt to fix it while at
> > it.
>
> Oh I had a look at this and the docker based jobs on GitHub do not have a
> git repository so installing git means we now get a repository not found
> error from save_good_tree() instead. We should probably make
> save_good_tree() and check_unignored_build_artifacts() return early if there
> isn't a repository but that's orthogonal to this series.
Yup, done now.
> Looking at the test output from the link in your cover letter we should we
> should also install apache2[1] and gnupg[2]
True. Also missing are Bash, CVS, the Perl CGI module, Subversion and
Perforce. I'll add them as available in the respective distro's
repositories.
Patrick
> [1] https://gitlab.com/gitlab-org/git/-/jobs/5349205374#L1444
> [2] https://gitlab.com/gitlab-org/git/-/jobs/5349205374#L1167
>
> Best Wishes
>
> Phillip
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox