* Re: [PATCH 1/4] sequencer: Do not require `allow_empty` for redundant commit options
From: Phillip Wood @ 2024-01-29 10:55 UTC (permalink / raw)
To: Brian Lyles, phillip.wood; +Cc: git, me, newren, gitster
In-Reply-To: <CAHPHrSf=UkR9+hMfb7pp5Y6uHqa2pBrEf+cTLJv=z=BOFdL3rw@mail.gmail.com>
Hi Brian
On 28/01/2024 16:36, Brian Lyles wrote:
> [+cc Junio]
>
> On Sat, Jan 27, 2024 at 5:30 PM Brian Lyles <brianmlyles@gmail.com> wrote:
>
>>>> For some amount of backwards compatibility with the existing code and
>>>> tests, I have opted to preserve the behavior of returning 0 when:
>>>>
>>>> - `allow_empty` is specified, and
>>>> - either `is_index_unchanged` or `is_original_commit_empty` indicates an
>>>> error
>>>
>>> I'm not sure that is a good idea as it is hiding an error that we didn't
>>> hit before because we returned early.
>>
>> I think you're right -- Previously the error could not have been hit,
>> but now it can. An error is still an error, and we should handle it
>> regardless of how `allow_empty` was set. I'll address this in v2 by
>> simply returning the error.
>
> As I dig into this more, I'm noticing that this may have unintended side
> effects that I'm unsure of. After making this change, I noticed a couple
> of failures in the cherry-pick test suite. The others may be a knock-on
> of this initial failure:
>
> expecting success of 3501.8 'cherry-pick on unborn branch':
> git checkout --orphan unborn &&
> git rm --cached -r . &&
> rm -rf * &&
> git cherry-pick initial &&
> git diff --quiet initial &&
> test_cmp_rev ! initial HEAD
>
> A extra_file
> Switched to a new branch 'unborn'
> rm 'extra_file'
> rm 'spoo'
> error: could not resolve HEAD commit
> fatal: cherry-pick failed
> not ok 8 - cherry-pick on unborn branch
> #
> # git checkout --orphan unborn &&
> # git rm --cached -r . &&
> # rm -rf * &&
> # git cherry-pick initial &&
> # git diff --quiet initial &&
> # test_cmp_rev ! initial HEAD
> #
>
> It looks like this is caused specifically by not hiding the error from
> `index_unchanged`
Oh dear, that's a pain. I haven't checked but suspect we already hit
this when running
git cherry-pick --allow-empty
on an orphan checkout. In do_pick_commit() we treat an error reading
HEAD as an unborn branch so I think we could do the same here. If the
branch is unborn then we can use the_hash_algo->empty_tree as the tree
to compare to.
Best Wishes
Phillip
^ permalink raw reply
* Re: [PATCH v2 1/6] t1300: make tests more robust with non-default ref backends
From: Patrick Steinhardt @ 2024-01-29 10:49 UTC (permalink / raw)
To: Christian Couder; +Cc: git, Taylor Blau, Eric Sunshine
In-Reply-To: <CAP8UFD2ZoV74jx3B_adwYENGtp9fCH_oVjW4QJXTZQJ0=_aeEA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1349 bytes --]
On Mon, Jan 29, 2024 at 11:32:53AM +0100, Christian Couder wrote:
> On Wed, Jan 24, 2024 at 9:52 AM Patrick Steinhardt <ps@pks.im> wrote:
> > On Tue, Jan 23, 2024 at 05:15:48PM +0100, Christian Couder wrote:
> > > On Wed, Jan 10, 2024 at 10:01 AM Patrick Steinhardt <ps@pks.im> wrote:
[snip]
> > Hm. I don't really know how to phrase this better. The preceding
> > paragraph already explains why we're discarding the extension and what
> > the consequence is. I added a sentence saying ", which will cause
> > failures when trying to access any refs."
>
> To me the preceding paragraph said that we are overwriting the config
> file, but I just don't see how for example the above test overwrites
> anything. So maybe I am missing something obvious, or maybe you don't
> quite mean "overwrite", but I don't see how the extension would be
> discarded by the test which only seems to add stuff.
It happens before already, outside of any tests. See line 1036:
```
cat > .git/config <<\EOF
[section "sub=section"]
val1 = foo=bar
val2 = foo\nbar
val3 = \n\n
val4 =
val5
EOF
```
Overall, I agree that this is rather hard to discover and that the tests
really could require a bigger refactoring to make them more independent
of each other.
I'll send another version that mentions this explicitly.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/6] t1300: make tests more robust with non-default ref backends
From: Christian Couder @ 2024-01-29 10:32 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Taylor Blau, Eric Sunshine
In-Reply-To: <ZbDP4ntRqrxX08yk@tanuki>
On Wed, Jan 24, 2024 at 9:52 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Tue, Jan 23, 2024 at 05:15:48PM +0100, Christian Couder wrote:
> > On Wed, Jan 10, 2024 at 10:01 AM Patrick Steinhardt <ps@pks.im> wrote:
> > >
> > > The t1300 test suite exercises the git-config(1) tool. To do so we
> > > overwrite ".git/config" to contain custom contents.
> >
> > Here "we" means "tests in t1300" I guess.
> >
> > > While this is easy
> > > enough to do, it may create problems when using a non-default repository
> > > format because we also overwrite the repository format version as well
> > > as any potential extensions.
> >
> > But I am not sure that "we" in the above sentence is also "tests in
> > t1300". I think overwriting the repo format version and potential
> > extensions is done by other tests, right? Anyway it would be nice to
> > clarify this.
> >
> > > With the upcoming "reftable" ref backend
> > > the result is that we may try to access refs via the "files" backend
> > > even though the repository has been initialized with the "reftable"
> > > backend.
> >
> > Not sure here also what "we" is. When could refs be accessed via the
> > "files" backend even though the repo was initialized with the
> > "reftable" backend?
>
> Yeah, I've rephrased all of these to sey "the tests" or something
> similar.
>
> > Does this mean that some of the tests in t1300 try to access refs via
> > the "files" backend while we may want to run all the tests using the
> > reftable backend?
>
> Exactly. We overwrite the ".git/config", which contains the "refStorage"
> extension that tells us to use the "reftable" backend. So the extension
> is gone, and thus Git would fall back to use the "files" backend
> instead, which will fail.
Let's take a look at this test:
test_expect_success 'check split_cmdline return' "
git config alias.split-cmdline-fix 'echo \"' &&
test_must_fail git split-cmdline-fix &&
echo foo > foo &&
git add foo &&
git commit -m 'initial commit' &&
git config branch.main.mergeoptions 'echo \"' &&
test_must_fail git merge main
"
I don't really see how it overwrites anything. When putting some debug
commands before and after that test, it looks like the config file
contains the following before that test:
---
[section "sub=section"]
val1 = foo=bar
val2 = foo\nbar
val3 = \n\n
val4 =
val5
[section]
val = foo \t bar
---
and the following after that test:
---
[section "sub=section"]
val1 = foo=bar
val2 = foo\nbar
val3 = \n\n
val4 =
val5
[section]
val = foo \t bar
[alias]
split-cmdline-fix = echo \"
[branch "main"]
mergeoptions = echo \"
---
So it doesn't look like it overwrites anything. To me it just adds
stuff at the end of the file.
> > > Refactor tests which access the refdb to be more robust by using their
> > > own separate repositories, which allows us to be more careful and not
> > > discard required extensions.
> >
> > Not sure what exactly is discarding extensions. Also robust is not
> > very clear. It would be better to give at least an example of how
> > things could fail.
>
> Hm. I don't really know how to phrase this better. The preceding
> paragraph already explains why we're discarding the extension and what
> the consequence is. I added a sentence saying ", which will cause
> failures when trying to access any refs."
To me the preceding paragraph said that we are overwriting the config
file, but I just don't see how for example the above test overwrites
anything. So maybe I am missing something obvious, or maybe you don't
quite mean "overwrite", but I don't see how the extension would be
discarded by the test which only seems to add stuff.
^ permalink raw reply
* Re: [PATCH] reftable: honor core.fsync
From: Patrick Steinhardt @ 2024-01-29 9:48 UTC (permalink / raw)
To: John Cai via GitGitGadget; +Cc: git, John Cai
In-Reply-To: <pull.1654.git.git.1706035870956.gitgitgadget@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1948 bytes --]
On Tue, Jan 23, 2024 at 06:51:10PM +0000, John Cai via GitGitGadget wrote:
> From: John Cai <johncai86@gmail.com>
>
> While the reffiles backend honors configured fsync settings, the
> reftable backend does not. Address this by fsyncing reftable files using
> the write-or-die api's fsync_component() in two places: when we
> add additional entries into the table, and when we close the reftable
> writer.
>
> This commits adds a flush function pointer as a new member of
> reftable_writer because we are not sure that the first argument to the
> *write function pointer always contains a file descriptor. In the case of
> strbuf_add_void, the first argument is a buffer. This way, we can pass
> in a corresponding flush function that knows how to flush depending on
> which writer is being used.
>
> This patch does not contain tests as they will need to wait for another
> patch to start to exercise the reftable backend. At that point, the
> tests will be added to observe that fsyncs are happening when the
> reftable is in use.
>
> Signed-off-by: John Cai <johncai86@gmail.com>
I noticed that we missed syncing the "tables.list" file when performing
auto-compaction. The below patch is needed on top of what we already
have.
The topic is currently in `next`, but not yet in `master`, so we might
still squash it in. Junio, please let me know whether you want to do so
or whether I shall send this fix-up as a new patch. Thanks!
Patrick
diff --git a/reftable/stack.c b/reftable/stack.c
index ab295341cc..b17cfb9516 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -1018,6 +1018,10 @@ static int stack_compact_range(struct reftable_stack *st, int first, int last,
unlink(new_table_path.buf);
goto done;
}
+
+ fsync_component_or_die(FSYNC_COMPONENT_REFERENCE, lock_file_fd,
+ lock_file_name.buf);
+
err = close(lock_file_fd);
lock_file_fd = -1;
if (err < 0) {
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* Re: what should "git clean -n -f [-d] [-x] <pattern>" do?
From: Sergey Organov @ 2024-01-29 9:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Elijah Newren, git
In-Reply-To: <xmqqzfwrjdul.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Sergey Organov <sorganov@gmail.com> writes:
>
>> I'm still arguing in favor of fixing "-n", and I believe a fix is needed
>> independently from decision about "-f -f".
>
> Even though I do not personally like it, I do not think "which
> between do-it (f) and do-not-do-it (n) do you want to use?" is
> broken. It sometimes irritates me to find "git clean" (without "-f"
> or "-n", and with clean.requireForce not disabled) complain, and I
> personally think "git clean" when clean.requireForce is in effect
> and no "-n" or "-f" were given should pretend as if "-n" were given.
As a note, I'd consider to get rid of 'clean.requireForce' anyway, as
its default value provides safe reasonably behaving environment, and I
fail to see why anybody would need to set it to 'false'.
Thanks,
-- Sergey Organov
^ permalink raw reply
* [PATCH 2/2] t/Makefile: get UNIT_TESTS list from C sources
From: Jeff King @ 2024-01-29 3:19 UTC (permalink / raw)
To: git; +Cc: Phillip Wood
In-Reply-To: <20240129031540.GA2433764@coredump.intra.peff.net>
We decide on the set of unit tests to run by asking make to expand the
wildcard "t/unit-tests/bin/*". One unfortunate outcome of this is that
we'll run anything in that directory, even if it is leftover cruft from
a previous build. This isn't _quite_ as bad as it sounds, since in
theory the unit test executables are self-contained (so if they passed
before, they'll pass even though they now have nothing to do with the
checked out version of Git). But at the very least it's wasteful, and if
they _do_ fail it can be quite confusing to understand why they are
being run at all.
This wildcarding presumably came from our handling of the regular
shell-script tests, which match "t[0-9][0-9][0-9][0-9]-*.sh". But the
difference there is that those are actual tracked files. So if you
checkout a different commit, they'll go away. Whereas the contents of
unit-tests/bin are ignored (so not only do they stick around, but you
are not even warned of the stale files via "git status").
This patch fixes the situation by looking for the actual unit-test
source files and then massaging those names into the final executable
names. This has two additional benefits:
1. It will notice if we failed to build one or more unit-tests for
some reason (wheras the current code just runs whatever made it to
the bin/ directory).
2. The wildcard should avoid other build cruft, like the pdb files we
worked around in 0df903d402 (unit-tests: do not mistake `.pdb`
files for being executable, 2023-09-25).
Our new wildcard does make an assumption that unit tests are build from
C sources. It would be a bit cleaner if we consulted UNIT_TEST_PROGRAMS
from the top-level Makefile. But doing so is tricky unless we reorganize
that Makefile to split the source file lists into include-able subfiles.
That might be worth doing in general, but in the meantime, the
assumptions made by the wildcard here seems reasonable.
Signed-off-by: Jeff King <peff@peff.net>
---
I of course hit this when moving between "next" and "master" for an
up-and-coming unit-test file which sometimes failed.
t/Makefile | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/t/Makefile b/t/Makefile
index b7a6fefe28..c5c6e2ef6b 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -42,7 +42,9 @@ TPERF = $(sort $(wildcard perf/p[0-9][0-9][0-9][0-9]-*.sh))
TINTEROP = $(sort $(wildcard interop/i[0-9][0-9][0-9][0-9]-*.sh))
CHAINLINTTESTS = $(sort $(patsubst chainlint/%.test,%,$(wildcard chainlint/*.test)))
CHAINLINT = '$(PERL_PATH_SQ)' chainlint.pl
-UNIT_TESTS = $(sort $(filter-out %.pdb unit-tests/bin/t-basic%,$(wildcard unit-tests/bin/t-*)))
+UNIT_TEST_SOURCES = $(wildcard unit-tests/t-*.c)
+UNIT_TEST_PROGRAMS = $(patsubst unit-tests/%.c,unit-tests/bin/%,$(UNIT_TEST_SOURCES))
+UNIT_TESTS = $(sort $(filter-out unit-tests/bin/t-basic%,$(UNIT_TEST_PROGRAMS)))
# `test-chainlint` (which is a dependency of `test-lint`, `test` and `prove`)
# checks all tests in all scripts via a single invocation, so tell individual
--
2.43.0.797.g29b680fc68
^ permalink raw reply related
* [PATCH 1/2] Makefile: use order-only prereq for UNIT_TEST_BIN
From: Jeff King @ 2024-01-29 3:18 UTC (permalink / raw)
To: git; +Cc: Phillip Wood
In-Reply-To: <20240129031540.GA2433764@coredump.intra.peff.net>
We build the UNIT_TEST_BIN directory (t/unit-tests/bin) on the fly with
"mkdir -p". And so the recipe for UNIT_TEST_PROGS, which put their
output in that directory, depend on UNIT_TEST_BIN to make sure it's
there.
But using a normal dependency leads to weird outcomes, because the
timestamp of the directory is important. For example, try this:
$ make
[...builds everything...]
[now re-build one unit test]
$ touch t/unit-tests/t-ctype.c
$ make
SUBDIR templates
CC t/unit-tests/t-ctype.o
LINK t/unit-tests/bin/t-ctype
So far so good. Now running make again should build nothing. But it
doesn't!
$ make
SUBDIR templates
LINK t/unit-tests/bin/t-basic
LINK t/unit-tests/bin/t-mem-pool
LINK t/unit-tests/bin/t-strbuf
Er, what? Let's rebuild again:
$ make
SUBDIR templates
LINK t/unit-tests/bin/t-ctype
Weird. And now we ping-pong back and forth forever:
$ make
SUBDIR templates
LINK t/unit-tests/bin/t-basic
LINK t/unit-tests/bin/t-mem-pool
LINK t/unit-tests/bin/t-strbuf
$ make
SUBDIR templates
LINK t/unit-tests/bin/t-ctype
What happens is that writing t/unit-tests/bin/t-ctype updates the mtime
of the directory t/unit-tests/bin. And then on the next invocation of
make, all of those other tests are now older and so get rebuilt. And
back and forth forever.
We can fix this by using an order-only prereq. This is a GNU-ism that
tells make to only care that the dependency exists at all, and to ignore
its mtime. It was designed for exactly this sort of situation (the
documentation example even uses "mkdir").
We already rely on GNU make, so that's not a problem. This particular
feature was added in GNU make 3.80, released in October 2002. This is
obviously quite old by date, but it's also worth thinking about macOS,
as Apple stopped updating packages that switched to GPLv3 tools. In this
their dev tools ship GNU make 3.81, which is recent enough.
If it is a problem, there are two alternatives:
- we can just "mkdir -p" in the recipe to build the individual
binaries. This will mean some redundant "mkdir" calls, but only when
actually invoking the compiler.
- we could stop making the directory on the fly, and just add it with
a .gitignore of "*". This would work fine, but might be awkward when
moving back and forth in history.
Signed-off-by: Jeff King <peff@peff.net>
---
I may be overly paranoid about the ".gitignore" strategy. I feel like
I've been bitten by this in the past by things switching from source to
build (I think with git-remote-testgit). But that's an actual built
file. Git would probably be OK with the "bin/" directory coming and
going as a tracked entity, because the index really only cares about
the file "bin/.gitignore". Still, this make fix was easy enough.
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 1a62e48759..958f4cd0bf 100644
--- a/Makefile
+++ b/Makefile
@@ -3866,7 +3866,7 @@ fuzz-all: $(FUZZ_PROGRAMS)
$(UNIT_TEST_BIN):
@mkdir -p $(UNIT_TEST_BIN)
-$(UNIT_TEST_PROGS): $(UNIT_TEST_BIN)/%$X: $(UNIT_TEST_DIR)/%.o $(UNIT_TEST_DIR)/test-lib.o $(GITLIBS) GIT-LDFLAGS $(UNIT_TEST_BIN)
+$(UNIT_TEST_PROGS): $(UNIT_TEST_BIN)/%$X: $(UNIT_TEST_DIR)/%.o $(UNIT_TEST_DIR)/test-lib.o $(GITLIBS) GIT-LDFLAGS | $(UNIT_TEST_BIN)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \
$(filter %.o,$^) $(filter %.a,$^) $(LIBS)
--
2.43.0.797.g29b680fc68
^ permalink raw reply related
* [PATCH 0/2] some unit-test Makefile polishing
From: Jeff King @ 2024-01-29 3:15 UTC (permalink / raw)
To: git; +Cc: Phillip Wood
The patches fixes two small hiccups I found with the unit-tests. Neither
is a show-stopper, but mostly just small quality-of-life fixes.
[1/2]: Makefile: use order-only prereq for UNIT_TEST_BIN
[2/2]: t/Makefile: get UNIT_TESTS list from C sources
Makefile | 2 +-
t/Makefile | 4 +++-
2 files changed, 4 insertions(+), 2 deletions(-)
-Peff
^ permalink raw reply
* [PATCH] diff: handle NULL meta-info when spawning external diff
From: Jeff King @ 2024-01-29 1:57 UTC (permalink / raw)
To: Wilfred Hughes; +Cc: git
In-Reply-To: <CAFXAjY7XcL1APhLRXU8TO96z=f7957f2ieK56dHVsXUay55vpg@mail.gmail.com>
On Sun, Jan 28, 2024 at 12:24:39PM -0800, Wilfred Hughes wrote:
> It looks like git crashes if diff.external is set and the user
> compares files that have different permissions. Here's a repro:
>
> $ mkdir demo
> $ cd demo
> $ git init .
> Initialized empty Git repository in /tmp/demo/.git/
>
> $ git config diff.external /bin/echo
> $ touch foo bar
> $ chmod 755 foo
> $ git diff --no-ext-diff --no-index foo bar
> diff --git 1/foo 2/bar
> old mode 100755
> new mode 100644
>
> $ git diff --no-index foo bar
> zsh: segmentation fault (core dumped) git diff --no-index foo bar
Thanks for providing a simple reproduction recipe. There's a pretty
straight-forward fix below, though it leaves open some question of
whether there's another bug lurking with --no-index (but either way, I
think we'd want this simple fix as a first step).
-- >8 --
Subject: diff: handle NULL meta-info when spawning external diff
Running this:
$ touch foo bar
$ chmod +x foo
$ git -c diff.external=echo diff --ext-diff --no-index foo bar
results in a segfault. The issue is that run_diff_cmd() passes a NULL
"xfrm_msg" variable to run_external_diff(), which feeds it to
strvec_push(), causing the segfault. The bug dates back to 82fbf269b9
(run_external_diff: use an argv_array for the command line, 2014-04-19),
though it mostly only ever worked accidentally. Before then, we just
stuck the NULL pointer into a "const char **" array, so our NULL ended
up acting as an extra end-of-argv sentinel (which was OK, because it was
the last thing in the array).
Curiously, though, this is only a problem with --no-index. We set up
xfrm_msg by calling fill_metainfo(). This result may be empty, or may
have text like "index 1234..5678\n", "rename from foo\nrename from
bar\n", etc. In run_external_diff(), we only look at xfrm_msg if the
"other" variable is not NULL. That variable is set when the paths of the
two sides of the diff pair aren't the same (in which case the
destination path becomes "other"). So normally it would kick in only for
a rename, in which case xfrm_msg should not be NULL (it would have the
rename information in it).
But with a "--no-index" of two blobs, we of course have two different
pathnames, and thus end up with a non-NULL "other" filename (which is
always just a repeat of the file2-name), but possibly a NULL xfrm_msg.
So how to fix it? I have a feeling that --no-index always passing
"other" to the external diff command is probably a bug. There was no
rename, and the name is always redundant with existing information we
pass (and this may even cause us to pass a useless "xfrm_msg" that
contains an "index 1234..5678" line). So one option would be to change
that behavior. We don't seem to have ever documented the "other" or
"xfrm_msg" parameters for external diffs.
But I'm not sure what fallout we might have from changing that behavior
now. So this patch takes the less-risky option, and simply teaches
run_external_diff() to avoid passing xfrm_msg when it's NULL. That makes
it agnostic to whether "other" and "xfrm_msg" always come as a pair. It
fixes the segfault now, and if we want to change the --no-index "other"
behavior on top, it will handle that, too.
Reported-by: Wilfred Hughes <me@wilfred.me.uk>
Signed-off-by: Jeff King <peff@peff.net>
---
diff.c | 3 ++-
t/t4053-diff-no-index.sh | 12 ++++++++++++
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/diff.c b/diff.c
index a89a6a6128..ccfa1fca0d 100644
--- a/diff.c
+++ b/diff.c
@@ -4384,7 +4384,8 @@ static void run_external_diff(const char *pgm,
add_external_diff_name(o->repo, &cmd.args, two);
if (other) {
strvec_push(&cmd.args, other);
- strvec_push(&cmd.args, xfrm_msg);
+ if (xfrm_msg)
+ strvec_push(&cmd.args, xfrm_msg);
}
}
diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
index 5ce345d309..651ec77660 100755
--- a/t/t4053-diff-no-index.sh
+++ b/t/t4053-diff-no-index.sh
@@ -205,6 +205,18 @@ test_expect_success POSIXPERM,SYMLINKS 'diff --no-index normalizes: mode not lik
test_cmp expected actual
'
+test_expect_success POSIXPERM 'external diff with mode-only change' '
+ echo content >not-executable &&
+ echo content >executable &&
+ chmod +x executable &&
+ echo executable executable $(test_oid zero) 100755 \
+ not-executable $(test_oid zero) 100644 not-executable \
+ >expect &&
+ test_expect_code 1 git -c diff.external=echo diff \
+ --no-index executable not-executable >actual &&
+ test_cmp expect actual
+'
+
test_expect_success "diff --no-index treats '-' as stdin" '
cat >expect <<-EOF &&
diff --git a/- b/a/1
--
2.43.0.794.g6ebf4d0024
^ permalink raw reply related
* [PATCH v4 8/8] completion: add tests for git-bisect
From: Britton Leo Kerin @ 2024-01-28 22:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Patrick Steinhardt, Britton Leo Kerin
In-Reply-To: <20240128223447.342493-1-britton.kerin@gmail.com>
There aren't any tests for completion of git bisect and it's
subcommands.
Add tests.
---
t/t9902-completion.sh | 135 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 135 insertions(+)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index aa9a614de3..698e278450 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1259,6 +1259,141 @@ test_expect_success 'git switch - with no options, complete local branches and u
EOF
'
+test_expect_success 'git bisect - when not bisecting, complete only replay and start subcommands' '
+ test_completion "git bisect " <<-\EOF
+ replay Z
+ start Z
+ EOF
+'
+
+test_expect_success 'git bisect - complete options to start subcommand' '
+ test_completion "git bisect start --" <<-\EOF
+ --term-new Z
+ --term-bad Z
+ --term-old Z
+ --term-good Z
+ --no-checkout Z
+ --first-parent Z
+ EOF
+'
+
+test_expect_success 'setup for git-bisect tests requiring a repo' '
+ git init git-bisect &&
+ (
+ cd git-bisect &&
+ echo "initial contents" >file &&
+ git add file &&
+ git commit -am "Initial commit" &&
+ git tag initial &&
+ echo "new line" >>file &&
+ git commit -am "First change" &&
+ echo "another new line" >>file &&
+ git commit -am "Second change" &&
+ git tag final
+ )
+'
+
+test_expect_success 'git bisect - start subcommand arguments before double-dash are completed as revs' '
+ (
+ cd git-bisect &&
+ test_completion "git bisect start " <<-\EOF
+ HEAD Z
+ final Z
+ initial Z
+ master Z
+ EOF
+ )
+'
+
+# Note that these arguments are <pathspec>s, which in practice the fallback
+# completion (not the git completion) later ends up completing as paths.
+test_expect_success 'git bisect - start subcommand arguments after double-dash are not completed' '
+ (
+ cd git-bisect &&
+ test_completion "git bisect start final initial -- " ""
+ )
+'
+
+test_expect_success 'setup for git-bisect tests requiring ongoing bisection' '
+ (
+ cd git-bisect &&
+ git bisect start --term-new=custom_new --term-old=custom_old final initial
+ )
+'
+
+test_expect_success 'git-bisect - when bisecting all subcommands are candidates' '
+ (
+ cd git-bisect &&
+ 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
+ replay Z
+ log Z
+ run Z
+ help Z
+ EOF
+ )
+'
+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-bisect - git-log options to visualize subcommand are candidates' '
+ (
+ cd git-bisect &&
+ # The completion used for git-log and here does not complete
+ # every git-log option, so rather than hope to stay in sync
+ # with exactly what it does we will just spot-test here.
+ test_completion "git bisect visualize --sta" <<-\EOF &&
+ --stat Z
+ EOF
+ test_completion "git bisect visualize --summar" <<-\EOF
+ --summary Z
+ EOF
+ )
+'
+
+test_expect_success 'git-bisect - view subcommand is not a candidate' '
+ (
+ cd git-bisect &&
+ test_completion "git bisect vi" <<-\EOF
+ visualize Z
+ EOF
+ )
+'
+
+test_expect_success 'git-bisect - existing view subcommand is recognized and enables completion of git-log options' '
+ (
+ cd git-bisect &&
+ # The completion used for git-log and here does not complete
+ # every git-log option, so rather than hope to stay in sync
+ # with exactly what it does we will just spot-test here.
+ test_completion "git bisect view --sta" <<-\EOF &&
+ --stat Z
+ EOF
+ test_completion "git bisect view --summar" <<-\EOF
+ --summary 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
* [PATCH v4 7/8] completion: bisect: recognize but do not complete view subcommand
From: Britton Leo Kerin @ 2024-01-28 22:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Patrick Steinhardt, Britton Leo Kerin
In-Reply-To: <20240128223447.342493-1-britton.kerin@gmail.com>
The "view" alias for the visualize subcommand is neither completed nor
recognized. It's undesirable to complete it because it's first letters
are the same as for visualize, making completion less rather than more
efficient without adding much in the way of interface discovery.
However, it needs to be recognized in order to enable log option
completion for it.
Recognize but do not complete the view command by creating and using
separate lists of completable_subcommands and all_subcommands.
Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
contrib/completion/git-completion.bash | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index ae16e742a4..0cf1a5a393 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1462,12 +1462,19 @@ _git_bisect ()
# 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")"
+ #
+ # We want to recognize 'view' but not complete it, because it overlaps
+ # with 'visualize' too much and is just an alias for it.
+ #
+ local completable_subcommands="start bad new $term_bad good old $term_good terms skip reset visualize replay log run help"
+ local all_subcommands="$completable_subcommands view"
+
+ local subcommand="$(__git_find_on_cmdline "$all_subcommands")"
+
if [ -z "$subcommand" ]; then
__git_find_repo_path
if [ -f "$__git_repo_path"/BISECT_START ]; then
- __gitcomp "$subcommands"
+ __gitcomp "$completable_subcommands"
else
__gitcomp "replay start"
fi
@@ -1490,7 +1497,7 @@ _git_bisect ()
__gitcomp "--term-good --term-old --term-bad --term-new"
return
;;
- visualize)
+ visualize|view)
__git_complete_log_opts
return
;;
--
2.43.0
^ permalink raw reply related
* [PATCH v4 6/8] completion: bisect: complete log opts for visualize subcommand
From: Britton Leo Kerin @ 2024-01-28 22:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Patrick Steinhardt, Britton Leo Kerin
In-Reply-To: <20240128223447.342493-1-britton.kerin@gmail.com>
Arguments passed to the "visualize" subcommand of git-bisect(1) get
forwarded to git-log(1). It thus supports the same options as git-log(1)
would, but our Bash completion script does not know to handle this.
Make completion of porcelain git-log options and option arguments to the
visualize subcommand work by calling __git_complete_log_opts when the
start of an option to the subcommand is seen (visualize doesn't support
any options besides the git-log options).
Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
contrib/completion/git-completion.bash | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 41c76c1246..ae16e742a4 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1490,6 +1490,10 @@ _git_bisect ()
__gitcomp "--term-good --term-old --term-bad --term-new"
return
;;
+ visualize)
+ __git_complete_log_opts
+ return
+ ;;
bad|new|"$term_bad"|good|old|"$term_good"|reset|skip)
__git_complete_refs
;;
--
2.43.0
^ permalink raw reply related
* [PATCH v4 5/8] completion: log: use __git_complete_log_opts
From: Britton Leo Kerin @ 2024-01-28 22:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Patrick Steinhardt, Britton Leo Kerin
In-Reply-To: <20240128223447.342493-1-britton.kerin@gmail.com>
Use the new __git_complete_log_opts function to handle option and
optiona rgument completion in _git_log.
Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
contrib/completion/git-completion.bash | 95 +-------------------------
1 file changed, 3 insertions(+), 92 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index dfd504c37e..41c76c1246 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2195,98 +2195,9 @@ _git_log ()
__git_has_doubledash && return
__git_find_repo_path
- local merge=""
- if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
- merge="--merge"
- fi
- case "$prev,$cur" in
- -L,:*:*)
- return # fall back to Bash filename completion
- ;;
- -L,:*)
- __git_complete_symbol --cur="${cur#:}" --sfx=":"
- return
- ;;
- -G,*|-S,*)
- __git_complete_symbol
- return
- ;;
- esac
- case "$cur" in
- --pretty=*|--format=*)
- __gitcomp "$__git_log_pretty_formats $(__git_pretty_aliases)
- " "" "${cur#*=}"
- return
- ;;
- --date=*)
- __gitcomp "$__git_log_date_formats" "" "${cur##--date=}"
- return
- ;;
- --decorate=*)
- __gitcomp "full short no" "" "${cur##--decorate=}"
- return
- ;;
- --diff-algorithm=*)
- __gitcomp "$__git_diff_algorithms" "" "${cur##--diff-algorithm=}"
- return
- ;;
- --submodule=*)
- __gitcomp "$__git_diff_submodule_formats" "" "${cur##--submodule=}"
- return
- ;;
- --ws-error-highlight=*)
- __gitcomp "$__git_ws_error_highlight_opts" "" "${cur##--ws-error-highlight=}"
- return
- ;;
- --no-walk=*)
- __gitcomp "sorted unsorted" "" "${cur##--no-walk=}"
- return
- ;;
- --diff-merges=*)
- __gitcomp "$__git_diff_merges_opts" "" "${cur##--diff-merges=}"
- return
- ;;
- --*)
- __gitcomp "
- $__git_log_common_options
- $__git_log_shortlog_options
- $__git_log_gitk_options
- $__git_log_show_options
- --root --topo-order --date-order --reverse
- --follow --full-diff
- --abbrev-commit --no-abbrev-commit --abbrev=
- --relative-date --date=
- --pretty= --format= --oneline
- --show-signature
- --cherry-mark
- --cherry-pick
- --graph
- --decorate --decorate= --no-decorate
- --walk-reflogs
- --no-walk --no-walk= --do-walk
- --parents --children
- --expand-tabs --expand-tabs= --no-expand-tabs
- $merge
- $__git_diff_common_options
- "
- return
- ;;
- -L:*:*)
- return # fall back to Bash filename completion
- ;;
- -L:*)
- __git_complete_symbol --cur="${cur#-L:}" --sfx=":"
- return
- ;;
- -G*)
- __git_complete_symbol --pfx="-G" --cur="${cur#-G}"
- return
- ;;
- -S*)
- __git_complete_symbol --pfx="-S" --cur="${cur#-S}"
- return
- ;;
- esac
+ __git_complete_log_opts
+ [ -z "$COMPREPLY" ] || return
+
__git_complete_revlist
}
--
2.43.0
^ permalink raw reply related
* [PATCH v4 1/8] completion: bisect: complete bad, new, old, and help subcommands
From: Britton Leo Kerin @ 2024-01-28 22:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Patrick Steinhardt, Britton Leo Kerin
In-Reply-To: <20240128223447.342493-1-britton.kerin@gmail.com>
The bad, new, old and help subcommands to git-bisect(1) are not
completed.
Add the bad, new, old, and help subcommands to the appropriate lists
such that the commands and their possible ref arguments are completed.
Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
contrib/completion/git-completion.bash | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 185b47d802..06d0b156e7 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1449,7 +1449,7 @@ _git_bisect ()
{
__git_has_doubledash && return
- local subcommands="start bad good skip reset visualize replay log run"
+ local subcommands="start bad new good old skip reset visualize replay log run help"
local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
__git_find_repo_path
@@ -1462,7 +1462,7 @@ _git_bisect ()
fi
case "$subcommand" in
- bad|good|reset|skip|start)
+ bad|new|good|old|reset|skip|start)
__git_complete_refs
;;
*)
--
2.43.0
^ permalink raw reply related
* [PATCH v4 0/8] completion: improvements for git-bisect
From: Britton Leo Kerin @ 2024-01-28 22:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Patrick Steinhardt, Britton Leo Kerin
In-Reply-To: <20240118204323.1113859-1-britton.kerin@gmail.com>
Relative to v3 this reworks the commit contents and descriptions
according to review suggestions, removes unnecessary case statements and
precondition, adds option completion for the terms subcommand, and adds
tests.
Britton Leo Kerin (8):
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: log: use __git_complete_log_opts
completion: bisect: complete log opts for visualize subcommand
completion: bisect: recognize but do not complete view subcommand
completion: add tests for git-bisect
contrib/completion/git-completion.bash | 65 ++++++++++--
t/t9902-completion.sh | 135 +++++++++++++++++++++++++
2 files changed, 193 insertions(+), 7 deletions(-)
Range-diff against v3:
1: e16264bfb9 ! 1: 66153024c3 completion: complete new old actions, start opts
@@ Metadata
Author: Britton Leo Kerin <britton.kerin@gmail.com>
## Commit message ##
- completion: complete new old actions, start opts
+ completion: bisect: complete bad, new, old, and help subcommands
+
+ The bad, new, old and help subcommands to git-bisect(1) are not
+ completed.
+
+ Add the bad, new, old, and help subcommands to the appropriate lists
+ such that the commands and their possible ref arguments are completed.
Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
@@ contrib/completion/git-completion.bash: _git_bisect ()
__git_has_doubledash && return
- local subcommands="start bad good skip reset visualize replay log run"
-+ local subcommands="start bad new good old terms skip reset visualize replay log run help"
++ local subcommands="start bad new good old skip reset visualize replay log run help"
local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
__git_find_repo_path
@@ contrib/completion/git-completion.bash: _git_bisect ()
case "$subcommand" in
- bad|good|reset|skip|start)
-+ start)
-+ case "$cur" in
-+ --*)
-+ __gitcomp "--term-new --term-bad --term-old --term-good --first-parent --no-checkout"
-+ return
-+ ;;
-+ *)
-+ ;;
-+ esac
-+ ;;
-+ esac
-+
-+ case "$subcommand" in
+ bad|new|good|old|reset|skip|start)
__git_complete_refs
;;
2: 130abe3460 < -: ---------- completion: git-log opts to bisect visualize
-: ---------- > 2: 7eb8c842a3 completion: bisect: complete custom terms and related options
-: ---------- > 3: 5f5076bb93 completion: bisect: complete missing --first-parent and --no-checkout options
3: d659ace9c2 ! 4: c8ffa0e915 completion: move to maintain define-before-use
@@ Metadata
Author: Britton Leo Kerin <britton.kerin@gmail.com>
## Commit message ##
- completion: move to maintain define-before-use
+ completion: new function __git_complete_log_opts
+
+ The options accepted by git-log are also accepted by at least one other
+ command (git-bisect). Prepare to factor out the common option and
+ option argument completion code by defining a new function.
Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
## contrib/completion/git-completion.bash ##
-@@ contrib/completion/git-completion.bash: _git_archive ()
- __git_complete_file
- }
+@@ contrib/completion/git-completion.bash: __git_diff_merges_opts="off none on first-parent 1 separate m combined c dense-c
+ __git_log_pretty_formats="oneline short medium full fuller reference email raw format: tformat: mboxrd"
+ __git_log_date_formats="relative iso8601 iso8601-strict rfc2822 short local default human raw unix auto: format:"
-+# Options that go well for log, shortlog and gitk
-+__git_log_common_options="
-+ --not --all
-+ --branches --tags --remotes
-+ --first-parent --merges --no-merges
-+ --max-count=
-+ --max-age= --since= --after=
-+ --min-age= --until= --before=
-+ --min-parents= --max-parents=
-+ --no-min-parents --no-max-parents
-+"
-+# Options that go well for log and gitk (not shortlog)
-+__git_log_gitk_options="
-+ --dense --sparse --full-history
-+ --simplify-merges --simplify-by-decoration
-+ --left-right --notes --no-notes
-+"
-+# Options that go well for log and shortlog (not gitk)
-+__git_log_shortlog_options="
-+ --author= --committer= --grep=
-+ --all-match --invert-grep
-+"
-+# Options accepted by log and show
-+__git_log_show_options="
-+ --diff-merges --diff-merges= --no-diff-merges --dd --remerge-diff
-+"
-+
-+__git_diff_merges_opts="off none on first-parent 1 separate m combined c dense-combined cc remerge r"
-+
-+__git_log_pretty_formats="oneline short medium full fuller reference email raw format: tformat: mboxrd"
-+__git_log_date_formats="relative iso8601 iso8601-strict rfc2822 short local default human raw unix auto: format:"
-+
-+# Check for only porcelain (i.e. not git-rev-list) option (not argument)
-+# and selected option argument completions for git-log options and if any
-+# are found put them in COMPREPLY. COMPREPLY must be empty at the start,
-+# and will be empty on return if no candidates are found.
++# Complete porcelain (i.e. not git-rev-list) options and at least some
++# option arguments accepted by git-log. Note that this same set of options
++# are also accepted by some other git commands besides git-log.
+__git_complete_log_opts ()
+{
-+ [ -z "$COMPREPLY" ] || return 1 # Precondition
++ COMPREPLY=""
+
+ local merge=""
+ if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
@@ contrib/completion/git-completion.bash: _git_archive ()
+ esac
+}
+
- _git_bisect ()
- {
- __git_has_doubledash && return
-@@ contrib/completion/git-completion.bash: _git_ls_tree ()
- __git_complete_file
- }
-
--# Options that go well for log, shortlog and gitk
--__git_log_common_options="
-- --not --all
-- --branches --tags --remotes
-- --first-parent --merges --no-merges
-- --max-count=
-- --max-age= --since= --after=
-- --min-age= --until= --before=
-- --min-parents= --max-parents=
-- --no-min-parents --no-max-parents
--"
--# Options that go well for log and gitk (not shortlog)
--__git_log_gitk_options="
-- --dense --sparse --full-history
-- --simplify-merges --simplify-by-decoration
-- --left-right --notes --no-notes
--"
--# Options that go well for log and shortlog (not gitk)
--__git_log_shortlog_options="
-- --author= --committer= --grep=
-- --all-match --invert-grep
--"
--# Options accepted by log and show
--__git_log_show_options="
-- --diff-merges --diff-merges= --no-diff-merges --dd --remerge-diff
--"
--
--__git_diff_merges_opts="off none on first-parent 1 separate m combined c dense-combined cc remerge r"
--
--__git_log_pretty_formats="oneline short medium full fuller reference email raw format: tformat: mboxrd"
--__git_log_date_formats="relative iso8601 iso8601-strict rfc2822 short local default human raw unix auto: format:"
--
--
--# Check for only porcelain (i.e. not git-rev-list) option (not argument)
--# and selected option argument completions for git-log options and if any
--# are found put them in COMPREPLY. COMPREPLY must be empty at the start,
--# and will be empty on return if no candidates are found.
--__git_complete_log_opts ()
--{
-- [ -z "$COMPREPLY" ] || return 1 # Precondition
--
-- local merge=""
-- if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
-- merge="--merge"
-- fi
-- case "$prev,$cur" in
-- -L,:*:*)
-- return # fall back to Bash filename completion
-- ;;
-- -L,:*)
-- __git_complete_symbol --cur="${cur#:}" --sfx=":"
-- return
-- ;;
-- -G,*|-S,*)
-- __git_complete_symbol
-- return
-- ;;
-- esac
-- case "$cur" in
-- --pretty=*|--format=*)
-- __gitcomp "$__git_log_pretty_formats $(__git_pretty_aliases)
-- " "" "${cur#*=}"
-- return
-- ;;
-- --date=*)
-- __gitcomp "$__git_log_date_formats" "" "${cur##--date=}"
-- return
-- ;;
-- --decorate=*)
-- __gitcomp "full short no" "" "${cur##--decorate=}"
-- return
-- ;;
-- --diff-algorithm=*)
-- __gitcomp "$__git_diff_algorithms" "" "${cur##--diff-algorithm=}"
-- return
-- ;;
-- --submodule=*)
-- __gitcomp "$__git_diff_submodule_formats" "" "${cur##--submodule=}"
-- return
-- ;;
-- --ws-error-highlight=*)
-- __gitcomp "$__git_ws_error_highlight_opts" "" "${cur##--ws-error-highlight=}"
-- return
-- ;;
-- --no-walk=*)
-- __gitcomp "sorted unsorted" "" "${cur##--no-walk=}"
-- return
-- ;;
-- --diff-merges=*)
-- __gitcomp "$__git_diff_merges_opts" "" "${cur##--diff-merges=}"
-- return
-- ;;
-- --*)
-- __gitcomp "
-- $__git_log_common_options
-- $__git_log_shortlog_options
-- $__git_log_gitk_options
-- $__git_log_show_options
-- --root --topo-order --date-order --reverse
-- --follow --full-diff
-- --abbrev-commit --no-abbrev-commit --abbrev=
-- --relative-date --date=
-- --pretty= --format= --oneline
-- --show-signature
-- --cherry-mark
-- --cherry-pick
-- --graph
-- --decorate --decorate= --no-decorate
-- --walk-reflogs
-- --no-walk --no-walk= --do-walk
-- --parents --children
-- --expand-tabs --expand-tabs= --no-expand-tabs
-- $merge
-- $__git_diff_common_options
-- "
-- return
-- ;;
-- -L:*:*)
-- return # fall back to Bash filename completion
-- ;;
-- -L:*)
-- __git_complete_symbol --cur="${cur#-L:}" --sfx=":"
-- return
-- ;;
-- -G*)
-- __git_complete_symbol --pfx="-G" --cur="${cur#-G}"
-- return
-- ;;
-- -S*)
-- __git_complete_symbol --pfx="-S" --cur="${cur#-S}"
-- return
-- ;;
-- esac
--}
--
_git_log ()
{
__git_has_doubledash && return
4: c5bee633b2 < -: ---------- completion: custom git-bisect terms
5: 2bd0cb26f1 < -: ---------- completion: git-bisect view recognized but not completed
-: ---------- > 5: 733613d1ed completion: log: use __git_complete_log_opts
-: ---------- > 6: 06f5973b3b completion: bisect: complete log opts for visualize subcommand
-: ---------- > 7: 1dc9323f24 completion: bisect: recognize but do not complete view subcommand
-: ---------- > 8: 451b7a4467 completion: add tests for git-bisect
--
2.43.0
^ permalink raw reply
* [PATCH v4 3/8] completion: bisect: complete missing --first-parent and --no-checkout options
From: Britton Leo Kerin @ 2024-01-28 22:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Patrick Steinhardt, Britton Leo Kerin
In-Reply-To: <20240128223447.342493-1-britton.kerin@gmail.com>
The --first-parent and --no-checkout options to the start subcommand of
git-bisect(1) are not completed.
Enable completion of the --first-parent and --no-checkout options to the
start subcommand.
Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
contrib/completion/git-completion.bash | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8baf330824..2ed600244a 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1478,7 +1478,7 @@ _git_bisect ()
start)
case "$cur" in
--*)
- __gitcomp "--term-new --term-bad --term-old --term-good"
+ __gitcomp "--first-parent --no-checkout --term-new --term-bad --term-old --term-good"
return
;;
*)
--
2.43.0
^ permalink raw reply related
* [PATCH v4 4/8] completion: new function __git_complete_log_opts
From: Britton Leo Kerin @ 2024-01-28 22:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Patrick Steinhardt, Britton Leo Kerin
In-Reply-To: <20240128223447.342493-1-britton.kerin@gmail.com>
The options accepted by git-log are also accepted by at least one other
command (git-bisect). Prepare to factor out the common option and
option argument completion code by defining a new function.
Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
contrib/completion/git-completion.bash | 101 +++++++++++++++++++++++++
1 file changed, 101 insertions(+)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2ed600244a..dfd504c37e 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2089,6 +2089,107 @@ __git_diff_merges_opts="off none on first-parent 1 separate m combined c dense-c
__git_log_pretty_formats="oneline short medium full fuller reference email raw format: tformat: mboxrd"
__git_log_date_formats="relative iso8601 iso8601-strict rfc2822 short local default human raw unix auto: format:"
+# Complete porcelain (i.e. not git-rev-list) options and at least some
+# option arguments accepted by git-log. Note that this same set of options
+# are also accepted by some other git commands besides git-log.
+__git_complete_log_opts ()
+{
+ COMPREPLY=""
+
+ local merge=""
+ if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
+ merge="--merge"
+ fi
+ case "$prev,$cur" in
+ -L,:*:*)
+ return # fall back to Bash filename completion
+ ;;
+ -L,:*)
+ __git_complete_symbol --cur="${cur#:}" --sfx=":"
+ return
+ ;;
+ -G,*|-S,*)
+ __git_complete_symbol
+ return
+ ;;
+ esac
+ case "$cur" in
+ --pretty=*|--format=*)
+ __gitcomp "$__git_log_pretty_formats $(__git_pretty_aliases)
+ " "" "${cur#*=}"
+ return
+ ;;
+ --date=*)
+ __gitcomp "$__git_log_date_formats" "" "${cur##--date=}"
+ return
+ ;;
+ --decorate=*)
+ __gitcomp "full short no" "" "${cur##--decorate=}"
+ return
+ ;;
+ --diff-algorithm=*)
+ __gitcomp "$__git_diff_algorithms" "" "${cur##--diff-algorithm=}"
+ return
+ ;;
+ --submodule=*)
+ __gitcomp "$__git_diff_submodule_formats" "" "${cur##--submodule=}"
+ return
+ ;;
+ --ws-error-highlight=*)
+ __gitcomp "$__git_ws_error_highlight_opts" "" "${cur##--ws-error-highlight=}"
+ return
+ ;;
+ --no-walk=*)
+ __gitcomp "sorted unsorted" "" "${cur##--no-walk=}"
+ return
+ ;;
+ --diff-merges=*)
+ __gitcomp "$__git_diff_merges_opts" "" "${cur##--diff-merges=}"
+ return
+ ;;
+ --*)
+ __gitcomp "
+ $__git_log_common_options
+ $__git_log_shortlog_options
+ $__git_log_gitk_options
+ $__git_log_show_options
+ --root --topo-order --date-order --reverse
+ --follow --full-diff
+ --abbrev-commit --no-abbrev-commit --abbrev=
+ --relative-date --date=
+ --pretty= --format= --oneline
+ --show-signature
+ --cherry-mark
+ --cherry-pick
+ --graph
+ --decorate --decorate= --no-decorate
+ --walk-reflogs
+ --no-walk --no-walk= --do-walk
+ --parents --children
+ --expand-tabs --expand-tabs= --no-expand-tabs
+ $merge
+ $__git_diff_common_options
+ "
+ return
+ ;;
+ -L:*:*)
+ return # fall back to Bash filename completion
+ ;;
+ -L:*)
+ __git_complete_symbol --cur="${cur#-L:}" --sfx=":"
+ return
+ ;;
+ -G*)
+ __git_complete_symbol --pfx="-G" --cur="${cur#-G}"
+ return
+ ;;
+ -S*)
+ __git_complete_symbol --pfx="-S" --cur="${cur#-S}"
+ return
+ ;;
+ esac
+}
+
_git_log ()
{
__git_has_doubledash && return
--
2.43.0
^ permalink raw reply related
* [PATCH v4 2/8] completion: bisect: complete custom terms and related options
From: Britton Leo Kerin @ 2024-01-28 22:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Patrick Steinhardt, Britton Leo Kerin
In-Reply-To: <20240128223447.342493-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
them if a bisection is in progress and adding them to the list of
subcommands.
Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
contrib/completion/git-completion.bash | 32 ++++++++++++++++++++++++--
1 file changed, 30 insertions(+), 2 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 06d0b156e7..8baf330824 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_START ]; 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
;;
*)
--
2.43.0
^ permalink raw reply related
* [PATCH v3] credential/wincred: store oauth_refresh_token
From: M Hickford via GitGitGadget @ 2024-01-28 21:25 UTC (permalink / raw)
To: git
Cc: Jeff King, Taylor Blau, Matthew John Cheetham, git-for-windows,
Jakub Bereżański, patthoyts, M Hickford, M Hickford
In-Reply-To: <pull.1534.v2.git.1699251395093.gitgitgadget@gmail.com>
From: M Hickford <mirth.hickford@gmail.com>
a5c7656 (credential: new attribute oauth_refresh_token) introduced
a new confidential credential attribute and added support to
credential-cache. Later 0ce02e2f (credential/libsecret: store new
attributes, 2023-06-16) added support in credential-libsecret.
To add support in credential-wincred, we encode the new attribute in the
CredentialBlob, separated by newline:
hunter2
oauth_refresh_token=xyzzy
This is extensible and backwards compatible. The credential protocol
already assumes that attribute values do not contain newlines.
This fixes test "helper (wincred) gets oauth_refresh_token" when
t0303-credential-external.sh is run with
GIT_TEST_CREDENTIAL_HELPER=wincred. This test was added in a5c76569e7
(credential: new attribute oauth_refresh_token, 2023-04-21).
Alternatives considered: store oauth_refresh_token in a wincred
attribute. This would be insecure because wincred assumes attribute
values to be non-confidential.
Signed-off-by: M Hickford <mirth.hickford@gmail.com>
---
credential/wincred: store oauth_refresh_token
Patch v3 fixes a maybe-uninitialized warning.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1534%2Fhickford%2Fwincred-refresh-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1534/hickford/wincred-refresh-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1534
Range-diff vs v2:
1: 12f092f45b1 ! 1: a324c3aac41 credential/wincred: store oauth_refresh_token
@@ Commit message
credential/wincred: store oauth_refresh_token
a5c7656 (credential: new attribute oauth_refresh_token) introduced
- a new confidential credential attribute for helpers to store.
+ a new confidential credential attribute and added support to
+ credential-cache. Later 0ce02e2f (credential/libsecret: store new
+ attributes, 2023-06-16) added support in credential-libsecret.
- We encode the new attribute in the CredentialBlob, separated by
- newline:
+ To add support in credential-wincred, we encode the new attribute in the
+ CredentialBlob, separated by newline:
hunter2
oauth_refresh_token=xyzzy
@@ contrib/credential/wincred/git-credential-wincred.c: static void get_credential(
+ }
+ line = wcstok_s(NULL, L"\r\n", &remaining_lines);
+ }
++ free(secret);
+ } else {
+ write_item("password",
+ (LPCWSTR)creds[i]->CredentialBlob,
@@ contrib/credential/wincred/git-credential-wincred.c: static void get_credential(
for (int j = 0; j < creds[i]->AttributeCount; j++) {
attr = creds[i]->Attributes + j;
if (!wcscmp(attr->Keyword, L"git_password_expiry_utc")) {
-@@ contrib/credential/wincred/git-credential-wincred.c: static void get_credential(void)
- break;
- }
- }
-+ free(secret);
- break;
- }
-
@@ contrib/credential/wincred/git-credential-wincred.c: static void store_credential(void)
{
CREDENTIALW cred;
.../wincred/git-credential-wincred.c | 46 ++++++++++++++++---
1 file changed, 40 insertions(+), 6 deletions(-)
diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c
index 4cd56c42e24..4be0d58cd89 100644
--- a/contrib/credential/wincred/git-credential-wincred.c
+++ b/contrib/credential/wincred/git-credential-wincred.c
@@ -35,7 +35,7 @@ static void *xmalloc(size_t size)
}
static WCHAR *wusername, *password, *protocol, *host, *path, target[1024],
- *password_expiry_utc;
+ *password_expiry_utc, *oauth_refresh_token;
static void write_item(const char *what, LPCWSTR wbuf, int wlen)
{
@@ -140,6 +140,11 @@ static void get_credential(void)
DWORD num_creds;
int i;
CREDENTIAL_ATTRIBUTEW *attr;
+ WCHAR *secret;
+ WCHAR *line;
+ WCHAR *remaining_lines;
+ WCHAR *part;
+ WCHAR *remaining_parts;
if (!CredEnumerateW(L"git:*", 0, &num_creds, &creds))
return;
@@ -149,9 +154,24 @@ static void get_credential(void)
if (match_cred(creds[i], 0)) {
write_item("username", creds[i]->UserName,
creds[i]->UserName ? wcslen(creds[i]->UserName) : 0);
- write_item("password",
- (LPCWSTR)creds[i]->CredentialBlob,
- creds[i]->CredentialBlobSize / sizeof(WCHAR));
+ if (creds[i]->CredentialBlobSize > 0) {
+ secret = xmalloc(creds[i]->CredentialBlobSize);
+ wcsncpy_s(secret, creds[i]->CredentialBlobSize, (LPCWSTR)creds[i]->CredentialBlob, creds[i]->CredentialBlobSize / sizeof(WCHAR));
+ line = wcstok_s(secret, L"\r\n", &remaining_lines);
+ write_item("password", line, line ? wcslen(line) : 0);
+ while(line != NULL) {
+ part = wcstok_s(line, L"=", &remaining_parts);
+ if (!wcscmp(part, L"oauth_refresh_token")) {
+ write_item("oauth_refresh_token", remaining_parts, remaining_parts ? wcslen(remaining_parts) : 0);
+ }
+ line = wcstok_s(NULL, L"\r\n", &remaining_lines);
+ }
+ free(secret);
+ } else {
+ write_item("password",
+ (LPCWSTR)creds[i]->CredentialBlob,
+ creds[i]->CredentialBlobSize / sizeof(WCHAR));
+ }
for (int j = 0; j < creds[i]->AttributeCount; j++) {
attr = creds[i]->Attributes + j;
if (!wcscmp(attr->Keyword, L"git_password_expiry_utc")) {
@@ -170,16 +190,26 @@ static void store_credential(void)
{
CREDENTIALW cred;
CREDENTIAL_ATTRIBUTEW expiry_attr;
+ WCHAR *secret;
+ int wlen;
if (!wusername || !password)
return;
+ if (oauth_refresh_token) {
+ wlen = _scwprintf(L"%s\r\noauth_refresh_token=%s", password, oauth_refresh_token);
+ secret = xmalloc(sizeof(WCHAR) * wlen);
+ _snwprintf_s(secret, sizeof(WCHAR) * wlen, wlen, L"%s\r\noauth_refresh_token=%s", password, oauth_refresh_token);
+ } else {
+ secret = _wcsdup(password);
+ }
+
cred.Flags = 0;
cred.Type = CRED_TYPE_GENERIC;
cred.TargetName = target;
cred.Comment = L"saved by git-credential-wincred";
- cred.CredentialBlobSize = (wcslen(password)) * sizeof(WCHAR);
- cred.CredentialBlob = (LPVOID)password;
+ cred.CredentialBlobSize = wcslen(secret) * sizeof(WCHAR);
+ cred.CredentialBlob = (LPVOID)_wcsdup(secret);
cred.Persist = CRED_PERSIST_LOCAL_MACHINE;
cred.AttributeCount = 0;
cred.Attributes = NULL;
@@ -194,6 +224,8 @@ static void store_credential(void)
cred.TargetAlias = NULL;
cred.UserName = wusername;
+ free(secret);
+
if (!CredWriteW(&cred, 0))
die("CredWrite failed");
}
@@ -265,6 +297,8 @@ static void read_credential(void)
password = utf8_to_utf16_dup(v);
else if (!strcmp(buf, "password_expiry_utc"))
password_expiry_utc = utf8_to_utf16_dup(v);
+ else if (!strcmp(buf, "oauth_refresh_token"))
+ oauth_refresh_token = utf8_to_utf16_dup(v);
/*
* Ignore other lines; we don't know what they mean, but
* this future-proofs us when later versions of git do
base-commit: b50a608ba20348cb3dfc16a696816d51780e3f0f
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2] merge-tree: accept 3 trees as arguments
From: Johannes Schindelin via GitGitGadget @ 2024-01-28 20:34 UTC (permalink / raw)
To: git; +Cc: Eric Sunshine, Johannes Schindelin, Johannes Schindelin
In-Reply-To: <pull.1647.git.1706277694231.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
When specifying a merge base explicitly, there is actually no good
reason why the inputs need to be commits: that's only needed if the
merge base has to be deduced from the commit graph.
This commit is best viewed with `--color-moved
--color-moved-ws=allow-indentation-change`.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
merge-tree: accept 3 trees as arguments
I was asked to implement this at $dayjob and it seems like a feature
that might be useful to other users, too.
Changes since v1:
* Fixed a typo in the manual page, simplified the part after the
semicolon.
* Simplified the test case.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1647%2Fdscho%2Fallow-merge-tree-to-accept-3-trees-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1647/dscho/allow-merge-tree-to-accept-3-trees-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1647
Range-diff vs v1:
1: 29234754c06 ! 1: 233a25e9071 merge-tree: accept 3 trees as arguments
@@ Documentation/git-merge-tree.txt: OPTIONS
currently not supported. This option is incompatible with `--stdin`.
++
+As the merge-base is provided directly, <branch1> and <branch2> do not need
-+o specify commits; it is sufficient if they specify trees.
++to specify commits; trees are enough.
[[OUTPUT]]
OUTPUT
@@ t/t4301-merge-tree-write-tree.sh: test_expect_success 'check the input format wh
'
+test_expect_success '--merge-base with tree OIDs' '
-+ git merge-tree --merge-base=side1^ side1 side3 >tree &&
-+ tree=$(cat tree) &&
-+ git merge-tree --merge-base=side1^^{tree} side1^{tree} side3^{tree} >tree2 &&
-+ tree2=$(cat tree2) &&
-+ test $tree = $tree2
++ git merge-tree --merge-base=side1^ side1 side3 >with-commits &&
++ git merge-tree --merge-base=side1^^{tree} side1^{tree} side3^{tree} >with-trees &&
++ test_cmp with-commits with-trees
+'
+
test_done
Documentation/git-merge-tree.txt | 5 +++-
builtin/merge-tree.c | 42 +++++++++++++++++++-------------
t/t4301-merge-tree-write-tree.sh | 6 +++++
3 files changed, 35 insertions(+), 18 deletions(-)
diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt
index b50acace3bc..dd388fa21d5 100644
--- a/Documentation/git-merge-tree.txt
+++ b/Documentation/git-merge-tree.txt
@@ -64,10 +64,13 @@ OPTIONS
share no common history. This flag can be given to override that
check and make the merge proceed anyway.
---merge-base=<commit>::
+--merge-base=<tree-ish>::
Instead of finding the merge-bases for <branch1> and <branch2>,
specify a merge-base for the merge, and specifying multiple bases is
currently not supported. This option is incompatible with `--stdin`.
++
+As the merge-base is provided directly, <branch1> and <branch2> do not need
+to specify commits; trees are enough.
[[OUTPUT]]
OUTPUT
diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index 3bdec53fbe5..cbd8e15af6d 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -429,35 +429,43 @@ static int real_merge(struct merge_tree_options *o,
struct merge_options opt;
copy_merge_options(&opt, &o->merge_options);
- parent1 = get_merge_parent(branch1);
- if (!parent1)
- help_unknown_ref(branch1, "merge-tree",
- _("not something we can merge"));
-
- parent2 = get_merge_parent(branch2);
- if (!parent2)
- help_unknown_ref(branch2, "merge-tree",
- _("not something we can merge"));
-
opt.show_rename_progress = 0;
opt.branch1 = branch1;
opt.branch2 = branch2;
if (merge_base) {
- struct commit *base_commit;
struct tree *base_tree, *parent1_tree, *parent2_tree;
- base_commit = lookup_commit_reference_by_name(merge_base);
- if (!base_commit)
- die(_("could not lookup commit '%s'"), merge_base);
+ /*
+ * We actually only need the trees because we already
+ * have a merge base.
+ */
+ struct object_id base_oid, head_oid, merge_oid;
+
+ if (repo_get_oid_treeish(the_repository, merge_base, &base_oid))
+ die(_("could not parse as tree '%s'"), merge_base);
+ base_tree = parse_tree_indirect(&base_oid);
+ if (repo_get_oid_treeish(the_repository, branch1, &head_oid))
+ die(_("could not parse as tree '%s'"), branch1);
+ parent1_tree = parse_tree_indirect(&head_oid);
+ if (repo_get_oid_treeish(the_repository, branch2, &merge_oid))
+ die(_("could not parse as tree '%s'"), branch2);
+ parent2_tree = parse_tree_indirect(&merge_oid);
opt.ancestor = merge_base;
- base_tree = repo_get_commit_tree(the_repository, base_commit);
- parent1_tree = repo_get_commit_tree(the_repository, parent1);
- parent2_tree = repo_get_commit_tree(the_repository, parent2);
merge_incore_nonrecursive(&opt, base_tree, parent1_tree, parent2_tree, &result);
} else {
+ parent1 = get_merge_parent(branch1);
+ if (!parent1)
+ help_unknown_ref(branch1, "merge-tree",
+ _("not something we can merge"));
+
+ parent2 = get_merge_parent(branch2);
+ if (!parent2)
+ help_unknown_ref(branch2, "merge-tree",
+ _("not something we can merge"));
+
/*
* Get the merge bases, in reverse order; see comment above
* merge_incore_recursive in merge-ort.h
diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh
index 12ac4368736..c5628c4e613 100755
--- a/t/t4301-merge-tree-write-tree.sh
+++ b/t/t4301-merge-tree-write-tree.sh
@@ -945,4 +945,10 @@ test_expect_success 'check the input format when --stdin is passed' '
test_cmp expect actual
'
+test_expect_success '--merge-base with tree OIDs' '
+ git merge-tree --merge-base=side1^ side1 side3 >with-commits &&
+ git merge-tree --merge-base=side1^^{tree} side1^{tree} side3^{tree} >with-trees &&
+ test_cmp with-commits with-trees
+'
+
test_done
base-commit: e02ecfcc534e2021aae29077a958dd11c3897e4c
--
gitgitgadget
^ permalink raw reply related
* Git segfaults with diff.external and comparing files with different permissions
From: Wilfred Hughes @ 2024-01-28 20:24 UTC (permalink / raw)
To: git
Hi folks
It looks like git crashes if diff.external is set and the user
compares files that have different permissions. Here's a repro:
$ mkdir demo
$ cd demo
$ git init .
Initialized empty Git repository in /tmp/demo/.git/
$ git config diff.external /bin/echo
$ touch foo bar
$ chmod 755 foo
$ git diff --no-ext-diff --no-index foo bar
diff --git 1/foo 2/bar
old mode 100755
new mode 100644
$ git diff --no-index foo bar
zsh: segmentation fault (core dumped) git diff --no-index foo bar
This was originally reported as a difftastic bug[1] but it seems to
occur regardless of the value of diff.external, hence my repro with
/bin/echo.
[1] https://github.com/Wilfred/difftastic/issues/615
[System Info]
git version:
git version 2.43.0
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
uname: Linux 6.7.0-zen3-1-zen #1 ZEN SMP PREEMPT_DYNAMIC Sat, 13 Jan
2024 14:36:54 +0000 x86_64
compiler info: gnuc: 13.2
libc info: glibc: 2.38
$SHELL (typically, interactive shell): /bin/zsh
Thanks
Wilfred
^ permalink raw reply
* [PATCH 5/5] completion: add an use __git_compute_second_level_config_vars_for_section
From: Philippe Blain via GitGitGadget @ 2024-01-28 20:02 UTC (permalink / raw)
To: git; +Cc: Philippe Blain, Philippe Blain
In-Reply-To: <pull.1660.git.git.1706472173.gitgitgadget@gmail.com>
From: Philippe Blain <levraiphilippeblain@gmail.com>
In a previous commit we removed some hardcoded config variable names from
function __git_complete_config_variable_name in the completion script by
introducing a new function,
__git_compute_first_level_config_vars_for_section.
The remaining hardcoded config variables are "second level"
configuration variables, meaning 'branch.<name>.upstream',
'remote.<name>.url', etc. where <name> is a user-defined name.
Making use of the new --config-all-for-completion flag to 'git help'
introduced in the previous commit, add a new function,
__git_compute_second_level_config_vars_for_section. This function takes
as argument a config section name and computes the corresponding
second-level config variables, i.e. those that contain a '<' which
indicates the start of a placeholder. Note that as in
__git_compute_first_level_config_vars_for_section added previsouly, we
use indirect expansion instead of associative arrays to stay compatible
with Bash 3 on which macOS is stuck for licensing reasons.
Use this new function and the variables it defines in
__git_complete_config_variable_name to remove hardcoded config
variables, and add a test to verify the new function. Use a single
'case' for all sections with second-level variables names, since the
code for each of them is now exactly the same.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
contrib/completion/git-completion.bash | 71 ++++++++------------------
t/t9902-completion.sh | 10 ++++
2 files changed, 31 insertions(+), 50 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2934ceb7637..0e8fd63bfdb 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2596,6 +2596,13 @@ __git_compute_config_vars ()
__git_config_vars="$(git help --config-for-completion)"
}
+__git_config_vars_all=
+__git_compute_config_vars_all ()
+{
+ test -n "$__git_config_vars_all" ||
+ __git_config_vars_all="$(git help --config-all-for-completion)"
+}
+
__git_compute_first_level_config_vars_for_section ()
{
section="$1"
@@ -2605,6 +2612,15 @@ __git_compute_first_level_config_vars_for_section ()
printf -v "__git_first_level_config_vars_for_section_${section}" %s "$(echo "$__git_config_vars" | grep -E "^${section}\.[a-z]" | awk -F. '{print $2}')"
}
+__git_compute_second_level_config_vars_for_section ()
+{
+ section="$1"
+ __git_compute_config_vars_all
+ local this_section="__git_second_level_config_vars_for_section_${section}"
+ test -n "${!this_section}" ||
+ printf -v "__git_second_level_config_vars_for_section_${section}" %s "$(echo "$__git_config_vars_all" | grep -E "^${section}\.<" | awk -F. '{print $3}')"
+}
+
__git_config_sections=
__git_compute_config_sections ()
{
@@ -2749,10 +2765,13 @@ __git_complete_config_variable_name ()
done
case "$cur_" in
- branch.*.*)
+ branch.*.*|guitool.*.*|difftool.*.*|man.*.*|mergetool.*.*|remote.*.*|submodule.*.*|url.*.*)
local pfx="${cur_%.*}."
cur_="${cur_##*.}"
- __gitcomp "remote pushRemote merge mergeOptions rebase" "$pfx" "$cur_" "$sfx"
+ local section="${pfx%.*.}"
+ __git_compute_second_level_config_vars_for_section "${section}"
+ local this_section="__git_second_level_config_vars_for_section_${section}"
+ __gitcomp "${!this_section}" "$pfx" "$cur_" "$sfx"
return
;;
branch.*)
@@ -2765,33 +2784,6 @@ __git_complete_config_variable_name ()
__gitcomp_nl_append "${!this_section}" "$pfx" "$cur_" "${sfx:- }"
return
;;
- guitool.*.*)
- local pfx="${cur_%.*}."
- cur_="${cur_##*.}"
- __gitcomp "
- argPrompt cmd confirm needsFile noConsole noRescan
- prompt revPrompt revUnmerged title
- " "$pfx" "$cur_" "$sfx"
- return
- ;;
- difftool.*.*)
- local pfx="${cur_%.*}."
- cur_="${cur_##*.}"
- __gitcomp "cmd path" "$pfx" "$cur_" "$sfx"
- return
- ;;
- man.*.*)
- local pfx="${cur_%.*}."
- cur_="${cur_##*.}"
- __gitcomp "cmd path" "$pfx" "$cur_" "$sfx"
- return
- ;;
- mergetool.*.*)
- local pfx="${cur_%.*}."
- cur_="${cur_##*.}"
- __gitcomp "cmd path trustExitCode" "$pfx" "$cur_" "$sfx"
- return
- ;;
pager.*)
local pfx="${cur_%.*}."
cur_="${cur_#*.}"
@@ -2799,15 +2791,6 @@ __git_complete_config_variable_name ()
__gitcomp_nl "$__git_all_commands" "$pfx" "$cur_" "${sfx:- }"
return
;;
- remote.*.*)
- local pfx="${cur_%.*}."
- cur_="${cur_##*.}"
- __gitcomp "
- url proxy fetch push mirror skipDefaultUpdate
- receivepack uploadpack tagOpt pushurl
- " "$pfx" "$cur_" "$sfx"
- return
- ;;
remote.*)
local pfx="${cur_%.*}."
cur_="${cur_#*.}"
@@ -2818,12 +2801,6 @@ __git_complete_config_variable_name ()
__gitcomp_nl_append "${!this_section}" "$pfx" "$cur_" "${sfx:- }"
return
;;
- submodule.*.*)
- local pfx="${cur_%.*}."
- cur_="${cur_##*.}"
- __gitcomp "url update branch fetchRecurseSubmodules ignore active" "$pfx" "$cur_" "$sfx"
- return
- ;;
submodule.*)
local pfx="${cur_%.*}."
cur_="${cur_#*.}"
@@ -2834,12 +2811,6 @@ __git_complete_config_variable_name ()
__gitcomp_nl_append "${!this_section}" "$pfx" "$cur_" "${sfx:- }"
return
;;
- url.*.*)
- local pfx="${cur_%.*}."
- cur_="${cur_##*.}"
- __gitcomp "insteadOf pushInsteadOf" "$pfx" "$cur_" "$sfx"
- return
- ;;
*.*)
__git_compute_config_vars
__gitcomp "$__git_config_vars" "" "$cur_" "$sfx"
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index f28d8f531b7..24ff786b273 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -2593,6 +2593,16 @@ test_expect_success 'git config - variable name - __git_compute_first_level_conf
submodule.recurse Z
EOF
'
+test_expect_success 'git config - variable name - __git_compute_second_level_config_vars_for_section' '
+ test_completion "git config branch.main." <<-\EOF
+ branch.main.description Z
+ branch.main.remote Z
+ branch.main.pushRemote Z
+ branch.main.merge Z
+ branch.main.mergeOptions Z
+ branch.main.rebase Z
+ EOF
+'
test_expect_success 'git config - value' '
test_completion "git config color.pager " <<-\EOF
--
gitgitgadget
^ permalink raw reply related
* [PATCH 4/5] builtin/help: add --config-all-for-completion
From: Philippe Blain via GitGitGadget @ 2024-01-28 20:02 UTC (permalink / raw)
To: git; +Cc: Philippe Blain, Philippe Blain
In-Reply-To: <pull.1660.git.git.1706472173.gitgitgadget@gmail.com>
From: Philippe Blain <levraiphilippeblain@gmail.com>
There is currently no machine-friendly way to show _all_ configuration
variables from the command line. 'git help --config' does show them all,
but it also sets up the pager. 'git help --config-for-completion' omits
some variables (those containing wildcards, for example) and 'git help
--config-section-for-completion' shows only top-level section names.
In a following commit we will want to have access to a list of all
configuration variables from the Bash completion script. As such, add a
new mode for the command, HELP_ACTION_CONFIG_ALL_FOR_COMPLETION,
triggered by the new option '--config-all-for-completion'. In this mode,
show all variables, just as HELP_ACTION_CONFIG, but do not set up the
pager.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
builtin/help.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/builtin/help.c b/builtin/help.c
index dc1fbe2b986..dacaeb10bf4 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -50,6 +50,7 @@ static enum help_action {
HELP_ACTION_DEVELOPER_INTERFACES,
HELP_ACTION_CONFIG_FOR_COMPLETION,
HELP_ACTION_CONFIG_SECTIONS_FOR_COMPLETION,
+ HELP_ACTION_CONFIG_ALL_FOR_COMPLETION,
} cmd_mode;
static const char *html_path;
@@ -86,6 +87,8 @@ static struct option builtin_help_options[] = {
HELP_ACTION_CONFIG_FOR_COMPLETION, PARSE_OPT_HIDDEN),
OPT_CMDMODE_F(0, "config-sections-for-completion", &cmd_mode, "",
HELP_ACTION_CONFIG_SECTIONS_FOR_COMPLETION, PARSE_OPT_HIDDEN),
+ OPT_CMDMODE_F(0, "config-all-for-completion", &cmd_mode, "",
+ HELP_ACTION_CONFIG_ALL_FOR_COMPLETION, PARSE_OPT_HIDDEN),
OPT_END(),
};
@@ -670,6 +673,10 @@ int cmd_help(int argc, const char **argv, const char *prefix)
opt_mode_usage(argc, "--config-for-completion", help_format);
list_config_help(SHOW_CONFIG_VARS);
return 0;
+ case HELP_ACTION_CONFIG_ALL_FOR_COMPLETION:
+ opt_mode_usage(argc, "--config-all-for-completion", help_format);
+ list_config_help(SHOW_CONFIG_HUMAN);
+ return 0;
case HELP_ACTION_USER_INTERFACES:
opt_mode_usage(argc, "--user-interfaces", help_format);
list_user_interfaces_help();
--
gitgitgadget
^ permalink raw reply related
* [PATCH 3/5] completion: add and use __git_compute_first_level_config_vars_for_section
From: Philippe Blain via GitGitGadget @ 2024-01-28 20:02 UTC (permalink / raw)
To: git; +Cc: Philippe Blain, Philippe Blain
In-Reply-To: <pull.1660.git.git.1706472173.gitgitgadget@gmail.com>
From: Philippe Blain <levraiphilippeblain@gmail.com>
The function __git_complete_config_variable_name in the Bash completion
script hardcodes several config variable names. These variables are
those in config section where user-defined names can appear, such as
"branch.<name>". These sections are treated first by the case statement,
and the two last "catch all" cases are used for other sections, making
use of the __git_compute_config_vars and __git_compute_config_sections
function, which omit listing any variables containing wildcards or
placeholders. Having hardcoded config variables introduces the risk of
the completion code becoming out of sync with the actual config
variables accepted by Git.
To avoid these hardcoded config variables, introduce a new function,
__git_compute_first_level_config_vars_for_section, making use of the
existing __git_config_vars variable. This function takes as argument a
config section name and computes the matching "first level" config
variables for that section, i.e. those _not_ containing any placeholder,
like 'branch.autoSetupMerge, 'remote.pushDefault', etc. Use this
function and the variables it defines in the 'branch.*', 'remote.*' and
'submodule.*' switches of the case statement instead of hardcoding the
corresponding config variables. Note that we use indirect expansion
instead of associative arrays because those are not supported in Bash 3,
on which macOS is stuck for licensing reasons.
Add a test to make sure the new function works correctly by verfying it
lists all 'submodule' config variables. This has the downside that this
test must be updated when new 'submodule' configuration are added, but
this should be a small burden since it happens infrequently.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
contrib/completion/git-completion.bash | 24 +++++++++++++++++++++---
t/t9902-completion.sh | 11 +++++++++++
2 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8af9bc3f4e1..2934ceb7637 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2596,6 +2596,15 @@ __git_compute_config_vars ()
__git_config_vars="$(git help --config-for-completion)"
}
+__git_compute_first_level_config_vars_for_section ()
+{
+ section="$1"
+ __git_compute_config_vars
+ local this_section="__git_first_level_config_vars_for_section_${section}"
+ test -n "${!this_section}" ||
+ printf -v "__git_first_level_config_vars_for_section_${section}" %s "$(echo "$__git_config_vars" | grep -E "^${section}\.[a-z]" | awk -F. '{print $2}')"
+}
+
__git_config_sections=
__git_compute_config_sections ()
{
@@ -2749,8 +2758,11 @@ __git_complete_config_variable_name ()
branch.*)
local pfx="${cur_%.*}."
cur_="${cur_#*.}"
+ local section="${pfx%.}"
__gitcomp_direct "$(__git_heads "$pfx" "$cur_" ".")"
- __gitcomp_nl_append $'autoSetupMerge\nautoSetupRebase\n' "$pfx" "$cur_" "${sfx:- }"
+ __git_compute_first_level_config_vars_for_section "${section}"
+ local this_section="__git_first_level_config_vars_for_section_${section}"
+ __gitcomp_nl_append "${!this_section}" "$pfx" "$cur_" "${sfx:- }"
return
;;
guitool.*.*)
@@ -2799,8 +2811,11 @@ __git_complete_config_variable_name ()
remote.*)
local pfx="${cur_%.*}."
cur_="${cur_#*.}"
+ local section="${pfx%.}"
__gitcomp_nl "$(__git_remotes)" "$pfx" "$cur_" "."
- __gitcomp_nl_append "pushDefault" "$pfx" "$cur_" "${sfx:- }"
+ __git_compute_first_level_config_vars_for_section "${section}"
+ local this_section="__git_first_level_config_vars_for_section_${section}"
+ __gitcomp_nl_append "${!this_section}" "$pfx" "$cur_" "${sfx:- }"
return
;;
submodule.*.*)
@@ -2812,8 +2827,11 @@ __git_complete_config_variable_name ()
submodule.*)
local pfx="${cur_%.*}."
cur_="${cur_#*.}"
+ local section="${pfx%.}"
__gitcomp_nl "$(__git config -f "$(__git rev-parse --show-toplevel)/.gitmodules" --get-regexp 'submodule.*.path' | awk -F. '{print $2}')" "$pfx" "$cur_" "."
- __gitcomp_nl_append $'alternateErrorStrategy\nfetchJobs\nactive\nalternateLocation\nrecurse\npropagateBranches' "$pfx" "$cur_" "${sfx:- }"
+ __git_compute_first_level_config_vars_for_section "${section}"
+ local this_section="__git_first_level_config_vars_for_section_${section}"
+ __gitcomp_nl_append "${!this_section}" "$pfx" "$cur_" "${sfx:- }"
return
;;
url.*.*)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 35eb534fdda..f28d8f531b7 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -2583,6 +2583,17 @@ test_expect_success 'git config - variable name include' '
EOF
'
+test_expect_success 'git config - variable name - __git_compute_first_level_config_vars_for_section' '
+ test_completion "git config submodule." <<-\EOF
+ submodule.active Z
+ submodule.alternateErrorStrategy Z
+ submodule.alternateLocation Z
+ submodule.fetchJobs Z
+ submodule.propagateBranches Z
+ submodule.recurse Z
+ EOF
+'
+
test_expect_success 'git config - value' '
test_completion "git config color.pager " <<-\EOF
false Z
--
gitgitgadget
^ permalink raw reply related
* [PATCH 2/5] completion: complete 'submodule.*' config variables
From: Philippe Blain via GitGitGadget @ 2024-01-28 20:02 UTC (permalink / raw)
To: git; +Cc: Philippe Blain, Philippe Blain
In-Reply-To: <pull.1660.git.git.1706472173.gitgitgadget@gmail.com>
From: Philippe Blain <philippe.blain@canada.ca>
In the Bash completion script, function
__git_complete_config_variable_name completes config variables and has
special logic to deal with config variables involving user-defined
names, like branch.<name>.* and remote.<name>.*.
This special logic is missing for submodule-related config variables.
Add the appropriate branches to the case statement, making use of the
in-tree '.gitmodules' to list relevant submodules.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
contrib/completion/git-completion.bash | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 159a4fd8add..8af9bc3f4e1 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2803,6 +2803,19 @@ __git_complete_config_variable_name ()
__gitcomp_nl_append "pushDefault" "$pfx" "$cur_" "${sfx:- }"
return
;;
+ submodule.*.*)
+ local pfx="${cur_%.*}."
+ cur_="${cur_##*.}"
+ __gitcomp "url update branch fetchRecurseSubmodules ignore active" "$pfx" "$cur_" "$sfx"
+ return
+ ;;
+ submodule.*)
+ local pfx="${cur_%.*}."
+ cur_="${cur_#*.}"
+ __gitcomp_nl "$(__git config -f "$(__git rev-parse --show-toplevel)/.gitmodules" --get-regexp 'submodule.*.path' | awk -F. '{print $2}')" "$pfx" "$cur_" "."
+ __gitcomp_nl_append $'alternateErrorStrategy\nfetchJobs\nactive\nalternateLocation\nrecurse\npropagateBranches' "$pfx" "$cur_" "${sfx:- }"
+ return
+ ;;
url.*.*)
local pfx="${cur_%.*}."
cur_="${cur_##*.}"
--
gitgitgadget
^ 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