* [PATCH 1/3] doc: git-rev-parse: enforce command-line description syntax
From: Jean-Noël Avila via GitGitGadget @ 2024-02-20 22:32 UTC (permalink / raw)
To: git; +Cc: Jean-Noël Avila, Jean-Noël Avila
In-Reply-To: <pull.1670.git.1708468374.gitgitgadget@gmail.com>
From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>
git-rev-parse(1) manpage is completely off with respect to the
command-line description syntax with badly formatted placeholders and
malformed alternatives.
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
---
Documentation/git-rev-parse.txt | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 546faf90177..5d83dd36da1 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -9,7 +9,7 @@ git-rev-parse - Pick out and massage parameters
SYNOPSIS
--------
[verse]
-'git rev-parse' [<options>] <args>...
+'git rev-parse' [<options>] <arg>...
DESCRIPTION
-----------
@@ -130,7 +130,7 @@ for another option.
'git diff-{asterisk}'). In contrast to the `--sq-quote` option,
the command input is still interpreted as usual.
---short[=length]::
+--short[=<length>]::
Same as `--verify` but shortens the object name to a unique
prefix with at least `length` characters. The minimum length
is 4, the default is the effective value of the `core.abbrev`
@@ -165,9 +165,9 @@ Options for Objects
--all::
Show all refs found in `refs/`.
---branches[=pattern]::
---tags[=pattern]::
---remotes[=pattern]::
+--branches[=<pattern>]::
+--tags[=<pattern>]::
+--remotes[=<pattern>]::
Show all branches, tags, or remote-tracking branches,
respectively (i.e., refs found in `refs/heads`,
`refs/tags`, or `refs/remotes`, respectively).
@@ -176,7 +176,7 @@ If a `pattern` is given, only refs matching the given shell glob are
shown. If the pattern does not contain a globbing character (`?`,
`*`, or `[`), it is turned into a prefix match by appending `/*`.
---glob=pattern::
+--glob=<pattern>::
Show all refs matching the shell glob pattern `pattern`. If
the pattern does not start with `refs/`, this is automatically
prepended. If the pattern does not contain a globbing
@@ -197,7 +197,7 @@ respectively, and they must begin with `refs/` when applied to `--glob`
or `--all`. If a trailing '/{asterisk}' is intended, it must be given
explicitly.
---exclude-hidden=[fetch|receive|uploadpack]::
+--exclude-hidden=(fetch|receive|uploadpack)::
Do not include refs that would be hidden by `git-fetch`,
`git-receive-pack` or `git-upload-pack` by consulting the appropriate
`fetch.hideRefs`, `receive.hideRefs` or `uploadpack.hideRefs`
@@ -314,17 +314,17 @@ The following options are unaffected by `--path-format`:
Other Options
~~~~~~~~~~~~~
---since=datestring::
---after=datestring::
+--since=<datestring>::
+--after=<datestring>::
Parse the date string, and output the corresponding
--max-age= parameter for 'git rev-list'.
---until=datestring::
---before=datestring::
+--until=<datestring>::
+--before=<datestring>::
Parse the date string, and output the corresponding
--min-age= parameter for 'git rev-list'.
-<args>...::
+<arg>...::
Flags and parameters to be parsed.
--
gitgitgadget
^ permalink raw reply related
* [PATCH 2/3] doc: git-clone fix missing placeholder end carret
From: Jean-Noël Avila via GitGitGadget @ 2024-02-20 22:32 UTC (permalink / raw)
To: git; +Cc: Jean-Noël Avila, Jean-Noël Avila
In-Reply-To: <pull.1670.git.1708468374.gitgitgadget@gmail.com>
From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
---
Documentation/git-clone.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 6e43eb9c205..0c07720c6f4 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -311,7 +311,7 @@ or `--mirror` is given)
The result is Git repository can be separated from working
tree.
---ref-format=<ref-format::
+--ref-format=<ref-format>::
Specify the given ref storage format for the repository. The valid values are:
+
--
gitgitgadget
^ permalink raw reply related
* [PATCH 3/3] doc: add some missing sentence dots.
From: Jean-Noël Avila via GitGitGadget @ 2024-02-20 22:32 UTC (permalink / raw)
To: git; +Cc: Jean-Noël Avila, Jean-Noël Avila
In-Reply-To: <pull.1670.git.1708468374.gitgitgadget@gmail.com>
From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
---
Documentation/config/diff.txt | 4 ++--
Documentation/git-fast-export.txt | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/config/diff.txt b/Documentation/config/diff.txt
index bd5ae0c3378..6c7e09a1ef5 100644
--- a/Documentation/config/diff.txt
+++ b/Documentation/config/diff.txt
@@ -223,5 +223,5 @@ diff.colorMoved::
diff.colorMovedWS::
When moved lines are colored using e.g. the `diff.colorMoved` setting,
- this option controls the `<mode>` how spaces are treated
- for details of valid modes see '--color-moved-ws' in linkgit:git-diff[1].
+ this option controls the `<mode>` how spaces are treated.
+ For details of valid modes see '--color-moved-ws' in linkgit:git-diff[1].
diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt
index 4643ddbe68f..752e4b9b01d 100644
--- a/Documentation/git-fast-export.txt
+++ b/Documentation/git-fast-export.txt
@@ -48,7 +48,7 @@ When asking to 'abort' (which is the default), this program will die
when encountering such a tag. With 'drop' it will omit such tags from
the output. With 'rewrite', if the tagged object is a commit, it will
rewrite the tag to tag an ancestor commit (via parent rewriting; see
-linkgit:git-rev-list[1])
+linkgit:git-rev-list[1]).
-M::
-C::
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH 2/3] doc: git-clone fix missing placeholder end carret
From: Eric Sunshine @ 2024-02-20 22:39 UTC (permalink / raw)
To: Jean-Noël Avila via GitGitGadget; +Cc: git, Jean-Noël Avila
In-Reply-To: <7d85bfe6c5511d3918dd84365249797abe6fa04b.1708468374.git.gitgitgadget@gmail.com>
On Tue, Feb 20, 2024 at 5:33 PM Jean-Noël Avila via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> doc: git-clone fix missing placeholder end carret
I believe you meant: s/carret/caret/
In English, I think this symbol is typically called an "angle bracket".
Either way, probably not worth a reroll since it's just a commit-message nit.
> Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
>
> diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
> @@ -311,7 +311,7 @@ or `--mirror` is given)
> ---ref-format=<ref-format::
> +--ref-format=<ref-format>::
^ permalink raw reply
* Re: [PATCH 3/3] doc: add some missing sentence dots.
From: Junio C Hamano @ 2024-02-20 22:50 UTC (permalink / raw)
To: Jean-Noël Avila via GitGitGadget; +Cc: git, Jean-Noël Avila
In-Reply-To: <1faa934a152d2eedc8949c1e17aff787614770e2.1708468374.git.gitgitgadget@gmail.com>
"Jean-Noël Avila via GitGitGadget" <gitgitgadget@gmail.com> writes:
> Subject: Re: [PATCH 3/3] doc: add some missing sentence dots.
"sentence dots." -> "full stop at the end of sentence" or something?
> diff --git a/Documentation/config/diff.txt b/Documentation/config/diff.txt
> index bd5ae0c3378..6c7e09a1ef5 100644
> --- a/Documentation/config/diff.txt
> +++ b/Documentation/config/diff.txt
> @@ -223,5 +223,5 @@ diff.colorMoved::
>
> diff.colorMovedWS::
> When moved lines are colored using e.g. the `diff.colorMoved` setting,
> - this option controls the `<mode>` how spaces are treated
> - for details of valid modes see '--color-moved-ws' in linkgit:git-diff[1].
> + this option controls the `<mode>` how spaces are treated.
> + For details of valid modes see '--color-moved-ws' in linkgit:git-diff[1].
Very good. Thanks for spotting this one.
> diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt
> index 4643ddbe68f..752e4b9b01d 100644
> --- a/Documentation/git-fast-export.txt
> +++ b/Documentation/git-fast-export.txt
> @@ -48,7 +48,7 @@ When asking to 'abort' (which is the default), this program will die
> when encountering such a tag. With 'drop' it will omit such tags from
> the output. With 'rewrite', if the tagged object is a commit, it will
> rewrite the tag to tag an ancestor commit (via parent rewriting; see
> -linkgit:git-rev-list[1])
> +linkgit:git-rev-list[1]).
Ditto.
^ permalink raw reply
* Re: [PATCH 1/3] doc: git-rev-parse: enforce command-line description syntax
From: Junio C Hamano @ 2024-02-20 22:57 UTC (permalink / raw)
To: Jean-Noël Avila via GitGitGadget; +Cc: git, Jean-Noël Avila
In-Reply-To: <17b0284c379e62a756e1bba008f4671f6afc0ad9.1708468374.git.gitgitgadget@gmail.com>
"Jean-Noël Avila via GitGitGadget" <gitgitgadget@gmail.com> writes:
> [verse]
> -'git rev-parse' [<options>] <args>...
> +'git rev-parse' [<options>] <arg>...
Good. The "or more" is signalled by the ellipsis, not "args" being
plural.
> ---short[=length]::
> +--short[=<length>]::
> Same as `--verify` but shortens the object name to a unique
> prefix with at least `length` characters. The minimum length
This same comment applies throughout this patch, but in other places
when we use <placeholder> in the option argument description, don't
we use the same <placeholder> in text as well? I am wondering if
the `length` (typeset in fixed-width) should become <length>. What
do other recent[*] documentation pages commonly do?
Side note: I say "recent" because rev-parse doc is one of
the oldest ones that did not get typesetting attention they
deserve, compared to more recent ones that got nitpicked
while they were written and updated.
> ---branches[=pattern]::
> ---tags[=pattern]::
> ---remotes[=pattern]::
> +--branches[=<pattern>]::
> +--tags[=<pattern>]::
> +--remotes[=<pattern>]::
> Show all branches, tags, or remote-tracking branches,
> respectively (i.e., refs found in `refs/heads`,
> `refs/tags`, or `refs/remotes`, respectively).
Ditto. We see `pattern` that may want to become <pattern> in the
description (after the post context of this hunk).
> ---glob=pattern::
> +--glob=<pattern>::
> Show all refs matching the shell glob pattern `pattern`. If
> the pattern does not start with `refs/`, this is automatically
> prepended. If the pattern does not contain a globbing
Ditto.
> ---exclude-hidden=[fetch|receive|uploadpack]::
> +--exclude-hidden=(fetch|receive|uploadpack)::
> Do not include refs that would be hidden by `git-fetch`,
> `git-receive-pack` or `git-upload-pack` by consulting the appropriate
> `fetch.hideRefs`, `receive.hideRefs` or `uploadpack.hideRefs`
Good.
> ---since=datestring::
> ---after=datestring::
> +--since=<datestring>::
> +--after=<datestring>::
> Parse the date string, and output the corresponding
> --max-age= parameter for 'git rev-list'.
Good, modulo possibly "date string" -> "<datestring>".
> ---until=datestring::
> ---before=datestring::
> +--until=<datestring>::
> +--before=<datestring>::
> Parse the date string, and output the corresponding
> --min-age= parameter for 'git rev-list'.
Ditto.
> -<args>...::
> +<arg>...::
> Flags and parameters to be parsed.
Good.
^ permalink raw reply
* Re: [PATCH v4] documentation: send-email: use camel case consistently
From: Junio C Hamano @ 2024-02-20 22:57 UTC (permalink / raw)
To: Dragan Simic; +Cc: git, code
In-Reply-To: <88f1fe08c3047e14090957093ee8d98b0f60cb6c.1708467601.git.dsimic@manjaro.org>
Dragan Simic <dsimic@manjaro.org> writes:
> -sendemail.smtpsslcertpath::
> +sendemail.smtpSSLCertPath::
> -sendemail.signedoffbycc::
> +sendemail.signedOffByCc::
> -sendemail.suppresscc::
> +sendemail.suppressCc::
> -sendemail.tocmd::
> +sendemail.toCmd::
> -sendemail.signedoffcc (deprecated)::
> - Deprecated alias for `sendemail.signedoffbycc`.
> +sendemail.signedOffCc (deprecated)::
> + Deprecated alias for `sendemail.signedOffByCc`.
All look good.
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index d1ef6a204e68..8264f8738093 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -138,7 +138,7 @@ Note that no attempts whatsoever are made to validate the encoding.
>
> --compose-encoding=<encoding>::
> Specify encoding of compose message. Default is the value of the
> - 'sendemail.composeencoding'; if that is unspecified, UTF-8 is assumed.
> + 'sendemail.composeEncoding'; if that is unspecified, UTF-8 is assumed.
Good.
> @@ -174,7 +174,7 @@ Sending
> Specify a command to run to send the email. The command should
> be sendmail-like; specifically, it must support the `-i` option.
> The command will be executed in the shell if necessary. Default
> - is the value of `sendemail.sendmailcmd`. If unspecified, and if
> + is the value of `sendemail.sendmailCmd`. If unspecified, and if
Good.
> @@ -269,7 +269,7 @@ must be used for each option.
> certificates concatenated together: see verify(1) -CAfile and
> -CApath for more information on these). Set it to an empty string
> to disable certificate verification. Defaults to the value of the
> - `sendemail.smtpsslcertpath` configuration variable, if set, or the
> + `sendemail.smtpSSLCertPath` configuration variable, if set, or the
Good.
> @@ -313,7 +313,7 @@ Automating
> Specify a command to execute once per patch file which
> should generate patch file specific "To:" entries.
> Output of this command must be single email address per line.
> - Default is the value of 'sendemail.tocmd' configuration value.
> + Default is the value of 'sendemail.toCmd' configuration value.
Good.
> - cc list. Default is the value of `sendemail.signedoffbycc` configuration
> + cc list. Default is the value of `sendemail.signedOffByCc` configuration
Good.
> - for each email set. Default is the value of 'sendemail.cccover'
> + for each email set. Default is the value of 'sendemail.ccCover'
Good.
> - for each email set. Default is the value of 'sendemail.tocover'
> + for each email set. Default is the value of 'sendemail.toCover'
Good.
> -Default is the value of `sendemail.suppresscc` configuration value; if
> +Default is the value of `sendemail.suppressCc` configuration value; if
Good.
> - See 'sendemail.aliasesfile' for more information about aliases.
> + See 'sendemail.aliasesFile' for more information about aliases.
Good.
This version looks very good to me. Thanks. Will queue.
^ permalink raw reply
* Re: [PATCH 2/3] doc: git-clone fix missing placeholder end carret
From: Junio C Hamano @ 2024-02-20 23:01 UTC (permalink / raw)
To: Eric Sunshine
Cc: Jean-Noël Avila via GitGitGadget, git, Jean-Noël Avila
In-Reply-To: <CAPig+cSFK_sVZn5p3Bw12HOEF4Z99niPGMHp1Dp2w2EUjjZ4zw@mail.gmail.com>
Eric Sunshine <sunshine@sunshineco.com> writes:
> On Tue, Feb 20, 2024 at 5:33 PM Jean-Noël Avila via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>> doc: git-clone fix missing placeholder end carret
>
> I believe you meant: s/carret/caret/
>
> In English, I think this symbol is typically called an "angle bracket".
Yes, caret is "^".
>> diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
>> @@ -311,7 +311,7 @@ or `--mirror` is given)
>> ---ref-format=<ref-format::
>> +--ref-format=<ref-format>::
A closing angle bracket is what is missing there. Thanks for
reading carefully.
^ permalink raw reply
* Re: [PATCH v4] documentation: send-email: use camel case consistently
From: Dragan Simic @ 2024-02-21 0:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, code
In-Reply-To: <xmqqle7elp62.fsf@gitster.g>
On 2024-02-20 23:57, Junio C Hamano wrote:
>
> This version looks very good to me. Thanks. Will queue.
Great, thank you! I'm glad we've reached this state of the entire
patch.
^ permalink raw reply
* Re: [PATCH] documentation: send-email: use camel case consistently
From: Chris Torek @ 2024-02-21 0:43 UTC (permalink / raw)
To: Dragan Simic; +Cc: Junio C Hamano, git
In-Reply-To: <9d0022ba5666223af94bbf450909b1ba@manjaro.org>
On Tue, Feb 20, 2024 at 8:42 AM Dragan Simic <dsimic@manjaro.org> wrote:
> I've never ever seen anyone referring to email headers as "TO", "CC" or
> "BCC". It's always referred to as "To", "Cc" and "Bcc".
I used some email system (back in the early 1980s) that did that. It
felt weird even then. I can't remember if it was some CSMail (CSNet)
or MH(Rand Mail Handler) version that did it.
> Thus, "cc" stems from the old age of literal carbon copies ...
That's correct. However:
> and "bcc" was seemingly coined when email took over.
"Blind Carbon Copies" predated email, but required adding the
notation separately, if it was to be added at all. (I'm just old enough
to remember using carbon copies myself, but not old enough to
know what Standard Office Practice was at that time.)
Whether adding a "bcc" notation was common I don't know;
it seems it would be easier to leave it off if you made, say, one
original and a total of 2 copies, one "blinded".
(As your Wikipedia link notes, there was a practical limit to how
many carbon copies one could make in the first place.)
Chris
^ permalink raw reply
* Re: [PATCH v2 1/1] completion: don't complete revs when --no-format-patch
From: Junio C Hamano @ 2024-02-21 1:34 UTC (permalink / raw)
To: Britton Kerin; +Cc: Patrick Steinhardt, git
In-Reply-To: <CAC4O8c88Z3ZqxH2VVaNPpEGB3moL5dJcg3cOWuLWwQ_hLrJMtA@mail.gmail.com>
Britton Kerin <britton.kerin@gmail.com> writes:
>> While this second hunk here makes perfect sense to me, there is no
>> explanation why we need to change `__git_find_last_on_cmdline ()`. It's
>> already used with "--guess --no-guess" in another place, so I would
>> think that it ought to work alright for this usecase, too. Or is it that
>> the existing callsite of this function is buggy, too? If so, we should
>> likely fix that in a separate patch together with a test.
>>
>> Also, adding a test for git-send-email that exercises this new behaviour
>> would be very much welcome, too.
>
> I'll look this one over again and add some tests eventually.
Thank you, both.
^ permalink raw reply
* Re: [PATCH 3/4] completion: reflog with implicit "show"
From: Junio C Hamano @ 2024-02-21 1:46 UTC (permalink / raw)
To: Rubén Justo; +Cc: Git List
In-Reply-To: <dd106d87-3363-426a-90a2-16e1f2d04661@gmail.com>
Rubén Justo <rjusto@gmail.com> writes:
>> So, when we see something that could be a subcommand we complete it
>> as a subcommand and return. In your example, how should
>>
>> $ git reflog def<TAB>
>>
>> work? We try to see if there is a subcommand that begins with
>> "def", we see nothing matching, and then run __git_complete_refs?
>> What if the branch you created earlier were not named "default" but,
>> say, "delmonte", and you did "git reflog del<TAB>"? Shouldn't the
>> user be offered "delete" and "delmonte" as potential completions?
>>
>> > __git_send_email_confirm_options="always never auto cc compose"
>> > diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
>> > index aa9a614de3..231e17f378 100755
>> > --- a/t/t9902-completion.sh
>> > +++ b/t/t9902-completion.sh
>> > @@ -2618,6 +2618,14 @@ test_expect_success 'git clone --config= - value' '
>> > EOF
>> > '
>> >
>> > +test_expect_success 'git reflog show' '
>> > + test_when_finished "git checkout -" &&
>> > + git checkout -b shown &&
>> > + test_completion "git reflog sho" "show " &&
>>
>> IOW, shouldn't this offer both show and shown?
>
> What should we do?
I would imagine that we should make the above offer both "show"
(because it can be what the user meant) and "shown" (because it can
also be what the user meant)? But thinking of it again, because
"show" is a prefix of "shown", this should offer "show" and then
another <TAB> would offer "show" and "shown". At least, that is
what I would expect from the usual bash completion behaviour, which
would look like:
$ mkdir /var/tmp/scratch && cd /var/tmp/scratch
$ : >show 2>shown
$ echo sho<TAB>
The <TAB> makes the above line expand to
$ echo show
and place the curser immediately after 'w' (without a space after it).
Giving another <TAB> at this point offers two possible candidates.
$ echo show<TAB>
show shown
So, I'd expect a similar completion to happen, i.e.
$ git reflog sho<TAB>
-->
$ git reflog show
because there are two candidates, "show" and "shown", and I can type
another <TAB> at that point to see the two candidates.
$ git reflog show<TAB>
show shown
If the branch name were "shot" instead of "shown", then the possible
choices would become "show" and "shot", so we'd skip one step from
the above and immediately get the two candidates against "sho<TAB>".
$ git reflog sho<TAB>
shot show
That is what I would expect.
Thanks.
^ permalink raw reply
* Segfault: git show-branch --reflog refs/pullreqs/1
From: Yasushi SHOJI @ 2024-02-21 1:48 UTC (permalink / raw)
To: Git Mailing List
Hi all,
Does anyone see a segfault on `git show-branch --reflog refs/pullreqs/1`?
It seems files_reflog_path() creates a wrong path with the above command
using REF_WORKTREE_SHARED.
refs/pullreqs is from GitHub if you have the following line in .git/config:
fetch = +refs/pull/*/head:refs/pullreqs/*
(gdb) bt
#0 __pthread_kill_implementation (threadid=<optimized out>,
signo=signo@entry=6, no_tid=no_tid@entry=0)
at ./nptl/pthread_kill.c:44
#1 0x00007ffff7e101cf in __pthread_kill_internal (signo=6,
threadid=<optimized out>) at ./nptl/pthread_kill.c:78
#2 0x00007ffff7dc2472 in __GI_raise (sig=sig@entry=6) at
../sysdeps/posix/raise.c:26
#3 0x00007ffff7dac4b2 in __GI_abort () at ./stdlib/abort.c:79
#4 0x00007ffff7dad1ed in __libc_message (fmt=fmt@entry=0x7ffff7f1f78c
"%s\n") at ../sysdeps/posix/libc_fatal.c:150
#5 0x00007ffff7e19ae5 in malloc_printerr
(str=str@entry=0x7ffff7f1d22c "free(): invalid pointer") at
./malloc/malloc.c:5658
#6 0x00007ffff7e1b864 in _int_free (av=<optimized out>, p=<optimized
out>, have_lock=have_lock@entry=0) at ./malloc/malloc.c:4432
#7 0x00007ffff7e1e1df in __GI___libc_free (mem=<optimized out>) at
./malloc/malloc.c:3367
#8 0x0000555555625ae2 in cmd_show_branch (ac=<optimized out>,
av=<optimized out>, prefix=<optimized out>)
at builtin/show-branch.c:801
#9 0x0000555555572250 in run_builtin (argv=0x7fffffffe080, argc=3,
p=0x5555558ca198 <commands+2712>) at git.c:469
#10 handle_builtin (argc=3, argv=0x7fffffffe080) at git.c:724
#11 0x00005555555731b4 in run_argv (argcp=argcp@entry=0x7fffffffde6c,
argv=argv@entry=0x7fffffffde60) at git.c:788
#12 0x0000555555573d38 in cmd_main (argc=<optimized out>,
argc@entry=4, argv=<optimized out>, argv@entry=0x7fffffffe078)
at git.c:923
#13 0x0000555555571f10 in main (argc=4, argv=0x7fffffffe078) at common-main.c:62
Best,
--
yashi
^ permalink raw reply
* Re: Bug: impure renames may be reported as pure renames
From: Elijah Newren @ 2024-02-21 2:12 UTC (permalink / raw)
To: Mário Guimarães; +Cc: git
In-Reply-To: <CAF7CYk6dERu7Lb0iKeq7zwtZVVd_bG2FMZReSDeejcu1JGiaFw@mail.gmail.com>
On Tue, Feb 20, 2024 at 12:58 PM Mário Guimarães
<mario.luis.guimaraes@gmail.com> wrote:
>
> Hello Git community,
>
> please see the report below of what may be a bug.
>
> Yours sincerely
> Mário Guimarães
>
> ======================================================
> Thank you for filling out a Git bug report!
> Please answer the following questions to help us understand your issue.
>
> What did you do before the bug happened? (Steps to reproduce your issue)
>
> In the rust-lang/rust repository (I cloned today from GitHub), if we
> run the command
>
> git diff-tree -r -M a04e649^2 a04e649 --
> tests/ui/issues/issue-83190.rs
> tests/ui/type-alias-impl-trait/nested-rpit-with-lifetimes.rs
>
> we get this result
>
> :100644 100644 da931c3edaf6f9de6805e771f2b3b28edd27001f
> 11b659eec97323ea5190dac1771c7ca3241861e7 R100
> tests/ui/issues/issue-83190.rs
> tests/ui/type-alias-impl-trait/nested-rpit-with-lifetimes.rs
>
> However, the source and destination files of the rename are not 100%
> equal. If we run this other command
>
> git diff -M a04e649^2 a04e649 -- tests/ui/issues/issue-83190.rs
> tests/ui/type-alias-impl-trait/nested-rpit-with-lifetimes.rs
>
> we get the following result
>
> diff --git a/tests/ui/issues/issue-83190.rs
> b/tests/ui/type-alias-impl-trait/nested-rpit-with-lifetimes.rs
> similarity index 100%
> rename from tests/ui/issues/issue-83190.rs
> rename to tests/ui/type-alias-impl-trait/nested-rpit-with-lifetimes.rs
> index da931c3edaf..11b659eec97 100644
> --- a/tests/ui/issues/issue-83190.rs
> +++ b/tests/ui/type-alias-impl-trait/nested-rpit-with-lifetimes.rs
> @@ -1,7 +1,7 @@
> -// check-pass
> -
> // Regression test for issue #83190, triggering an ICE in borrowck.
>
> +// check-pass
> +
> pub trait Any {}
> impl<T> Any for T {}
Heh, good point. And more generally, due to how the similarity checks
work (split the file into lines/chunks, hash each to an integer, then
sort the list of integers and compare the list of integers between two
files), whenever you keep all the original lines of a file but permute
their order, you will see a 100% match.
Maybe a simple hack like the below would suffice to fix (untested --
anyone want to test it out for me?):
diff --git a/diff.c b/diff.c
index a2def45644b..6b7b3a8b9af 100644
--- a/diff.c
+++ b/diff.c
@@ -4405,7 +4405,7 @@ static void run_external_diff(const char *pgm,
static int similarity_index(struct diff_filepair *p)
{
- return p->score * 100 / MAX_SCORE;
+ return p->score * 100 / (MAX_SCORE+1);
}
static const char *diff_abbrev_oid(const struct object_id *oid, int abbrev)
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 5a6e2bcac71..3228a898e0b 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -314,7 +314,7 @@ static int find_identical_files(struct hashmap *srcs,
break;
}
if (best) {
- record_rename_pair(dst_index, best->index, MAX_SCORE);
+ record_rename_pair(dst_index, best->index, MAX_SCORE+1);
renames++;
}
return renames;
Which is based on the premise that 100% of source lines can match 100%
of destination lines and result in a score of MAX_SCORE, so we
manually set the scoring to MAX_SCORE+1 when we detect exact renames
based on matching hashes and then use that higher value as the divisor
when computing percentages. (Note that there are several other places
where "MAX_SCORE" is used and which I'm pretty sure we would _not_
want to replace with "MAX_SCORE+1", so if you don't like my patch and
want to redefine MAX_SCORE, then all those other locations would need
to be adjusted instead.)
^ permalink raw reply related
* Re: Git rebasing attribute the change to the wrong file [bug]
From: Elijah Newren @ 2024-02-21 2:18 UTC (permalink / raw)
To: John Gabo; +Cc: git
In-Reply-To: <CALPZ9FGr8K=AdqOzX2P7A3T8+_V25SOjcHA5kY-Fg5TUFEvh7w@mail.gmail.com>
On Tue, Feb 20, 2024 at 12:02 PM John Gabo <charleforalia@gmail.com> wrote:
>
> Hello,
> I think I found a bug in git, where a change get attributed to the
> wrong file when doing a rebasing.
>
> # Short summary
> Assume `master_branch`
> ```
> .
> └── parent_folder
> ├── project_a
> │ └── file.txt
> └── project_b
> └── file.txt # the 2 file.txt have the same content on master
> ```
> Assume `feature_branch` from master
> ```
> .
> └── parent_folder
> ├── project_a
> │ └── file.txt
> └── project_b
> └── file.txt # this file got modified with a feature
> ```
> Assume `refactor_branch` from master
> ```
> .
> └── refactored_parent_folder # got renamed
> ├── project_a
> │ └── file.txt
> └── project_b
> └── file.txt
> ```
> then rebase `feature_branch` on `refactor_branch`,
>
> ```
> .
> └── refactored_parent_folder
> ├── project_a
> │ └── file.txt # the feature got moved here !
> └── project_b
> └── file.txt # instead of here, where it should be !
> ```
You may want to read through the Support section of
https://git.github.io/rev_news/2024/01/31/edition-107/. You are
dealing with inexact renames rather than exact, but it's still the
same basic issue: git is detecting renames based on file similarity
and you have multiple equally valid targets for it to choose, so it
simply picks one. As such, it might be considered a limitation of
rename detection that you can't manually enforce which files are
paired, but it's not what we'd classify as a bug since it did
correctly pick one of the files with the most similar contents.
^ permalink raw reply
* Re: Git in GSoC 2024
From: Kaartic Sivaraam @ 2024-02-21 5:02 UTC (permalink / raw)
To: Christian Couder, Patrick Steinhardt, Karthik Nayak, git
Cc: Taylor Blau, Junio C Hamano, Victoria Dye
In-Reply-To: <26cf6320-7ead-4ca0-b4b8-ca7008cae401@gmail.com>
Hello Christian, Patrick, Karthik and all,
On 06/02/24 00:09, Kaartic Sivaraam wrote:
> Hi Christian,
>
> On 05/02/24 22:37, Christian Couder wrote:
>> Hi Kaartic, Patrick, Karthik and all,
>>
>> Thanks for creating the page!
>>
>> I have just applied the patch Patrick sent to the mailing list with
>> the ideas related to reftable.
>>
>
> Thank you! I've now successfully submitted the application for Git using
> the Ideas page we have :-)
>
> Let's hope that we get selected this year. We should know about that by
> February 21 - 18:00 UTC.
>
I think it's time we start being prepared for potential contributors who
are interested in contributing to Git via GSoC. ;-)
On a more important note, we need to assign Org Admins and mentors for
the 2024 program. For now, I'm the only Org Admin for the 2024 program.
Christian, could you do the following so that I could add you as an Org
Admin for the 2024 program?
"Visit https://g.co/gsoc site, click on the 2024 bar which will display
the 2024 Program Rules and Org Member agreement, read and agree to the
same."
For mentors, as Patrick and Karthik are new to the program, we need to
invite them first and only after they've accepted the invitation can we
add them to the program.
Karthik, I was able to get your Gmail address and have sent an invite to
you. Could you accept the same after reading through the program rules
and member agreement?
Patrick, could you kindly share with me your Gmail address so that I could
invite you to the program?
--
Sivaraam
PS: Apologies if you've got this email multiple times. My email client has not
yet been able to deliver the message to the list. So, I'm trying other ways to
get it done. Sorry for the noise.
^ permalink raw reply
* Re: [PATCH v4] mergetools: vimdiff: use correct tool's name when reading mergetool config
From: Kipras Melnikovas @ 2024-02-21 5:20 UTC (permalink / raw)
To: gitster; +Cc: git, greenfoo, kipras
In-Reply-To: <xmqq8r3fyhhg.fsf@gitster.g>
On 24/02/19 06:52PM, Junio C Hamano wrote:
> That's curious. This is v4 and no changes from v3?
My bad. The patches are slightly different [1], I just managed to screw up the
range-diff args [2]. Won't happen again [3].
Sorry, I know this is wasting your time; this whole submission should've
been a single reroll not 3. I'll be more thorough next time.
Thank you for your time, help and quick responses.
Kipras
---
[3] https://github.com/kiprasmel/git-reroll/commit/3aa14a2860b19f86bb51f70ce0d5cec99a5aca59#diff-59ce06aeaf7a77de81c8f24c0301d95d017a83fdaec8bc1dce9877af708d1ad3R213-R227
[2]
What essentially happened was: I marked a new checkpoint "v4" *before*
sending the email, and ran format-patch with --range-diff "<checkpoint>..@",
which of course was "v4 against v4" instead of "v3 against v4"..
[1] fwiw,
Range-diff against v3:
1: 60be87c3d5 ! 1: 0018c7e18c mergetools: vimdiff: use correct tool's name when reading mergetool config
@@ Documentation/config/mergetool.txt: mergetool.meld.useAutoMerge::
- windows appear. Applies even if you are using Neovim (`nvim`) or
- gVim (`gvim`) as the merge tool. See BACKEND SPECIFIC HINTS section
+mergetool.<vimdiff variant>.layout::
-+ Git's vimdiff backend uses this variable to control how the split windows of
-+ `<vimdiff variant>` appear. Here `<vimdiff variant>` is any of `vimdiff`,
-+ `nvimdiff`, `gvimdiff`. To configure the layout and use the tool, see the
-+ `BACKEND SPECIFIC HINTS`
++ Configure the split window layout for vimdiff's `<variant>`, which is any of `vimdiff`,
++ `nvimdiff`, `gvimdiff`.
++ Upon launching `git mergetool` with `--tool=<variant>` (or without `--tool`
++ if `merge.tool` is configured as `<variant>`), Git will consult
++ `mergetool.<variant>.layout` to determine the tool's layout. If the
++ variant-specific configuration is not available, `vimdiff`'s is used as
++ fallback. If that too is not available, a default layout with 4 windows
++ will be used. To configure the layout, see the `BACKEND SPECIFIC HINTS`
+ifdef::git-mergetool[]
+ section.
+endif::[]
@@ Documentation/config/mergetool.txt: mergetool.meld.useAutoMerge::
mergetool.hideResolved::
During a merge, Git will automatically resolve as many conflicts as
+ ## Documentation/mergetools/vimdiff.txt ##
+@@ Documentation/mergetools/vimdiff.txt: Instead of `--tool=vimdiff`, you can also use one of these other variants:
+
+ When using these variants, in order to specify a custom layout you will have to
+ set configuration variables `mergetool.gvimdiff.layout` and
+-`mergetool.nvimdiff.layout` instead of `mergetool.vimdiff.layout`
++`mergetool.nvimdiff.layout` instead of `mergetool.vimdiff.layout` (though the
++latter will be used as fallback if the variant-specific one is not set).
+
+ In addition, for backwards compatibility with previous Git versions, you can
+ also append `1`, `2` or `3` to either `vimdiff` or any of the variants (ex:
+
## mergetools/vimdiff ##
@@ mergetools/vimdiff: diff_cmd_help () {
Documentation/config/mergetool.txt | 19 +++++++++++++------
Documentation/mergetools/vimdiff.txt | 3 ++-
mergetools/vimdiff | 12 ++++++++++--
3 files changed, 25 insertions(+), 9 deletions(-)
^ permalink raw reply
* Re: Git in GSoC 2024
From: Patrick Steinhardt @ 2024-02-21 6:19 UTC (permalink / raw)
To: Kaartic Sivaraam
Cc: Christian Couder, Karthik Nayak, git, Taylor Blau, Junio C Hamano,
Victoria Dye
In-Reply-To: <CA+ARAtqicQkhKFcTxoT+GWMhCxnV-BNqd0oOcn2YwznfFnnRPw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1898 bytes --]
On Wed, Feb 21, 2024 at 10:32:25AM +0530, Kaartic Sivaraam wrote:
> Hello Christian, Patrick, Karthik and all,
>
> On 06/02/24 00:09, Kaartic Sivaraam wrote:
> > Hi Christian,
> >
> > On 05/02/24 22:37, Christian Couder wrote:
> >> Hi Kaartic, Patrick, Karthik and all,
> >>
> >> Thanks for creating the page!
> >>
> >> I have just applied the patch Patrick sent to the mailing list with
> >> the ideas related to reftable.
> >>
> >
> > Thank you! I've now successfully submitted the application for Git using
> > the Ideas page we have :-)
> >
> > Let's hope that we get selected this year. We should know about that by
> > February 21 - 18:00 UTC.
> >
>
> I think it's time we start being prepared for potential contributors who
> are interested in contributing to Git via GSoC. ;-)
>
> On a more important note, we need to assign Org Admins and mentors for
> the 2024 program. For now, I'm the only Org Admin for the 2024 program.
>
> Christian, could you do the following so that I could add you as an Org
> Admin for the 2024 program?
>
> "Visit https://g.co/gsoc site, click on the 2024 bar which will display
> the 2024 Program Rules and Org Member agreement, read and agree to the
> same."
>
> For mentors, as Patrick and Karthik are new to the program, we need to
> invite them first and only after they've accepted the invitation can we
> add them to the program.
>
> Karthik, I was able to get your Gmail address and have sent an invite to
> you. Could you accept the same after reading through the program rules
> and member agreement?
>
> Patrick, could you kindly share with me your Gmail address so that I could
> invite you to the program?
You can use my GitLab mail address for this: psteinhardt@gitlab.com.
Thanks for handling the administrative parts, and happy to hear that the
Git project has been accepted.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] rebase: make warning less passive aggressive
From: Patrick Steinhardt @ 2024-02-21 7:14 UTC (permalink / raw)
To: Harmen Stoppels via GitGitGadget; +Cc: git, Harmen Stoppels
In-Reply-To: <pull.1669.git.1708442603395.gitgitgadget@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1989 bytes --]
On Tue, Feb 20, 2024 at 03:23:21PM +0000, Harmen Stoppels via GitGitGadget wrote:
> From: Harmen Stoppels <me@harmenstoppels.nl>
>
> When you run `git rebase --continue` when no rebase is in progress, git
> outputs `fatal: no rebase in progress?` which is not a question but a
> statement. This commit makes it appear as a statement.
>
> Signed-off-by: Harmen Stoppels <me@harmenstoppels.nl>
> ---
> rebase: make warning less passive aggressive
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1669%2Fhaampie%2Ffix%2Fpassive-agressive-message-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1669/haampie/fix/passive-agressive-message-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1669
>
> builtin/rebase.c | 2 +-
> po/bg.po | 2 +-
> po/ca.po | 2 +-
> po/de.po | 2 +-
> po/el.po | 2 +-
> po/es.po | 2 +-
> po/fr.po | 2 +-
> po/id.po | 2 +-
> po/it.po | 2 +-
> po/ko.po | 2 +-
> po/pl.po | 2 +-
> po/pt_PT.po | 2 +-
> po/ru.po | 2 +-
> po/sv.po | 2 +-
> po/tr.po | 2 +-
> po/uk.po | 2 +-
> po/vi.po | 2 +-
> po/zh_CN.po | 2 +-
> po/zh_TW.po | 2 +-
> 19 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 5b086f651a6..415783c4a21 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -1254,7 +1254,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
> die(_("options '%s' and '%s' cannot be used together"), "--root", "--fork-point");
>
> if (options.action != ACTION_NONE && !in_progress)
> - die(_("No rebase in progress?"));
> + die(_("No rebase in progress"));
While we're at it changing this message, do we also want to convert it
to start with a lower-case letter so that it aligns with our error
message style?
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 1/3] doc: git-rev-parse: enforce command-line description syntax
From: Jean-Noël Avila @ 2024-02-21 7:41 UTC (permalink / raw)
To: Junio C Hamano, Jean-Noël Avila via GitGitGadget; +Cc: git
In-Reply-To: <xmqqsf1mlp6f.fsf@gitster.g>
Le 20/02/2024 à 23:57, Junio C Hamano a écrit :
> "Jean-Noël Avila via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> [verse]
>> -'git rev-parse' [<options>] <args>...
>> +'git rev-parse' [<options>] <arg>...
>
> Good. The "or more" is signalled by the ellipsis, not "args" being
> plural.
>
>> ---short[=length]::
>> +--short[=<length>]::
>> Same as `--verify` but shortens the object name to a unique
>> prefix with at least `length` characters. The minimum length
>
> This same comment applies throughout this patch, but in other places
> when we use <placeholder> in the option argument description, don't
> we use the same <placeholder> in text as well? I am wondering if
> the `length` (typeset in fixed-width) should become <length>. What
> do other recent[*] documentation pages commonly do?
This is another part of the inconsistences in documentation that I'd
like to tackle (hopefully, not in another life).
Using angle brackets for placeholders everywhere they appear is a visual
link to the preceding syntax description, but may feel a bit heavy on
some cases. Anyway, I'm all for applying the rule everywhere, for the
sake of consistency.
Backticks and single quotes are used indistinctively (by the way,
asciidoctor does not process single quotes as markup) and are not used
everywhere they should. Using backticks is also a good hint for
translators to mean "verbatim, do not translate". Obviously, the
placeholders ask for translation, so the backtick rule should not apply
to them, even if another formating would be welcome : _<placeholder>_
for instance?
>
> Side note: I say "recent" because rev-parse doc is one of
> the oldest ones that did not get typesetting attention they
> deserve, compared to more recent ones that got nitpicked
> while they were written and updated.
>
>> ---branches[=pattern]::
>> ---tags[=pattern]::
>> ---remotes[=pattern]::
>> +--branches[=<pattern>]::
>> +--tags[=<pattern>]::
>> +--remotes[=<pattern>]::
>> Show all branches, tags, or remote-tracking branches,
>> respectively (i.e., refs found in `refs/heads`,
>> `refs/tags`, or `refs/remotes`, respectively).
>
> Ditto. We see `pattern` that may want to become <pattern> in the
> description (after the post context of this hunk).
>
>> ---glob=pattern::
>> +--glob=<pattern>::
>> Show all refs matching the shell glob pattern `pattern`. If
>> the pattern does not start with `refs/`, this is automatically
>> prepended. If the pattern does not contain a globbing
>
> Ditto.
>
>> ---exclude-hidden=[fetch|receive|uploadpack]::
>> +--exclude-hidden=(fetch|receive|uploadpack)::
>> Do not include refs that would be hidden by `git-fetch`,
>> `git-receive-pack` or `git-upload-pack` by consulting the appropriate
>> `fetch.hideRefs`, `receive.hideRefs` or `uploadpack.hideRefs`
>
> Good.
>
>> ---since=datestring::
>> ---after=datestring::
>> +--since=<datestring>::
>> +--after=<datestring>::
>> Parse the date string, and output the corresponding
>> --max-age= parameter for 'git rev-list'.
>
> Good, modulo possibly "date string" -> "<datestring>".
>
>> ---until=datestring::
>> ---before=datestring::
>> +--until=<datestring>::
>> +--before=<datestring>::
>> Parse the date string, and output the corresponding
>> --min-age= parameter for 'git rev-list'.
>
> Ditto.
>
>> -<args>...::
>> +<arg>...::
>> Flags and parameters to be parsed.
>
> Good.
^ permalink raw reply
* Re: [PATCH] documentation: send-email: use camel case consistently
From: Dragan Simic @ 2024-02-21 7:50 UTC (permalink / raw)
To: Chris Torek; +Cc: Junio C Hamano, git
In-Reply-To: <CAPx1GvcGhZLqHVz9=ZW-w+ebP64-8FpPSb_ef7ygXzDDTze2bA@mail.gmail.com>
On 2024-02-21 01:43, Chris Torek wrote:
> On Tue, Feb 20, 2024 at 8:42 AM Dragan Simic <dsimic@manjaro.org>
> wrote:
>> I've never ever seen anyone referring to email headers as "TO", "CC"
>> or
>> "BCC". It's always referred to as "To", "Cc" and "Bcc".
>
> I used some email system (back in the early 1980s) that did that. It
> felt weird even then. I can't remember if it was some CSMail (CSNet)
> or MH(Rand Mail Handler) version that did it.
That's interesting, it shows that different variants were used in the
very early days of email. Maybe even the all-lowercase "cc" and "bcc"
variants were used somewhere, at least because RFC2076 (better said,
the RFCs that predate it) specifies them.
>> Thus, "cc" stems from the old age of literal carbon copies ...
>
> That's correct. However:
>
>> and "bcc" was seemingly coined when email took over.
>
> "Blind Carbon Copies" predated email, but required adding the
> notation separately, if it was to be added at all. (I'm just old enough
> to remember using carbon copies myself, but not old enough to
> know what Standard Office Practice was at that time.)
Thanks for the correction. You're right, I was lazy enough not to
check that blind carbon copies predate the age of email. [1]
I'm also old enough to remember the literal carbon copies, I even made
a few dozens of them myself on a mechanical typewriter. They usually
left me with dirty fingertips. :) Though, I'm also not old enough to
know what the common office practice was like back then.
> Whether adding a "bcc" notation was common I don't know;
> it seems it would be easier to leave it off if you made, say, one
> original and a total of 2 copies, one "blinded".
>
> (As your Wikipedia link notes, there was a practical limit to how
> many carbon copies one could make in the first place.)
Exactly, it was the limitation of mechanical typewriters. Perhaps the
limit was around four or five carbon copies.
[1] https://en.wikipedia.org/wiki/Blind_carbon_copy
^ permalink raw reply
* [PATCH] git-remote.txt: fix typo
From: Jakub Wilk @ 2024-02-21 8:35 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Abhradeep Chakraborty
Signed-off-by: Jakub Wilk <jwilk@jwilk.net>
---
Documentation/git-remote.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
index 1dec314834..932a5c3ea4 100644
--- a/Documentation/git-remote.txt
+++ b/Documentation/git-remote.txt
@@ -35,7 +35,7 @@ OPTIONS
-v::
--verbose::
Be a little more verbose and show remote url after name.
- For promisor remotes, also show which filter (`blob:none` etc.)
+ For promisor remotes, also show which filters (`blob:none` etc.)
are configured.
NOTE: This must be placed between `remote` and subcommand.
--
2.39.2
^ permalink raw reply related
* Re: Segfault: git show-branch --reflog refs/pullreqs/1
From: Jeff King @ 2024-02-21 8:42 UTC (permalink / raw)
To: Yasushi SHOJI; +Cc: Denton Liu, Junio C Hamano, Git Mailing List
In-Reply-To: <CAELBRWK-bZTV0qx6_34HAgpmYwy+5Zo2E0M+4B6yZJJ3CqweTw@mail.gmail.com>
On Wed, Feb 21, 2024 at 10:48:25AM +0900, Yasushi SHOJI wrote:
> Does anyone see a segfault on `git show-branch --reflog refs/pullreqs/1`?
>
> It seems files_reflog_path() creates a wrong path with the above command
> using REF_WORKTREE_SHARED.
I can trigger a segfault, but I think the issue is simply a ref that has
no reflog. Here's a simple reproduction:
$ git init
$ git commit --allow-empty -m foo
$ rm -rf .git/logs
$ git show-branch --reflog
Segmentation fault
The bug is in read_ref_at(). When asked for the reflog at position "0",
it calls refs_for_each_reflog_ent_reverse() with a special callback, but
does not check that it actually found anything! So we return "0" for
success, but all of the returned fields are garbage (including the
pointer to reflog message, which is where I see the segfault).
The bug was introduced by 6436a20284 (refs: allow @{n} to work with
n-sized reflog, 2021-01-07). Probably the fix is something like:
diff --git a/refs.c b/refs.c
index 03968ad787..c2a48f8188 100644
--- a/refs.c
+++ b/refs.c
@@ -945,6 +945,8 @@ static int read_ref_at_ent_newest(struct object_id *ooid, struct object_id *noid
set_read_ref_cutoffs(cb, timestamp, tz, message);
oidcpy(cb->oid, noid);
+ cb->reccnt++;
+ cb->found_it = 1;
/* We just want the first entry */
return 1;
}
@@ -980,12 +982,10 @@ int read_ref_at(struct ref_store *refs, const char *refname,
cb.cutoff_cnt = cutoff_cnt;
cb.oid = oid;
- if (cb.cnt == 0) {
+ if (cb.cnt == 0)
refs_for_each_reflog_ent_reverse(refs, refname, read_ref_at_ent_newest, &cb);
- return 0;
- }
-
- refs_for_each_reflog_ent_reverse(refs, refname, read_ref_at_ent, &cb);
+ else
+ refs_for_each_reflog_ent_reverse(refs, refname, read_ref_at_ent, &cb);
if (!cb.reccnt) {
if (flags & GET_OID_QUIETLY)
but that breaks t1508.35, which explicitly tests for branch@{0} to work
with an empty reflog file (added by that same commit). The code in
get_oid_basic() to parse reflogs doesn't suffer from the same bugs: it
checks up front that the reflog file exists, it preloads the output oid
with the current ref value, and it doesn't look at other fields (like
the reflog message).
So I'm not sure if read_ref_at() needs to be made safer, or if
cmd_show_branch() needs to learn the same tricks as get_oid_basic().
Those are the only two callers of read_ref_at().
Beyond that confusion, I noticed we do not have many tests for
show-branch, and none for "--reflog". So I thought to add a basic one
where we _do_ have an actual reflog to show. But wow, this has been
broken for some time. I found at least two issues trying to run a test
like:
diff --git a/t/t3207-show-branch-reflog.sh b/t/t3207-show-branch-reflog.sh
new file mode 100755
index 0000000000..7f52c8dcb1
--- /dev/null
+++ b/t/t3207-show-branch-reflog.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+
+test_description='show-branch reflog tests'
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ test_commit base &&
+ git checkout -b branch &&
+ test_commit one &&
+ git reset --hard HEAD^ &&
+ test_commit two &&
+ test_commit three
+'
+
+test_expect_success '--reflog shows reflog entries' '
+ cat >expect <<-\EOF &&
+ ! [branch@{0}] (0 seconds ago) commit: three
+ ! [branch@{1}] (60 seconds ago) commit: two
+ ! [branch@{2}] (2 minutes ago) reset: moving to HEAD^
+ ! [branch@{3}] (2 minutes ago) commit: one
+ ----
+ + [refs/heads/branch@{0}] three
+ ++ [refs/heads/branch@{1}] two
+ + [refs/heads/branch@{3}] one
+ ++++ [refs/heads/branch@{2}] base
+ EOF
+ # the output always contains relative timestamps; use
+ # a known time to get deterministic results
+ GIT_TEST_DATE_NOW=$test_tick \
+ git show-branch --reflog branch >actual &&
+ test_cmp expect actual
+'
+
+test_done
The first is that "show-branch" does not print the correct reflog
message, and you get output like this:
! [branch@{0}] (0 seconds ago) (none)
! [branch@{1}] (0 seconds ago) (none)
! [branch@{2}] (60 seconds ago) (none)
! [branch@{3}] (2 minutes ago) (none)
Once upon a time, read_ref_at() returned the whole reflog line, and
show-branch had to find the tab-separator. But since 4207ed285f (refs.c:
change read_ref_at to use the reflog iterators, 2014-06-03), it returns
just the actual message (curiously, with the newline still attached). So
we need something like this to fix it:
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index d6d2dabeca..b678b9fedb 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -761,7 +761,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
for (i = 0; i < reflog; i++) {
char *logmsg;
char *nth_desc;
- const char *msg;
+ char *eol;
timestamp_t timestamp;
int tz;
@@ -771,15 +771,13 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
reflog = i;
break;
}
- msg = strchr(logmsg, '\t');
- if (!msg)
- msg = "(none)";
- else
- msg++;
+ eol = strchr(logmsg, '\n');
+ if (eol)
+ *eol = '\0';
reflog_msg[i] = xstrfmt("(%s) %s",
show_date(timestamp, tz,
DATE_MODE(RELATIVE)),
- msg);
+ logmsg);
free(logmsg);
nth_desc = xstrfmt("%s@{%d}", *av, base+i);
Easy enough. But the output is still subtly wrong! Now we're back to
6436a20284 (refs: allow @{n} to work with n-sized reflog, 2021-01-07)
again. Before that commit, applying the fix above gives the expected
output from my test:
! [branch@{0}] (0 seconds ago) commit: three
! [branch@{1}] (60 seconds ago) commit: two
! [branch@{2}] (2 minutes ago) reset: moving to HEAD^
! [branch@{3}] (2 minutes ago) commit: one
but afterwards, entries higher than one are all shifted (so 1 is a
duplicate of 0, 2 is the old 1, and so on):
! [branch@{0}] (0 seconds ago) commit: three
! [branch@{1}] (0 seconds ago) commit: three
! [branch@{2}] (60 seconds ago) commit: two
! [branch@{3}] (2 minutes ago) reset: moving to HEAD^
I am still trying to wrap my head around how it can get such wrong
results for show-branch when asking for "git rev-parse branch@{0}", etc,
are correct. I think it is that "rev-parse branch@{0}" is only looking
at the output oid and does not consider the reflog message at all. So I
think it is subtly broken, but in a way that happens to work for that
caller. But I'm not sure of the correct fix. At least not at this time
of night.
Cc-ing folks involved in 6436a20284.
-Peff
^ permalink raw reply related
* Re: Git in GSoC 2024
From: Christian Couder @ 2024-02-21 8:43 UTC (permalink / raw)
To: Kaartic Sivaraam
Cc: Patrick Steinhardt, Karthik Nayak, git, Taylor Blau,
Junio C Hamano, Victoria Dye
In-Reply-To: <CA+ARAtqicQkhKFcTxoT+GWMhCxnV-BNqd0oOcn2YwznfFnnRPw@mail.gmail.com>
Hi Kaartic,
On Wed, Feb 21, 2024 at 6:02 AM Kaartic Sivaraam
<kaartic.sivaraam@gmail.com> wrote:
> Christian, could you do the following so that I could add you as an Org
> Admin for the 2024 program?
Yeah, sure.
> "Visit https://g.co/gsoc site, click on the 2024 bar which will display
> the 2024 Program Rules and Org Member agreement, read and agree to the
> same."
Done. My status is now "Accepted", but yeah I think you need to add me
to the GSoC 2024 program now.
> For mentors, as Patrick and Karthik are new to the program, we need to
> invite them first and only after they've accepted the invitation can we
> add them to the program.
Thanks for inviting them!
^ permalink raw reply
* Re: Git in GSoC 2024
From: Karthik Nayak @ 2024-02-21 8:59 UTC (permalink / raw)
To: Kaartic Sivaraam, Christian Couder, Patrick Steinhardt, git
Cc: Taylor Blau, Junio C Hamano, Victoria Dye
In-Reply-To: <CA+ARAtqicQkhKFcTxoT+GWMhCxnV-BNqd0oOcn2YwznfFnnRPw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 372 bytes --]
Hello,
Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:
> Hello Christian, Patrick, Karthik and all,
> Karthik, I was able to get your Gmail address and have sent an invite to
> you. Could you accept the same after reading through the program rules
> and member agreement?
>
I confirm the same, and have signed up.
Thanks for handling everything here.
- Karthik
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox