* Re: [PATCH v2 0/5] Avoid hang if curl needs eof twice + minor related improvements
From: Jiří Hruška @ 2023-11-27 13:39 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git, Jeff King, Junio C Hamano
In-Reply-To: <20231115232824.2507648-1-jonathantanmy@google.com>
> I've already reviewed 1/5 so I'll summarize what I think of the rest.
Thank you for your time looking at everything, Jonathan.
I'm sorry for the delay in my reply, I was away for a while and also trying
to get clarity from the cURL side, summarized in [1].
[1] https://lore.kernel.org/git/CAGE_+C5u9H8m5faK1vXKk6QTyjcHgKHqxOZy5ptzsYbF_0yrCQ@mail.gmail.com/
Most importantly, the bug has been apparently in libcurl only for ~1 year and
it can no longer manifest for a few months. Therefore, the main point of my
changes is a bit moot and we might reconsider what is useful and what is not.
> I'm a bit reluctant to include 2/5 and 4/5.
> I think 3/5 [...], part of 4/5 [...] and 5/5 have significant benefit and
> should be included, though.
Fair enough. I will update everything based on your and other's comments and
my new learnings, then the final decision can be made about each part.
^ permalink raw reply
* Re: [PATCH v2 5/5] http: reset CURLOPT_POSTFIELDSIZE_LARGE between requests
From: Jiří Hruška @ 2023-11-27 13:21 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Jeff King, Jonathan Tan, Junio C Hamano
In-Reply-To: <ZVRovA9OSfY5odhy@tanuki>
> It feels quite easy for this list to grow stale whenever we start to set
> a new option somewhere else. Is there a specific reason why we can't
> instead use `curl_easy_reset()` here?
Good point, makes sense. Thanks, Patrick, I will use that instead.
^ permalink raw reply
* Re: [PATCH] remote-curl: avoid hang if curl asks for more data after eof
From: Jiří Hruška @ 2023-11-27 13:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Tan, git, Jeff King
In-Reply-To: <CAGE_+C7h6HOh+ptdhwJ3MNn5HWFoc1WF-foLRrew6FJEo_yupg@mail.gmail.com>
I have discussed this with Daniel Stenberg himself in this thread:
https://curl.se/mail/lib-2023-11/0017.html
The summary is:
- It was never intended/expected that CURLOPT_READFUNCTION would be called
more after already returning EOF once. libcurl has been modified to never
do this anymore (in master, to be released in what comes after 8.4.0).
- The problem has been tracked down to a particular case that could reproduce
only if using HTTP/2 and only in cURL versions 7.84.0 – 8.1.2 (inclusive,
roughly between June 2022 and July 2023). The same misbehavior is still
technically possible in newer versions, but not happening in practice.
Therefore, I believe nothing strongly _needs_ to be done on the Git side.
^ permalink raw reply
* Re: [PATCH] i18n: factorize even more 'incompatible options' messages
From: Junio C Hamano @ 2023-11-27 13:12 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List, Jean-Noël Avila
In-Reply-To: <e6eb12e4-bb63-473c-9c2f-965a4d5981ad@web.de>
René Scharfe <l.s.r@web.de> writes:
> Continue the work of 12909b6b8a (i18n: turn "options are incompatible"
> into "cannot be used together", 2022-01-05) and a699367bb8 (i18n:
> factorize more 'incompatible options' messages, 2022-01-31) to use the
> same parameterized error message for reporting incompatible command line
> options. This reduces the number of strings to translate and makes the
> UI slightly more consistent.
Will queue. Thanks.
^ permalink raw reply
* Re: [PATCH] i18n: factorize even more 'incompatible options' messages
From: Junio C Hamano @ 2023-11-27 13:11 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List, Jean-Noël Avila
In-Reply-To: <e6eb12e4-bb63-473c-9c2f-965a4d5981ad@web.de>
René Scharfe <l.s.r@web.de> writes:
> Continue the work of 12909b6b8a (i18n: turn "options are incompatible"
> into "cannot be used together", 2022-01-05) and a699367bb8 (i18n:
> factorize more 'incompatible options' messages, 2022-01-31) to use the
> same parameterized error message for reporting incompatible command line
> options. This reduces the number of strings to translate and makes the
> UI slightly more consistent.
Will queue. Thanks.
^ permalink raw reply
* Re: [PATCH] column: release strbuf and string_list after use
From: Junio C Hamano @ 2023-11-27 12:04 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List
In-Reply-To: <f087137d-a5aa-487e-a1cb-0ad7117b38ed@web.de>
René Scharfe <l.s.r@web.de> writes:
> Releasing strbuf and string_list just before exiting is not strictly
> necessary, but it gets rid of false positives reported by leak checkers,
> which can then be more easily used to show that the column-printing
> machinery behind print_columns() are free of leaks.
Thanks for being careful. Will queue.
^ permalink raw reply
* --end-of-options inconsistently available?!
From: Sven Strickroth @ 2023-11-27 11:22 UTC (permalink / raw)
To: git
Hi,
gitcli(7) states:
> Because -- disambiguates revisions and paths in some commands, it cannot be used for those commands to separate options and revisions. You can use --end-of-options for this (it also works for commands that do not distinguish between revisions in paths, in which case it is simply an alias for --).
However, when I use this for certain commands it fails:
$ git reset --end-of-options HEAD --
fatal: option '--end-of-options' must come before non-option arguments
$ git rev-parse --symbolic-full-name --end-of-options master
--end-of-options
refs/heads/master
Here, the output also contains "--end-of-options" as if it is a
reference (same for "--")
$ git checkout -f --end-of-options HEAD~1 -- afile.txt
fatal: only one reference expected, 2 given.
Best,
Sven
^ permalink raw reply
* Re: Fix git-send-email.perl w.r.t. recent Getopt::Long update
From: H.Merijn Brand @ 2023-11-27 8:38 UTC (permalink / raw)
To: Junio C Hamano, git
In-Reply-To: <xmqqzfz03tbn.fsf@gitster.g>
[-- Attachment #1: Type: text/plain, Size: 2548 bytes --]
On Mon, 27 Nov 2023 09:58:52 +0900, Junio C Hamano <gitster@pobox.com> wrote:
> "H.Merijn Brand" <linux@tux.freedom.nl> writes:
>
> > From the Getopt::Long changes:
> > ```
> > Changes in version 2.55
> > -----------------------
> > * Fix long standing bug that duplicate options were not detected when
> > the options differ in case while ignore_case is in effect.
> > This will now yield a warning and become a fatal error in a future
> > release.
> > ```
> >
> > Current version is 2.57
>
> This patch looks like duplicate of
>
> https://lore.kernel.org/git/20231116193014.470420-1-tmz@pobox.com/
>
> perhaps independently discovered and worked on. Thanks for caring.
>
> One downside of unconditional upgrade of the call is, of course,
> that it would no longer work for those with older Getopt::Long that
> did not support the "!" suffix. Fortunately, Getopt::Long 2.33
> started shipping with Perl 5.8.1 that is more than 20 years old, so
> with the series we accepted, we also have a change to bump the
> required version of Perl from 5.8.0 to 5.8.1 to make it clear that
> it is deliberate that we drop the support for anything older at the
> same time.
The is a no-issue ...
Just the 'use Getopt::Long' is enough to guarantee a working version:
The '!' was already implemented in version 2.10 (April 1997):
--8<---
=item !
Option does not take an argument and may be negated, i.e. prefixed by
"no". E.g. "foo!" will allow B<--foo> (with value 1) and B<-nofoo>
(with value 0).
The option variable will be set to 1, or 0 if negated.
-->8---
Looking at the ChangeLog, a reliable behavior of '!' was available
since version 2.22 (march 2000):
--8<---
Changes in version 2.22
-----------------------
* Fixes a bug in the combination of aliases and negation.
Old: "foo|bar!" allowed negation on foo, but not on bar.
New: "foo|bar!" allows negation on foo and bar.
Caveat: "foo|f!", with bundling, issues the warning that negation on
a short option is ignored. To obtain the desired behaviour, use
"foo!" => \$opt_foo, "f" => \$opt_foo
or
"foo|f" => \$opt_foo, "nofoo" => sub { $opt_foo = 0 }
Remember that this is _only_ required when bundling is in effect.
-->8---
--
H.Merijn Brand https://tux.nl Perl Monger http://amsterdam.pm.org/
using perl5.00307 .. 5.37 porting perl5 on HP-UX, AIX, and Linux
https://tux.nl/email.html http://qa.perl.org https://www.test-smoke.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] checkout: forbid "-B <branch>" from touching a branch used elsewhere
From: Junio C Hamano @ 2023-11-27 1:51 UTC (permalink / raw)
To: Phillip Wood; +Cc: Willem Verstraeten, git
In-Reply-To: <bf848477-b4dd-49d3-8e4b-de0fc3948570@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
>> diff --git a/builtin/checkout.c b/builtin/checkout.c
>> index b4ab972c5a..8a8ad23e98 100644
>> --- a/builtin/checkout.c
>> +++ b/builtin/checkout.c
>> @@ -1600,6 +1600,13 @@ static int checkout_branch(struct checkout_opts *opts,
>> if (new_branch_info->path && !opts->force_detach && !opts->new_branch)
>> die_if_switching_to_a_branch_in_use(opts, new_branch_info->path);
>> + /* "git checkout -B <branch>" */
>> + if (opts->new_branch_force) {
>> + char *full_ref = xstrfmt("refs/heads/%s", opts->new_branch);
>> + die_if_switching_to_a_branch_in_use(opts, full_ref);
>> + free(full_ref);
>
> At the moment this is academic as neither of the test scripts changed
> by this patch are leak free and so I don't think we need to worry
> about it but it raises an interesting question about how we should
> handle memory leaks when dying. Leaving the leak when dying means that
> a test script that tests an expected failure will never be leak free
> but using UNLEAK() would mean we miss a leak being introduced in the
> successful case should the call to "free()" ever be removed.
Is there a leak here? The piece of memory is pointed at by an on-stack
variable full_ref when leak sanitizer starts scanning the heap and
the stack just before the process exits due to die, so I do not see
a reason to worry about this particular variable over all the other
on stack variables we accumulated before the control reached this
point of the code.
Are you worried about optimizing compilers that behave more cleverly
than their own good to somehow lose the on-stack reference to
full_ref while calling die_if_switching_to_a_branch_in_use()? We
might need to squelch them with UNLEAK() but that does not mean we
have to remove the free() we see above, and I suspect a more
productive use of our time to solve that issue is ensure that our
leak-sanitizing build will not triger such an unwanted optimization
anyway.
Thanks.
^ permalink raw reply
* Re: [PATCH 4/4] completion: avoid user confusion in non-cone mode
From: Junio C Hamano @ 2023-11-27 1:39 UTC (permalink / raw)
To: Elijah Newren; +Cc: Elijah Newren via GitGitGadget, git
In-Reply-To: <CABPp-BE73cDq-DBKzsw3R0awiKn5J5LCfdXRULHyPbrS9GEn4Q@mail.gmail.com>
Elijah Newren <newren@gmail.com> writes:
>> > if [[ "$using_cone" == "true" ]]; then
>> > __gitcomp_directories
>>
>> Hmph, doesn't "Providing the files and directories currently present
>> is thus always wrong." apply equally to cone mode?
>
> Absolutely, it definitely applies to cone mode. We (mostly) fixed
> that a long time ago, making it not complete on the files &
> directories currently present. In particular, the
> __gitcomp_directories() function highlighted here completes on the
> output of `git ls-tree -z -d --name-only HEAD`.
Thanks; what I missed was exactly what __gitcomp_directories does
not do you explained above.
>> > + # 4. Provides no completions when run from a
>> > + # subdirectory of the repository root. (If we
>> > + # did provide file/directory completions, the
>> > + # user would just get a "please run from the
>> > + # toplevel directory" error message when they
>> > + # ran it. *Further*, if the user did rerun
>> > + # the command from the toplevel, the
>> > + # completions we previously provided would
>> > + # likely be wrong as they'd be relative to the
>> > + # subdirectory rather than the repository
>> > + # root. That could lead to users getting a
>> > + # nasty surprise based on trying to use a
>> > + # command we helped them create.)
>>
>> Hmph, would an obvious alternative to (1) check against the HEAD (or
>> the index) to see if the prefix string matches an entity at the
>> current directory level, and then (2) to prefix the result of the
>> previous step with "/$(git rev-parse --show-prefix)" work? That is
>> something like this:
>>
>> $ cd t
>> $ git sparse-checkout add help<TAB>
>> ->
>> $ git sparse-checkout add /t/helper/
>
> I thought bash-completion was only for completions, not for startings
> as well. Was I mistaken?
To my mind, the completion is what I as an end user get when I type
<TAB> to help me formulate input that is acceptable by the command.
As I said, I consider it a bug (or UI mistake) in the a command if
it pretends to work inside a subdirecctory but complains when it is
given a path relative to the current directory, so I'd rather prefer
the approach to "fix" the underlying command, but if that is too
much work or cannot be done for whatever reason, the second best
would be to turn whatever we can do to help the end-user input into
a form that is accepted by the command without changing what the
input means. If it takes more than "appending at the end", that is
fine, at least by me as an end user.
If you are saying "completion code can only append at the end
because we can only return strings to be appended, not the entire
strings, to the readline machinery, so mucking with the start of the
string is not doable", then sorry---I accept that what we cannot do
cannot be done, and in that case you are "not mistaken".
But from the existing use of COMPREPLY[], it didn't look that way
(it seems __gitcomp is equipped to take fixed prefix to all
candidates by passing it in $2 and used to complete names of
configuration variables in a section, but it seems to me that it can
be repurposed when prefixing "$(git rev-parse --show-prefix)" to a
given pathname relative to the $cwd). And if that can be done, then
you are "not mistaken", but merely being dogmatic and limiting what
your code can do yourself.
>> Another more fundamental approach to avoid "confusion" this bullet
>> item tries to side step might be to *fix* the command that gets
>> completed. As "git sparse-checkout --help" is marked as
>> EXPERIMENTAL in capital letters, we should be able to say "what was
>> traditionally known as 'add' is from now on called 'add-pattern' and
>> command line completion would not get in the way; the 'add'
>> subcommand now takes only literal paths, not patterns, that are
>> relative to the current directory" if we wanted to.
>
> That's interesting...but it opens up a new can of worms:
> * Would we also need both `set-patterns` and `set`, in addition to
> `add-patterns` and `add`?
If "set" has a similar UI issue that confuses end-users, then sure,
I do not see a reason why we want to leave it confusing---the
experimental labelling is to allow us to fix these warts more
easily, no?
> * In cone mode, the paths passed are literal directories (and only
> directories; no individual files), but the thing added is a
> telescoping "cone" of leading directories as well. Does this make it
> potentially confusing to users to say that `add` only takes literal
> paths?
I do not know.
> * In cone mode (the default), should `add-patterns` just be an
> error, since no pattern specification is allowed?
I do not really care. "add-patterns" is a potential tool you can
use to reduce friction while fixing the UI warts in an experimental
command.
> * In the git-sparse-checkout manual, for performance reasons, we
> recommend users _not_ specify individual paths in non-cone mode.
> Would our recommendation then be to just not use `add` or `set` and
> only use `add-patterns` and `set-patterns`?
Very likely. If the desired behaviour from the command can only be
had by castrating features, then such a recommendation would not
mean much to end-users anyway, though.
> If so, what have we
> accomplished by adding the new names?
It is valuable for those who do need to go against recommendation
(because the recommendation robs usability from them way too much),
will have much less confusing and working completion when they use
'add' or 'set', no?
> Maybe I'm missing something about your suggestion, but this seems much
> more complex than the simple solution we implemented in bb8b5e9a90d
> ("sparse-checkout: pay attention to prefix for {set, add}",
> 2022-02-19) for the !core_sparse_checkout_cone case.
Oh, if we do honor the $(git rev-parse --show-prefix), then that
changes the equation somewhat. I got an impression from your log
message or cover letter that it wasn't the case, and that was where
the "if the command is so broken, then completion can add it for the
user" and "if the command is so broken, then fix it to take relative
paths" came from.
^ permalink raw reply
* Re: Fix git-send-email.perl w.r.t. recent Getopt::Long update
From: Junio C Hamano @ 2023-11-27 0:58 UTC (permalink / raw)
To: H.Merijn Brand; +Cc: git
In-Reply-To: <20231124103932.31ca7688@pc09>
"H.Merijn Brand" <linux@tux.freedom.nl> writes:
> From the Getopt::Long changes:
> ```
> Changes in version 2.55
> -----------------------
> * Fix long standing bug that duplicate options were not detected when
> the options differ in case while ignore_case is in effect.
> This will now yield a warning and become a fatal error in a future
> release.
> ```
>
> Current version is 2.57
This patch looks like duplicate of
https://lore.kernel.org/git/20231116193014.470420-1-tmz@pobox.com/
perhaps independently discovered and worked on. Thanks for caring.
One downside of unconditional upgrade of the call is, of course,
that it would no longer work for those with older Getopt::Long that
did not support the "!" suffix. Fortunately, Getopt::Long 2.33
started shipping with Perl 5.8.1 that is more than 20 years old, so
with the series we accepted, we also have a change to bump the
required version of Perl from 5.8.0 to 5.8.1 to make it clear that
it is deliberate that we drop the support for anything older at the
same time.
^ permalink raw reply
* Re: [PATCH v3 0/4] Switch links to https
From: Junio C Hamano @ 2023-11-27 0:49 UTC (permalink / raw)
To: Josh Soref; +Cc: Elijah Newren, Josh Soref via GitGitGadget, git, Eric Sunshine
In-Reply-To: <CACZqfqAq0ijtYO9-1q6h2KQMyxHgLpFOg--d4c9F4xT0nMkvUg@mail.gmail.com>
Josh Soref <jsoref@gmail.com> writes:
> Elijah Newren wrote:
>> As stated elsewhere, I'd be fine with using the archived link if the
>> justification presented in the series for using archived links was
>> consistent and mentioned both reasons for changes. But, I think this
>> series is fine to merge down as-is if you don't want to go through the
>> trouble. Especially given how long you've waited.
>
> I'm clearly still contributing, so I can come back later and cross
> that bridge...
>
>> Anyway, I checked through every link in this series; it all looks good to me.
>
> Let's take this as-is. Thanks for taking the time to re-check every
> link, I know exactly how tedious that is :).
Thanks, both. Will queue.
^ permalink raw reply
* Re: [PATCH] i18n: factorize even more 'incompatible options' messages
From: Eric Sunshine @ 2023-11-26 17:49 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List, Jean-Noël Avila
In-Reply-To: <e6eb12e4-bb63-473c-9c2f-965a4d5981ad@web.de>
On Sun, Nov 26, 2023 at 6:57 AM René Scharfe <l.s.r@web.de> wrote:
> Continue the work of 12909b6b8a (i18n: turn "options are incompatible"
> into "cannot be used together", 2022-01-05) and a699367bb8 (i18n:
> factorize more 'incompatible options' messages, 2022-01-31) to use the
> same parameterized error message for reporting incompatible command line
> options. This reduces the number of strings to translate and makes the
> UI slightly more consistent.
Thanks for tackling this.
A couple additional instances recently slipped into `show-ref.c` which
were caught during review[1,2] but nevertheless made it to
"master"[3,4]. This patch, of course, doesn't need to address those,
but if rerolling for some other reason, perhaps they can be included,
as well(?).
[1]: https://lore.kernel.org/git/CAPig+cSrp7vZuy7D_ENHKZKZzF4OSmCtfYNHPGMtS1Hj6gArDw@mail.gmail.com/
[2]: https://lore.kernel.org/git/CAPig+cRTOMie0rUf=Mhbo9e2EXf-_2kQyMeqpB9OCRB1MZZ1rw@mail.gmail.com/
[3]: 199970e72f (builtin/show-ref: ensure mutual exclusiveness of
subcommands, 2023-10-31)
[4]: 9080a7f178 (builtin/show-ref: add new mode to check for reference
existence, 2023-10-31)
^ permalink raw reply
* No automatic continue of rebasing with rerere
From: Vovodroid @ 2023-11-26 12:08 UTC (permalink / raw)
To: git@vger.kernel.org
In-Reply-To: <511253473.8084883.1701000481697.ref@mail.yahoo.com>
Hi,
when running non-interactive rebasing (i.e. no -i option) with rerere turned on, already resolved conflicts and recorded conflicts causing Git to return exit code 1, as if error occurred.
Also "git.exe rebase --continue" still opens log editor unless GIT_EDITOR environment variable is set to true.
Such behavior causes to user to perform a lot of unnecessary Git runs, or clicking if some GUI shell, like in GitExtension, is used.
See https://github.com/gitextensions/gitextensions/issues/11382 for discussion and screenshots.
Regards.
^ permalink raw reply
* [PATCH] i18n: factorize even more 'incompatible options' messages
From: René Scharfe @ 2023-11-26 11:57 UTC (permalink / raw)
To: Git List; +Cc: Jean-Noël Avila
Continue the work of 12909b6b8a (i18n: turn "options are incompatible"
into "cannot be used together", 2022-01-05) and a699367bb8 (i18n:
factorize more 'incompatible options' messages, 2022-01-31) to use the
same parameterized error message for reporting incompatible command line
options. This reduces the number of strings to translate and makes the
UI slightly more consistent.
Signed-off-by: René Scharfe <l.s.r@web.de>
---
builtin/clone.c | 4 +++-
builtin/merge-tree.c | 3 ++-
parse-options.c | 3 ++-
revision.c | 4 ++--
t/t0040-parse-options.sh | 8 ++++----
t/t1006-cat-file.sh | 2 +-
t/t4301-merge-tree-write-tree.sh | 2 +-
t/t5606-clone-options.sh | 2 +-
8 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index c6357af949..45a5070268 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -965,7 +965,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
}
if (bundle_uri && deepen)
- die(_("--bundle-uri is incompatible with --depth, --shallow-since, and --shallow-exclude"));
+ die(_("options '%s' and '%s' cannot be used together"),
+ "--bundle-uri",
+ "--depth/--shallow-since/--shallow-exclude");
repo_name = argv[0];
diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index a35e0452d6..a4aa6013c5 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -577,7 +577,8 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)
if (o.mode == MODE_TRIVIAL)
die(_("--trivial-merge is incompatible with all other options"));
if (merge_base)
- die(_("--merge-base is incompatible with --stdin"));
+ die(_("options '%s' and '%s' cannot be used together"),
+ "--merge-base", "--stdin");
line_termination = '\0';
while (strbuf_getline_lf(&buf, stdin) != EOF) {
struct strbuf **split;
diff --git a/parse-options.c b/parse-options.c
index e0c94b0546..c3955847f4 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -279,7 +279,8 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
opt_name = optnamearg(opt, arg, flags);
other_opt_name = optnamearg(elem->opt, elem->arg, elem->flags);
- error(_("%s is incompatible with %s"), opt_name, other_opt_name);
+ error(_("options '%s' and '%s' cannot be used together"),
+ opt_name, other_opt_name);
free(opt_name);
free(other_opt_name);
return -1;
diff --git a/revision.c b/revision.c
index 00d5c29bfc..b2861474b1 100644
--- a/revision.c
+++ b/revision.c
@@ -2384,8 +2384,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
revs->left_right = 1;
} else if (!strcmp(arg, "--left-only")) {
if (revs->right_only)
- die("--left-only is incompatible with --right-only"
- " or --cherry");
+ die(_("options '%s' and '%s' cannot be used together"),
+ "--left-only", "--right-only/--cherry");
revs->left_only = 1;
} else if (!strcmp(arg, "--right-only")) {
if (revs->left_only)
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 8fdef88b65..ec974867e4 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -376,7 +376,7 @@ test_expect_success 'OPT_CMDMODE() detects incompatibility (1)' '
test_must_be_empty output &&
test_grep "mode1" output.err &&
test_grep "mode2" output.err &&
- test_grep "is incompatible with" output.err
+ test_grep "cannot be used together" output.err
'
test_expect_success 'OPT_CMDMODE() detects incompatibility (2)' '
@@ -384,7 +384,7 @@ test_expect_success 'OPT_CMDMODE() detects incompatibility (2)' '
test_must_be_empty output &&
test_grep "mode2" output.err &&
test_grep "set23" output.err &&
- test_grep "is incompatible with" output.err
+ test_grep "cannot be used together" output.err
'
test_expect_success 'OPT_CMDMODE() detects incompatibility (3)' '
@@ -392,7 +392,7 @@ test_expect_success 'OPT_CMDMODE() detects incompatibility (3)' '
test_must_be_empty output &&
test_grep "mode2" output.err &&
test_grep "set23" output.err &&
- test_grep "is incompatible with" output.err
+ test_grep "cannot be used together" output.err
'
test_expect_success 'OPT_CMDMODE() detects incompatibility (4)' '
@@ -401,7 +401,7 @@ test_expect_success 'OPT_CMDMODE() detects incompatibility (4)' '
test_must_be_empty output &&
test_grep "mode2" output.err &&
test_grep "mode34.3" output.err &&
- test_grep "is incompatible with" output.err
+ test_grep "cannot be used together" output.err
'
test_expect_success 'OPT_COUNTUP() with PARSE_OPT_NODASH works' '
diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index d73a0be1b9..271c5e4fd3 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -6,7 +6,7 @@ test_description='git cat-file'
test_cmdmode_usage () {
test_expect_code 129 "$@" 2>err &&
- grep "^error:.*is incompatible with" err
+ grep "^error: .* cannot be used together" err
}
for switches in \
diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh
index b2c8a43fce..12ac436873 100755
--- a/t/t4301-merge-tree-write-tree.sh
+++ b/t/t4301-merge-tree-write-tree.sh
@@ -887,7 +887,7 @@ test_expect_success '--stdin with both a successful and a conflicted merge' '
test_expect_success '--merge-base is incompatible with --stdin' '
test_must_fail git merge-tree --merge-base=side1 --stdin 2>expect &&
- grep "^fatal: --merge-base is incompatible with --stdin" expect
+ grep "^fatal: .*merge-base.*stdin.* cannot be used together" expect
'
# specify merge-base as parent of branch2
diff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh
index fc4bbd9daf..a400bcca62 100755
--- a/t/t5606-clone-options.sh
+++ b/t/t5606-clone-options.sh
@@ -64,7 +64,7 @@ test_expect_success 'disallows --bundle-uri with shallow options' '
for option in --depth=1 --shallow-since=01-01-2000 --shallow-exclude=HEAD
do
test_must_fail git clone --bundle-uri=bundle $option from to 2>err &&
- grep "bundle-uri is incompatible" err || return 1
+ grep "bundle-uri.* cannot be used together" err || return 1
done
'
--
2.43.0
^ permalink raw reply related
* [PATCH] column: release strbuf and string_list after use
From: René Scharfe @ 2023-11-26 11:57 UTC (permalink / raw)
To: Git List
Releasing strbuf and string_list just before exiting is not strictly
necessary, but it gets rid of false positives reported by leak checkers,
which can then be more easily used to show that the column-printing
machinery behind print_columns() are free of leaks.
Signed-off-by: René Scharfe <l.s.r@web.de>
---
builtin/column.c | 2 ++
t/t9002-column.sh | 1 +
2 files changed, 3 insertions(+)
diff --git a/builtin/column.c b/builtin/column.c
index a83be8bc99..e80218f81f 100644
--- a/builtin/column.c
+++ b/builtin/column.c
@@ -56,5 +56,7 @@ int cmd_column(int argc, const char **argv, const char *prefix)
string_list_append(&list, sb.buf);
print_columns(&list, colopts, &copts);
+ strbuf_release(&sb);
+ string_list_clear(&list, 0);
return 0;
}
diff --git a/t/t9002-column.sh b/t/t9002-column.sh
index 6d3dbde3fe..348cc40658 100755
--- a/t/t9002-column.sh
+++ b/t/t9002-column.sh
@@ -1,6 +1,7 @@
#!/bin/sh
test_description='git column'
+TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
test_expect_success 'setup' '
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] builtin/reflog.c: fix dry-run option short name
From: Junio C Hamano @ 2023-11-26 8:44 UTC (permalink / raw)
To: Josh Brobst; +Cc: git
In-Reply-To: <20231126000514.85509-1-josh@brob.st>
Josh Brobst <josh@brob.st> writes:
> The documentation for reflog states that the --dry-run option of the
> expire and delete subcommands has a corresponding short name, -n.
> However, 33d7bdd645 (builtin/reflog.c: use parse-options api for expire,
> delete subcommands, 2022-01-06) did not include this short name in the
> new options parsing.
>
> Re-add the short name in the new dry-run option definitions.
Perfect. Thanks for spotting and doing necessary archaeology.
^ permalink raw reply
* [PATCH v2 4/4] completion: avoid user confusion in non-cone mode
From: Elijah Newren via GitGitGadget @ 2023-11-26 7:51 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, SZEDER Gábor, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1349.v2.git.1700985086.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>
It is tempting to think of "files and directories" of the current
directory as valid inputs to the add and set subcommands of git
sparse-checkout. However, in non-cone mode, they often aren't and using
them as potential completions leads to *many* forms of confusion:
Issue #1. It provides the *wrong* files and directories.
For
git sparse-checkout add
we always want to add files and directories not currently in our sparse
checkout, which means we want file and directories not currently present
in the current working tree. Providing the files and directories
currently present is thus always wrong.
For
git sparse-checkout set
we have a similar problem except in the subset of cases where we are
trying to narrow our checkout to a strict subset of what we already
have. That is not a very common scenario, especially since it often
does not even happen to be true for the first use of the command; for
years we required users to create a sparse-checkout via
git sparse-checkout init
git sparse-checkout set <args...>
(or use a clone option that did the init step for you at clone time).
The init command creates a minimal sparse-checkout with just the
top-level directory present, meaning the set command has to be used to
expand the checkout. Thus, only in a special and perhaps unusual cases
would any of the suggestions from normal file and directory completion
be appropriate.
Issue #2: Suggesting patterns that lead to warnings is unfriendly.
If the user specifies any regular file and omits the leading '/', then
the sparse-checkout command will warn the user that their command is
problematic and suggest they use a leading slash instead.
Issue #3: Completion gets confused by leading '/', and provides wrong paths.
Users often want to anchor their patterns to the toplevel of the
repository, especially when listing individual files. There are a
number of reasons for this, but notably even sparse-checkout encourages
them to do so (as noted above). However, if users do so (via adding a
leading '/' to their pattern), then bash completion will interpret the
leading slash not as a request for a path at the toplevel of the
repository, but as a request for a path at the root of the filesytem.
That means at best that completion cannot help with such paths, and if
it does find any completions, they are almost guaranteed to be wrong.
Issue #4: Suggesting invalid patterns from subdirectories is unfriendly.
There is no per-directory equivalent to .gitignore with
sparse-checkouts. There is only a single worktree-global
$GIT_DIR/info/sparse-checkout file. As such, paths to files must be
specified relative to the toplevel of a repository. Providing
suggestions of paths that are relative to the current working directory,
as bash completion defaults to, is wrong when the current working
directory is not the worktree toplevel directory.
Issue #5: Paths with special characters will be interpreted incorrectly
The entries in the sparse-checkout file are patterns, not paths. While
most paths also qualify as patterns (though even in such cases it would
be better for users to not use them directly but prefix them with a
leading '/'), there are a variety of special characters that would need
special escaping beyond the normal shell escaping: '*', '?', '\', '[',
']', and any leading '#' or '!'. If completion suggests any such paths,
users will likely expect them to be treated as an exact path rather than
as a pattern that might match some number of files other than 1.
Because of the combination of the above issues, turn completion off for
the `set` and `add` subcommands of `sparse-checkout` when in non-cone
mode, but leave a NEEDSWORK comment specifying what could theoretically
be done if someone wanted to provide completion rules that were more
helpful than harmful.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
contrib/completion/git-completion.bash | 62 ++++++++++++++++++++++++++
1 file changed, 62 insertions(+)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c614e5d4f07..1e07dc8b73e 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3138,6 +3138,68 @@ _git_sparse_checkout ()
fi
if [[ "$using_cone" == "true" ]]; then
__gitcomp_directories
+ else
+ # NEEDSWORK: It might be useful to provide a
+ # completion function which:
+ #
+ # 1. Provides completions based on
+ # files/directories that exist in HEAD (or in
+ # the index since sparse-index isn't possible
+ # in non-cone mode), not just those currently
+ # present in the working tree. Bash's
+ # default file and directory completion is
+ # totally useless for "git sparse-checkout
+ # add" because of this. It is likewise
+ # problematic for "git sparse-checkout set"
+ # except in those subset of cases when trying
+ # to narrow scope to a strict subset of what
+ # you already have checked out.
+ #
+ # 2. Always provides file/directory completions
+ # with a prepended leading '/', so that
+ # files/directories are only searched at the
+ # relevant level rather than throughout all
+ # trees in the hierarchy. Doing this also
+ # avoids suggesting the user run a
+ # sparse-checkout command that will result in
+ # a warning be thrown at the user.
+ #
+ # 3. Does not accidentally search the root of
+ # the filesystem when a path with a leading
+ # slash is specified. ("git sparse-checkout
+ # add /ho<TAB>" should not complete to
+ # "/home" but to e.g. "/hooks" if there is a
+ # "hooks" in the top of the repository.)
+ #
+ # 4. Provides no completions when run from a
+ # subdirectory of the repository root. (If we
+ # did provide file/directory completions, the
+ # user would just get a "please run from the
+ # toplevel directory" error message when they
+ # ran it. *Further*, if the user did rerun
+ # the command from the toplevel, the
+ # completions we previously provided would
+ # likely be wrong as they'd be relative to the
+ # subdirectory rather than the repository
+ # root. That could lead to users getting a
+ # nasty surprise based on trying to use a
+ # command we helped them create.)
+ #
+ # 5. Provides escaped completions for any paths
+ # containing a '*', '?', '\', '[', ']', or
+ # leading '#' or '!'. (These characters might
+ # already be escaped to protect from the
+ # shell, but they need an *extra* layer of
+ # escaping to prevent the pattern parsing in
+ # Git from seeing them as special characters.)
+ #
+ # Of course, this would be a lot of work, so for now,
+ # just avoid the many forms of user confusion that
+ # could be caused by providing bad completions by
+ # providing a fake completion to avoid falling back to
+ # bash's normal file and directory completion.
+
+ COMPREPLY=( "" )
fi
esac
}
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 3/4] completion: avoid misleading completions in cone mode
From: Elijah Newren via GitGitGadget @ 2023-11-26 7:51 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, SZEDER Gábor, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1349.v2.git.1700985086.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>
The "set" and "add" subcommands of "sparse-checkout", when in cone mode,
should only complete on directories. For bash_completion in general,
when no completions are returned for any subcommands, it will often fall
back to standard completion of files and directories as a substitute.
That is not helpful here. Since we have already looked for all valid
completions, if none are found then falling back to standard bash file
and directory completion is at best actively misleading. In fact, there
are three different ways it can be actively misleading. Add a long
comment in the code about how that fallback behavior can deceive, and
disable the fallback by returning a fake result as the sole completion.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
contrib/completion/git-completion.bash | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 7aa66c19ede..c614e5d4f07 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3090,6 +3090,26 @@ __gitcomp_directories ()
# No possible further completions any deeper, so assume we're at
# a leaf directory and just consider it complete
__gitcomp_direct_append "$cur "
+ elif [[ $_found == 0 ]]; then
+ # No possible completions found. Avoid falling back to
+ # bash's default file and directory completion, because all
+ # valid completions have already been searched and the
+ # fallbacks can do nothing but mislead. In fact, they can
+ # mislead in three different ways:
+ # 1) Fallback file completion makes no sense when asking
+ # for directory completions, as this function does.
+ # 2) Fallback directory completion is bad because
+ # e.g. "/pro" is invalid and should NOT complete to
+ # "/proc".
+ # 3) Fallback file/directory completion only completes
+ # on paths that exist in the current working tree,
+ # i.e. which are *already* part of their
+ # sparse-checkout. Thus, normal file and directory
+ # completion is always useless for "git
+ # sparse-checkout add" and is also probelmatic for
+ # "git sparse-checkout set" unless using it to
+ # strictly narrow the checkout.
+ COMPREPLY=( "" )
fi
}
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 2/4] completion: fix logic for determining whether cone mode is active
From: Elijah Newren via GitGitGadget @ 2023-11-26 7:51 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, SZEDER Gábor, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1349.v2.git.1700985086.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>
_git_sparse_checkout() was checking whether we were in cone mode by
checking whether either:
A) core.sparseCheckoutCone was "true"
B) "--cone" was specified on the command line
This code has 2 bugs I didn't catch in my review at the time
1) core.sparseCheckout must be "true" for core.sparseCheckoutCone to
be relevant (which matters since "git sparse-checkout disable"
only unsets core.sparseCheckout, not core.sparseCheckoutCone)
2) The presence of "--no-cone" should override any config setting
Further, I forgot to update this logic as part of 2d95707a02
("sparse-checkout: make --cone the default", 2022-04-22) for the new
default.
Update the code for the new default and make it be more careful in
determining whether to complete based on cone mode or non-cone mode.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
contrib/completion/git-completion.bash | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index b8661701718..7aa66c19ede 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3097,6 +3097,7 @@ _git_sparse_checkout ()
{
local subcommands="list init set disable add reapply"
local subcommand="$(__git_find_on_cmdline "$subcommands")"
+ local using_cone=true
if [ -z "$subcommand" ]; then
__gitcomp "$subcommands"
return
@@ -3107,8 +3108,15 @@ _git_sparse_checkout ()
__gitcomp_builtin sparse-checkout_$subcommand "" "--"
;;
set,*|add,*)
- if [ "$(__git config core.sparseCheckoutCone)" == "true" ] ||
- [ -n "$(__git_find_on_cmdline --cone)" ]; then
+ if [[ "$(__git config core.sparseCheckout)" == "true" &&
+ "$(__git config core.sparseCheckoutCone)" == "false" &&
+ -z "$(__git_find_on_cmdline --cone)" ]]; then
+ using_cone=false
+ fi
+ if [[ -n "$(__git_find_on_cmdline --no-cone)" ]]; then
+ using_cone=false
+ fi
+ if [[ "$using_cone" == "true" ]]; then
__gitcomp_directories
fi
esac
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 0/4] Sparse checkout completion fixes
From: Elijah Newren via GitGitGadget @ 2023-11-26 7:51 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, SZEDER Gábor, Elijah Newren
In-Reply-To: <pull.1349.git.1700761448.gitgitgadget@gmail.com>
This fixes a few issues with tab completion for the sparse-checkout command,
specifically with the "add" and "set" subcommands.
As noted in v1, the 4th patch implements a somewhat suboptimal solution that
at least improves the situation, while also documenting with a code comment
a much more involved alternative solution that we could consider in the
future.
Changes since v1:
* Use __git wrapper function to squelch errors, as suggested by SZEDER
Gábor
* note that we could use the index or HEAD for the more involved solution
in patch 4.
[1] https://lore.kernel.org/git/xmqqv8yjz5us.fsf@gitster.g/
Elijah Newren (4):
completion: squelch stray errors in sparse-checkout completion
completion: fix logic for determining whether cone mode is active
completion: avoid misleading completions in cone mode
completion: avoid user confusion in non-cone mode
contrib/completion/git-completion.bash | 96 +++++++++++++++++++++++++-
1 file changed, 93 insertions(+), 3 deletions(-)
base-commit: 564d0252ca632e0264ed670534a51d18a689ef5d
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1349%2Fnewren%2Fsparse-checkout-completion-fixes-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1349/newren/sparse-checkout-completion-fixes-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1349
Range-diff vs v1:
1: 591c7b8d73b ! 1: 97e20e3b99d completion: squelch stray errors in sparse-checkout completion
@@ Commit message
fatal: ../: '../' is outside repository at '/home/newren/floss/git'
is written to stderr, which munges the users view of their own command.
- Squelch such messages.
+ Squelch such messages by using the __git() wrapper, designed for this
+ purpose; see commit e15098a314 (completion: consolidate silencing errors
+ from git commands, 2017-02-03) for more on the wrapper.
Signed-off-by: Elijah Newren <newren@gmail.com>
@@ contrib/completion/git-completion.bash: __gitcomp_directories ()
_found=1
fi
- done < <(git ls-tree -z -d --name-only HEAD $_tmp_dir)
-+ done < <(git ls-tree -z -d --name-only HEAD $_tmp_dir 2>/dev/null)
++ done < <(__git ls-tree -z -d --name-only HEAD $_tmp_dir)
if [[ $_found == 0 ]] && [[ "$cur" =~ /$ ]]; then
# No possible further completions any deeper, so assume we're at
2: a15fb054579 = 2: 212ba35ed46 completion: fix logic for determining whether cone mode is active
3: e8cc5c54e60 = 3: 1cbbcd9097c completion: avoid misleading completions in cone mode
4: fe8669a3f4f ! 4: 604f21dc827 completion: avoid user confusion in non-cone mode
@@ contrib/completion/git-completion.bash: _git_sparse_checkout ()
+ # completion function which:
+ #
+ # 1. Provides completions based on
-+ # files/directories that exist in HEAD, not
-+ # just those currently present in the working
-+ # tree. Bash's default file and directory
-+ # completion is totally useless for "git
-+ # sparse-checkout add" because of this. It is
-+ # likewise problematic for "git
-+ # sparse-checkout set" except in those subset
-+ # of cases when trying to narrow scope to a
-+ # strict subset of what you already have
-+ # checked out.
++ # files/directories that exist in HEAD (or in
++ # the index since sparse-index isn't possible
++ # in non-cone mode), not just those currently
++ # present in the working tree. Bash's
++ # default file and directory completion is
++ # totally useless for "git sparse-checkout
++ # add" because of this. It is likewise
++ # problematic for "git sparse-checkout set"
++ # except in those subset of cases when trying
++ # to narrow scope to a strict subset of what
++ # you already have checked out.
+ #
+ # 2. Always provides file/directory completions
+ # with a prepended leading '/', so that
--
gitgitgadget
^ permalink raw reply
* [PATCH v2 1/4] completion: squelch stray errors in sparse-checkout completion
From: Elijah Newren via GitGitGadget @ 2023-11-26 7:51 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, SZEDER Gábor, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1349.v2.git.1700985086.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>
If, in the root of a project, one types
git sparse-checkout set --cone ../<TAB>
then an error message of the form
fatal: ../: '../' is outside repository at '/home/newren/floss/git'
is written to stderr, which munges the users view of their own command.
Squelch such messages by using the __git() wrapper, designed for this
purpose; see commit e15098a314 (completion: consolidate silencing errors
from git commands, 2017-02-03) for more on the wrapper.
Signed-off-by: Elijah Newren <newren@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 13a39ebd2e7..b8661701718 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3084,7 +3084,7 @@ __gitcomp_directories ()
COMPREPLY+=("$c/")
_found=1
fi
- done < <(git ls-tree -z -d --name-only HEAD $_tmp_dir)
+ done < <(__git ls-tree -z -d --name-only HEAD $_tmp_dir)
if [[ $_found == 0 ]] && [[ "$cur" =~ /$ ]]; then
# No possible further completions any deeper, so assume we're at
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH] doc: make the gitfile syntax easier to discover
From: Junio C Hamano @ 2023-11-26 3:04 UTC (permalink / raw)
To: Marcel Krause; +Cc: git mailing list
In-Reply-To: <20231124194711.563720-1-mk+copyleft@pimpmybyte.de>
Marcel Krause <mk+copyleft@pimpmybyte.de> writes:
> It took way too long for me to find the syntax expected for a gitfile.
> My search engine found the gitglossary manpage which defined the term
> but had no hints about syntax.
> Thus here I add a mention of gitrepository-layout.
Everything you wrote is not very interesting or relevant story we
want to see in order to explain and justify this change. The title
itself is sufficient, i.e. it had poor visibility, and you fix it by
giving it better visibility.
A more relevant is why you needed to find out what the former should
be in the first place. "git submodule init" and "git worktree add"
would create them as necessary without you needing to know about the
exact implementation.
> Signed-off-by: Marcel Krause <mk+copyleft@pimpmybyte.de>
> ---
> Documentation/gitrepository-layout.txt | 8 ++++----
> Documentation/glossary-content.txt | 1 +
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/gitrepository-layout.txt b/Documentation/gitrepository-layout.txt
> index 1a2ef4c150..c52b8564e3 100644
> --- a/Documentation/gitrepository-layout.txt
> +++ b/Documentation/gitrepository-layout.txt
> @@ -23,10 +23,10 @@ A Git repository comes in two different flavours:
>
> *Note*: Also you can have a plain text file `.git` at the root of
> your working tree, containing `gitdir: <path>` to point at the real
> -directory that has the repository. This mechanism is often used for
> -a working tree of a submodule checkout, to allow you in the
> -containing superproject to `git checkout` a branch that does not
> -have the submodule. The `checkout` has to remove the entire
> +directory that has the repository. This mechanism is called a 'gitfile'
> +and is often used for a working tree of a submodule checkout, to allow
> +you in the containing superproject to `git checkout` a branch that
> +does not have the submodule. The `checkout` has to remove the entire
> submodule working tree, without losing the submodule repository.
Do not unnecessary rewrap existing text, only to insert a few words,
to force reviewers read a lot more than needed.
> diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
> index 5a537268e2..e5f55bf670 100644
> --- a/Documentation/glossary-content.txt
> +++ b/Documentation/glossary-content.txt
> @@ -184,6 +184,7 @@ current branch integrates with) obviously do not work, as there is no
> [[def_gitfile]]gitfile::
> A plain file `.git` at the root of a working tree that
> points at the directory that is the real repository.
> + See linkgit:gitrepository-layout for the syntax.
Running "git grep linkgit:gitrepository-layout" would help you find
the right way to spell this one, I think.
Thanks.
^ permalink raw reply
* Re: [PATCH 1/1] git-send-email causes failures because of wrong option specifications
From: Eric Sunshine @ 2023-11-26 2:39 UTC (permalink / raw)
To: H.Merijn Brand - Tux; +Cc: git
In-Reply-To: <20231125094429.12025-2-linux@tux.freedom.nl>
On Sat, Nov 25, 2023 at 4:52 AM H.Merijn Brand - Tux
<linux@tux.freedom.nl> wrote:
> ...
> git-2.43.0 🐧 perl -Iperl git-send-email.perl --help
> Duplicate specification "cc-cover|cc-cover!" for option "cc-cover"
> Duplicate specification "no-cc-cover" for option "no-cc-cover"
> ...
> Signed-off-by: H.Merijn Brand - Tux <linux@tux.freedom.nl>
> ---
> diff --git a/git-send-email.perl b/git-send-email.perl
> @@ -506,36 +505,27 @@ sub config_regexp {
> - "no-signed-off-cc|no-signed-off-by-cc" => sub {$signed_off_by_cc = 0},
> - "cc-cover|cc-cover!" => \$cover_cc,
> - "no-cc-cover" => sub {$cover_cc = 0},
> - "to-cover|to-cover!" => \$cover_to,
> - "no-to-cover" => sub {$cover_to = 0},
> + "cc-cover!" => \$cover_cc,
> + "to-cover!" => \$cover_to,
Thanks for submitting a patch to address this issue. It matches very
closely an earlier patch[1] addressing the same purpose which has
already made it into Junio's "next" branch.
[1]: https://lore.kernel.org/git/20231116193014.470420-3-tmz@pobox.com/
^ permalink raw reply
* [PATCH] builtin/reflog.c: fix dry-run option short name
From: Josh Brobst @ 2023-11-26 0:05 UTC (permalink / raw)
To: git; +Cc: Josh Brobst
The documentation for reflog states that the --dry-run option of the
expire and delete subcommands has a corresponding short name, -n.
However, 33d7bdd645 (builtin/reflog.c: use parse-options api for expire,
delete subcommands, 2022-01-06) did not include this short name in the
new options parsing.
Re-add the short name in the new dry-run option definitions.
Signed-off-by: Josh Brobst <josh@brob.st>
---
builtin/reflog.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/reflog.c b/builtin/reflog.c
index df63a5892e..a39962df69 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -248,7 +248,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
int verbose = 0;
reflog_expiry_should_prune_fn *should_prune_fn = should_expire_reflog_ent;
const struct option options[] = {
- OPT_BIT(0, "dry-run", &flags, N_("do not actually prune any entries"),
+ OPT_BIT('n', "dry-run", &flags, N_("do not actually prune any entries"),
EXPIRE_REFLOGS_DRY_RUN),
OPT_BIT(0, "rewrite", &flags,
N_("rewrite the old SHA1 with the new SHA1 of the entry that now precedes it"),
@@ -368,7 +368,7 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)
int verbose = 0;
const struct option options[] = {
- OPT_BIT(0, "dry-run", &flags, N_("do not actually prune any entries"),
+ OPT_BIT('n', "dry-run", &flags, N_("do not actually prune any entries"),
EXPIRE_REFLOGS_DRY_RUN),
OPT_BIT(0, "rewrite", &flags,
N_("rewrite the old SHA1 with the new SHA1 of the entry that now precedes it"),
--
2.42.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