* Re: [RFC][PATCH 0/32] SHA256 and SHA1 interoperability
From: brian m. carlson @ 2023-09-10 15:38 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: git, Junio C Hamano
In-Reply-To: <87sf7ol0z3.fsf@email.froward.int.ebiederm.org>
[-- Attachment #1: Type: text/plain, Size: 2175 bytes --]
On 2023-09-08 at 23:05:52, Eric W. Biederman wrote:
>
> I would like to see the SHA256 transition happen so I started playing
> with the k2204-transition-interop branch of brian m. carlson's tree.
>
> Before I go farther I need to some other folks to look at this and see
> if this is a general direction that the git project can stand.
I'm really excited to see this and I think it's a great way forward.
I've taken a brief look at each patch, and I don't see anything that
should be a dealbreaker. I left a few comments, although I think your
mailserver is blocking mine at the moment, so you may not have received
them (hopefully you can read them on the list in the interim).
You may also feel free to simply adjust the commit message for the
patches of mine you've modified without needing to document that you've
changed them. I expect that you will have changed them when you submit
them, if only to resolve conflicts. After all, Junio does so all the
time.
> This patchset is not complete it does not implement converting a
> received pack of the compatibility hash into the hash function of the
> repository, nor have I written any automated tests. Both need to happen
> before this is finalized.
Speaking of tests, one set of tests I had intended to write and think
should be written, but had not yet implemented, is tests for
round-tripping objects. That is, the SHA-1 value we get for a revision
in a pure SHA-1 repository should obviously be the same as the SHA-1
value we get in a SHA-256 repository in interop mode, and we should be
able to use the `test_oid_cache` functionality to hard-code the desired
objects. I think it would be also helpful to do this for fixed objects
that are doubly-signed (with both algorithms) as well, since that's a
tricky edge case that we'll want to avoid breaking. Other edge cases
will include things like merge commits, including octopus merges.
But overall, I think this is a great improvement, and I'm very excited
to see someone picking up some of this work and moving it forward.
Thanks for doing so.
--
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 02/32] doc hash-function-transition: Replace compatObjectFormat with compatMap
From: brian m. carlson @ 2023-09-10 14:34 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: git, Junio C Hamano
In-Reply-To: <20230908231049.2035003-2-ebiederm@xmission.com>
[-- Attachment #1: Type: text/plain, Size: 2248 bytes --]
On 2023-09-08 at 23:10:19, Eric W. Biederman wrote:
> Ir makes a lot of sense for the hash algorithm that determines how all
Minor nit: "It".
> diff --git a/Documentation/technical/hash-function-transition.txt b/Documentation/technical/hash-function-transition.txt
> index 4b937480848a..10572c5794f9 100644
> --- a/Documentation/technical/hash-function-transition.txt
> +++ b/Documentation/technical/hash-function-transition.txt
> @@ -148,14 +148,14 @@ Detailed Design
> Repository format extension
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~
> A SHA-256 repository uses repository format version `1` (see
> -Documentation/technical/repository-version.txt) with extensions
> -`objectFormat` and `compatObjectFormat`:
> +Documentation/technical/repository-version.txt) with the extension
> +`objectFormat`, and an optional core.compatMap configuration.
>
> [core]
> repositoryFormatVersion = 1
> + compatMap = on
> [extensions]
> objectFormat = sha256
> - compatObjectFormat = sha1
While I'm in favour of an approach that uses the compat map, the
situation we've implemented here doesn't specify the extra hash
algorithm. We want this approach to work just as well for moving from
SHA-1 to SHA-256 as it might for a future transition from SHA-256 to,
say, SHA-3-512, if that becomes necessary.
Making a future transition easier has been a goal of my SHA-256 work
(because who wants to write several hundred patches in such a case?), so
my hope is we can keep that here as well by explicitly naming the
algorithm we're using.
I also wonder if an approach that doesn't use an extension is going to
be helpful. Say, that I have a repository that is using Git 3.x, which
supports interop, but I also need to use Git 2.x, which does not. While
it's true that Git 2.x can read my SHA-256 repository, it won't write
the appropriate objects into the map, and thus it will be practically
very difficult to actually use Git 3.x to push data to a repository of a
different hash function. We might well prefer to have Git 2.x not work
with the repository at all rather than have incomplete data preventing
us from, well, interoperating.
--
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 01/32] doc hash-file-transition: A map file for mapping between sha1 and sha256
From: brian m. carlson @ 2023-09-10 14:24 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: git, Junio C Hamano
In-Reply-To: <20230908231049.2035003-1-ebiederm@xmission.com>
[-- Attachment #1: Type: text/plain, Size: 1305 bytes --]
On 2023-09-08 at 23:10:18, Eric W. Biederman wrote:
> The v3 pack index file as documented has a lot of complexity making it
> difficult to implement correctly. I worked with bryan's preliminary
> implementation and it took several passes to get the bugs out.
>
> The complexity also requires multiple table look-ups to find all of
> the information that is needed to translate from one kind of oid to
> another. Which can't be good for cache locality.
>
> Even worse coming up with a new index file version requires making
> changes that have the potentialy to break anything that uses the index
> of a pack file.
>
> Instead of continuing to deal with the chance of braking things
> besides the oid mapping functionality, the additional complexity in
> the file format, and worry if the performance would be reasonable I
> stripped down the problem to it's fundamental complexity and came up
> with a file format that is exactly about mapping one kind of oid to
> another, and only supports two kinds of oids.
I think this is a fine approach, and as I'm sure you noticed from my
series, it's a lot more robust than trying to implement pack v3. I'd be
fine with going with this approach instead of pack v3.
--
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 2/2] parse-options: use and require int pointer for OPT_CMDMODE
From: Oswald Buddenhagen @ 2023-09-10 10:18 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List, Jeff King, Junio C Hamano
In-Reply-To: <e6d8a291-03de-cfd3-3813-747fc2cad145@web.de>
On Sat, Sep 09, 2023 at 11:14:20PM +0200, René Scharfe wrote:
>Convert the offending OPT_CMDMODE users and use the typed value_int
>point in the macro's definition to enforce that type for future ones.
>
that defeats -Wswitch[-enum], though.
the pedantically correct solution would be using setter callbacks.
regards
^ permalink raw reply
* Re: [PATCH] rebase -i: ignore signals when forking subprocesses
From: Oswald Buddenhagen @ 2023-09-10 10:05 UTC (permalink / raw)
To: phillip.wood
Cc: Jeff King, Phillip Wood via GitGitGadget, git,
Johannes Schindelin, Junio C Hamano
In-Reply-To: <9ba22d4b-3cbe-4d4a-8dba-bc3781e82222@gmail.com>
On Fri, Sep 08, 2023 at 10:59:06AM +0100, Phillip Wood wrote:
>Ah, I hadn't thought about "gc --auto". I was assuming that the calling
>code would see the child had been killed and exit but that's not always
>the case.
>
that's a quite reasonable assumption.
ignoring gc's exit status is ok-ish, but ignoring its termination signal
is absolutely not.
>On 07/09/2023 22:06, Jeff King wrote:
>> I think this really comes down to: does the user perceive the child
>> process as the current "main" process running in the foreground?
>>
that is indeed a key point here.
note that the shell doesn't enable job control in scripts, either.
>The child not dying is tricky, if it is in the same process group as
>git then even if git dies the I think the shell will wait for the child
>to exit before showing the prompt again so it is not clear to me that
>the user is disadvantaged by git ignoring SIGINT in that case.
>
there is no such thing as waiting for grandchildren. the grandchild is
reparented to init when the child exits.
there is a situation were one can be deadlocked by a non-exiting
grandchild: when doing a blocking read of the child's output past its
exit, when the grandchild has inherited stdout. but that's a
implementation bug in the parent. and not relevant here.
On Fri, Sep 08, 2023 at 02:11:51PM +0100, Phillip Wood wrote:
>On 08/09/2023 10:59, Phillip Wood wrote:
>>> I've never done it before, but from my reading we basically want to
>>> do
>>> (in the forked process before we exec):
>>>
>>> setsid();
>>> open("/dev/tty");
>>
>> Do we want a whole new session? As I understand it to launch a
>> foreground job shells put the child in its own process group and then
>> call tcsetpgrp() to change the foreground process group of the
>> controlling terminal to that of the child.
>
this would indeed be the right way if we wanted to isolate the children
more, but ...
>It is better for handling SIGINT and SIGQUIT when we don't want git to
>be killed but in complicates the handling of SIGTSTP and friends. [...]
>
... this shows that we really don't want that; we don't want to
replicate interactive shell behavior. that is even before the divergence
on windows.
so i think your patch is approaching things the right way.
though blocking signals doesn't appear right - to ensure git's own clean
exit while it has no children, it must catch sigint anyway, and
temporarily ignoring it around spawning children sounds racy.
regards
^ permalink raw reply
* Re: [PATCH] diff --no-index: fix -R with stdin
From: Phillip Wood @ 2023-09-10 10:00 UTC (permalink / raw)
To: René Scharfe, Martin Storsjö, git; +Cc: Phillip Wood, Junio C Hamano
In-Reply-To: <22fdfa3b-f90e-afcc-667c-705fb7670245@web.de>
On 09/09/2023 23:12, René Scharfe wrote:
> When -R is given, queue_diff() swaps the mode and name variables of the
> two files to produce a reverse diff. 1e3f26542a (diff --no-index:
> support reading from named pipes, 2023-07-05) added variables that
> indicate whether files are special, i.e named pipes or - for stdin.
> These new variables were not swapped, though, which broke the handling
> of stdin with with -R. Swap them like the other metadata variables.
>
> Reported-by: Martin Storsjö <martin@martin.st>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
> Great bug report, thank you!
Yes thank you Martin for reporting this and thank you to René for fixing
it - I saw the report just before I went to bed and it was already fixed
by the time I got up! The patch looks good and thanks for adding a test.
Best Wishes
Phillip
> diff-no-index.c | 1 +
> t/t4053-diff-no-index.sh | 19 +++++++++++++++++++
> 2 files changed, 20 insertions(+)
>
> diff --git a/diff-no-index.c b/diff-no-index.c
> index 8aead3e332..e7041b89e3 100644
> --- a/diff-no-index.c
> +++ b/diff-no-index.c
> @@ -232,6 +232,7 @@ static int queue_diff(struct diff_options *o,
> if (o->flags.reverse_diff) {
> SWAP(mode1, mode2);
> SWAP(name1, name2);
> + SWAP(special1, special2);
> }
>
> d1 = noindex_filespec(name1, mode1, special1);
> diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
> index 6781cc9078..5f059f65fc 100755
> --- a/t/t4053-diff-no-index.sh
> +++ b/t/t4053-diff-no-index.sh
> @@ -224,6 +224,25 @@ test_expect_success "diff --no-index treats '-' as stdin" '
> test_must_be_empty actual
> '
>
> +test_expect_success "diff --no-index -R treats '-' as stdin" '
> + cat >expect <<-EOF &&
> + diff --git b/a/1 a/-
> + index $(git hash-object --stdin <a/1)..$ZERO_OID 100644
> + --- b/a/1
> + +++ a/-
> + @@ -1 +1 @@
> + -1
> + +x
> + EOF
> +
> + test_write_lines x | test_expect_code 1 \
> + git -c core.abbrev=no diff --no-index -R -- - a/1 >actual &&
> + test_cmp expect actual &&
> +
> + test_write_lines 1 | git diff --no-index -R -- a/1 - >actual &&
> + test_must_be_empty actual
> +'
> +
> test_expect_success 'diff --no-index refuses to diff stdin and a directory' '
> test_must_fail git diff --no-index -- - a </dev/null 2>err &&
> grep "fatal: cannot compare stdin to a directory" err
> --
> 2.42.0
^ permalink raw reply
* Re: [bug] git clone command leaves orphaned ssh process
From: Max Amelchenko @ 2023-09-10 9:47 UTC (permalink / raw)
To: Bagas Sanjaya; +Cc: git, Hideaki Yoshifuji, Junio C Hamano
In-Reply-To: <ZP2DaQMA_aFvjQiR@debian.me>
Output of first ps aux command:
bash-4.2# ps aux
USER PID %CPU %MEM VSZ RSS TTY STAT START TIME COMMAND
root 1 0.0 0.0 715708 5144 pts/0 Ssl+ 09:43 0:00
/usr/local/bin/aws-lambda-rie /var/runtime/bootstrap
root 14 0.1 0.0 114096 3088 pts/1 Ss 09:43 0:00 bash
root 165 0.0 0.0 118296 3392 pts/1 R+ 09:45 0:00 ps aux
Output of second ps aux command (after running git clone):
bash-4.2# ps aux
USER PID %CPU %MEM VSZ RSS TTY STAT START TIME COMMAND
root 1 0.0 0.0 715708 5144 pts/0 Ssl+ 09:43 0:00
/usr/local/bin/aws-lambda-rie /var/runtime/bootstrap
root 14 0.0 0.0 114096 3088 pts/1 Ss 09:43 0:00 bash
root 167 0.5 0.0 0 0 pts/1 Z 09:46 0:00 [ssh] <defunct>
root 168 0.0 0.0 118296 3408 pts/1 R+ 09:46 0:00 ps aux
See the added ssh defunct process.
On Sun, Sep 10, 2023 at 11:50 AM Bagas Sanjaya <bagasdotme@gmail.com> wrote:
>
> On Sun, Sep 10, 2023 at 09:38:54AM +0300, Max Amelchenko wrote:
> > What did you do before the bug happened? (Steps to reproduce your issue)
> >
> > Run the command:
> > ps aux
> > Observe no ssh processes running on system.
> >
> > Run git clone against a non-existent hostname:
> > git clone -v --depth=1 -b 3.23.66
> > ssh://*****@*****lab-prod.server.sim.cloud/terraform/modules/aws-eks
> > /tmp/dest
> > Observe the command fails with:
> >
> > Could not resolve hostname *****lab-prod.server.sim.cloud: Name or
> > service not known
> >
> > Run:
> > ps aux
> >
> > Observe a defunct ssh process is left behind.
>
> On git current master on my system, I got sshd (server) processes instead:
>
> ```
> root 835 0.0 0.0 15500 3584 ? Ss 14:38 0:00 sshd: /usr/sbin/sshd -D [listener] 0 of 10-100 startups
> 165536 3865 0.0 0.0 8488 1408 ? Ss 14:39 0:00 sshd: /usr/sbin/sshd -D -e [listener] 0 of 10-100 startups
> 165536 4039 0.0 0.0 11308 1920 ? Ss 14:40 0:00 sshd: /usr/bin/sshd -D [listener] 0 of 10-100 startups
> 165536 4374 0.0 0.0 15404 1920 ? Ss 14:40 0:00 sshd: /usr/sbin/sshd -D [listener] 0 of 10-100 startups
> 165536 4399 0.0 0.0 15404 1792 ? Ss 14:40 0:00 sshd: /usr/sbin/sshd -D [listener] 0 of 10-100 startups
> 165536 4732 0.0 0.0 15404 2048 ? Ss 14:41 0:00 sshd: /usr/sbin/sshd -D [listener] 0 of 10-100 startups
> 165536 4943 0.0 0.0 18004 848 ? Ss 14:41 0:00 sshd: /usr/sbin/sshd -D [listener] 0 of 10-100 startups
> bagas 6841 0.0 0.0 7668 1092 ? Ss 14:43 0:00 /usr/bin/ssh-agent /usr/bin/im-launch /usr/bin/gnome-session
> bagas 6908 0.0 0.1 162780 5488 ? Ssl 14:43 0:00 /usr/libexec/gcr-ssh-agent /run/user/1000/gcr
>
> ```
>
> What is your ps output then?
>
> Thanks.
>
> --
> An old man doll... just what I always wanted! - Clara
^ permalink raw reply
* Re: [bug] git clone command leaves orphaned ssh process
From: Bagas Sanjaya @ 2023-09-10 8:50 UTC (permalink / raw)
To: Max Amelchenko, git; +Cc: Hideaki Yoshifuji, Junio C Hamano
In-Reply-To: <CAN47KsV0E+XC2F+TVKXnnJnkATRp7eM7=-ZJFyZcoTz9SJmcHQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1913 bytes --]
On Sun, Sep 10, 2023 at 09:38:54AM +0300, Max Amelchenko wrote:
> What did you do before the bug happened? (Steps to reproduce your issue)
>
> Run the command:
> ps aux
> Observe no ssh processes running on system.
>
> Run git clone against a non-existent hostname:
> git clone -v --depth=1 -b 3.23.66
> ssh://*****@*****lab-prod.server.sim.cloud/terraform/modules/aws-eks
> /tmp/dest
> Observe the command fails with:
>
> Could not resolve hostname *****lab-prod.server.sim.cloud: Name or
> service not known
>
> Run:
> ps aux
>
> Observe a defunct ssh process is left behind.
On git current master on my system, I got sshd (server) processes instead:
```
root 835 0.0 0.0 15500 3584 ? Ss 14:38 0:00 sshd: /usr/sbin/sshd -D [listener] 0 of 10-100 startups
165536 3865 0.0 0.0 8488 1408 ? Ss 14:39 0:00 sshd: /usr/sbin/sshd -D -e [listener] 0 of 10-100 startups
165536 4039 0.0 0.0 11308 1920 ? Ss 14:40 0:00 sshd: /usr/bin/sshd -D [listener] 0 of 10-100 startups
165536 4374 0.0 0.0 15404 1920 ? Ss 14:40 0:00 sshd: /usr/sbin/sshd -D [listener] 0 of 10-100 startups
165536 4399 0.0 0.0 15404 1792 ? Ss 14:40 0:00 sshd: /usr/sbin/sshd -D [listener] 0 of 10-100 startups
165536 4732 0.0 0.0 15404 2048 ? Ss 14:41 0:00 sshd: /usr/sbin/sshd -D [listener] 0 of 10-100 startups
165536 4943 0.0 0.0 18004 848 ? Ss 14:41 0:00 sshd: /usr/sbin/sshd -D [listener] 0 of 10-100 startups
bagas 6841 0.0 0.0 7668 1092 ? Ss 14:43 0:00 /usr/bin/ssh-agent /usr/bin/im-launch /usr/bin/gnome-session
bagas 6908 0.0 0.1 162780 5488 ? Ssl 14:43 0:00 /usr/libexec/gcr-ssh-agent /run/user/1000/gcr
```
What is your ps output then?
Thanks.
--
An old man doll... just what I always wanted! - Clara
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* [PATCH] maintenance(systemd): support the Windows Subsystem for Linux
From: Johannes Schindelin via GitGitGadget @ 2023-09-10 8:30 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
When running in the Windows Subsystem for Linux (WSL), it is usually
necessary to use the Git Credential Manager for authentication when
performing the background fetches.
This requires interoperability between the Windows Subsystem for Linux
and the Windows host to work, which uses so-called vsocks, i.e. sockets
intended for communcations between virtual machines and the host they
are running on.
However, when Git is configured to run background maintenance via
`systemd`, the address families available to those maintenance processes
are restricted, and did not include `AF_VSOCK`. This leads to problems
e.g. when a background fetch tries to access github.com:
systemd[437]: Starting Optimize Git repositories data...
git[747387]: WSL (747387) ERROR: UtilBindVsockAnyPort:285: socket failed 97
git[747381]: fatal: could not read Username for 'https://github.com': No such device or address
git[747381]: error: failed to prefetch remotes
git[747381]: error: task 'prefetch' failed
systemd[437]: git-maintenance@hourly.service: Main process exited, code=exited, status=1/FAILURE
systemd[437]: git-maintenance@hourly.service: Failed with result 'exit-code'.
systemd[437]: Failed to start Optimize Git repositories data.
Address this (pun intended) by adding the `AF_VSOCK` address family to
the allow list.
This fixes https://github.com/microsoft/git/issues/604.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
maintenance(systemd): support the Windows Subsystem for Linux
https://github.com/microsoft/git/issues/604 reports a bug where git
maintenance start does not work in the Windows Subsystem for Linux
(WSL), and this patch fixes it.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1586%2Fdscho%2Fscheduled-maintenance-in-wsl-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1586/dscho/scheduled-maintenance-in-wsl-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1586
builtin/gc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 5c4315f0d81..719cae9a88a 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -2397,7 +2397,7 @@ static int systemd_timer_write_unit_templates(const char *exec_path)
"LockPersonality=yes\n"
"MemoryDenyWriteExecute=yes\n"
"NoNewPrivileges=yes\n"
- "RestrictAddressFamilies=AF_UNIX AF_INET AF_INET6\n"
+ "RestrictAddressFamilies=AF_UNIX AF_INET AF_INET6 AF_VSOCK\n"
"RestrictNamespaces=yes\n"
"RestrictRealtime=yes\n"
"RestrictSUIDSGID=yes\n"
base-commit: 43c8a30d150ecede9709c1f2527c8fba92c65f40
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH] rebase -i: ignore signals when forking subprocesses
From: Johannes Schindelin @ 2023-09-10 8:24 UTC (permalink / raw)
To: Phillip Wood
Cc: Phillip Wood via GitGitGadget, git, Jeff King, Junio C Hamano
In-Reply-To: <4fecbf96-aa04-41bd-a589-bf7c368d586c@gmail.com>
Hi Phillip,
On Fri, 8 Sep 2023, Phillip Wood wrote:
> On 07/09/2023 13:57, Johannes Schindelin wrote:
> >
> > On Thu, 7 Sep 2023, Phillip Wood via GitGitGadget wrote:
> >
> > > Having written this I started thinking about what happens when
> > > we fork hooks, merge strategies and merge drivers. I now wonder
> > > if it would be better to change run_command() instead - are
> > > there any cases where we actually want git to be killed when
> > > the user interrupts a child process?
> >
> > I am not sure that we can rely on arbitrary hooks to do the right
> > thing upon Ctrl+C, which is to wrap up and leave. So I _guess_ that we
> > will have to leave it an opt-in.
>
> Peff pointed out it doesn't play well with "gc --auto" either. Do you have any
> thoughts (particularly about the implications for Windows) on his suggestion
> to put the child in it's own session, or putting the child in its own process
> group and making that the foreground process group of the controlling
> terminal?
The concept of "sessions" does not really translate well into the Windows
world. Neither does the concept of a "process group".
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH] completion(switch/checkout): treat --track and -t the same
From: Johannes Schindelin @ 2023-09-10 8:22 UTC (permalink / raw)
To: Todd Zullinger; +Cc: Junio C Hamano, Johannes Schindelin via GitGitGadget, git
In-Reply-To: <ZPtISYVufU0MazO_@pobox.com>
Hi Todd,
On Fri, 8 Sep 2023, Todd Zullinger wrote:
> Junio C Hamano wrote:
> > "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> > writes:
> >> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> >> index 8835e16e811..df8bc44c285 100755
> >> --- a/t/t9902-completion.sh
> >> +++ b/t/t9902-completion.sh
> >> @@ -1622,14 +1622,22 @@ test_expect_success 'git checkout - with -d, complete only references' '
> >> '
> >>
> >> test_expect_success 'git switch - with --track, complete only remote branches' '
> >> - test_completion "git switch --track " <<-\EOF
> >> +: test_completion "git switch --track " <<-\EOF &&
>
> Is this new leading ":" intended? It looks out of place
> (though perhaps I just don't unerstand the context well
> enough).
Thanks for catching this. It is a debugging left-over, when I wanted to
make sure that the `-t` validation I added would run immediately.
I see that Junio helpfully dropped it before merging down to `next`, so I
will refrain from sending a v2.
Ciao,
Johannes
>
> >> + other/branch-in-other Z
> >> + other/main-in-other Z
> >> + EOF
> >> + test_completion "git switch -t " <<-\EOF
> >> other/branch-in-other Z
> >> other/main-in-other Z
> >> EOF
> >> '
> >
> > So, this demonstrates that '-t' behaves the same way as '--track'.
>
> --
> Todd
>
^ permalink raw reply
* [bug] git clone command leaves orphaned ssh process
From: Max Amelchenko @ 2023-09-10 6:38 UTC (permalink / raw)
To: git
What did you do before the bug happened? (Steps to reproduce your issue)
Run the command:
ps aux
Observe no ssh processes running on system.
Run git clone against a non-existent hostname:
git clone -v --depth=1 -b 3.23.66
ssh://*****@*****lab-prod.server.sim.cloud/terraform/modules/aws-eks
/tmp/dest
Observe the command fails with:
Could not resolve hostname *****lab-prod.server.sim.cloud: Name or
service not known
Run:
ps aux
Observe a defunct ssh process is left behind.
What did you expect to happen? (Expected behavior)
I expected the command to quit without leaving any processes behind.
What happened instead? (Actual behavior)
The command quit and left a defunct ssh process on the system.
What's different between what you expected and what actually happened?
I don't want zombie processes left after any git command (either failed or not).
Anything else you want to add:
These processes are zombie orphaned, meaning we're stuck with them
until system reboot (which is bad).
Please review the rest of the bug report below.
You can delete any lines you don't wish to share.
[System Info]
git version:
git version 2.40.1
cpu: aarch64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
compiler info: gnuc: 7.3
libc info: glibc: 2.26
$SHELL (typically, interactive shell): <unset>
[Enabled Hooks]
not run from a git repository - no hooks to show
^ permalink raw reply
* Re: [PATCH v4 11/15] replay: use standard revision ranges
From: Linus Arver @ 2023-09-10 3:20 UTC (permalink / raw)
To: Christian Couder, git
Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
Toon Claes, Christian Couder
In-Reply-To: <owlyo7icl1g3.fsf@fine.c.googlers.com>
Linus Arver <linusa@google.com> writes:
>> +update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH}
>> +update refs/heads/branch2 ${NEW_branch2_HASH} ${OLD_branch2_HASH}
>> +update refs/heads/branch3 ${NEW_branch3_HASH} ${OLD_branch3_HASH}
>> +------------
>> +
>> +This will simultaneously rebase branch1, branch2, and branch3 -- all
>> +commits they have since base, playing them on top of origin/main.
>
> How about
>
> This will rebase the commits in `branch1`, `branch2`, and `branch3`
> (excluding those in `base`), preplaying them on top of `origin/main`.
Oops, I meant "replaying" not "preplaying". But also, perhaps the
following is simpler?
This will replay the commits in `branch1`, `branch2`, and `branch3`
(excluding those in `base`), on top of `origin/main`.
^ permalink raw reply
* Re: [PATCH] completion: improve doc for complex aliases
From: Eric Sunshine @ 2023-09-10 2:02 UTC (permalink / raw)
To: Philippe Blain via GitGitGadget; +Cc: git, Steffen Prohaska, Philippe Blain
In-Reply-To: <pull.1585.git.1694274592854.gitgitgadget@gmail.com>
On Sat, Sep 9, 2023 at 12:25 PM Philippe Blain via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> The completion code can be told to use a particular completion for
> aliases that shell out by using ': git <cmd> ;' as the first command of
> the alias. This only works if <cmd> and the semicolon are separated by a
> space. The examples have that space but it's not clear if it's just for
> style or if it's mandatory.
>
> Explicitely mention it.
s/Explicitely/Explicitly/
> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
^ permalink raw reply
* Re: [PATCH 1/2] test-lib: prevent misuses of --invert-exit-code
From: Eric Sunshine @ 2023-09-10 1:59 UTC (permalink / raw)
To: Rubén Justo; +Cc: Git List, Ævar Arnfjörð Bjarmason
In-Reply-To: <1a60a1ca-0ef0-ecf5-d0aa-a28d7c148a82@gmail.com>
On Sat, Sep 9, 2023 at 7:08 PM Rubén Justo <rjusto@gmail.com> wrote:
> GIT_TEST_PASSING_SANITIZE_LEAK=true and GIT_TEST_SANITIZE_LEAK_LOG=true
> use internnlly the --invert-exit-code machinery. Therefore if the user
s/internnlly/internally/
> wants to use --invert-exit-code in combination with them, the result
> will be confusing.
>
> For the same reason, we are already using BAIL_OUT if the user tries to
> combine GIT_TEST_PASSING_SANITIZE_LEAK=check with --invert-exit-code.
>
> Let's do the same for GIT_TEST_PASSING_SANITIZE_LEAK=true and
> GIT_TEST_SANITIZE_LEAK_LOG=true.
>
> Signed-off-by: Rubén Justo <rjusto@gmail.com>
^ permalink raw reply
* [PATCH 2/2] test-lib: fix GIT_TEST_SANITIZE_LEAK_LOG
From: Rubén Justo @ 2023-09-09 23:09 UTC (permalink / raw)
To: Git List; +Cc: Ævar Arnfjörð Bjarmason
In-Reply-To: <68522960-edda-26d3-ddca-cee63f2d859e@gmail.com>
GIT_TEST_SANITIZE_LEAK_LOG=true with a test that leaks, will make the
test return zero unintentionally:
$ git checkout v2.40.1
$ make SANITIZE=leak
$ make -C t GIT_TEST_SANITIZE_LEAK_LOG=true t3200-branch.sh
...
With GIT_TEST_SANITIZE_LEAK_LOG=true our logs revealed a memory leak, exit non-zero!
# faked up failures as TODO & now exiting with 0 due to --invert-exit-code
Let's use invert_exit_code only if needed.
Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
t/test-lib.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 88a34fba67..87cfea9e9a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1263,7 +1263,8 @@ check_test_results_san_file_ () {
then
say "As TEST_PASSES_SANITIZE_LEAK=true isn't set the above leak is 'ok' with GIT_TEST_PASSING_SANITIZE_LEAK=check" &&
invert_exit_code=t
- else
+ elif test "$test_failure" = 0
+ then
say "With GIT_TEST_SANITIZE_LEAK_LOG=true our logs revealed a memory leak, exit non-zero!" &&
invert_exit_code=t
fi
--
2.40.1
^ permalink raw reply related
* [PATCH 1/2] test-lib: prevent misuses of --invert-exit-code
From: Rubén Justo @ 2023-09-09 23:08 UTC (permalink / raw)
To: Git List; +Cc: Ævar Arnfjörð Bjarmason
In-Reply-To: <68522960-edda-26d3-ddca-cee63f2d859e@gmail.com>
GIT_TEST_PASSING_SANITIZE_LEAK=true and GIT_TEST_SANITIZE_LEAK_LOG=true
use internnlly the --invert-exit-code machinery. Therefore if the user
wants to use --invert-exit-code in combination with them, the result
will be confusing.
For the same reason, we are already using BAIL_OUT if the user tries to
combine GIT_TEST_PASSING_SANITIZE_LEAK=check with --invert-exit-code.
Let's do the same for GIT_TEST_PASSING_SANITIZE_LEAK=true and
GIT_TEST_SANITIZE_LEAK_LOG=true.
Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
t/test-lib.sh | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 293caf0f20..88a34fba67 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1557,15 +1557,25 @@ then
say "in GIT_TEST_PASSING_SANITIZE_LEAK=check mode, setting --invert-exit-code for TEST_PASSES_SANITIZE_LEAK != true"
invert_exit_code=t
fi
- elif test -z "$passes_sanitize_leak" &&
- test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false
+ elif test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false
then
- skip_all="skipping $this_test under GIT_TEST_PASSING_SANITIZE_LEAK=true"
- test_done
+ if test -n "$invert_exit_code"
+ then
+ BAIL_OUT "cannot use --invert-exit-code under GIT_TEST_PASSING_SANITIZE_LEAK=true"
+ elif test -z "$passes_sanitize_leak"
+ then
+ skip_all="skipping $this_test under GIT_TEST_PASSING_SANITIZE_LEAK=true"
+ test_done
+ fi
fi
if test_bool_env GIT_TEST_SANITIZE_LEAK_LOG false
then
+ if test -n "$invert_exit_code"
+ then
+ BAIL_OUT "cannot use --invert-exit-code and GIT_TEST_SANITIZE_LEAK_LOG=true"
+ fi
+
if ! mkdir -p "$TEST_RESULTS_SAN_DIR"
then
BAIL_OUT "cannot create $TEST_RESULTS_SAN_DIR"
--
2.40.1
^ permalink raw reply related
* [PATCH 0/2] fix GIT_TEST_SANITIZE_LEAK_LOG=true
From: Rubén Justo @ 2023-09-09 23:03 UTC (permalink / raw)
To: Git List; +Cc: Ævar Arnfjörð Bjarmason
While using "make test" I noticed that invert_exit_code used by
GIT_TEST_SANITIZE_LEAK_LOG=true produces unexpected results.
$ git checkout v2.40.1
$ make test SANITIZE=leak T=t3200-branch.sh # fails
$ make test SANITIZE=leak GIT_TEST_SANITIZE_LEAK_LOG=true T=t3200-branch.sh # succeeds
Rubén Justo (2):
test-lib: prevent misuses of --invert-exit-code
test-lib: fix GIT_TEST_SANITIZE_LEAK_LOG
t/test-lib.sh | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
--
2.40.1
^ permalink raw reply
* Bug: git submodule set-url does not handle name != path correctly
From: Jan Alexander Steffens (heftig) @ 2023-09-09 22:26 UTC (permalink / raw)
To: git
In-Reply-To: <cafa28e0bfe1f603204a74f619ac713518989a40.camel@archlinux.org>
On Mon, 2023-07-24 at 06:21 +0200, Jan Alexander Steffens (heftig)
wrote:
> Hi,
>
> I've encountered this problem with the flatpak-builder repository:
>
> https://github.com/flatpak/flatpak-builder/blob/main/.gitmodules
>
> [submodule "libglnx"]
> path = subprojects/libglnx
> url = https://gitlab.gnome.org/GNOME/libglnx.git
> [submodule "debugedit"]
> path = subprojects/debugedit
> url = https://sourceware.org/git/debugedit.git
>
> After 'git submodule init', using 'git submodule set-url libglnx foo'
> successfully modifies .gitmodules but does not touch .git/config.
> However, a subsequent 'git submodule sync' does sync the modified url
> to the local config.
>
> I've investigated a bit and it seems 'git submodule set-url' calls
> sync_submodule with "libglnx" as the path, which does not work, while
> 'git submodule sync' calls it with "subprojects/libglnx" as the path,
> which does work.
>
> Greetings,
> Jan
Friendly bump.
The docs say set-url needs the path to the submodule, not its name, so
it should be 'git submodule set-url subprojects/libglnx foo'.
However, using that actually creates a new
'submodule.subprojects/libglnx.url' option instead of modifying the
existing 'submodule.libglnx.url'.
It looks like set-url needs to translate the path to the name for
modifying .gitmodules but does not do so.
'git submodule set-branch' is also affected by this.
^ permalink raw reply
* [PATCH] diff --no-index: fix -R with stdin
From: René Scharfe @ 2023-09-09 22:12 UTC (permalink / raw)
To: Martin Storsjö, git; +Cc: Phillip Wood, Junio C Hamano
In-Reply-To: <d42579a0-f438-9b4c-97e4-58724dbe4a4@martin.st>
When -R is given, queue_diff() swaps the mode and name variables of the
two files to produce a reverse diff. 1e3f26542a (diff --no-index:
support reading from named pipes, 2023-07-05) added variables that
indicate whether files are special, i.e named pipes or - for stdin.
These new variables were not swapped, though, which broke the handling
of stdin with with -R. Swap them like the other metadata variables.
Reported-by: Martin Storsjö <martin@martin.st>
Signed-off-by: René Scharfe <l.s.r@web.de>
---
Great bug report, thank you!
diff-no-index.c | 1 +
t/t4053-diff-no-index.sh | 19 +++++++++++++++++++
2 files changed, 20 insertions(+)
diff --git a/diff-no-index.c b/diff-no-index.c
index 8aead3e332..e7041b89e3 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -232,6 +232,7 @@ static int queue_diff(struct diff_options *o,
if (o->flags.reverse_diff) {
SWAP(mode1, mode2);
SWAP(name1, name2);
+ SWAP(special1, special2);
}
d1 = noindex_filespec(name1, mode1, special1);
diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
index 6781cc9078..5f059f65fc 100755
--- a/t/t4053-diff-no-index.sh
+++ b/t/t4053-diff-no-index.sh
@@ -224,6 +224,25 @@ test_expect_success "diff --no-index treats '-' as stdin" '
test_must_be_empty actual
'
+test_expect_success "diff --no-index -R treats '-' as stdin" '
+ cat >expect <<-EOF &&
+ diff --git b/a/1 a/-
+ index $(git hash-object --stdin <a/1)..$ZERO_OID 100644
+ --- b/a/1
+ +++ a/-
+ @@ -1 +1 @@
+ -1
+ +x
+ EOF
+
+ test_write_lines x | test_expect_code 1 \
+ git -c core.abbrev=no diff --no-index -R -- - a/1 >actual &&
+ test_cmp expect actual &&
+
+ test_write_lines 1 | git diff --no-index -R -- a/1 - >actual &&
+ test_must_be_empty actual
+'
+
test_expect_success 'diff --no-index refuses to diff stdin and a directory' '
test_must_fail git diff --no-index -- - a </dev/null 2>err &&
grep "fatal: cannot compare stdin to a directory" err
--
2.42.0
^ permalink raw reply related
* Re: [PATCH] grep: use OPT_INTEGER_F for --max-depth
From: René Scharfe @ 2023-09-09 21:14 UTC (permalink / raw)
To: Jeff King; +Cc: Git List
In-Reply-To: <cec47733-5b15-6ca7-adaf-7f3216ad178b@web.de>
Am 09.09.23 um 00:28 schrieb René Scharfe:
> Am 07.09.23 um 22:40 schrieb Jeff King:
>
> I was thinking more about something like the solutions discussed at
> https://stackoverflow.com/questions/47674663/variable-arguments-inside-a-macro.
> It allows selecting variants based on argument count.
>
> That could look like this (untested except on https://godbolt.org/; the
> EVALs are needed for MSVC for some reason):
>
> #define OPT_INTEGER_FULL(s, l, v, h, f, d) { \
> .type = OPTION_INTEGER, \
> .short_name = (s), \
> .long_name = (l), \
> .value = (v), \
> .argh = N_("n"), \
> .help = (h), \
> .flags = (f), \
> .defval = (d), \
> }
> #define OPT_INTEGER_4(s, l, v, h) \
> OPT_INTEGER_FULL(s, l, v, h, 0, 0)
> #define OPT_INTEGER_5(s, l, v, h, f) \
> OPT_INTEGER_FULL(s, l, v, h, f, 0)
> #define OPT_INTEGER_6(s, l, v, h, f, d) \
> OPT_INTEGER_FULL(s, l, v, h, (f) | PARSE_OPT_OPTARG, d)
> #define EVAL(x) x
> #define SEVENTH(_1, _2, _3, _4, _5, _6, x, ...) x
> #define OPT_INTEGER(...) \
> EVAL(EVAL(SEVENTH(__VA_ARGS__, OPT_INTEGER_6, OPT_INTEGER_5, OPT_INTEGER_4, 0))(__VA_ARGS__))
>
> So OPT_INTEGER(s, l, v, h) would be the same as before. Add an argument
> and it becomes current OPT_INTEGER_F, add another one and it acts as
> your _OPTARG_F variant.
Should we actually do something like that? Probably not. At least it
doesn't help with my goals of simplicity and safety. (I get sidetracked
so easily..)
>> So yeah, I think you'd really want a separate macro. The combinations
>> start to add up (or multiply up, if you prefer ;) ). They _could_ be
>> generated mechanically, I think, as they can all be implemented in terms
>> of a master macro that knows about all features:
>>
>> #define OPT_BOOL_F(s, l, v, h, f) OPT_BOOL_ALL(s, l, v, h, f, 0)
>> #define OPT_BOOL(s, l, v, h, f) OPT_BOOL_F(s, l, v, h, 0)
>
> The "f" arg needs to go...
>
>> #define OPT_BOOL_OPTARG_F(s, l, v, h, arg) OPT_BOOL_ALL(s, l, v, h, f | PARSE_OPT_OPTARG, arg)
>
> ... here, possibly.
>
>> #define OPT_BOOL_OPTARG(s, l, v, h, arg) OPT_BOOL_OPTARG_F(s, l, v, h, 0, arg)
Or we could use designated initializers directly. It would improve
readability at the cost of some verbosity. We could make it a bit
less verbose by by setting some flags implicitly based on type (e.g.
set PARSE_OPT_OPTARG if defval is set for an OPTION_INTEGER option).
René
^ permalink raw reply
* [PATCH 2/2] parse-options: use and require int pointer for OPT_CMDMODE
From: René Scharfe @ 2023-09-09 21:14 UTC (permalink / raw)
To: Git List; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <2d6f3d74-687a-2d40-5c0c-abc396aef80f@web.de>
Some uses of OPT_CMDMODE provide a pointer to an enum. It is
dereferenced as an int pointer in parse-options.c::get_value(). These
two types are incompatible, though -- the storage size of an enum can
vary between platforms. C23 would allow us to specify the underlying
type of the different enums, making them compatible, but with C99 the
easiest safe option is to actually use int as the value type.
Convert the offending OPT_CMDMODE users and use the typed value_int
point in the macro's definition to enforce that type for future ones.
Signed-off-by: René Scharfe <l.s.r@web.de>
---
builtin/am.c | 2 +-
builtin/help.c | 5 +++--
builtin/ls-tree.c | 2 +-
builtin/rebase.c | 2 +-
builtin/replace.c | 3 ++-
builtin/stripspace.c | 2 +-
parse-options.h | 2 +-
7 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index 202040b62e..ebb72ebaaa 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2269,7 +2269,7 @@ enum resume_type {
};
struct resume_mode {
- enum resume_type mode;
+ int mode;
enum show_patch_type sub_mode;
};
diff --git a/builtin/help.c b/builtin/help.c
index dc1fbe2b98..e8aedb932c 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -42,7 +42,7 @@ enum show_config_type {
SHOW_CONFIG_SECTIONS,
};
-static enum help_action {
+enum help_action {
HELP_ACTION_ALL = 1,
HELP_ACTION_GUIDES,
HELP_ACTION_CONFIG,
@@ -50,7 +50,8 @@ static enum help_action {
HELP_ACTION_DEVELOPER_INTERFACES,
HELP_ACTION_CONFIG_FOR_COMPLETION,
HELP_ACTION_CONFIG_SECTIONS_FOR_COMPLETION,
-} cmd_mode;
+};
+static int cmd_mode;
static const char *html_path;
static int verbose = 1;
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 209d2dc0d5..6f8c43f729 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -346,7 +346,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
int i, full_tree = 0;
int full_name = !prefix || !*prefix;
read_tree_fn_t fn = NULL;
- enum ls_tree_cmdmode cmdmode = MODE_DEFAULT;
+ int cmdmode = MODE_DEFAULT;
int null_termination = 0;
struct ls_tree_options options = { 0 };
const struct option ls_tree_options[] = {
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 50cb85751f..d11e749579 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -111,7 +111,7 @@ struct rebase_options {
REBASE_INTERACTIVE_EXPLICIT = 1<<4,
} flags;
struct strvec git_am_opts;
- enum action action;
+ int action;
char *reflog_action;
int signoff;
int allow_rerere_autoupdate;
diff --git a/builtin/replace.c b/builtin/replace.c
index da59600ad2..d0063d3feb 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -553,7 +553,8 @@ int cmd_replace(int argc, const char **argv, const char *prefix)
MODE_GRAFT,
MODE_CONVERT_GRAFT_FILE,
MODE_REPLACE
- } cmdmode = MODE_UNSPECIFIED;
+ };
+ int cmdmode = MODE_UNSPECIFIED;
struct option options[] = {
OPT_CMDMODE('l', "list", &cmdmode, N_("list replace refs"), MODE_LIST),
OPT_CMDMODE('d', "delete", &cmdmode, N_("delete replace refs"), MODE_DELETE),
diff --git a/builtin/stripspace.c b/builtin/stripspace.c
index 7b700a9fb1..f6de0b17dc 100644
--- a/builtin/stripspace.c
+++ b/builtin/stripspace.c
@@ -32,7 +32,7 @@ enum stripspace_mode {
int cmd_stripspace(int argc, const char **argv, const char *prefix)
{
struct strbuf buf = STRBUF_INIT;
- enum stripspace_mode mode = STRIP_DEFAULT;
+ int mode = STRIP_DEFAULT;
int nongit;
const struct option options[] = {
diff --git a/parse-options.h b/parse-options.h
index 5e7475bd2d..349c3fca04 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -262,7 +262,7 @@ struct option {
.type = OPTION_SET_INT, \
.short_name = (s), \
.long_name = (l), \
- .value = (v), \
+ .value_int = (v), \
.help = (h), \
.flags = PARSE_OPT_CMDMODE|PARSE_OPT_NOARG|PARSE_OPT_NONEG | (f), \
.defval = (i), \
--
2.42.0
^ permalink raw reply related
* [PATCH 1/2] parse-options: add int value pointer to struct option
From: René Scharfe @ 2023-09-09 21:10 UTC (permalink / raw)
To: Git List; +Cc: Jeff King, Junio C Hamano
Add an int pointer, value_int, to struct option to provide a typed value
pointer for the various integer options. It allows type checks at
compile time, which is not possible with the void pointer, value. Its
use is optional for now.
Signed-off-by: René Scharfe <l.s.r@web.de>
---
parse-options.c | 34 +++++++++++++++++++---------------
parse-options.h | 2 ++
2 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index e8e076c3a6..2552745804 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -82,10 +82,11 @@ static enum parse_opt_result opt_command_mode_error(
* already, and report that this is not compatible with it.
*/
for (that = all_opts; that->type != OPTION_END; that++) {
+ int *value_int = opt->value_int ? opt->value_int : opt->value;
if (that == opt ||
!(that->flags & PARSE_OPT_CMDMODE) ||
that->value != opt->value ||
- that->defval != *(int *)opt->value)
+ that->defval != *value_int)
continue;
if (that->long_name)
@@ -109,6 +110,7 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
const char *s, *arg;
const int unset = flags & OPT_UNSET;
int err;
+ int *value_int = opt->value_int ? opt->value_int : opt->value;
if (unset && p->opt)
return error(_("%s takes no value"), optname(opt, flags));
@@ -122,7 +124,7 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
* is not a grave error, so let it pass.
*/
if ((opt->flags & PARSE_OPT_CMDMODE) &&
- *(int *)opt->value && *(int *)opt->value != opt->defval)
+ *value_int && *value_int != opt->defval)
return opt_command_mode_error(opt, all_opts, flags);
switch (opt->type) {
@@ -131,33 +133,33 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
case OPTION_BIT:
if (unset)
- *(int *)opt->value &= ~opt->defval;
+ *value_int &= ~opt->defval;
else
- *(int *)opt->value |= opt->defval;
+ *value_int |= opt->defval;
return 0;
case OPTION_NEGBIT:
if (unset)
- *(int *)opt->value |= opt->defval;
+ *value_int |= opt->defval;
else
- *(int *)opt->value &= ~opt->defval;
+ *value_int &= ~opt->defval;
return 0;
case OPTION_BITOP:
if (unset)
BUG("BITOP can't have unset form");
- *(int *)opt->value &= ~opt->extra;
- *(int *)opt->value |= opt->defval;
+ *value_int &= ~opt->extra;
+ *value_int |= opt->defval;
return 0;
case OPTION_COUNTUP:
- if (*(int *)opt->value < 0)
- *(int *)opt->value = 0;
- *(int *)opt->value = unset ? 0 : *(int *)opt->value + 1;
+ if (*value_int < 0)
+ *value_int = 0;
+ *value_int = unset ? 0 : *value_int + 1;
return 0;
case OPTION_SET_INT:
- *(int *)opt->value = unset ? 0 : opt->defval;
+ *value_int = unset ? 0 : opt->defval;
return 0;
case OPTION_STRING:
@@ -206,11 +208,11 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
}
case OPTION_INTEGER:
if (unset) {
- *(int *)opt->value = 0;
+ *value_int = 0;
return 0;
}
if (opt->flags & PARSE_OPT_OPTARG && !p->opt) {
- *(int *)opt->value = opt->defval;
+ *value_int = opt->defval;
return 0;
}
if (get_arg(p, opt, flags, &arg))
@@ -218,7 +220,7 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
if (!*arg)
return error(_("%s expects a numerical value"),
optname(opt, flags));
- *(int *)opt->value = strtol(arg, (char **)&s, 10);
+ *value_int = strtol(arg, (char **)&s, 10);
if (*s)
return error(_("%s expects a numerical value"),
optname(opt, flags));
@@ -483,6 +485,8 @@ static void parse_options_check(const struct option *opts)
if (opts->type == OPTION_SET_INT && !opts->defval &&
opts->long_name && !(opts->flags & PARSE_OPT_NONEG))
optbug(opts, "OPTION_SET_INT 0 should not be negatable");
+ if (opts->value && opts->value_int)
+ optbug(opts, "only a single value type supported");
switch (opts->type) {
case OPTION_COUNTUP:
case OPTION_BIT:
diff --git a/parse-options.h b/parse-options.h
index 57a7fe9d91..5e7475bd2d 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -158,6 +158,8 @@ struct option {
parse_opt_ll_cb *ll_callback;
intptr_t extra;
parse_opt_subcommand_fn *subcommand_fn;
+
+ int *value_int;
};
#define OPT_BIT_F(s, l, v, h, b, f) { \
--
2.42.0
^ permalink raw reply related
* Regression in git diff with stdin, with -R
From: Martin Storsjö @ 2023-09-09 20:42 UTC (permalink / raw)
To: git; +Cc: Phillip Wood
Hi,
Since 1e3f26542a6ecd3006c2c0d5ccc0bae4a700f7e5, "diff --no-index: support
reading from named pipes", one usecase about diffing with stdin has
broken.
I see that this patch was preceded by adding some extra tests around
diffing with stdin - but one case seem to have been missed.
"git diff --no-index - regularfile" still works fine as it did before,
also "git diff --no-index regularfile -" also still works. (I.e. stdin can
either be the first or second file argument - both work.)
However if using the -R option to reverse the diff direction, i.e. "git
diff --no-index -R - regularfile" or "git diff --no-index -R regularfile
-", I'm now getting the following error:
fatal: stat '-': No such file or directory
// Martin
^ permalink raw reply
* [PATCH v2] status: fix branch shown when not only bisecting
From: Rubén Justo @ 2023-09-09 20:12 UTC (permalink / raw)
To: Git List, Junio C Hamano
In-Reply-To: <48745298-f12b-8efb-4e48-90d2c22a8349@gmail.com>
In 83c750acde (wt-status.*: better advice for git status added,
2012-06-05), git-status received new informative messages to describe
the ongoing work in a worktree.
These messages were enhanced in 0722c805d6 (status: show the branch name
if possible in in-progress info, 2013-02-03), to show, if possible, the
branch where the operation was initiated.
Since then, we show incorrect information when several operations are in
progress and one of them is bisect:
$ git checkout -b foo
$ GIT_SEQUENCE_EDITOR='echo break >' git rebase -i HEAD~
$ git checkout -b bar
$ git bisect start
$ git status
...
You are currently editing a commit while rebasing branch 'bar' on '...'.
You are currently bisecting, started from branch 'bar'.
...
Note that we erroneously say "while rebasing branch 'bar'" when we
should be referring to "foo".
This must have gone unnoticed for so long because it must be unusual to
start a bisection while another operation is in progress. And even less
usual to involve different branches.
It caught my attention reviewing a leak introduced in 8b87cfd000
(wt-status: move strbuf into read_and_strip_branch(), 2013-03-16).
A simple change to deal with this situation can be to record in struct
wt_status_state, the branch where the bisect starts separately from the
branch related to other operations.
Let's do it and so we'll be able to display correct information and
we'll avoid the leak as well.
Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
Let's try again.
branch.c | 4 ++--
ref-filter.c | 2 +-
t/t7512-status-help.sh | 28 ++++++++++++++++++++++++++++
worktree.c | 4 ++--
wt-status.c | 7 ++++---
wt-status.h | 1 +
6 files changed, 38 insertions(+), 8 deletions(-)
diff --git a/branch.c b/branch.c
index 06f7af9dd4..534594f7f8 100644
--- a/branch.c
+++ b/branch.c
@@ -420,9 +420,9 @@ static void prepare_checked_out_branches(void)
wt_status_state_free_buffers(&state);
if (wt_status_check_bisect(wt, &state) &&
- state.branch) {
+ state.bisecting_from) {
struct strbuf ref = STRBUF_INIT;
- strbuf_addf(&ref, "refs/heads/%s", state.branch);
+ strbuf_addf(&ref, "refs/heads/%s", state.bisecting_from);
old = strmap_put(¤t_checked_out_branches,
ref.buf,
xstrdup(wt->path));
diff --git a/ref-filter.c b/ref-filter.c
index 1bfaf20fbf..7fd5548e93 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2124,7 +2124,7 @@ char *get_head_description(void)
state.detached_from);
} else if (state.bisect_in_progress)
strbuf_addf(&desc, _("(no branch, bisect started on %s)"),
- state.branch);
+ state.bisecting_from);
else if (state.detached_from) {
if (state.detached_at)
strbuf_addf(&desc, _("(HEAD detached at %s)"),
diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index c2ab8a444a..802f8f704c 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -692,6 +692,34 @@ EOF
'
+test_expect_success 'status when bisecting while rebasing' '
+ git reset --hard main &&
+ test_when_finished "git rebase --abort" &&
+ ONTO=$(git rev-parse --short HEAD^) &&
+ FAKE_LINES="break" git rebase -i HEAD^ &&
+ test_when_finished "git checkout -" &&
+ git checkout -b bisect_while_rebasing &&
+ test_when_finished "git bisect reset" &&
+ git bisect start &&
+ cat >expected <<EOF &&
+On branch bisect_while_rebasing
+Last command done (1 command done):
+ break
+No commands remaining.
+You are currently editing a commit while rebasing branch '\''bisect'\'' on '\''$ONTO'\''.
+ (use "git commit --amend" to amend the current commit)
+ (use "git rebase --continue" once you are satisfied with your changes)
+
+You are currently bisecting, started from branch '\''bisect_while_rebasing'\''.
+ (use "git bisect reset" to get back to the original branch)
+
+nothing to commit (use -u to show untracked files)
+EOF
+ git status --untracked-files=no >actual &&
+ test_cmp expected actual
+'
+
+
test_expect_success 'status when rebase --apply conflicts with statushints disabled' '
git reset --hard main &&
git checkout -b statushints_disabled &&
diff --git a/worktree.c b/worktree.c
index b8cf29e6a1..360e2b1866 100644
--- a/worktree.c
+++ b/worktree.c
@@ -395,9 +395,9 @@ int is_worktree_being_bisected(const struct worktree *wt,
memset(&state, 0, sizeof(state));
found_bisect = wt_status_check_bisect(wt, &state) &&
- state.branch &&
+ state.bisecting_from &&
skip_prefix(target, "refs/heads/", &target) &&
- !strcmp(state.branch, target);
+ !strcmp(state.bisecting_from, target);
wt_status_state_free_buffers(&state);
return found_bisect;
}
diff --git a/wt-status.c b/wt-status.c
index d03dfab9e4..dec28e8124 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -861,6 +861,7 @@ void wt_status_state_free_buffers(struct wt_status_state *state)
FREE_AND_NULL(state->branch);
FREE_AND_NULL(state->onto);
FREE_AND_NULL(state->detached_from);
+ FREE_AND_NULL(state->bisecting_from);
}
static void wt_longstatus_print_unmerged(struct wt_status *s)
@@ -1569,10 +1570,10 @@ static void show_revert_in_progress(struct wt_status *s,
static void show_bisect_in_progress(struct wt_status *s,
const char *color)
{
- if (s->state.branch)
+ if (s->state.bisecting_from)
status_printf_ln(s, color,
_("You are currently bisecting, started from branch '%s'."),
- s->state.branch);
+ s->state.bisecting_from);
else
status_printf_ln(s, color,
_("You are currently bisecting."));
@@ -1733,7 +1734,7 @@ int wt_status_check_bisect(const struct worktree *wt,
if (!stat(worktree_git_path(wt, "BISECT_LOG"), &st)) {
state->bisect_in_progress = 1;
- state->branch = get_branch(wt, "BISECT_START");
+ state->bisecting_from = get_branch(wt, "BISECT_START");
return 1;
}
return 0;
diff --git a/wt-status.h b/wt-status.h
index ab9cc9d8f0..819dcad723 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -94,6 +94,7 @@ struct wt_status_state {
char *branch;
char *onto;
char *detached_from;
+ char *bisecting_from;
struct object_id detached_oid;
struct object_id revert_head_oid;
struct object_id cherry_pick_head_oid;
--
2.40.1
^ 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