* Re: GFW fails with ST3 on Windows 11
From: Konstantin Khomoutov @ 2023-10-27 12:29 UTC (permalink / raw)
To: git
In-Reply-To: <cd248c66-fc71-4783-9195-02c8811744b8@app.fastmail.com>
On Fri, Oct 27, 2023 at 02:31:32PM +0700, John wrote:
> I have been using Sublime Text 3 as the editor on Git for Windows for years,
> on Windows 10. I recently purchased a Windows 11 machine. On that machine,
> when I give GFW the following command, I get the response shown:
>
> $ git commit -a
> […]
> hint: Waiting for your editor to close the file… C:\Program Files\Sublime Text 3\sublime_text.exe: C:Program: command not found
> error: There was a problem with the editor ‘C:\Program Files\Sublime Text 3\sublime_text.exe’.
> Please supply the message using either -m or -F option.
>
> Does anyone know of a fix? To repeat, it’s only a problem on Windows 11.
What if you change the contents of the GIT_EDITOR (or EDITOR - whichever is
set) environment variable to contain forward slashes? That is, for the
pathname to read "C:/Program Files/..."?
If that does not work, what if you replace "C:/" with "/c/" - that is, for the
pathname to read "/c/Program files/..."?
Please note that depending on where this variable is set (in the "System"
block of setting or "User") you might need to reboot or relogin,
correspondingly in order for the setting to be applied to your environment.
Also please check the core.editor configuration setting - it might be set,
and then you should try to modify it instead [3].
While we're at it, such issues are better discussed on the dedicated mailing
list for GfW [1]. Also check the project's issue tracker [2].
1. https://groups.google.com/g/git-for-windows/
2. https://github.com/git-for-windows/git/issues/
3. https://github.com/git-for-windows/git/issues/782
^ permalink raw reply
* Re: [PATCH v2 5/5] ci: add support for GitLab CI
From: Patrick Steinhardt @ 2023-10-27 11:57 UTC (permalink / raw)
To: phillip.wood; +Cc: git, Oswald Buddenhagen
In-Reply-To: <ZTucxlBR1VrJPuSK@tanuki>
[-- Attachment #1: Type: text/plain, Size: 2435 bytes --]
On Fri, Oct 27, 2023 at 01:19:35PM +0200, 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:
[snip]
> > > 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
> > > @@ -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,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?
>
> 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.
>
> I'll have a look at whether I can add another patch on top that adjusts
> `ci/run-docker-build.sh` to do rootless builds, which would also make it
> more obvious why we now need to install sudo and shadow. And I'll make
> sure to document why we now need to have Git around.
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.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Constant recompilation when GENERATE_COMPILATION_DATABASE is set
From: brian m. carlson @ 2023-10-27 11:45 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 1002 bytes --]
I typically use clangd with Git for the nice language server protocol
(LSP) support. I noticed that when GENERATE_COMPILATION_DATABASE is set
(which is used so clangd can find the proper flags), `make` rebuilds all
of the files every time, even if I do it back to back. This persists
even after a `git clean -dxf`.
Obviously, this is not great, since it means the turnaround time to
compile changes is slower. Unfortunately, I'm not a huge expert in make
and I'm unsure what the right solution is here. I'd appreciate anyone's
thoughts on how to improve this.
This is on a Debian sid (unstable) amd64 system with the following
configuration. The version of clang in use is Debian's 16.0.6.
----
NO_OPENSSL=1
DC_SHA1=1
NETTLE_SHA256=1
DEVELOPER=1
NO_SVN_TESTS=1
NO_PYTHON=1
USE_ASCIIDOCTOR=1
USE_ASCIIDOCTOR_MANPAGE=1
CC=clang
GENERATE_COMPILATION_DATABASE=yes
CSPRNG_METHOD=getrandom getentropy
----
--
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply
* Re: GFW fails with ST3 on Windows 11
From: brian m. carlson @ 2023-10-27 11:24 UTC (permalink / raw)
To: John, git
In-Reply-To: <e1de1948-8604-4c8f-961b-d214efa4ed84@app.fastmail.com>
[-- Attachment #1: Type: text/plain, Size: 2221 bytes --]
On 2023-10-27 at 09:59:47, John wrote:
> Brian,
>
> Thanks very much for your suggestions. Unfortunately, after each command, I continue to get the same error messages noted in my original email.
>
> Any Windows 11 experts out there--please help.
>
> John
>
> On Fri, 27 Oct 2023, at 16:25, brian m. carlson wrote:
> > On 2023-10-27 at 07:31:32, John wrote:
> >> I have been using Sublime Text 3 as the editor on Git for Windows for years, on Windows 10. I recently purchased a Windows 11 machine. On that machine, when I give GFW the following command, I get the response shown:
> >>
> >> $ git commit -a
> >> […]
> >> hint: Waiting for your editor to close the file… C:\Program Files\Sublime Text 3\sublime_text.exe: C:Program: command not found
> >> error: There was a problem with the editor ‘C:\Program Files\Sublime Text 3\sublime_text.exe’.
> >> Please supply the message using either -m or -F option.
> >
> > I think the "command not found" indicates that you may have specified
> > your editor incorrectly. In Git Bash, can you try the following
> > commands, one at a time, and then commit between each one to see if one
> > of them works?
> >
> > git config --global core.editor '"C:/Program Files/Sublime Text
> > 3/sublime_text.exe"'
> > git config --global core.editor '"C:\Program Files\Sublime Text
> > 3\sublime_text.exe"'
> >
> > I believe that both should work here.
> >
> > My guess is that this is the situation hinted at in the FAQ[0], where,
> > because the editor is passed to the shell (bash), you need to use double
> > quotes to prevent it from thinking that you have a program named
> > "C:\Program" with arguments "Files\Sublime", "Text", and
> > "3\sublime_text.exe".
> >
> > Hopefully this works; if not, please let us know, and hopefully someone
> > on the list who uses Windows can provide some help. I'm afraid that, as
> > a Linux user, I'm not able to provide more than a guess here.
> >
> > [0] https://git-scm.com/docs/gitfaq#configure-editor
It looks like this was sent to just me and not the list, so forwarding
it back for additional assistance.
--
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply
* Re: [PATCH v2 5/5] ci: add support for GitLab CI
From: Patrick Steinhardt @ 2023-10-27 11:19 UTC (permalink / raw)
To: phillip.wood; +Cc: git, Oswald Buddenhagen
In-Reply-To: <0d889da1-7fd8-4e21-965f-6222e4433ecf@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 10754 bytes --]
On Fri, Oct 27, 2023 at 11:19:04AM +0100, Phillip Wood wrote:
> On 27/10/2023 10:25, Patrick Steinhardt wrote:
> > 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.
>
> I agree we don't want to support too many CI platforms but I think adding
> support for GitLab is good as it helps to stop us being too tied to GitHub
> Actions (which should make it easier if we ever need to transition to a
> different platform in the future) and provides an alternative for
> contributors who want to use a different platform.
>
> > 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].
>
> Having someone committed to on-going maintenance is great.
>
> > 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.
> >
> > [1]: https://gitlab.com/gitlab-org/git
> >
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> > .gitlab-ci.yml | 51 +++++++++++++++++++++++++++++++
> > ci/install-docker-dependencies.sh | 9 +++++-
> > ci/lib.sh | 49 +++++++++++++++++++++++++++++
> > 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..43d3a961fa0
> > --- /dev/null
> > +++ b/.gitlab-ci.yml
> > @@ -0,0 +1,51 @@
> > +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 --home-dir "${CI_PROJECT_DIR}"
> > + - chown -R builder "${CI_PROJECT_DIR}"
> > + - sudo --preserve-env --set-home --user=builder ./ci/run-build-and-tests.sh
>
> It's really good that you're running the tests as an unprivileged user. This
> is something we used to do when we were using Travis that got lost in the
> transition to Azure Pipelines which means some tests that rely on httpd are
> now skipped as they refuse to run as root. ci/run-docker-build.sh is
> currently bit-rotting, I wonder if it is possible to update it so that we
> can run the dockerized tests in the same way on all CI platforms.
>
> > + 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: linux-musl
> > + image: alpine:latest
> > + artifacts:
> > + paths:
> > + - t/failed-test-artifacts
> > + when: on_failure
>
> This file is pleasingly small.
>
> > 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
> > @@ -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,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?
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.
I'll have a look at whether I can add another patch on top that adjusts
`ci/run-docker-build.sh` to do rootless builds, which would also make it
more obvious why we now need to install sudo and shadow. And I'll make
sure to document why we now need to have Git around.
> > ;;
> > +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}}
> > + ;;
> > 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 9ffdf743903..f518df7e2cb 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
> > + echo -e "\e[0Ksection_start:$(date +%s):$(echo "$1" | tr ' ' _)\r\e[0K$1"
> > + trap "end_group '$1'" EXIT
> > + set -x
> > + }
> > +
> > + end_group () {
> > + 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"
> > + trap - EXIT
> > + }
> > else
> > begin_group () { :; }
> > end_group () { :; }
> > @@ -203,6 +219,39 @@ then
> > MAKEFLAGS="$MAKEFLAGS --jobs=10"
> > test windows != "$CI_OS_NAME" ||
> > GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS"
> > +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:*|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"
>
> I guess making this explicit is helpful, otherwise someone may wonder why
> CI_JOB_ID is not being set.
>
> > + 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 : -)
> > +
> > + 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"
>
> This last paragraph feels like it should be common to all the CI providers.
> There are some small differences but if we're going to support several
> providers it would be nice to set the common options centrally. I'm pretty
> sure the --jobs=10 we use for the GitHub and Azure is not optimal, using
> $(nproc) is nice so long as it is supported on all the images we use.
True enough, I can have a go at it. I'm rather cautious around updating
the build infra as I cannot easily verify that things continue to work
for GitHub. But this part here might be a good candidate for deduping.
> I had a quick glance through the previous patches and they all look like
> nice cleanups that make our ci support less dependent on a single provider.
> This series looks like a nice addition to our CI support.
>
> Best Wishes
>
> Phillip
Thanks!
Patrick
>
> > 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
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* RE: [EXT] Re: ls-remote bug
From: Lior Zeltzer @ 2023-10-27 11:16 UTC (permalink / raw)
To: Bagas Sanjaya, Git Mailing List
Cc: Andrzej Hunt, Ævar Arnfjörð Bjarmason,
Junio C Hamano
In-Reply-To: <ZTsNfgJV1JN2y-Aw@debian.me>
The reproduction ,as I wrote in the code, should be done with few threads in parallel
Each working on a list of ~10 repos with each repo containing a lot of refs/tags (~1000)
All this should be against gerrit (my gerrit is 3.8.0)
Also read the notes below regarding the code that was moved between 2.31.8 and 2.32.0 in ls-remote.c file
I can elaborate more, in a zoom meeting.
10x
Lior.
-----Original Message-----
From: Bagas Sanjaya <bagasdotme@gmail.com>
Sent: Friday, October 27, 2023 4:08 AM
To: Lior Zeltzer <liorz@marvell.com>; Git Mailing List <git@vger.kernel.org>
Cc: Andrzej Hunt <ajrhunt@google.com>; Ævar Arnfjörð Bjarmason <avarab@gmail.com>; Junio C Hamano <gitster@pobox.com>
Subject: [EXT] Re: ls-remote bug
External Email
----------------------------------------------------------------------
On Tue, Oct 24, 2023 at 10:55:24AM +0000, Lior Zeltzer wrote:
>
> >uname -a
> Linux dc3lp-veld0045 3.10.0-1160.21.1.el7.x86_64 #1 SMP Tue Mar 16
> 18:28:22 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
>
> Gerrit version :
> 3.8.0
>
> Bug description :
> When running ls-remote : sometime data gets cut in the middle
>
> Reproducing :
> You need a few files with a few repo names (I used 4 files with 10
> repos each) Call then l1..l4 And the code below just cd into each of
> them does ls-remote twice and compares the data Doing it in parallel
> on all lists.
> Data received in both ls-remotes should be the same , if not, it
> prints *** Repos should contain a lot of tags and refs
What repo did you find this regression? Did you mean linux.git (Linux kernel)?
>
> Note :
> 1. without stderr redirection (2>&1) all works well 2. On local repos
> (not through gerrit) all works well
>
> I compared various git vers and found the bug to be between 2.31.8 and
> 2.32.0 Comparing ls-remote.c file between those vers gave me :
>
> Lines :
> if (transport_disconnect(transport))
> return 1;
>
> moved to end of sub
>
> copying ls-remote.c from 2.31.8 to 2.32.0 - fixed the bug
>
>
>
> Code reproducing bug :
>
> #!/proj/mislcad/areas/DAtools/tools/perl/5.10.1/bin/perl -w use
> strict; use Cwd qw(cwd);
>
> my $count = 4;
> for my $f (1..$count) {
> my $child = fork();
> if (!$child) {
> my $curr = cwd();
>
> my @repos = `cat l$f`;
> foreach my $repo (@repos) {
> chomp $repo;
> print "$repo\n";
> chdir($repo);
> my $remote_tags_str = `git ls-remote 2>&1`;
> my $remote_tags_str2 = `git ls-remote 2>&1 `;
> chdir($curr);
> if ( $remote_tags_str ne $remote_tags_str2) {
> print "***\n";
> }
> }
>
> exit(0);
> }
> }
> while (wait != -1) {}
> 1;
>
I tried reproducing this regression by:
```
$ cd /path/to/git.git
$ git ls-remote 2>&1 > /tmp/root.list
$ cd builtin/
$ git ls-remote 2>&1 > /tmp/builtin.list $ cd ../ $ git diff --no-index /tmp/root.list /tmp/builtin.list ```
And indeed, the diff was empty (which meant that both listings are same).
Confused...
--
An old man doll... just what I always wanted! - Clara
^ permalink raw reply
* Re: [PATCH 5/5] ci: add support for GitLab CI
From: Patrick Steinhardt @ 2023-10-27 11:11 UTC (permalink / raw)
To: Oswald Buddenhagen; +Cc: git
In-Reply-To: <ZTuVr54KxhdaIf+F@ugly>
[-- Attachment #1: Type: text/plain, Size: 1260 bytes --]
On Fri, Oct 27, 2023 at 12:49:19PM +0200, Oswald Buddenhagen wrote:
> On Fri, Oct 27, 2023 at 10:17:33AM +0200, Patrick Steinhardt wrote:
> > On Thu, Oct 26, 2023 at 11:07:08AM +0200, Oswald Buddenhagen wrote:
> > > you should probably choose a less generic name for the jobs, at least
> > > debian-*.
> >
> > The names are all preexisting, so I cannot change them.
> >
> aren't they coming from the yml file? would adjusting them in the company
> setup be an unreasonable effort?
They come from the ".gitlab-ci.yml" file, but I have to reuse the exact
names that GitHub Actions already uses or otherwise we're not testing
for the same thing. The preexisting CI scripts for Git expect exactly
those names.
I do agree that they may benefit from a redesign so that they're more
explicit. But I don't think this patch series here is where we should do
that refactoring.
> > I don't quite know what you mean by counter-intuitive patch structure.
> >
> it looked like you're adding the function to the github branch, not to the
> freshly added gitlab branch. of course that's just a diffing artifact.
Ah, thought you meant the larger "structure of how things were layed
out". Agreed and fixed in v2 of the patch series.
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: Oswald Buddenhagen @ 2023-10-27 11:01 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <37a507e9b255dd40c0536438dfe3fa05b067f08f.1698398590.git.ps@pks.im>
On Fri, Oct 27, 2023 at 11:25:41AM +0200, Patrick Steinhardt wrote:
>+ export GIT_PROVE_OPTS="--timer --jobs $(nproc)"
>+ export GIT_TEST_OPTS="--verbose-log -x"
>
fwiw (as this is again only copied), export with assignment is a
bash-ism (though (d)ash started to accept this syntax some time ago),
and not all of the including scripts ask for bash (i didn't check
whether they are using these functions, but the inconsistency is an
armed trap).
regards
^ permalink raw reply
* Re: [PATCH 5/5] ci: add support for GitLab CI
From: Oswald Buddenhagen @ 2023-10-27 10:49 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <ZTtyHScKkMg4qnMH@tanuki>
On Fri, Oct 27, 2023 at 10:17:33AM +0200, Patrick Steinhardt wrote:
>On Thu, Oct 26, 2023 at 11:07:08AM +0200, Oswald Buddenhagen wrote:
>> you should probably choose a less generic name for the jobs, at least
>> debian-*.
>
>The names are all preexisting, so I cannot change them.
>
aren't they coming from the yml file? would adjusting them in the
company setup be an unreasonable effort?
>I don't quite know what you mean by counter-intuitive patch structure.
>
it looked like you're adding the function to the github branch, not to
the freshly added gitlab branch. of course that's just a diffing
artifact.
regards
^ permalink raw reply
* Re: [PATCH 5/5] ci: add support for GitLab CI
From: Oswald Buddenhagen @ 2023-10-27 10:43 UTC (permalink / raw)
To: Phillip Wood; +Cc: Patrick Steinhardt, git
In-Reply-To: <d62b2e38-6e24-4661-b51d-2ecc59eae256@crinan.ddns.net>
On Fri, Oct 27, 2023 at 11:22:35AM +0100, Phillip Wood wrote:
>On 27/10/2023 09:17, Patrick Steinhardt wrote:
>>>> + CI_BRANCH="$CI_COMMIT_REF_NAME"
>>>> + CI_COMMIT="$CI_COMMIT_SHA"
>>>>
>>> assignments need no quoting to prevent word splitting.
>>> repeats below.
>>>
>>>> + case "$CI_JOB_IMAGE" in
>>>>
>>> ... as does the selector in case statements.
>>
>> True, but I'm simply matching the coding style in this script.
>
>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. of course, many people (apparently you included) don't know these
subtleties, so just quoting everything isn't the worst idea (though it
would backfire with some *really* old buggy shells, but this doesn't
need to concern us).
regards
^ permalink raw reply
* Re: [PATCH 5/5] ci: add support for GitLab CI
From: Phillip Wood @ 2023-10-27 10:22 UTC (permalink / raw)
To: Patrick Steinhardt, Oswald Buddenhagen; +Cc: git
In-Reply-To: <ZTtyHScKkMg4qnMH@tanuki>
On 27/10/2023 09:17, Patrick Steinhardt wrote:
>>> + CI_BRANCH="$CI_COMMIT_REF_NAME"
>>> + CI_COMMIT="$CI_COMMIT_SHA"
>>>
>> assignments need no quoting to prevent word splitting.
>> repeats below.
>>
>>> + case "$CI_JOB_IMAGE" in
>>>
>> ... as does the selector in case statements.
>
> True, but I'm simply matching the coding style in this script.
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.
>>> --- 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"
>>>
>> you're just following the precedent, but imo it's more legible to quote the
>> entire string, not just the variable expansion. the code doesn't even agree
>> with itself, as the line directly above apparently agrees with me.
>>
>> regards
>
> Yeah, as you say, this is another case where I follow precedent. I
> honestly don't quite care which way we go in this case.
Yes, if you're following existing practice I don't think this is worth
worrying about.
Best Wishes
Phillip
^ permalink raw reply
* Re: [PATCH v2 5/5] ci: add support for GitLab CI
From: Phillip Wood @ 2023-10-27 10:19 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Oswald Buddenhagen
In-Reply-To: <37a507e9b255dd40c0536438dfe3fa05b067f08f.1698398590.git.ps@pks.im>
On 27/10/2023 10:25, Patrick Steinhardt wrote:
> 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.
I agree we don't want to support too many CI platforms but I think
adding support for GitLab is good as it helps to stop us being too tied
to GitHub Actions (which should make it easier if we ever need to
transition to a different platform in the future) and provides an
alternative for contributors who want to use a different platform.
> 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].
Having someone committed to on-going maintenance is great.
> 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.
>
> [1]: https://gitlab.com/gitlab-org/git
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> .gitlab-ci.yml | 51 +++++++++++++++++++++++++++++++
> ci/install-docker-dependencies.sh | 9 +++++-
> ci/lib.sh | 49 +++++++++++++++++++++++++++++
> 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..43d3a961fa0
> --- /dev/null
> +++ b/.gitlab-ci.yml
> @@ -0,0 +1,51 @@
> +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 --home-dir "${CI_PROJECT_DIR}"
> + - chown -R builder "${CI_PROJECT_DIR}"
> + - sudo --preserve-env --set-home --user=builder ./ci/run-build-and-tests.sh
It's really good that you're running the tests as an unprivileged user.
This is something we used to do when we were using Travis that got lost
in the transition to Azure Pipelines which means some tests that rely on
httpd are now skipped as they refuse to run as root.
ci/run-docker-build.sh is currently bit-rotting, I wonder if it is
possible to update it so that we can run the dockerized tests in the
same way on all CI platforms.
> + 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: linux-musl
> + image: alpine:latest
> + artifacts:
> + paths:
> + - t/failed-test-artifacts
> + when: on_failure
This file is pleasingly small.
> 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
> @@ -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,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?
> ;;
> +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}}
> + ;;
> 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 9ffdf743903..f518df7e2cb 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
> + echo -e "\e[0Ksection_start:$(date +%s):$(echo "$1" | tr ' ' _)\r\e[0K$1"
> + trap "end_group '$1'" EXIT
> + set -x
> + }
> +
> + end_group () {
> + 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"
> + trap - EXIT
> + }
> else
> begin_group () { :; }
> end_group () { :; }
> @@ -203,6 +219,39 @@ then
> MAKEFLAGS="$MAKEFLAGS --jobs=10"
> test windows != "$CI_OS_NAME" ||
> GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS"
> +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:*|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"
I guess making this explicit is helpful, otherwise someone may wonder
why CI_JOB_ID is not being set.
> + 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 : -)
> +
> + 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"
This last paragraph feels like it should be common to all the CI
providers. There are some small differences but if we're going to
support several providers it would be nice to set the common options
centrally. I'm pretty sure the --jobs=10 we use for the GitHub and Azure
is not optimal, using $(nproc) is nice so long as it is supported on all
the images we use.
I had a quick glance through the previous patches and they all look like
nice cleanups that make our ci support less dependent on a single
provider. This series looks like a nice addition to our CI support.
Best Wishes
Phillip
> 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
^ permalink raw reply
* Re: GFW fails with ST3 on Windows 11
From: brian m. carlson @ 2023-10-27 9:25 UTC (permalink / raw)
To: John; +Cc: git
In-Reply-To: <cd248c66-fc71-4783-9195-02c8811744b8@app.fastmail.com>
[-- Attachment #1: Type: text/plain, Size: 1663 bytes --]
On 2023-10-27 at 07:31:32, John wrote:
> I have been using Sublime Text 3 as the editor on Git for Windows for years, on Windows 10. I recently purchased a Windows 11 machine. On that machine, when I give GFW the following command, I get the response shown:
>
> $ git commit -a
> […]
> hint: Waiting for your editor to close the file… C:\Program Files\Sublime Text 3\sublime_text.exe: C:Program: command not found
> error: There was a problem with the editor ‘C:\Program Files\Sublime Text 3\sublime_text.exe’.
> Please supply the message using either -m or -F option.
I think the "command not found" indicates that you may have specified
your editor incorrectly. In Git Bash, can you try the following
commands, one at a time, and then commit between each one to see if one
of them works?
git config --global core.editor '"C:/Program Files/Sublime Text 3/sublime_text.exe"'
git config --global core.editor '"C:\Program Files\Sublime Text 3\sublime_text.exe"'
I believe that both should work here.
My guess is that this is the situation hinted at in the FAQ[0], where,
because the editor is passed to the shell (bash), you need to use double
quotes to prevent it from thinking that you have a program named
"C:\Program" with arguments "Files\Sublime", "Text", and
"3\sublime_text.exe".
Hopefully this works; if not, please let us know, and hopefully someone
on the list who uses Windows can provide some help. I'm afraid that, as
a Linux user, I'm not able to provide more than a guess here.
[0] https://git-scm.com/docs/gitfaq#configure-editor
--
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply
* [PATCH v2 5/5] ci: add support for GitLab CI
From: Patrick Steinhardt @ 2023-10-27 9:25 UTC (permalink / raw)
To: git; +Cc: Oswald Buddenhagen
In-Reply-To: <cover.1698398590.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 6955 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.
[1]: https://gitlab.com/gitlab-org/git
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
.gitlab-ci.yml | 51 +++++++++++++++++++++++++++++++
ci/install-docker-dependencies.sh | 9 +++++-
ci/lib.sh | 49 +++++++++++++++++++++++++++++
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..43d3a961fa0
--- /dev/null
+++ b/.gitlab-ci.yml
@@ -0,0 +1,51 @@
+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 --home-dir "${CI_PROJECT_DIR}"
+ - 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: 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 d0bc19d3bb3..1cd92db1876 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,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
;;
+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}}
+ ;;
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 9ffdf743903..f518df7e2cb 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
+ echo -e "\e[0Ksection_start:$(date +%s):$(echo "$1" | tr ' ' _)\r\e[0K$1"
+ trap "end_group '$1'" EXIT
+ set -x
+ }
+
+ end_group () {
+ 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"
+ trap - EXIT
+ }
else
begin_group () { :; }
end_group () { :; }
@@ -203,6 +219,39 @@ then
MAKEFLAGS="$MAKEFLAGS --jobs=10"
test windows != "$CI_OS_NAME" ||
GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS"
+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:*|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 : -)
+
+ 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
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 v2 4/5] ci: split out logic to set up failed test artifacts
From: Patrick Steinhardt @ 2023-10-27 9:25 UTC (permalink / raw)
To: git; +Cc: Oswald Buddenhagen
In-Reply-To: <cover.1698398590.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 v2 3/5] ci: group installation of Docker dependencies
From: Patrick Steinhardt @ 2023-10-27 9:25 UTC (permalink / raw)
To: git; +Cc: Oswald Buddenhagen
In-Reply-To: <cover.1698398590.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 v2 2/5] ci: make grouping setup more generic
From: Patrick Steinhardt @ 2023-10-27 9:25 UTC (permalink / raw)
To: git; +Cc: Oswald Buddenhagen
In-Reply-To: <cover.1698398590.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 v2 0/5] ci: add GitLab CI definition
From: Patrick Steinhardt @ 2023-10-27 9:25 UTC (permalink / raw)
To: git; +Cc: Oswald Buddenhagen
In-Reply-To: <cover.1698305961.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 9177 bytes --]
Hi,
this is the second 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 v1:
- Improved commit messages.
- The `group ()` function is now generic across all the different CI
solutions to reduce duplication.
This should hopefully address the feedback from Oswald -- thanks again!
Patrick
[1]: <cover.1698305961.git.ps@pks.im>
Patrick Steinhardt (5):
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: add support for GitLab CI
.gitlab-ci.yml | 51 +++++++++++
ci/install-docker-dependencies.sh | 15 +++-
ci/lib.sh | 139 ++++++++++++++++++++----------
ci/print-test-failures.sh | 6 ++
4 files changed, 166 insertions(+), 45 deletions(-)
create mode 100644 .gitlab-ci.yml
Range-diff against v1:
1: 586a8d1003b ! 1: 4eb9cfc816b ci: reorder definitions for grouping functions
@@ Commit message
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 GitLab Actions, where we define the non-stub logic in the
+ _not_ being GitHub Actions, where we define the non-stub logic in the
else branch.
- Reorder the definitions such that we explicitly handle GitHub Actions.
+ Reorder the conditional branches such that we explicitly handle GitHub
+ Actions.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
2: ec390354f15 ! 2: 85617ef8577 ci: make grouping setup more generic
@@ Commit message
or not. This ensures we can more readily add support for additional CI
platforms.
- Second, 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.
+ 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: then
echo '::endgroup::' >&2
}
- trap end_group EXIT
-
- group () {
- set +x
+-
+- group () {
+- set +x
- begin_group "$1"
-+
-+ group="$1"
- shift
-+ begin_group "$group"
-+
- # work around `dash` not supporting `set -o pipefail`
- (
- "$@" 2>&1
-@@ ci/lib.sh: then
- sed 's/^\(\([^ ]*\):\([0-9]*\):\([0-9]*:\) \)\(error\|warning\): /::\5 file=\2,line=\3::\1/'
- res=$(cat exit.status)
- rm exit.status
+- 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
-+
-+ end_group "$group"
- return $res
- }
+- return $res
+- }
-
- begin_group "CI setup"
else
begin_group () { :; }
end_group () { :; }
-@@ ci/lib.sh: else
+
+- 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
+
3: a65d235dd3c ! 3: 57bbc50e3dc ci: group installation of Docker dependencies
@@ Metadata
## Commit message ##
ci: group installation of Docker dependencies
- Pull in "lib.sh" into "install-docker-dependencies.sh" such that we can
- set up proper groups for those dependencise. This allows the reader to
- collapse sections in the CI output on GitHub Actions (and later on on
- GitLab CI).
+ 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>
4: 4a864a1d174 ! 4: 5ab11d5236d ci: split out logic to set up failed test artifacts
@@ Commit message
ci: split out logic to set up failed test artifacts
We have some logic in place to create a directory with the output from
- failed tests, which will then subsequently be uploaded as CI artifact.
+ 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.
5: 35b07e5378d ! 5: 37a507e9b25 ci: add support for GitLab CI
@@ Commit message
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 sets a
+ 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 would otherwise 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 ensure to send better patch series upstream and thus
- reduce overhead for the maintainer.
+ 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
@@ ci/install-docker-dependencies.sh: linux32)
## ci/lib.sh ##
@@ ci/lib.sh: then
+ need_to_end_group=
echo '::endgroup::' >&2
}
-
-+ group () {
-+ set +x
-+
-+ 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
-+ }
+elif test true = "$GITLAB_CI"
+then
+ begin_group () {
@@ ci/lib.sh: then
+ echo -e "\e[0Ksection_end:$(date +%s):$(echo "$1" | tr ' ' _)\r\e[0K"
+ trap - EXIT
+ }
-+
- group () {
- set +x
-
+ else
+ begin_group () { :; }
+ end_group () { :; }
@@ ci/lib.sh: then
MAKEFLAGS="$MAKEFLAGS --jobs=10"
test windows != "$CI_OS_NAME" ||
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v2 1/5] ci: reorder definitions for grouping functions
From: Patrick Steinhardt @ 2023-10-27 9:25 UTC (permalink / raw)
To: git; +Cc: Oswald Buddenhagen
In-Reply-To: <cover.1698398590.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
* Re: [PATCH 5/5] ci: add support for GitLab CI
From: Patrick Steinhardt @ 2023-10-27 8:17 UTC (permalink / raw)
To: Oswald Buddenhagen; +Cc: git
In-Reply-To: <ZTosPCkpx/FMTDH5@ugly>
[-- Attachment #1: Type: text/plain, Size: 3281 bytes --]
On Thu, Oct 26, 2023 at 11:07:08AM +0200, Oswald Buddenhagen wrote:
> On Thu, Oct 26, 2023 at 10:00:20AM +0200, Patrick Steinhardt wrote:
> > project has. More esoteric jobs like "linux-TEST-vars" that also sets a
> ^ -s
>
> > 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 would otherwise fail to compile or pass tests in GitHub Workflows.
> ^^^^^^^^^^^^^^^
> that inverts the meaning
>
> > [...]
> > project to help us ensure to send better patch series upstream and thus
> ^^
> "we"
> > reduce overhead for the maintainer.
>
> > --- 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
> > ;;
> > +linux-*)
> >
> you should probably choose a less generic name for the jobs, at least
> debian-*.
The names are all preexisting, so I cannot change them. And the CI infra
does indeed rely on the exact names to choose what to do.
> > diff --git a/ci/lib.sh b/ci/lib.sh
> > index 33005854520..102e9d04a1f 100755
> > --- a/ci/lib.sh
> > +++ b/ci/lib.sh
> > @@ -15,6 +15,42 @@ then
> > echo '::endgroup::' >&2
> > }
> >
> > + group () {
> > [...]
> > + }
> >
> this counter-intutive patch structure reveals that the function is
> duplicated between github and gitlab. you may want to factor it out and
> alias it. or change the structure entirely (circling back to patch 1/5).
I don't quite know what you mean by counter-intuitive patch structure.
But regarding the duplication for the `group ()` function I agree, it's
a bit unfortunate. My first version did de-duplicate it indeed, but it
resulted in some weirdness in the stubbed case.
I'll revisit this.
> > + CI_BRANCH="$CI_COMMIT_REF_NAME"
> > + CI_COMMIT="$CI_COMMIT_SHA"
> >
> assignments need no quoting to prevent word splitting.
> repeats below.
>
> > + case "$CI_JOB_IMAGE" in
> >
> ... as does the selector in case statements.
True, but I'm simply matching the coding style in this script.
> > --- 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"
> >
> you're just following the precedent, but imo it's more legible to quote the
> entire string, not just the variable expansion. the code doesn't even agree
> with itself, as the line directly above apparently agrees with me.
>
> regards
Yeah, as you say, this is another case where I follow precedent. I
honestly don't quite care which way we go in this case.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 3/5] ci: group installation of Docker dependencies
From: Patrick Steinhardt @ 2023-10-27 8:17 UTC (permalink / raw)
To: Oswald Buddenhagen; +Cc: git
In-Reply-To: <ZTokgo1mQ7ZVH7GU@ugly>
[-- Attachment #1: Type: text/plain, Size: 702 bytes --]
On Thu, Oct 26, 2023 at 10:34:10AM +0200, Oswald Buddenhagen wrote:
> On Thu, Oct 26, 2023 at 10:00:12AM +0200, Patrick Steinhardt wrote:
> > Pull in "lib.sh" into "install-docker-dependencies.sh" such that we can
> > set up proper groups for those dependencise. This allows the reader to
> ^^ !!!
> > collapse sections in the CI output on GitHub Actions (and later on on
> > GitLab CI).
> >
> the structure of the text is kind of backwards - the fact that you need to
> pull in a lib is just a consequence, not the intent, which imo should come
> first. tough it mostly doesn't matter, as it's just one short paragraph.
>
> regards
Good point indeed. Will rewrite.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 1/5] ci: reorder definitions for grouping functions
From: Patrick Steinhardt @ 2023-10-27 8:17 UTC (permalink / raw)
To: Oswald Buddenhagen; +Cc: git
In-Reply-To: <ZToin/fQiZrmUJTS@ugly>
[-- Attachment #1: Type: text/plain, Size: 1045 bytes --]
On Thu, Oct 26, 2023 at 10:26:07AM +0200, Oswald Buddenhagen wrote:
> On Thu, Oct 26, 2023 at 10:00:03AM +0200, Patrick Steinhardt wrote:
> > [...]
> > _not_ being GitLab Actions, where we define the non-stub logic in the
> >
> you meant GitHub here.
>
> > else branch.
> >
> > Reorder the definitions such that we explicitly handle GitHub Actions.
> >
> i'd say something like "the conditional branches". imo that makes it clearer
> that you're actually talking about code, not some markup or whatever.
> for that matter, this is my overall impression of the commit message - it
> seems way too detached from the near-trivial fact that you're just slightly
> adjusting the code structure to make it easier to implement a cascade (aka a
> switch).
>
> regards
The change is trivial indeed, but even a trivial change needs a reason
why it should be done. Maybe it's too long, maybe it isn't... I'm happy
to take suggestions.
But anyway, I've adopted both of the other two suggestions you made,
thanks.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v5 3/3] rev-list: add commit object support in `--missing` option
From: Karthik Nayak @ 2023-10-27 7:59 UTC (permalink / raw)
To: karthik.188; +Cc: git, gitster, ps
In-Reply-To: <ZTtXzg4NGJZzAqfS@tanuki>
The `--missing` object option in rev-list currently works only with
missing blobs/trees. For missing commits the revision walker fails with
a fatal error.
Let's extend the functionality of `--missing` option to also support
commit objects. This is done by adding a `missing_objects` field to
`rev_info`. This field is an `oidset` to which we'll add the missing
commits as we encounter them. The revision walker will now continue the
traversal and call `show_commit()` even for missing commits. In rev-list
we can then check if the commit is a missing commit and call the
existing code for parsing `--missing` objects.
A scenario where this option would be used is to find the boundary
objects between different object directories. Consider a repository with
a main object directory (GIT_OBJECT_DIRECTORY) and one or more alternate
object directories (GIT_ALTERNATE_OBJECT_DIRECTORIES). In such a
repository, using the `--missing=print` option while disabling the
alternate object directory allows us to find the boundary objects
between the main and alternate object directory.
Helped-by: Patrick Steinhardt <ps@pks.im>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
builtin/rev-list.c | 6 +++
list-objects.c | 3 ++
revision.c | 16 +++++++-
revision.h | 4 ++
t/t6022-rev-list-missing.sh | 74 +++++++++++++++++++++++++++++++++++++
5 files changed, 101 insertions(+), 2 deletions(-)
create mode 100755 t/t6022-rev-list-missing.sh
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 98542e8b3c..181353dcf5 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -149,6 +149,12 @@ static void show_commit(struct commit *commit, void *data)
display_progress(progress, ++progress_counter);
+ if (revs->do_not_die_on_missing_objects &&
+ oidset_contains(&revs->missing_commits, &commit->object.oid)) {
+ finish_object__ma(&commit->object);
+ return;
+ }
+
if (show_disk_usage)
total_disk_usage += get_object_disk_usage(&commit->object);
diff --git a/list-objects.c b/list-objects.c
index 47296dff2f..f4e1104b56 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -389,6 +389,9 @@ static void do_traverse(struct traversal_context *ctx)
*/
if (!ctx->revs->tree_objects)
; /* do not bother loading tree */
+ else if (ctx->revs->do_not_die_on_missing_objects &&
+ oidset_contains(&ctx->revs->missing_commits, &commit->object.oid))
+ ;
else if (repo_get_commit_tree(the_repository, commit)) {
struct tree *tree = repo_get_commit_tree(the_repository,
commit);
diff --git a/revision.c b/revision.c
index 219dc76716..00d5c29bfc 100644
--- a/revision.c
+++ b/revision.c
@@ -6,6 +6,7 @@
#include "object-name.h"
#include "object-file.h"
#include "object-store-ll.h"
+#include "oidset.h"
#include "tag.h"
#include "blob.h"
#include "tree.h"
@@ -1112,6 +1113,9 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
if (commit->object.flags & ADDED)
return 0;
+ if (revs->do_not_die_on_missing_objects &&
+ oidset_contains(&revs->missing_commits, &commit->object.oid))
+ return 0;
commit->object.flags |= ADDED;
if (revs->include_check &&
@@ -1168,7 +1172,8 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
for (parent = commit->parents; parent; parent = parent->next) {
struct commit *p = parent->item;
int gently = revs->ignore_missing_links ||
- revs->exclude_promisor_objects;
+ revs->exclude_promisor_objects ||
+ revs->do_not_die_on_missing_objects;
if (repo_parse_commit_gently(revs->repo, p, gently) < 0) {
if (revs->exclude_promisor_objects &&
is_promisor_object(&p->object.oid)) {
@@ -1176,7 +1181,11 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
break;
continue;
}
- return -1;
+
+ if (revs->do_not_die_on_missing_objects)
+ oidset_insert(&revs->missing_commits, &p->object.oid);
+ else
+ return -1; /* corrupt repository */
}
if (revs->sources) {
char **slot = revision_sources_at(revs->sources, p);
@@ -3109,6 +3118,7 @@ void release_revisions(struct rev_info *revs)
clear_decoration(&revs->merge_simplification, free);
clear_decoration(&revs->treesame, free);
line_log_free(revs);
+ oidset_clear(&revs->missing_commits);
}
static void add_child(struct rev_info *revs, struct commit *parent, struct commit *child)
@@ -3800,6 +3810,8 @@ int prepare_revision_walk(struct rev_info *revs)
FOR_EACH_OBJECT_PROMISOR_ONLY);
}
+ oidset_init(&revs->missing_commits, 0);
+
if (!revs->reflog_info)
prepare_to_use_bloom_filter(revs);
if (!revs->unsorted_input)
diff --git a/revision.h b/revision.h
index c73c92ef40..94c43138bc 100644
--- a/revision.h
+++ b/revision.h
@@ -4,6 +4,7 @@
#include "commit.h"
#include "grep.h"
#include "notes.h"
+#include "oidset.h"
#include "pretty.h"
#include "diff.h"
#include "commit-slab-decl.h"
@@ -373,6 +374,9 @@ struct rev_info {
/* Location where temporary objects for remerge-diff are written. */
struct tmp_objdir *remerge_objdir;
+
+ /* Missing commits to be tracked without failing traversal. */
+ struct oidset missing_commits;
};
/**
diff --git a/t/t6022-rev-list-missing.sh b/t/t6022-rev-list-missing.sh
new file mode 100755
index 0000000000..40265a4f66
--- /dev/null
+++ b/t/t6022-rev-list-missing.sh
@@ -0,0 +1,74 @@
+#!/bin/sh
+
+test_description='handling of missing objects in rev-list'
+
+TEST_PASSES_SANITIZE_LEAK=true
+. ./test-lib.sh
+
+# We setup the repository with two commits, this way HEAD is always
+# available and we can hide commit 1.
+test_expect_success 'create repository and alternate directory' '
+ test_commit 1 &&
+ test_commit 2 &&
+ test_commit 3
+'
+
+for obj in "HEAD~1" "HEAD~1^{tree}" "HEAD:1.t"
+do
+ test_expect_success "rev-list --missing=error fails with missing object $obj" '
+ oid="$(git rev-parse $obj)" &&
+ path=".git/objects/$(test_oid_to_path $oid)" &&
+
+ mv "$path" "$path.hidden" &&
+ test_when_finished "mv $path.hidden $path" &&
+
+ test_must_fail git rev-list --missing=error --objects \
+ --no-object-names HEAD
+ '
+done
+
+for obj in "HEAD~1" "HEAD~1^{tree}" "HEAD:1.t"
+do
+ for action in "allow-any" "print"
+ do
+ test_expect_success "rev-list --missing=$action with missing $obj" '
+ oid="$(git rev-parse $obj)" &&
+ path=".git/objects/$(test_oid_to_path $oid)" &&
+
+ # Before the object is made missing, we use rev-list to
+ # get the expected oids.
+ git rev-list --objects --no-object-names \
+ HEAD ^$obj >expect.raw &&
+
+ # Blobs are shared by all commits, so evethough a commit/tree
+ # might be skipped, its blob must be accounted for.
+ if [ $obj != "HEAD:1.t" ]; then
+ echo $(git rev-parse HEAD:1.t) >>expect.raw &&
+ echo $(git rev-parse HEAD:2.t) >>expect.raw
+ fi &&
+
+ mv "$path" "$path.hidden" &&
+ test_when_finished "mv $path.hidden $path" &&
+
+ git rev-list --missing=$action --objects --no-object-names \
+ HEAD >actual.raw &&
+
+ # When the action is to print, we should also add the missing
+ # oid to the expect list.
+ case $action in
+ allow-any)
+ ;;
+ print)
+ grep ?$oid actual.raw &&
+ echo ?$oid >>expect.raw
+ ;;
+ esac &&
+
+ sort actual.raw >actual &&
+ sort expect.raw >expect &&
+ test_cmp expect actual
+ '
+ done
+done
+
+test_done
--
2.42.0
^ permalink raw reply related
* Re: [PATCH v5 3/3] rev-list: add commit object support in `--missing` option
From: Karthik Nayak @ 2023-10-27 7:54 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, gitster
In-Reply-To: <ZTtXzg4NGJZzAqfS@tanuki>
On Fri, Oct 27, 2023 at 8:25 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> We unconditionally clear the oidset now, so shouldn't we also
> unconditionally initialize it?
>
You're right. I should have added that in.
Junio, I'm replying to this email with the amended commit for v5. If
you want me to reroll
v6, I could do that too. Thanks!
^ permalink raw reply
* GFW fails with ST3 on Windows 11
From: John @ 2023-10-27 7:31 UTC (permalink / raw)
To: git
I have been using Sublime Text 3 as the editor on Git for Windows for years, on Windows 10. I recently purchased a Windows 11 machine. On that machine, when I give GFW the following command, I get the response shown:
$ git commit -a
[…]
hint: Waiting for your editor to close the file… C:\Program Files\Sublime Text 3\sublime_text.exe: C:Program: command not found
error: There was a problem with the editor ‘C:\Program Files\Sublime Text 3\sublime_text.exe’.
Please supply the message using either -m or -F option.
Does anyone know of a fix? To repeat, it’s only a problem on Windows 11.
^ 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