* Re: What's cooking in git.git (Feb 2024, #02; Fri, 2)
From: Phillip Wood @ 2024-02-06 13:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Patrick Steinhardt, Karthik Nayak
In-Reply-To: <xmqqjznimrbe.fsf@gitster.g>
Hi Junio
On 05/02/2024 23:20, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
>
>> I'm concerned that the UI could use some improvement. If I understand
>> correctly the proposal is to make
>>
>> git for-each-ref
>>
>> and
>>
>> git for-each-ref ""
>>
>> behave differently so that the latter prints the pseudorefs from the
>> current worktree and the former does not.
>
> I would actually think that is perfectly sensible. The optional
> arguments for-each-ref name "filtering patterns" and you can view
> the behaviour of the command as using "refs/" as the default
> filtering pattern when nothing is given. But it is easy to defeat
> the unfortunate and historical default filtering pattern, by saying
> "we do not limit to any hierarchy, anything goes" by giving "" as
> the prefix.
There is a logic to that if one ignores
"refs/{main-worktree,worktrees/$worktree}/*" not being shown. As Patrick
has pointed out the change in the handling of the empty prefix here is a
breaking change though [1]. To me, the handling of the empty prefix
feels inconsistent with the rest of the pattern space. The patterns "*"
and "*_HEAD" don't match anything and there is no way to filter a subset
of pseudorefs. If all we want is a way to show all the refs including
pseudorefs in a worktree then I think an option to do that which errored
out if a pattern is given would be a better approach. I'd prefer an
option (say "--include-pseudorefs") that included pseudorefs in the list
of refs to be filtered and allowed the user to filter them as they
wanted. That way we could later add a "--all-worktrees" option that
included refs/{main-worktree,worktrees/$worktree} in the list of refs to
be filtered as well.
Best Wishes
Phillip
[1] https://lore.kernel.org/git/ZcHEmyvvMR_b_Idl@tanuki/
^ permalink raw reply
* Re: git-gui desktop launcher
From: Marc Branchaud @ 2024-02-06 13:57 UTC (permalink / raw)
To: Johannes Sixt, Tobias Boesch; +Cc: git
In-Reply-To: <993e6823-7fa7-4130-8c0a-69ed31da5fbe@kdbg.org>
On 2024-02-06 01:50, Johannes Sixt wrote:
>
>> Comment=A portable graphical interface to Git
>
> I have two gripes with this Comment:
>
> - That the program is portable is irrelevant for the user. The word need
> not occur in this Comment.
>
> - I had hoped for a more precise description. In particular, when a
> program is advertised as "graphical interface to Git", then I would
> expect that it can do a bit more than initialize repositories and make
> commits. At a minimum, I would expect a history viewer; but Git Gui
> doesn't have one. Unless you count the two "Visualize" entries in the
> "Repository" menu that invoke gitk as such. So, I dunno.
Perhaps
A graphical tool for creating Git commits
?
M.
^ permalink raw reply
* Re: [PATCH v3 4/4] for-each-ref: avoid filtering on empty pattern
From: Karthik Nayak @ 2024-02-06 15:30 UTC (permalink / raw)
To: phillip.wood; +Cc: git, gitster, ps
In-Reply-To: <92ba680d-0b48-49f0-aafc-f503e5a5e0ea@gmail.com>
Hello,
On Tue, Feb 6, 2024 at 2:55 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> Hi Karthik
>
> On 06/02/2024 08:52, Karthik Nayak wrote:
> > On Mon, Feb 5, 2024 at 7:48 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
> >> On 29/01/2024 11:35, Karthik Nayak wrote:
> >>> When the user uses an empty string pattern (""), we don't match any refs
> >>> in git-for-each-ref(1). This is because in git-for-each-ref(1), we use
> >>> path based matching and an empty string doesn't match any path.
> >>>
> >>> In this commit we change this behavior by making empty string pattern
> >>> match all references. This is done by introducing a new flag
> >>> `FILTER_REFS_NO_FILTER` in `ref-filter.c`, which uses the newly
> >>> introduced `refs_for_each_all_refs()` function to iterate over all the
> >>> refs in the repository.
> >>
> >> It actually iterates over all the refs in the current worktree, not all
> >> the refs in the repository. I have to say that I find it extremely
> >> unintuitive that "" behaves differently to not giving a pattern. I
> >> wonder if we can find a better UI here - perhaps a command line option
> >> to include pseudorefs?
> >>
> >
> > As Patrick mentioned, this was discussed a while ago and we decided to
> > move forward with the `git for-each-ref ""` syntax.
>
> Thanks I'd missed that discussion. I see that at the end of that
> discussion Junio was concerned that the proposed "" did not account for
> "refs/worktrees/$worktree/*" [1] - how has that been resolved?
>
The current implementation (merged to next) prints all the refs from the current
worktree and doesn't support printing from other worktrees.
$ git worktree add ../wt-files @~10
Preparing worktree (detached HEAD 559d5138bc)
HEAD is now at 559d5138bc Merge branch 'jc/make-libpath-template' into next
$ cd ../wt-files/
direnv: unloading
$ git for-each-ref "" | grep -v "refs"
559d5138bc8b81354fd1bc3421ace614076e66de commit HEAD
559d5138bc8b81354fd1bc3421ace614076e66de commit ORIG_HEAD
$ git rebase -i @~3
Stopped at be65f9ef88... t/Makefile: get UNIT_TESTS list from C sources
You can amend the commit now, with
git commit --amend '-S'
Once you are satisfied with your changes, run
git rebase --continue
$ git for-each-ref "" | grep -v "refs"
be65f9ef88ff741454dcf10a0af6e384d0bf26cf commit HEAD
43f081b9c7dfb9c23e547ffee1778af0f30c5c4e commit ORIG_HEAD
be65f9ef88ff741454dcf10a0af6e384d0bf26cf commit REBASE_HEAD
^ permalink raw reply
* Bug: git submodule update doesn't give a prompt to add pubkey of remote repo host, and failed because of it.
From: DingJunyao @ 2024-02-06 16:07 UTC (permalink / raw)
To: git@vger.kernel.org
Git version: git version 2.43.0.windows.1
OS: Windows 11 22H2 (22621.3007)
(I don't know whether it's a Windows-specific bug)
I cloned a project from one remote repo (A), and the project has a submodule from the other remote repo (B). Both of them are connected to the remote repo of which by SSH, and I added my pubkey to both of the repo providers.
I executed the command in project directory:
git submodule init
git submodule update
When updating, the following error happened:
Cloning into '/path/to/submodule'...
Host key verification failed.
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
fatal: clone of 'user@host:repo/path.git' into submodule path '/path/to/submodule' failed
Failed to clone 'themes/anzhiyu-ding-mod'. Retry scheduled
Cloning into '/path/to/submodule'...
Host key verification failed.
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
fatal: clone of 'user@host:repo/path.git' into submodule path '/path/to/submodule' failed
Failed to clone 'path/to/submodule' a second time, aborting
I cloned another repo from B. When I did it, I found I hadn't added the pubkey of repo B to the list of known hosts before.
The authenticity of host 'host (XXX.XXX.XXX.XXX)' can't be established.
ED25519 key fingerprint is SHA256:XXXXXXXXXXXXX.
This key is not known by any other names.
Are you sure you want to continue connecting (yes/no/[fingerprint])? yes
Warning: Permanently added 'host' (ED25519) to the list of known hosts.
After that, I tried to execute `git submodule update` in the previous project and succeeded.
I think when I update submodule and I haven't added the key to the submodule remote repo, it needs to give a prompt to add the public key, just like cloning.
^ permalink raw reply
* Wrong exit code on failed SSH signing
From: Sergey Kosukhin @ 2024-02-06 16:24 UTC (permalink / raw)
To: git
Hello!
There seems to be a bug in the sign_buffer_ssh function in
gpg-interface.c: a possible exit code of ssh-keygen is 255, which is
returned as-is by sign_buffer_ssh. The problem is that, for example,
the function build_tag_object in builtin/tag.c considers only negative
values as a failure. Since 255 >= 0, the error message "unable to sign
the tag" is not emitted and git exits normally with zero exit code. It
might be enough to return -1 in sign_buffer_ssh if ret is not zero.
I am sorry if this has already been reported or taken care of. Thank you.
Best regards,
Sergey
^ permalink raw reply
* Re: [PATCH v3 4/4] for-each-ref: avoid filtering on empty pattern
From: Junio C Hamano @ 2024-02-06 17:03 UTC (permalink / raw)
To: Phillip Wood; +Cc: Karthik Nayak, phillip.wood, git, ps
In-Reply-To: <92ba680d-0b48-49f0-aafc-f503e5a5e0ea@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
> Thanks I'd missed that discussion. I see that at the end of that
> discussion Junio was concerned that the proposed "" did not account
> for "refs/worktrees/$worktree/*" [1] - how has that been resolved?
Ah, that is an excellent point.
If we plan to never allow showing refs/worktrees/ hierarchy, then
the "there is a default pattern, refs/, that gets used when there is
no user-specified patterns" model would be sufficient to allow
showing things that are directly beneath $GIT_DIR and are out of
refs/ hierarchy, but that does not explain why refs/worktrees/ is
omitted. I'll envision a design for a longer term later, but an
easy way out would be to add --include-worktree-refs option for
that, and at that point, adding --include-root-refs option for things
outside the refs/ hierarchy may become a lot more natural solution.
In the longer term, I suspect that we would want something similar
to the negative refspec magic (e.g., "git log ':!Documentation/'"
that shows things outside the named hierarchy) exposed to the API[*],
so that we can say
$ git for-each-ref --format=... \
refs/ !refs/heads/ !refs/tags/ !refs/remotes/
to show things under refs/ excluding the commonly used hierarchies,
and at that point, the current behaviour for "no limit" case can
again become explainable as having "refs/ !refs/worktrees/" as the
default. It still does not explain why "git for-each-ref refs/"
omits the refs/worktrees/ hierchy, unless the default limit pattern
rule were something like "unless you give a positive limit pattern
rule, then we use 'refs/' by default, and unless you give a negative
limit pattern rule, then we use '!refs/worktrees/' by default".
It then gives an easy explanation on the traditional behaviour, with
"" used for "including stuff outside refs/", and is more flexible.
The use of dashed-options to include hierachies that are by default
excluded (e.g. "--include-root-refs" and "--include-worktree-refs")
feels limiting, but should be sufficient for our needs, both current
(i.e. I want to see HEAD and FETCH_HEAD) and in the immediate future
(i.e. I want to see worktree refs from that worktree), and I can buy
that as a good alternative solution, at least in the shorter term.
I still worry that it may make introducing the negative ref patterns
harder, though. How does --include-worktree-refs=another to include
the worktree refs from another worktree in refs/worktrees/another
interact with a negative pattern that was given from the command
line that overlaps with it? Whatever interaction rules we define,
can we easily explain it in the documentation?
Just like "an empty string tells Git to include everything" is a
perfectly reasonable approach if we plan to never allow
refs/worktrees/ hierarchy, "dashed-options for selected hierarchies"
is a perfectly reasonable approach if we plan to never allow
negative limit patterns, I suspect. We should stop complexity at
some point, and the decision to never support negative limit
patterns might be the place to draw that line. I dunno.
[Footnote]
* Such an exclusion mechanism already exists and are used to hide
certain refs from being seen over the network by "git fetch" and
friends. I do not think it is plugged to the machinery used by
for-each-ref and friends, but it smells like a reasonably easy
thing to do.
^ permalink raw reply
* Re: [PATCH 1/4] notes: print note blob to stdout directly
From: Junio C Hamano @ 2024-02-06 17:52 UTC (permalink / raw)
To: Maarten Bosmans; +Cc: git, Teng Long, Maarten Bosmans
In-Reply-To: <CA+CvcKTtcHCCKucQ0h1dnaDAMNfErJ+a1CXEVi=ZE5dv57Tb3A@mail.gmail.com>
Maarten Bosmans <mkbosmans@gmail.com> writes:
>> I am not sure if we want to accept an approach that feels somewhat
>> narrow/short sighted, like this patch. When "git show" learns an
>> improved way to show blob objects, who will update the code this
>> patch touches to teach it to use the same improved way to show the
>> notes?
Another thing I forgot to mention was that I suspected the use of
"git show" was so that we can deal with notes trees whose leaves are
*not* blob objects. As our "git notes" machinery, including how the
initial contents are populated with "git notes add" and how existing
notes on the same object are merged, all assume and depend on that
the leaves are blobs, we can say "we'll just dump to stdout with
write_in_full()", but who knows what kind of custom crap random
third-party tools and IDEs try to use notes trees to store.
Even if we limit ourselves to blobs, they may not be suitable to be
dumped to tty (or a pager, for that matter), and I can see how
textconv could be used as a way out ("detect that the notes payload
is an image and spawn display" kind of hack in a repository full of
images and notes used to store thumbnails, perhaps).
> That is also a cool idea. That would probably use the functionality of
> the cat-file batch mode, right?
Not really. I was hoping that "git show" that can take multiple
objects from its command line would directly be used, or with a new
option that gives a separator between these objects.
Perhaps we want a new option, e.g., "git notes show --text" that
passes the contents of the leaf blob object to write_in_full(),
bypassing all the things "git show" does, while in a rare case when
the leaf we find is not a blob to invoke "git show". That might be
a safe approach to move forward, if we wanted to do this.
^ permalink raw reply
* Re: [PATCH] .github/PULL_REQUEST_TEMPLATE.md: add a note about single-commit PRs
From: Junio C Hamano @ 2024-02-06 17:56 UTC (permalink / raw)
To: Philippe Blain; +Cc: Philippe Blain via GitGitGadget, git, Johannes Schindelin
In-Reply-To: <b7789242-5eee-3b1b-54e7-cafce96f2340@gmail.com>
Philippe Blain <levraiphilippeblain@gmail.com> writes:
> Yes, it probably is possible, and Dscho suggested the same in [1] and [2].
Ah, great minds think alike, and Dscho's suggestion was made a few
years ago already.
> I'll maybe get a crack at it eventually, but I hope in the meantime this
> simple addition to the PR template will help a bit.
Perhaps.
^ permalink raw reply
* Re: [PATCH] branch: clarify <oldbranch> and <newbranch> terms further
From: Junio C Hamano @ 2024-02-06 18:28 UTC (permalink / raw)
To: Dragan Simic; +Cc: Kyle Lippincott, git
In-Reply-To: <8f588db87929b063462dbf4ff134adc7@manjaro.org>
Dragan Simic <dsimic@manjaro.org> writes:
>> I think I laid all this out and more in a separate message.
>> https://lore.kernel.org/git/xmqq8r4zln08.fsf@gitster.g/
>
> I agree about this as well, but that will perhaps be handled in some
> separate patch for the git-branch(1) man page.
If you truly agree with the longer term plan, which includes removal
of the bullet points your patch updates, then reviewing further on
that patch becomes a waste of time in the larger picture, doesn't
it?
"Will be deferred to some later time, let's take this small update
as-is" has already been said back then. Let's not do that again
this time.
Thanks.
^ permalink raw reply
* Re: [PATCH] t0091: allow test in a repository without tags
From: Junio C Hamano @ 2024-02-06 18:33 UTC (permalink / raw)
To: Martin Ågren; +Cc: git
In-Reply-To: <CAN0heSrX8zQnfk_abtgBreoc=a8Z+7E-jEHHUFmu6740L8p2Lw@mail.gmail.com>
Martin Ågren <martin.agren@gmail.com> writes:
> GIT-VERSION-GEN seems to be careful to only use tags on the wanted form.
> My build generates a git version of "2.43.GIT", no "unknown..." stuff.
>
> I don't doubt that you've hit this, I just wonder which piece of the
> puzzle I'm missing.
I wonder that too.
I was experimenting with "seen" with the reftable. I first created
a new and empty repository with "git init --ref-format=reftable" in
a brand new directory next to my primary working area, and then did
"git fetch --no-tags ../git.git/ master" or something to pull the
history without tags in. After that I thought I was careful to make
sure I only ran the "seen" version of Git (all my other installations
of Git are unaware of reftable, and the version of Git on my regular
$PATH is not from the "seen" branch), but perhaps I screwed up at
some point and the mistake got stuck in the version file, or something
silly like that, perhaps.
>
>> - # The beginning should match "git version --build-info" verbatim,
>> + # The beginning should match "git version --build-options" verbatim,
>
> Correct, my thinko-typo, thanks for correcting.
>
>> # but rather than checking bit-for-bit equality, just test some basics.
>> - grep "git version [0-9]." system &&
>> + grep "git version " system &&
>
> This matches the commit message, ok.
>
> Martin
^ permalink raw reply
* Re: [PATCH] Always check the return value of `repo_read_object_file()`
From: Junio C Hamano @ 2024-02-06 18:36 UTC (permalink / raw)
To: Karthik Nayak
Cc: Johannes Schindelin via GitGitGadget, git, Johannes Schindelin
In-Reply-To: <CAOLa=ZQOALZRNqp7dDH0qDWoHwo6_3G8VgVuMbb3C20UdJ4C5A@mail.gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
> Hello,
>
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:
>> diff --git a/bisect.c b/bisect.c
>> index f1273c787d9..f75e50c3397 100644
>> --- a/bisect.c
>> +++ b/bisect.c
>> @@ -158,6 +158,9 @@ static void show_list(const char *debug, int counted, int nr,
>> const char *subject_start;
>> int subject_len;
>>
>> + if (!buf)
>> + die(_("unable to read %s"), oid_to_hex(&commit->object.oid));
>> +
>
> Nit: We know that `repo_read_object_file()` fails on corrupt objects, so
> this means that this is only happening when the object doesn't exist. I
> wonder if it makes more sense to replace "unable to read %s" which is a
> little ambiguous with something like "object %q doesn't exist".
I am not sure if that is a good move in the longer run. We may
"fix" the called function to return NULL to allow callers to deal
with errors from object corruption better, at which time between
"doesn't exist" and "unable to read", the latter becomes far closer
to what actually happened (it is debatable if a corrupt thing really
exists in the first place, too).
> Otherwise, the patch looks good, thanks!
I haven't read the remainder of the patch, but to me this hunk looks
OK.
Thanks.
^ permalink raw reply
* Re: [PATCH] branch: clarify <oldbranch> and <newbranch> terms further
From: Dragan Simic @ 2024-02-06 18:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kyle Lippincott, git
In-Reply-To: <xmqqle7xih0s.fsf@gitster.g>
On 2024-02-06 19:28, Junio C Hamano wrote:
> Dragan Simic <dsimic@manjaro.org> writes:
>>> I think I laid all this out and more in a separate message.
>>> https://lore.kernel.org/git/xmqq8r4zln08.fsf@gitster.g/
>>
>> I agree about this as well, but that will perhaps be handled in some
>> separate patch for the git-branch(1) man page.
>
> If you truly agree with the longer term plan, which includes removal
> of the bullet points your patch updates, then reviewing further on
> that patch becomes a waste of time in the larger picture, doesn't
> it?
Hmm, it doesn't seem like a waste of time and effort to me, because
the first patch would move/add the descriptions of the <oldbranch>
and <newbranch> arguments to the descriptions of the branch copy and
rename operations in the "Options" section.
This needs to be done anyway, if you agree. The following patch would
either dissolve as many sentences from the "Description" section into
the "Options" section, or have those sentences converted into some
kind of more readable prose.
I hope you agree.
> "Will be deferred to some later time, let's take this small update
> as-is" has already been said back then. Let's not do that again
> this time.
Oh, I've also heard that too many times, and I also know where such
an approach usually leads. Nowhere. :)
^ permalink raw reply
* Re: [PATCH] Always check the return value of `repo_read_object_file()`
From: Junio C Hamano @ 2024-02-06 18:42 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: Johannes Schindelin via GitGitGadget, git, Johannes Schindelin
In-Reply-To: <ZcHW_bc6N5umk2G4@tanuki>
Patrick Steinhardt <ps@pks.im> writes:
>> mmfile[i].ptr = repo_read_object_file(the_repository,
>> &ce->oid, &type,
>> &size);
>> + if (!mmfile[i].ptr)
>> + die(_("unable to read %s"),
>> + oid_to_hex(&ce->oid));
>> mmfile[i].size = size;
>> }
>> }
>
> A few lines below this we check whether `mmfile[i].ptr` is `NULL` and
> replace it with the empty string if so. So this patch here is basically
> a change in behaviour where we now die instead of falling back to the
> empty value.
I think that one is trying to cope with cases where we genuinely do
not have all three variants, not "we thought we had this variant so
we tried to read it into mmfile[i].{ptr,size}, but it turns out that
the object name we had was bad". So the fallback code for an entirely
different case was masking the breakage the above hunk fixes, and
this being "rerere", it is better to be cautious than sorry.
Thanks for reading the original code carefully.
^ permalink raw reply
* Re: [PATCH] commit.c: ensure strchrnul() doesn't scan beyond range
From: Junio C Hamano @ 2024-02-06 18:44 UTC (permalink / raw)
To: René Scharfe
Cc: Chandra Pratap via GitGitGadget, git, Chandra Pratap,
Chandra Pratap
In-Reply-To: <ce83bd09-dbd2-4c9e-8197-6e4800935523@web.de>
René Scharfe <l.s.r@web.de> writes:
>> while (line && line < msg + len) {
>> const char *eol = strchrnul(line, '\n');
>> + assert(eol - line <= len);
>
> Something like this might work in Verse, but C is more simple-minded.
> You can't undo an out-of-bounds access after the fact, and assert()
> would be compiled out if the code is built with NDEBUG anyway.
Good comments. Thanks.
^ permalink raw reply
* Re: [PATCH v3 4/4] for-each-ref: avoid filtering on empty pattern
From: Junio C Hamano @ 2024-02-06 18:47 UTC (permalink / raw)
To: Phillip Wood; +Cc: Karthik Nayak, phillip.wood, git, ps
In-Reply-To: <xmqqle7xjzic.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Phillip Wood <phillip.wood123@gmail.com> writes:
>
>> Thanks I'd missed that discussion. I see that at the end of that
>> discussion Junio was concerned that the proposed "" did not account
>> for "refs/worktrees/$worktree/*" [1] - how has that been resolved?
>
> Ah, that is an excellent point.
> ...
> The use of dashed-options to include hierachies that are by default
> excluded (e.g. "--include-root-refs" and "--include-worktree-refs")
> feels limiting, but should be sufficient for our needs, both current
> (i.e. I want to see HEAD and FETCH_HEAD) and in the immediate future
> (i.e. I want to see worktree refs from that worktree), and I can buy
> that as a good alternative solution, at least in the shorter term.
>
> I still worry that it may make introducing the negative ref patterns
> harder, though. How does --include-worktree-refs=another to include
> the worktree refs from another worktree in refs/worktrees/another
> interact with a negative pattern that was given from the command
> line that overlaps with it? Whatever interaction rules we define,
> can we easily explain it in the documentation?
>
> Just like "an empty string tells Git to include everything" is a
> perfectly reasonable approach if we plan to never allow
> refs/worktrees/ hierarchy, "dashed-options for selected hierarchies"
> is a perfectly reasonable approach if we plan to never allow
> negative limit patterns, I suspect. We should stop complexity at
> some point, and the decision to never support negative limit
> patterns might be the place to draw that line. I dunno.
For now, let's block the kn/for-all-refs topic until we figure out
the UI issue. Which means this (and the reftable integration that
started to depend on it) will not be in the upcoming release.
FWIW, I am leaning towards "a set of narrowly targetted command line
options (like "--include-root-refs")" approach over "a empty string
defeats the built-in default of 'refs/' limit".
Thanks.
^ permalink raw reply
* Re: git-gui desktop launcher
From: Junio C Hamano @ 2024-02-06 18:49 UTC (permalink / raw)
To: brian m. carlson; +Cc: Tobias Boesch, git
In-Reply-To: <ZcFhNPRprfMqeRu1@tapette.crustytoothpaste.net>
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
> I think such functionality would be generally useful, and probably be
> beneficial to a wide variety of distributors.
How have the various distros been packaging their binaries? Would
this change affect what they have done already?
As long as it does not conflict, I am all for it (i.e. we do it once
and everybody benefits).
Thanks.
^ permalink raw reply
* Re: git-gui desktop launcher
From: Dragan Simic @ 2024-02-06 19:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: brian m. carlson, Tobias Boesch, git
In-Reply-To: <xmqqle7xh1hc.fsf@gitster.g>
On 2024-02-06 19:49, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>
>> I think such functionality would be generally useful, and probably be
>> beneficial to a wide variety of distributors.
>
> How have the various distros been packaging their binaries? Would
> this change affect what they have done already?
>
> As long as it does not conflict, I am all for it (i.e. we do it once
> and everybody benefits).
AFAICT, Linux distributions provided their own version(s) of the
.desktop file. Perhaps the version provided by Fedora [1] could be
consulted, for example, to see what's already expected there, and
to provide parity in the version supplied by us.
[1]
https://koji.fedoraproject.org/koji/fileinfo?rpmID=37302272&filename=git-gui.desktop
^ permalink raw reply
* Re: is it a bug that git status show the in-progress 'edit' in an interactive rebase as 'done'?
From: Sergey Organov @ 2024-02-06 19:45 UTC (permalink / raw)
To: Oswald Buddenhagen; +Cc: Britton Kerin, git
In-Reply-To: <ZcIYz82iLxPOVR9Q@ugly>
Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:
> On Tue, Feb 06, 2024 at 01:02:43AM -0900, Britton Kerin wrote:
>>>Last command done (1 command done):
>>> edit 71b73de914 message for first commit
>>>...
>>>You are currently editing a commit while rebasing branch
>>>...
>>
>>This seems wrong, because until git rebase --continue has been done
>>the edit operation for the first commit is *ongoing* and it would be
>>much clearer for the output of status to accurately say so.
>>
> it makes a lot of more sense when you decompose 'edit' into 'pick'
> followed by 'break', which it essentially is. so from git's perspective,
> the command really _is_ already done. note that in this state, you can
> do all kinds of crazy things - including adding new commits (possibly by
> cherry-picking them) and even dropping already rewritten commits (using
> a hard reset). so in a way, the message above is even a bit too
> suggestive.
Yep. Maybe, if the rebase action itself were called "amend" rather than
"edit", it'd have been more clear and consistent thus less confusing.
Check:
git status
interactive rebase in progress; onto e79552d197
Last command done (1 command done):
amend 71b73de914 message for first commit
Next commands to do (6 remaining commands):
amend 3a478a7a08 message for second commit
pick fab7159cf4 message for third commit
(use "git rebase --edit-todo" to view and edit)
You are currently amending a commit while rebasing branch
'my_completion_updates' on 'e79552d197'.
(use "git commit --amend" to amend the current commit)
(use "git rebase --continue" once you are satisfied with your changes)
--
Sergey
^ permalink raw reply
* Re: is it a bug that git status show the in-progress 'edit' in an interactive rebase as 'done'?
From: Britton Kerin @ 2024-02-06 20:09 UTC (permalink / raw)
To: Oswald Buddenhagen; +Cc: git
In-Reply-To: <ZcIYz82iLxPOVR9Q@ugly>
On Tue, Feb 6, 2024 at 2:32 AM Oswald Buddenhagen
<oswald.buddenhagen@gmx.de> wrote:
>
> On Tue, Feb 06, 2024 at 01:02:43AM -0900, Britton Kerin wrote:
> >>Last command done (1 command done):
> >> edit 71b73de914 message for first commit
> >>...
> >>You are currently editing a commit while rebasing branch
> >>...
> >
> >This seems wrong, because until git rebase --continue has been done
> >the edit operation for the first commit is *ongoing* and it would be
> >much clearer for the output of status to accurately say so.
> >
> it makes a lot of more sense when you decompose 'edit' into 'pick'
> followed by 'break', which it essentially is. so from git's perspective,
> the command really _is_ already done. note that in this state, you can
well viewed this way the pick may be done but not the implicit break
> do all kinds of crazy things - including adding new commits (possibly by
> cherry-picking them) and even dropping already rewritten commits (using
> a hard reset). so in a way, the message above is even a bit too
> suggestive.
Yes. I'd handle this by changing the description of edit offered in
the comments in the todo to better reflect the possibilities. The
hints that git rebase -i (or --continue I guess) gives when it hits
the edit commit also don't reflect all the possibilities very well.
Britton
^ permalink raw reply
* Query about gitignore
From: Divyaditya Singh @ 2024-02-06 20:26 UTC (permalink / raw)
To: git
Hello there,
I hope you are having a wonderful day.
I apologize if this is inappropriate but I wanted to ask is there a way that I can make my .gitignore such that it ignores the entire parent directory of a matching file.
Thank You
DV
^ permalink raw reply
* Re: [PATCH 0/4] merge-tree: handle missing objects correctly
From: Junio C Hamano @ 2024-02-06 21:12 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
In-Reply-To: <pull.1651.git.1707212981.gitgitgadget@gmail.com>
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> I recently looked into issues where git merge-tree calls returned bogus data
> (in one instance returning an empty tree for non-empty merge parents). By
> the time I had a look at the corresponding repository, the issue was no
> longer reproducible, but a closer look at the code combined with some manual
> experimenting turned up the fact that missing tree objects aren't handled as
> errors by git merge-tree.
>
> While at it, I added a commit on top that tries to catch all remaining
> unchecked parse_tree() calls.
>
> This patch series is based on js/merge-tree-3-trees because I introduced
> three unchecked parse_tree() calls in that topic branch.
Thanks. All the added checks looked reasonable to me.
Will queue.
>
> Johannes Schindelin (4):
> merge-tree: fail with a non-zero exit code on missing tree objects
> merge-ort: do check `parse_tree()`'s return value
> t4301: verify that merge-tree fails on missing blob objects
> Always check `parse_tree*()`'s return value
>
> builtin/checkout.c | 19 ++++++++++++++++---
> builtin/clone.c | 3 ++-
> builtin/commit.c | 3 ++-
> builtin/merge-tree.c | 6 ++++++
> builtin/read-tree.c | 3 ++-
> builtin/reset.c | 4 ++++
> cache-tree.c | 4 ++--
> merge-ort.c | 16 +++++++++++-----
> merge-recursive.c | 3 ++-
> merge.c | 5 ++++-
> reset.c | 5 +++++
> sequencer.c | 4 ++++
> t/t4301-merge-tree-write-tree.sh | 24 ++++++++++++++++++++++++
> 13 files changed, 84 insertions(+), 15 deletions(-)
>
>
> base-commit: 5f43cf5b2e4b68386d3774bce880b0f74d801635
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1651%2Fdscho%2Fmerge-tree-and-missing-objects-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1651/dscho/merge-tree-and-missing-objects-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1651
^ permalink raw reply
* Re: git-gui desktop launcher
From: Tobias Boesch @ 2024-02-06 21:14 UTC (permalink / raw)
To: brian m. carlson; +Cc: git
In-Reply-To: <ZcFhNPRprfMqeRu1@tapette.crustytoothpaste.net>
5 Feb 2024 23:29:10 brian m. carlson <sandals@crustytoothpaste.net>:
> On 2024-02-05 at 20:12:10, Tobias Boesch wrote:
>>
>> .desktop file proposal
>>
>> [Desktop Entry]
>> Name=git gui
>
> I don't know whether this is the official name of the project or not.
> Perhaps someone else can comment on what the capitalization and
> punctuation of this entry should be.
>
From the official documentation [1] it reads "git-gui"
[1] https://git-scm.com/docs/git-gui
>> Comment=A portable graphical interface to Git
>> Exec=/bin/bash -c 'if [[ "$0" = "/bin/bash" ]]; then git gui; else cd
>> "$0" && git gui; fi' %F
>
> It's not guaranteed that bash even exists on the system, let alone that
> it's in /bin. For example, this wouldn't work on most of the BSDs.
> This would need to be templated using SHELL_PATH and written in POSIX
> sh (e.g., no `[[`).
>
I see. I'll try to look into it. If someone knows how to do that let me know.
>> Icon=/usr/share/git-gui/lib/git-gui.ico
>
> This would also need to be given an appropriate location based on the
> build parameters.
>
Is it about the build parameters of the build of git(-gui), or about the downstream distros are building?
So git leaves this empty and the packagers full this out?
>> Type=Application
>> Terminal=false
>> Categories=Development;
>>
>>
>> I think upstream has any interest to add this. Therefore I ask here."
>>
>> -------------------------
>>
>> The arch package maintainer proposed to try to to add this to upstream
>> before just putting it into the arch package.
>> Here I am asking if it could be added to git.
>
> If you wanted to send a suitable patch for the file such that it were
> appropriately built as part of the build process and installed, then we
> could probably accept it. Such patches are usually created by using
> `git format-patch` on one or multiple commits and then sent using `git
> send-email`. You can take a look at `Documentation/SubmittingPatches`
> for more details.
>
That is the plan. I don't know if I get the installation part fine for testing it, but sending a patch should be possible. Sure.
> I think such functionality would be generally useful, and probably be
> beneficial to a wide variety of distributors.
> --
Nice to hear. Thanks
^ permalink raw reply
* Re: Wrong exit code on failed SSH signing
From: Junio C Hamano @ 2024-02-06 21:25 UTC (permalink / raw)
To: Sergey Kosukhin; +Cc: git
In-Reply-To: <CAGMF1KiFNnr1nFBg2+mRqiurXpxOOXAcoWc0GciRKDoWzpJSkA@mail.gmail.com>
Sergey Kosukhin <skosukhin@gmail.com> writes:
> There seems to be a bug in the sign_buffer_ssh function in
> gpg-interface.c: a possible exit code of ssh-keygen is 255, which is
> returned as-is by sign_buffer_ssh. The problem is that, for example,
> the function build_tag_object in builtin/tag.c considers only negative
> values as a failure. Since 255 >= 0, the error message "unable to sign
> the tag" is not emitted and git exits normally with zero exit code. It
> might be enough to return -1 in sign_buffer_ssh if ret is not zero.
Thanks for noticing and an excellent initial diagnosis.
There are three callers of sign_buffer():
- send-pack.c:generate_push_cert() lets the user sign the push
certificate, and non-zero return from sign_buffer() is a sign
that we failed to sign.
- commit.c:sign_with_header() lets the user sign a commit object,
and non-zero return from sign_buffer() is taken as an error.
- builtin/tag.c:do_sign() calls sign_buffer() and propagates the
return value to its caller, which assumes that positive return
values are not errors.
It seems to me that what needs fixing is the last caller. Perhaps
inside "git tag" implementation, there is a local convention that
errors are signaled with negative values, and that is fine, but then
builtin/tag.c:do_sign() should be doing the same translation as
builtin/tag.c:verify_tag() does, I would say.
The latter calls gpg_verify_tag() and upon non-zero return,
translates that to a return of -1 to its caller, like so:
static int verify_tag(const char *name, const char *ref UNUSED,
const struct object_id *oid, void *cb_data)
{
int flags;
struct ref_format *format = cb_data;
flags = GPG_VERIFY_VERBOSE;
if (format->format)
flags = GPG_VERIFY_OMIT_STATUS;
if (gpg_verify_tag(oid, name, flags))
return -1;
...
So perhaps something like this with a proper log message would be a
better fix?
builtin/tag.c | 2 +-
gpg-interface.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git c/builtin/tag.c w/builtin/tag.c
index f036cf32f5..37473ac21f 100644
--- c/builtin/tag.c
+++ w/builtin/tag.c
@@ -153,7 +153,7 @@ static int verify_tag(const char *name, const char *ref UNUSED,
static int do_sign(struct strbuf *buffer)
{
- return sign_buffer(buffer, buffer, get_signing_key());
+ return sign_buffer(buffer, buffer, get_signing_key()) ? -1 : 0;
}
static const char tag_template[] =
diff --git c/gpg-interface.h w/gpg-interface.h
index 143cdc1c02..7cd98161f7 100644
--- c/gpg-interface.h
+++ w/gpg-interface.h
@@ -66,7 +66,7 @@ size_t parse_signed_buffer(const char *buf, size_t size);
* Create a detached signature for the contents of "buffer" and append
* it after "signature"; "buffer" and "signature" can be the same
* strbuf instance, which would cause the detached signature appended
- * at the end.
+ * at the end. Returns 0 on success, non-zero on failure.
*/
int sign_buffer(struct strbuf *buffer, struct strbuf *signature,
const char *signing_key);
^ permalink raw reply related
* [PATCH v6 0/7] completion: improvements for git-bisect
From: Britton Leo Kerin @ 2024-02-06 21:50 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Patrick Steinhardt, Britton Leo Kerin
In-Reply-To: <20240206020930.312164-1-britton.kerin@gmail.com>
Relative to v5 this makes the following actual changes:
* Treat COMPREPLY as an array on assignment and test, rather than
relying on the bash mechanism of implicitly acting on the first
element of the array.
* Whitespace fixes.
The commit message about __git_complete_log_opts has also been changed
to indicate that COMPREPLY is emptied and why, and a broken
Signed-off-by line fixed.
Britton Leo Kerin (7):
completion: tests: always use 'master' for default initial branch name
completion: bisect: complete bad, new, old, and help subcommands
completion: bisect: complete custom terms and related options
completion: bisect: complete missing --first-parent and --no-checkout
options
completion: new function __git_complete_log_opts
completion: bisect: complete log opts for visualize subcommand
completion: bisect: recognize but do not complete view subcommand
contrib/completion/git-completion.bash | 65 ++++++++++--
t/t9902-completion.sh | 141 +++++++++++++++++++++++++
2 files changed, 199 insertions(+), 7 deletions(-)
Range-diff against v5:
1: 71b73de914 = 1: 71b73de914 completion: tests: always use 'master' for default initial branch name
2: 3a478a7a08 ! 2: 7bc45bfc13 completion: bisect: complete bad, new, old, and help subcommands
@@ Commit message
such that the commands and their possible ref arguments are completed.
Add tests.
- Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.c
+ Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
## contrib/completion/git-completion.bash ##
@@ contrib/completion/git-completion.bash: _git_bisect ()
3: fab7159cf4 ! 3: be925327d3 completion: bisect: complete custom terms and related options
@@ t/t9902-completion.sh: test_expect_success 'git-bisect - when bisecting all subc
reset Z
visualize Z
@@ t/t9902-completion.sh: test_expect_success 'git-bisect - when bisecting all subcommands are candidates'
- EOF
)
'
+
+test_expect_success 'git-bisect - options to terms subcommand are candidates' '
+ (
+ cd git-bisect &&
@@ t/t9902-completion.sh: test_expect_success 'git-bisect - when bisecting all subc
+ )
+'
+
-
test_expect_success 'git checkout - completes refs and unique remote branches for DWIM' '
test_completion "git checkout " <<-\EOF
+ HEAD Z
4: 73f3343b94 = 4: c3141921e5 completion: bisect: complete missing --first-parent and --no-checkout options
5: a20846bbd3 ! 5: 092bfba6b1 completion: new function __git_complete_log_opts
@@ Commit message
completion: new function __git_complete_log_opts
The options accepted by git-log are also accepted by at least one other
- command (git-bisect). Factor the common option completion code into
- a new function and use it from _git_log.
+ command (git-bisect). Factor the common option completion code into a
+ new function and use it from _git_log. The new function leaves
+ COMPREPLY empty if no option candidates are found, so that callers can
+ safely check it to determine if completion for other arguments should be
+ attempted.
Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
@@ contrib/completion/git-completion.bash: __git_diff_merges_opts="off none on firs
{
- __git_has_doubledash && return
- __git_find_repo_path
-+ COMPREPLY=""
++ COMPREPLY=()
local merge=""
if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
@@ contrib/completion/git-completion.bash: _git_log ()
+ __git_find_repo_path
+
+ __git_complete_log_opts
-+ [ -z "$COMPREPLY" ] || return
++ [ ${#COMPREPLY[@]} -eq 0 ] || return
+
__git_complete_revlist
}
6: fe5545c9a3 ! 6: 9afd4d4e0f completion: bisect: complete log opts for visualize subcommand
@@ t/t9902-completion.sh: test_expect_success 'git-bisect - options to terms subcom
+ EOF
+ )
+'
-
++
test_expect_success 'git checkout - completes refs and unique remote branches for DWIM' '
test_completion "git checkout " <<-\EOF
+ HEAD Z
7: c9102ac532 = 7: dba916b31c completion: bisect: recognize but do not complete view subcommand
--
2.43.0
^ permalink raw reply
* [PATCH v6 3/7] completion: bisect: complete custom terms and related options
From: Britton Leo Kerin @ 2024-02-06 21:50 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Patrick Steinhardt, Britton Leo Kerin
In-Reply-To: <20240206215048.488344-1-britton.kerin@gmail.com>
git bisect supports the use of custom terms via the --term-(new|bad) and
--term-(old|good) options, but the completion code doesn't know about
these options or the new subcommands they define.
Add support for these options and the custom subcommands by checking for
BISECT_TERMS and adding them to the list of subcommands. Add tests.
Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
contrib/completion/git-completion.bash | 32 ++++++++++++++++++++++++--
t/t9902-completion.sh | 15 ++++++++++++
2 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 06d0b156e7..6a3d9c7760 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1449,7 +1449,20 @@ _git_bisect ()
{
__git_has_doubledash && return
- local subcommands="start bad new good old skip reset visualize replay log run help"
+ __git_find_repo_path
+
+ # If a bisection is in progress get the terms being used.
+ local term_bad term_good
+ if [ -f "$__git_repo_path"/BISECT_TERMS ]; then
+ term_bad=$(__git bisect terms --term-bad)
+ term_good=$(__git bisect terms --term-good)
+ fi
+
+ # We will complete any custom terms, but still always complete the
+ # more usual bad/new/good/old because git bisect gives a good error
+ # message if these are given when not in use, and that's better than
+ # silent refusal to complete if the user is confused.
+ local subcommands="start bad new $term_bad good old $term_good terms skip reset visualize replay log run help"
local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
__git_find_repo_path
@@ -1462,7 +1475,22 @@ _git_bisect ()
fi
case "$subcommand" in
- bad|new|good|old|reset|skip|start)
+ start)
+ case "$cur" in
+ --*)
+ __gitcomp "--term-new --term-bad --term-old --term-good"
+ return
+ ;;
+ *)
+ __git_complete_refs
+ ;;
+ esac
+ ;;
+ terms)
+ __gitcomp "--term-good --term-old --term-bad --term-new"
+ return
+ ;;
+ bad|new|"$term_bad"|good|old|"$term_good"|reset|skip)
__git_complete_refs
;;
*)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 7388c892cf..304903b1a7 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1321,9 +1321,12 @@ test_expect_success 'git-bisect - when bisecting all subcommands are candidates'
test_completion "git bisect " <<-\EOF
start Z
bad Z
+ custom_new Z
+ custom_old Z
new Z
good Z
old Z
+ terms Z
skip Z
reset Z
visualize Z
@@ -1335,6 +1338,18 @@ test_expect_success 'git-bisect - when bisecting all subcommands are candidates'
)
'
+test_expect_success 'git-bisect - options to terms subcommand are candidates' '
+ (
+ cd git-bisect &&
+ test_completion "git bisect terms --" <<-\EOF
+ --term-bad Z
+ --term-good Z
+ --term-new Z
+ --term-old Z
+ EOF
+ )
+'
+
test_expect_success 'git checkout - completes refs and unique remote branches for DWIM' '
test_completion "git checkout " <<-\EOF
HEAD Z
--
2.43.0
^ 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