* Re: [PATCH v2 5/5] ci: add support for GitLab CI
From: Phillip Wood @ 2023-10-27 13:17 UTC (permalink / raw)
To: Oswald Buddenhagen, Patrick Steinhardt; +Cc: git
In-Reply-To: <ZTuYjzOfG0y27Sho@ugly>
On 27/10/2023 12:01, Oswald Buddenhagen wrote:
> 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
Not according to
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#export
SYNOPSIS
export name[=word]...
DESCRIPTION
The shell shall give the export attribute to the variables
corresponding to the specified names, which shall cause them
to be in the environment of subsequently executed commands. If
the name of a variable is followed by = word, then the value
of that variable shall be set to word.
It is true that in our test suite we separate a variable assignment when
exporting. Presumably that is because someone reported that their shell
did not support the "export name=WORD" syntax in the past. As we're
already using this syntax with the same docker images in Github Actions
I think we can assume it is safe here.
Best Wishes
Phillip
^ permalink raw reply
* Re: [PATCH v3] git-rebase.txt: rewrite docu for fixup/squash (again)
From: Marc Branchaud @ 2023-10-27 13:14 UTC (permalink / raw)
To: Oswald Buddenhagen, git
Cc: Junio C Hamano, Phillip Wood, Taylor Blau, Christian Couder,
Charvi Mendiratta
In-Reply-To: <20231025102932.1202299-1-oswald.buddenhagen@gmx.de>
On 2023-10-25 06:29, Oswald Buddenhagen wrote:
> Create a clear top-down structure which makes it hopefully unambiguous
> what happens when.
>
> The behavior in the presence of multiple "fixup -c" is somewhat
> questionable, as arguably it would be better to complain about it rather
> than letting the last instance win. But for the time being we document
> the status quo, with a note that it is not guaranteed. Note that
> actually changing it would require --autosquash eliding the superseded
> uses.
I do not think this kind of editorializing belongs in the commit's
message, but this likely isn't the first commit message that expresses
an opinion.
> Also emphasize that the author info of the first commit is preserved
> even in the presence of "fixup -c", as this diverges from "git commit
> -c"'s behavior. New options matching the latter should be introduced for
> completeness.
>
> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
>
> ---
> v3:
> - adjust to reality, and elaborate in the commit message why it's
> arguably somewhat suboptimal
>
> i deliberated the 'command "pick"' word order swap suggested by marc,
> but while it improves things locally, it somehow doesn't flow with the
> "redundancy-reduced" last part of the sentence.
>
> v2:
> - slight adjustments inspired by marc. however, i left most things
> unchanged or even went in the opposite direction, because i assume the
> readers to be sufficiently context-sensitive, and the objective is
> merely to be not actively confusing. adding redundancy in the name of
> clarity would just make the text stylistically inferior and arguably
> harder to read.
>
> Cc: Junio C Hamano <gitster@pobox.com>
> Cc: Phillip Wood <phillip.wood123@gmail.com>
> Cc: Taylor Blau <me@ttaylorr.com>
> Cc: Christian Couder <christian.couder@gmail.com>
> Cc: Charvi Mendiratta <charvi077@gmail.com>
> Cc: Marc Branchaud <marcnarc@xiplink.com>
> ---
> Documentation/git-rebase.txt | 30 ++++++++++++++++--------------
> 1 file changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index e7b39ad244..578d1d34a6 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -890,20 +890,22 @@ command "pick" with the command "reword".
> To drop a commit, replace the command "pick" with "drop", or just
> delete the matching line.
>
> -If you want to fold two or more commits into one, replace the command
> -"pick" for the second and subsequent commits with "squash" or "fixup".
> -If the commits had different authors, the folded commit will be
> -attributed to the author of the first commit. The suggested commit
> -message for the folded commit is the concatenation of the first
> -commit's message with those identified by "squash" commands, omitting the
> -messages of commits identified by "fixup" commands, unless "fixup -c"
> -is used. In that case the suggested commit message is only the message
> -of the "fixup -c" commit, and an editor is opened allowing you to edit
> -the message. The contents (patch) of the "fixup -c" commit are still
> -incorporated into the folded commit. If there is more than one "fixup -c"
> -commit, the message from the final one is used. You can also use
> -"fixup -C" to get the same behavior as "fixup -c" except without opening
> -an editor.
> +If you want to fold two or more commits into one (that is, to combine
> +their contents/patches), replace the command "pick" for the second and
> +subsequent commits with "squash" or "fixup".
> +The commit message for the folded commit is the concatenation of the
> +message of the first commit with those of commits identified by "squash"
> +commands, omitting those of commits identified by "fixup" commands,
> +unless "fixup -c" is used. In the latter case, the message is obtained
> +only from the "fixup -c" commit (if multiple are present, the last one
> +takes precedence, but this should not be relied upon).
I like the overall phrasing here.
But I think you should remove the "but this should not be relied upon"
phrase. This reads as if Git's current behaviour is undefined, which
most definitely is not true.
Even changing this to something like "but this might change in the
future" is unhelpful. Everything in Git is subject to change over a
long-enough time span, so the same could be said about every aspect of Git.
Until the behaviour actually changes, it's perfectly fine for people to
use multiple "fixup -c" commands. There's no reason to scare them off
of it.
> +If the resulting commit message is a concatenation of multiple messages,
> +an editor is opened allowing you to edit it. This is also the case for a
> +message obtained via "fixup -c", while using "fixup -C" instead skips
> +the editor; this is analogous to the behavior of `git commit`.
> +The author information (including date/timestamp) always comes from
> +the first commit; this is the case even if "fixup -c/-C" is used,
> +contrary to what `git commit` does.
This phrasing is much better.
Thanks for putting up with my pedantry!
M.
^ permalink raw reply
* Re: [RESEND v2] git-rebase.txt: rewrite docu for fixup/squash (again)
From: Oswald Buddenhagen @ 2023-10-27 13:08 UTC (permalink / raw)
To: Marc Branchaud
Cc: git, Junio C Hamano, Phillip Wood, Christian Couder,
Charvi Mendiratta
In-Reply-To: <b71d066b-104a-4c60-9319-b3c635be6efc@xiplink.com>
On Fri, Oct 27, 2023 at 08:39:03AM -0400, Marc Branchaud wrote:
>On 2023-10-24 17:19, Oswald Buddenhagen wrote:
>>>> +The commit message for the folded commit is the concatenation of
>>>> the
>>>> +message of the first commit with those of commits identified by
>>>> "squash"
>>>
>>> s/message of the first commit/picked commit's message/
>>>
>> that does indeed sound better, but i think it's more confusing (and
>> potentially even more so when translated directly). i guess one could
>> use "pick'd commit's", but that's kind of ugly again.
>
>Let the translators worry about how to phrase it in other languages.
>
my experience tells me that this isn't a good idea. translations are
often done by people who have little domain knowledge of what they
translate. it's a good idea to guide them.
also, the english text is often read by people who barely understand
english, and will attempt literal translations in their head.
>In English "picked" is the right choice.
>
the squashed commits also fit the natural use of "picked", because
"picking" means "selecting". it's not advisable to use this potentially
ambiguous term when there is an unambiguous alternative way to identify
the commit available.
>You should not presume that other languages will want to use the word
>"pick" verbatim.
>
well, i actually should, because it's the command's own name, which
definitely shouldn't be translated.
regards
^ permalink raw reply
* Re: [PATCH 00/12] show-ref: introduce mode to check for ref existence
From: Phillip Wood @ 2023-10-27 13:06 UTC (permalink / raw)
To: Patrick Steinhardt, phillip.wood
Cc: Han-Wen Nienhuys, Junio C Hamano, git, Eric Sunshine
In-Reply-To: <ZTo2Aq2wAxx_6Laz@tanuki>
Hi Patrick
On 26/10/2023 10:48, Patrick Steinhardt wrote:
> On Wed, Oct 25, 2023 at 03:44:33PM +0100, Phillip Wood wrote:
>> On 25/10/2023 15:26, Han-Wen Nienhuys wrote:
>>> On Tue, Oct 24, 2023 at 9:17 PM Junio C Hamano <gitster@pobox.com> wrote:
>>>>
>>>> Patrick Steinhardt <ps@pks.im> writes:
>>>>
>>>>> this patch series introduces a new `--exists` mode to git-show-ref(1) to
>>>>> explicitly check for the existence of a reference, only.
>>>>
>>>> I agree that show-ref would be the best place for this feature (not
>>>> rev-parse, which is already a kitchen sink). After all, the command
>>>> was designed for validating refs in 358ddb62 (Add "git show-ref"
>>>> builtin command, 2006-09-15).
>>>>
>>>> Thanks. Hopefully I can take a look before I go offline.
>>>
>>> The series description doesn't say why users would care about this.
>>>
>>> If this is just to ease testing, I suggest adding functionality to a
>>> suitable test helper. Anything you add to git-show-ref is a publicly
>>> visible API that needs documentation and comes with a stability
>>> guarantee that is more expensive to maintain than test helper
>>> functionality.
>>
>> Does the new functionality provide a way for scripts to see if a branch is
>> unborn (i.e. has not commits yet)? I don't think we have a way to
>> distinguish between a ref that points to a missing object and an unborn
>> branch at the moment.
>
> You could do it with two commands:
>
> ```
> target=$(git symbolic-ref HEAD)
> git show-ref --exists "$target"
> case "$?" in
> 2)
> echo "unborn branch";;
> 0)
> echo "branch exists";;
> *)
> echo "could be anything, dunno";;
> esac
> ```
>
> While you could use git-rev-parse(1) instead of git-show-ref(1), you
> wouldn't be able to easily distinguish the case of a missing target
> reference and any other kind of error.
Thanks, it is helpful to have an example use outside of the test suite.
Best Wishes
Phillip
^ permalink raw reply
* Re: [PATCH v3 0/5] config-parse: create config parsing library
From: Junio C Hamano @ 2023-10-27 13:04 UTC (permalink / raw)
To: Josh Steadmon; +Cc: Jonathan Tan, Taylor Blau, git, calvinwan, glencbz
In-Reply-To: <ZTluW0Ha2W4NYaJd@google.com>
Josh Steadmon <steadmon@google.com> writes:
> Thanks everyone for the revived discussion here. I think I agree, this
> series is not going in the right direction. Additionally, our internal
> use case for this change has evaporated, so let's just drop the series.
> We can pick it up again later if interest returns.
OK. Let's scrap it for now.
The "internal use case" behind a proposed feature changing so
quickly is a bit worrying. What is good for this project should
ideally be good for everybody, not only for satisfying a particular
$CORP needs of the day. But I think the idea of giving enhanced
visibility into stakeholder companies directions and priorities
Emily (I think?) floated during the contributors' summit may help
reduce such a risk, hopefully.
Thanks.
^ permalink raw reply
* Re: [PATCH] send-email: move validation code below process_address_list
From: Junio C Hamano @ 2023-10-27 13:04 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Michael Strawbridge, Jeff King, Bagas Sanjaya, Git Mailing List
In-Reply-To: <20231025074317.r3sydthautjjsf5y@pengutronix.de>
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:
>> This fixes email address validation errors when the optional
>> perl module Email::Valid is installed and multiple addresses are passed
>> in on a single to/cc argument like --to=foo@example.com,bar@example.com.
>>
>> Reported-by: Bagas Sanjaya <bagasdotme@gmail.com>
>> Signed-off-by: Michael Strawbridge <michael.strawbridge@amd.com>
>
> If you do Fixes: trailers as the kernel does, this could get:
>
> Fixes: a8022c5f7b67 ("send-email: expose header information to git-send-email's sendemail-validate hook")
While referring to a concrete commit object name is great, we tend
not to use that particular trailer in this project; rather, we
prefer to see description on how and in what way the culprit change
was undesirable.
> I tested this patch on top of main (2e8e77cbac8a) and it fixes the
> regression I reported in a separate thread (where Jeff pointed out this
> patch as fixing it).
Great. Thanks.
^ permalink raw reply
* Re: [PATCH v2 5/5] ci: add support for GitLab CI
From: Phillip Wood @ 2023-10-27 13:02 UTC (permalink / raw)
To: Patrick Steinhardt, phillip.wood; +Cc: git, Oswald Buddenhagen
In-Reply-To: <ZTulo_rh_G-lfICQ@tanuki>
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. The
fact that we run the dockerized tests as root on the other CI platforms
is a regression from what we used to do. I'm not an expert but I think
for the builds that use docker we're essentially using the some build
environment regardless of the CI provider so it would make sense to
handle them all in the same way. I think the existing script uses "su"
but we could change it to use "sudo" like you're doing here.
Best Wishes
Phillip
^ permalink raw reply
* Re: using oldest date when squashing commits
From: Oswald Buddenhagen @ 2023-10-27 12:45 UTC (permalink / raw)
To: Marc Branchaud
Cc: Johannes Sixt, Junio C Hamano, Phillip Wood, phillip.wood, git
In-Reply-To: <70b8d4d8-f4b5-4cd7-b73a-1d7393d84266@xiplink.com>
On Fri, Oct 27, 2023 at 08:34:40AM -0400, Marc Branchaud wrote:
>I never use "fixup -C" (or -c), but I do use squash/fixup a lot. I
>find that I would prefer it if Git used the most recent Author date
>from the set of commits being combined, rather than preserving the
>picked commit's Author date.
>
that would be unreliable, as plain amends wouldn't be reflected. that
may be rare in your workflow, but still.
>Sometimes it takes quite a while for me to get a piece of work sorted
>out, and I would rather have the Author date in the end-result commit
>reflect the work's completion time than its initiation time.
>
afaict, you need to get used to `--amend --reset-author` all commits
before you push to achieve this reliably. that can be easily automated
by using -x with rebase -i (filter-repo (ex filter-branch) would also
work).
regards
^ permalink raw reply
* Re: [RESEND v2] git-rebase.txt: rewrite docu for fixup/squash (again)
From: Marc Branchaud @ 2023-10-27 12:39 UTC (permalink / raw)
To: Oswald Buddenhagen
Cc: git, Junio C Hamano, Phillip Wood, Christian Couder,
Charvi Mendiratta
In-Reply-To: <ZTg0zXkvSQ6L+4Oj@ugly>
On 2023-10-24 17:19, Oswald Buddenhagen wrote:
>
>>> +The commit message for the folded commit is the concatenation of the
>>> +message of the first commit with those of commits identified by
>>> "squash"
>>
>> s/message of the first commit/picked commit's message/
>>
> that does indeed sound better, but i think it's more confusing (and
> potentially even more so when translated directly). i guess one could
> use "pick'd commit's", but that's kind of ugly again.
Let the translators worry about how to phrase it in other languages. In
English "picked" is the right choice. You should not presume that other
languages will want to use the word "pick" verbatim.
M.
^ permalink raw reply
* Re: GFW fails with ST3 on Windows 11
From: Konstantin Khomoutov @ 2023-10-27 12:37 UTC (permalink / raw)
To: git
In-Reply-To: <cd248c66-fc71-4783-9195-02c8811744b8@app.fastmail.com>
On Fri, Oct 27, 2023 at 03:29:58PM +0300, Konstantin Khomoutov wrote:
[...]
> Also please check the core.editor configuration setting - it might be set,
> and then you should try to modify it instead.
[...]
This means Git's configuration setting.
You might run
git config --get-all --show-scope core.editor
in your shell to get the values of this setting printed, if set, prefixed with
the scope these values are defined in (see `git help config` or [1] for more
info).
1. https://www.git-scm.com/docs/git-config
^ permalink raw reply
* Re: using oldest date when squashing commits
From: Marc Branchaud @ 2023-10-27 12:34 UTC (permalink / raw)
To: Johannes Sixt, Junio C Hamano, Phillip Wood
Cc: Oswald Buddenhagen, phillip.wood, git
In-Reply-To: <59731c05-c3f6-4815-8411-783bb1c2aac4@kdbg.org>
On 2023-10-24 17:19, Johannes Sixt wrote:
> Am 24.10.23 um 19:30 schrieb Junio C Hamano:
>> Phillip Wood <phillip.wood123@gmail.com> writes:
>>> "fixup -c/-C" were conceived as a way to reword a commit message at
>>> the same time as optionally fixing up the commit's content.
>>
>> Yup, it still is a "fix", meaning the identity and the spirit of the
>> commit being fixed are unchanged.
>
> That's a pitty, because that is not at all what *I* use "fixup -C" for.
> To update the commit message, I use "squash" (or occasionally "reword").
> I use "fixup -C" after the following events:
>
> 1. Commit unfinished changes for whatever reason. Usually the commit
> message just says "WIP <topic>" because that's what it is.
> 2. Make a fixup commit for an earlier commit because doing the fixup now
> gets it out of the way, and often delaying it until after the completed
> change would cause merge conflicts.
> 3. Complete the WIP including the commit message.
>
> I would now use "fixup -C" on commit 3, because its metadata reflects
> reality more accurately than that of 1. Commit 3 often comes days after 1.
Speaking of the metadata ...
I never use "fixup -C" (or -c), but I do use squash/fixup a lot. I find
that I would prefer it if Git used the most recent Author date from the
set of commits being combined, rather than preserving the picked
commit's Author date. Sometimes it takes quite a while for me to get a
piece of work sorted out, and I would rather have the Author date in the
end-result commit reflect the work's completion time than its initiation
time.
The current behaviour means that when scanning through commits with
tools like gitk (which shows just the Author date in its list of
commits) I'll often see what I feel are inaccurate or confusing dates
there, and I use the Committer date (a bit less convenient in gitk) to
figure out when the work was actually "done". (Although the span
between the Author date from the start of the work and the Committer
date from the end of the work would roughly reflect how long the work
took to complete, I don't use Git for that kind of information.)
Anyway, this is a minor itch for me that I've never felt the need to
scratch. I just thought I'd mention it since the topic is being discussed.
M.
^ permalink raw reply
* 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
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