* rebase invoking pre-commit
From: Sean Allred @ 2023-12-21 20:58 UTC (permalink / raw)
To: Git List
Is there a current reason why pre-commit shouldn't be invoked during
rebase, or is this just waiting for a reviewable patch?
This was brought up before at [1] in 2015, but that thread so old at
this point that it seemed prudent to double-check before investing time
in a developing and testing a patch.
[1]: https://lore.kernel.org/git/1m55i3m.1fum4zo1fpnhncM%25lists@haller-berlin.de/
--
Sean Allred
^ permalink raw reply
* Re: [PATCH v2 2/9] CodingGuidelines: write punctuation marks
From: Junio C Hamano @ 2023-12-21 20:57 UTC (permalink / raw)
To: Josh Soref via GitGitGadget
Cc: git, Elijah Newren, René Scharfe, Phillip Wood, Josh Soref
In-Reply-To: <c0db8336e519cb43767effbe842dc8b97f914a4b.1703176866.git.gitgitgadget@gmail.com>
"Josh Soref via GitGitGadget" <gitgitgadget@gmail.com> writes:
Nobody seems to have commented on this step in the previous round,
but I do not understand what you mean by ...
> From: Josh Soref <jsoref@gmail.com>
>
> - Match style in Release Notes
... at all. The patch text is fine, though.
> Signed-off-by: Josh Soref <jsoref@gmail.com>
> ---
> Documentation/CodingGuidelines | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
> index af94ed3a75d..578587a4715 100644
> --- a/Documentation/CodingGuidelines
> +++ b/Documentation/CodingGuidelines
> @@ -578,7 +578,7 @@ Externally Visible Names
> . The variable name describes the effect of tweaking this knob.
>
> The section and variable names that consist of multiple words are
> - formed by concatenating the words without punctuations (e.g. `-`),
> + formed by concatenating the words without punctuation marks (e.g. `-`),
> and are broken using bumpyCaps in documentation as a hint to the
> reader.
^ permalink raw reply
* Re: [PATCH 2/2] ref-filter: support filtering of operational refs
From: Junio C Hamano @ 2023-12-21 20:40 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git, ps, christian.couder
In-Reply-To: <20231221170715.110565-3-karthik.188@gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
> With the upcoming introduction of the reftable backend, it becomes ever
> so important to provide the necessary tooling for printing all refs
> associated with a repository.
We have pseudoref (those all caps files outside the refs/ hierarchy)
as an official term defined in the glossary, and Patrick's reftable
work based on Han-Wen's work revealed the need to treat FETCH_HEAD
and MERGE_HEAD as "even more pecurilar than pseudorefs" that need
different term (tentatively called "special refs"). Please avoid
coming up with yet another random name "operational" without
discussing.
With a quick look at the table in this patch, "pseudorefs" appears
to be the closest word that people are already familiar with, I
think. A lot more reasonable thing to do may be to scan the
$GIT_DIR for files whose name satisfy refs.c:is_pseudoref_syntax()
and list them, instead of having a hardcoded list of these special
refs. In addition, when reftable and other backends that can
natively store things outside refs/ hierarchy is in use, they ought
to know what they have so enumerating these would not be an issue
for them without having such a hardcoded table of names.
^ permalink raw reply
* Re: Unable to install git-2.43.0 from source on macOS Big Sur 11.7.10
From: Jonathan Abrams @ 2023-12-21 19:40 UTC (permalink / raw)
To: Eric Sunshine; +Cc: git@vger.kernel.org
In-Reply-To: <CAPig+cQgTNq8rKiXm_dDha+Rz-=Z9O4_gvWLWdcPJe1yp=hQ8Q@mail.gmail.com>
After deactivating the conda environment, I get a different error. The commands I entered are:
conda deactivate
make configure
./configure --prefix=/usr
make V=1 all doc
This ends with the following error.
/usr/bin/perl ./build-docdep.perl >doc.dep
/Applications/Xcode.app/Contents/Developer/usr/bin/make -C ../ GIT-VERSION-FILE
make[2]: `GIT-VERSION-FILE' is up to date.
fatal: not a git repository (or any of the parent directories): .git
/Applications/Xcode.app/Contents/Developer/usr/bin/make -C ../ GIT-VERSION-FILE
make[2]: `GIT-VERSION-FILE' is up to date.
* new asciidoc flags
asciidoc -f asciidoc.conf -amanmanual='Git Manual' -amansource='Git 2.43.0' -arevdate='' -b xhtml11 -d manpage -o git-add.html git-add.txt
make[1]: asciidoc: No such file or directory
make[1]: *** [git-add.html] Error 1
make: *** [doc] Error 2
Your suggestion to run the following commands results in a different error. The suggested commands were:
cd git-2.43.0
conda deactivate
make distclean
make all
This ends with the following error.
GIT_VERSION = 2.43.0
* new build flags
CC oss-fuzz/fuzz-commit-graph.o
CC oss-fuzz/fuzz-pack-headers.o
CC oss-fuzz/fuzz-pack-idx.o
CC daemon.o
* new link flags
CC common-main.o
In file included from common-main.c:3:
./gettext.h:17:11: fatal error: 'libintl.h' file not found
# include <libintl.h>
^~~~~~~~~~~
1 error generated.
make: *** [common-main.o] Error 1
Looking around line 17 in ./gettext.h, I see this.
#ifndef NO_GETTEXT
# include <libintl.h>
Executing gettext --version returned nothing. I then downloaded gettext and installed it using the following commands.
./configure
make
make check
sudo make install
I then executed your suggested commands again.
conda deactivate
make distclean
make all
sudo make install
This installed GIT. Thank you for your guidance!
Jonathan S. Abrams
^ permalink raw reply
* Re: [PATCH/RFC] sparse-checkout: take care of "--end-of-options" in set/add
From: Junio C Hamano @ 2023-12-21 18:20 UTC (permalink / raw)
To: Jeff King; +Cc: Derrick Stolee, git, Josh Steadmon
In-Reply-To: <20231221083643.GA545870@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Which would mean that simply dropping --end-of-options from the list, as
> your patch does, would be similarly unsafe. It is papering over the
> problem of:
>
> git sparse-checkout --end-of-options foo
>
> but leaving:
>
> git sparse-checkout --end-of-options --foo
>
> broken.
Hmph, I do not understand. The latter case we want to take "--foo"
as the sole parameter to the subcommand, no?
> But the plot thickens! Curiously, in both of these cases, we do not do
> any further parsing of the options at all. We just treat them as
> pattern arguments with no special meaning.
Exactly.
> So your patch is actually OK, but I would argue that the correct fix
> here is to drop the use of PARSE_OPT_KEEP_UNKNOWN_OPT at all. I cannot
> find any indication in the history on why it was ever used. It was added
> when the command was converted to parse-options in 7bffca95ea
> (sparse-checkout: add '--stdin' option to set subcommand, 2019-11-21).
> Back then it was just called KEEP_UNKNOWN. Later it was renamed to
> KEEP_UNKNOWN_OPT in 99d86d60e5 (parse-options: PARSE_OPT_KEEP_UNKNOWN
> only applies to --options, 2022-08-19) to clarify that it was only about
> dashed options; we always keep non-option arguments.
Yes.
> So looking at that original patch, it makes me think that the author was
> confused about the mis-named option, and really just wanted to keep the
> non-option arguments. We never should have used the flag all along (and
> the other cases were cargo-culted within the file).
That is something only the original author can answer, by my working
theory has been they wanted to have a cheap way to allow any string,
even the ones that happen to begin with a dash, to be passed as one
of the patterns.
> There is one minor gotcha, though. Unlike most other Git commands,
> sparse-checkout will happily accept random option names and treat them
> as non-option arguments. E.g. this works:
>
> git sparse-checkout add --foo --bar
>
> and will add "--foo" and "--bar" as patterns. If we remove the flag,
> we'd be breaking that.
Exactly. The user _should_ protect these "patterns" by using
"--end-of-options" before "--foo" above, but we have been taking the
patterns with such a loose parser for quite some time, so tightening
would be taken as a regression X-<.
> But I'd argue that anybody relying on that is
> asking for trouble. For example, this does not work in the same way:
>
> git sparse-checkout add --skip-checks --foo
>
> because "--skip-checks" is a real option. Ditto for any other options,
> including those we add in the future. If you don't trust the contents of
> your arguments, you should be using "--" or "--end-of-options" to
> communicate the intent to the command.
Exactly. My response to the other message, with a new test,
summarizes my position.
Thanks.
^ permalink raw reply
* Re: [RFC/PATCH] archive: "--list" does not take further options
From: Junio C Hamano @ 2023-12-21 18:13 UTC (permalink / raw)
To: Jeff King; +Cc: René Scharfe, git
In-Reply-To: <20231221085948.GD545870@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Thu, Dec 21, 2023 at 08:30:36AM +0100, René Scharfe wrote:
>> ...
>> Don't we have one? It would affect other unsupported options as well,
>> and this seems to work just fine, e.g.:
>> ...
> Right. The whole idea of upload-archive is to spawn a separate writer
> process and mux the conversation (including errors) back over the wire.
Thanks, both. Just to tie the loose end, let me queue this and
merge it to 'next'.
----- >8 --------- >8 --------- >8 -----
Subject: [PATCH] archive: "--list" does not take further options
"git archive --list blah" should notice an extra command line
parameter that goes unused. Make it so.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
archive.c | 2 ++
t/t5000-tar-tree.sh | 10 ++++++++++
2 files changed, 12 insertions(+)
diff --git a/archive.c b/archive.c
index ca11db185b..8da820d1ce 100644
--- a/archive.c
+++ b/archive.c
@@ -685,6 +685,8 @@ static int parse_archive_args(int argc, const char **argv,
base = "";
if (list) {
+ if (argc)
+ die(_("extra command line parameter '%s'"), *argv);
for (i = 0; i < nr_archivers; i++)
if (!is_remote || archivers[i]->flags & ARCHIVER_REMOTE)
printf("%s\n", archivers[i]->name);
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index 4b4c3315d8..72b8d0ff02 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -124,6 +124,16 @@ test_expect_success 'setup' '
EOF
'
+test_expect_success '--list notices extra parameters' '
+ test_must_fail git archive --list blah &&
+ test_must_fail git archive --remote=. --list blah
+'
+
+test_expect_success 'end-of-options is correctly eaten' '
+ git archive --list --end-of-options &&
+ git archive --remote=. --list --end-of-options
+'
+
test_expect_success 'populate workdir' '
mkdir a &&
echo simple textfile >a/a &&
--
2.43.0-174-g055bb6e996
^ permalink raw reply related
* [ISSUE] `git push` keep asking for username when the global config already configured (wincred)
From: Andry @ 2023-12-21 16:16 UTC (permalink / raw)
To: Git
Hello Git,
Accidentally have found the issue when the Git is properly installed (git-scm.com) and configured over the global config under Window 8.
Btu if try to use the Cygwin (cygwin.com) installation for the Bash interpreter like this from the `myscript.bat`:
myscript.bat -> myscript.sh -> `git push ...`
Then the git asks for the username.
If directly run the command `git push ...`, then it works as expected without the username prompt.
The issue is around the HOME variable behind the Cygwin installation, when the Git thinks the configuration is in the Cygwin, instead of in the Windows users folder.
The workaround can be something like this:
```bash
# WORKAROUND:
# The `git push` asks for username under the bash shell call from the Windows cmd.exe script.
#
[[ -n "${COMSPEC+x}" ]] && unset HOME
```
>git --version
git version 2.43.0.windows.1
>winver
Version 6.2 (Build 9200)
^ permalink raw reply
* [PATCH 2/2] ref-filter: support filtering of operational refs
From: Karthik Nayak @ 2023-12-21 17:07 UTC (permalink / raw)
To: git; +Cc: gitster, ps, christian.couder, Karthik Nayak
In-Reply-To: <20231221170715.110565-1-karthik.188@gmail.com>
With the upcoming introduction of the reftable backend, it becomes ever
so important to provide the necessary tooling for printing all refs
associated with a repository.
While regular refs stored within the "refs/" namespace are currently
supported by multiple commands like git-for-each-ref(1),
git-show-ref(1). Neither support printing all the operational refs
within the repository.
This is crucial because with the reftable backend, all these refs will
also move to reftable. It would be necessary to identify all the refs
that are stored within the reftable since there is no easy way to do so
otherwise. This is because the reftable itself is a binary format and
all access will be via git. Unlike the filesystem backend, which allows
access directly via the filesystem.
This commit adds the necessary code to iterate over operational refs
present in a repository and provides a new filter flag
'FILTER_REFS_OPERATIONAL' to iterate over operational refs.
This flag can now be used to extend existing git commands to print
operational refs.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
ref-filter.c | 12 ++++++++++++
ref-filter.h | 4 +++-
refs.c | 14 ++++++++++++++
refs.h | 3 +++
4 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/ref-filter.c b/ref-filter.c
index fdaabb5bb4..307a512c27 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2756,6 +2756,10 @@ static int filter_ref_kind(struct ref_filter *filter, const char *refname)
filter->kind == FILTER_REFS_REMOTES ||
filter->kind == FILTER_REFS_TAGS)
return filter->kind;
+
+ if (filter->kind & FILTER_REFS_OPERATIONAL)
+ return filter->kind;
+
return ref_kind_from_refname(refname);
}
@@ -3032,6 +3036,14 @@ static int do_filter_refs(struct ref_filter *filter, unsigned int type, each_ref
if (!filter->kind)
die("filter_refs: invalid type");
else {
+ size_t i;
+ if (filter->kind & FILTER_REFS_OPERATIONAL) {
+ for (i = 0; i < operational_refs_max; i++) {
+ refs_single_ref(get_main_ref_store(the_repository),
+ operational_refs[i], fn, cb_data);
+ }
+ }
+
/*
* For common cases where we need only branches or remotes or tags,
* we only iterate through those refs. If a mix of refs is needed,
diff --git a/ref-filter.h b/ref-filter.h
index 0ce5af58ab..eec1d29514 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -23,7 +23,9 @@
#define FILTER_REFS_ALL (FILTER_REFS_TAGS | FILTER_REFS_BRANCHES | \
FILTER_REFS_REMOTES | FILTER_REFS_OTHERS)
#define FILTER_REFS_DETACHED_HEAD 0x0020
-#define FILTER_REFS_KIND_MASK (FILTER_REFS_ALL | FILTER_REFS_DETACHED_HEAD)
+#define FILTER_REFS_OPERATIONAL 0x0040
+#define FILTER_REFS_KIND_MASK (FILTER_REFS_ALL | FILTER_REFS_DETACHED_HEAD | \
+ FILTER_REFS_OPERATIONAL)
struct atom_value;
struct ref_sorting;
diff --git a/refs.c b/refs.c
index cebc5458d3..70f6854301 100644
--- a/refs.c
+++ b/refs.c
@@ -82,6 +82,20 @@ static unsigned char refname_disposition[256] = {
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 4, 4
};
+const char *operational_refs[] = {
+ "HEAD",
+ "ORIG_HEAD",
+ "REBASE_HEAD",
+ "REVERT_HEAD",
+ "CHERRY_PICK_HEAD",
+ "BISECT_HEAD",
+ "BISECT_EXPECTED_REV",
+ "NOTES_MERGE_PARTIAL",
+ "NOTES_MERGE_REF",
+};
+
+const int operational_refs_max = ARRAY_SIZE(operational_refs) - 1;
+
struct ref_namespace_info ref_namespace[] = {
[NAMESPACE_HEAD] = {
.ref = "HEAD",
diff --git a/refs.h b/refs.h
index e147f13a85..b01938a91a 100644
--- a/refs.h
+++ b/refs.h
@@ -15,6 +15,9 @@ int default_ref_storage_format(void);
int ref_storage_format_by_name(const char *name);
const char *ref_storage_format_to_name(int ref_storage_format);
+extern const char *operational_refs[];
+extern const int operational_refs_max;
+
/*
* Resolve a reference, recursively following symbolic refererences.
*
--
2.43.GIT
^ permalink raw reply related
* [PATCH 1/2] refs: introduce the `refs_single_ref` function
From: Karthik Nayak @ 2023-12-21 17:07 UTC (permalink / raw)
To: git; +Cc: gitster, ps, christian.couder, Karthik Nayak
In-Reply-To: <20231221170715.110565-1-karthik.188@gmail.com>
We currently provide the functionality to iterate over refs and call a
specific callback. This functionality currently supports iterating over
the entire "refs/*" space or directly the "HEAD" ref. This leaves
operational refs "ORIG_HEAD", "REBASE_HEAD" and so forth behind.
In the following commit, we introduce a mechanism to process all the
operational refs outside the "refs/*" space. To do this, we require a
function similar `refs_head_ref`, which can process a single specified
reference. This commit introduces `refs_single_ref` to fill in that gap.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
refs.c | 12 +++++++++---
refs.h | 2 ++
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/refs.c b/refs.c
index 59fea0d44b..cebc5458d3 100644
--- a/refs.c
+++ b/refs.c
@@ -1549,18 +1549,24 @@ const char *find_descendant_ref(const char *dirname,
return NULL;
}
-int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
+int refs_single_ref(struct ref_store *refs, const char *refname,
+ each_ref_fn fn, void *cb_data)
{
struct object_id oid;
int flag;
- if (refs_resolve_ref_unsafe(refs, "HEAD", RESOLVE_REF_READING,
+ if (refs_resolve_ref_unsafe(refs, refname, RESOLVE_REF_READING,
&oid, &flag))
- return fn("HEAD", &oid, flag, cb_data);
+ return fn(refname, &oid, flag, cb_data);
return 0;
}
+int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
+{
+ return refs_single_ref(refs, "HEAD", fn, cb_data);
+}
+
int head_ref(each_ref_fn fn, void *cb_data)
{
return refs_head_ref(get_main_ref_store(the_repository), fn, cb_data);
diff --git a/refs.h b/refs.h
index 4816e46846..e147f13a85 100644
--- a/refs.h
+++ b/refs.h
@@ -324,6 +324,8 @@ typedef int each_repo_ref_fn(struct repository *r,
* modifies the reference also returns a nonzero value to immediately
* stop the iteration. Returned references are sorted.
*/
+int refs_single_ref(struct ref_store *refs, const char *refname,
+ each_ref_fn fn, void *cb_data);
int refs_head_ref(struct ref_store *refs,
each_ref_fn fn, void *cb_data);
int refs_for_each_ref(struct ref_store *refs,
--
2.43.GIT
^ permalink raw reply related
* [RFC 0/2] Initial changes to support printing all refs
From: Karthik Nayak @ 2023-12-21 17:07 UTC (permalink / raw)
To: git; +Cc: gitster, ps, christian.couder, Karthik Nayak
With the upcoming introduction of the reftable backend, it becomes ever
so important to provide the necessary tooling for printing all refs
associated with a repository.
While regular refs stored within the "refs/" namespace are currently
supported by multiple commands like git-for-each-ref(1),
git-show-ref(1). Neither support printing all the operational refs
within the repository.
This is crucial because with the reftable backend, all these refs will
also move to reftable. It would be necessary to identify all the refs
that are stored within the reftable since there is no easy way to do so
otherwise. This is because the reftable itself is a binary format and
all access will be via git. Unlike the filesystem backend, which allows
access directly via the filesystem.
This patch series is an RFC to discuss the intent and direction to be
taken to implement tooling for printing all refs in a repository. The
patches in this series implement a rather simple way to do this, by
iterating over a static list of operational refs. The drawback of this
approach is that this could still miss refs in the reftable/files
backend which are not in "refs" namespace or part of the list. The
positive being that this is ref backend agnostic.
The alternate approach would be to patch this inside individual backends
respectively, in the reftable backend this is rather simple, since we
simply iterate over all refs and not filter for "refs" namespace. In the
files backend, this can be done in two ways:
1. static approach: similar to this patch series, the files backend
would iterate over a static list of operational refs apart from the
"refs" directory.
2. dynamic approach: iterate over all files in `.git` folder and print
any refs if encountered. This is rather tedious and error prone, since
any file could be mistaken for a reference if the content is reference
like.
Personally, I'm leaning towards implementing this functionality inside
individual backends respectively, because that would allow the reftable
backend to print all its refs while the current approach might miss some
refs. But with the files backend, it would be best to still use a static
list.
Over this, I'm also curious on what the mailing list thinks about
exposing this to the client side. Would an `--all` option for
git-for-each-ref(1) be sufficient?
Karthik Nayak (2):
refs: introduce the `refs_single_ref` function
ref-filter: support filtering of operational refs
ref-filter.c | 12 ++++++++++++
ref-filter.h | 4 +++-
refs.c | 26 +++++++++++++++++++++++---
refs.h | 5 +++++
4 files changed, 43 insertions(+), 4 deletions(-)
--
2.43.GIT
^ permalink raw reply
* Re: [PATCH/RFC] sparse-checkout: take care of "--end-of-options" in set/add
From: Junio C Hamano @ 2023-12-21 17:02 UTC (permalink / raw)
To: Jeff King; +Cc: Josh Steadmon, git
In-Reply-To: <20231221084011.GB545870@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Dec 20, 2023 at 06:46:51PM -0800, Junio C Hamano wrote:
>
>> Josh Steadmon <steadmon@google.com> writes:
>>
>> > I can confirm that this fixes an issue noticed by sparse-checkout users
>> > at $DAYJOB. Looks good to me. Thanks!
>>
>> Heh, there is another one that is not converted in the same file for
>> "check-rules" subcommand, so the posted patch is way incomplete, I
>> think.
>
> Yeah. I think it is in the same boat as the other two, in that I believe
> that the KEEP_UNKNOWN_OPT flag is counter-productive and should just be
> dropped.
If we dropped KEEP_UNKNOWN_OPT, however, the pattern that is
currently accepted will stop working, e.g.,
$ git sparse-checkout [add/set] --[no-]cone --foo bar
as we would barf with "--foo: unknown option", and the users who are
used to this sloppy command line parsing we had for the past few
years must now write "--end-of-options" before "--foo". After all,
the reason why the original authors of this code used KEEP_UNKNOWN
is likely to deal with path patterns that begin with dashes.
The patch in the message that started this thread may not be
correct, either, I am afraid. For either of these:
$ git sparse-checkout [add/set] --[no-]cone foo --end-of-options bar
$ git sparse-checkout [add/set] --[no-]cone --foo --end-of-options bar
we would see that "foo" (or "--foo") is not "--end-of-options", and
we end up using three patterns "foo" (or "--foo"),
"--end-of-options", and "bar", I suspect. I wonder if we should
notice the "foo" or "--foo" that were not understood and error out,
instead.
But after all, it is not absolutely necessary to notice and barf.
The ONLY practical use of the "--end-of-options" mechanism is to
allow us to write (this applies to any git subcommand):
#!/bin/sh
git cmd --hard --coded --options --end-of-options "$@"
in scripts to protect the intended operation from mistaking the
end-user input as options. And with a script written carefully to
do so, all the args that appear before "--end-of-options" would be
recognizable by the command line parser. On the other hand, such a
"notice and barf" would not help a script that is written without
"--end-of-options", when it is fed "$@" that happens to begin with
"--end-of-options". We would silently swallow "--end-of-options"
without any chance to notice the lack of "--end-of-options" in the
script. So I dunno.
Here is an additional test on top of [1/3]
<20231221065925.3234048-2-gitster@pobox.com>
that demonstrates and summarizes the idea.
t/t1090-sparse-checkout-scope.sh | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git c/t/t1090-sparse-checkout-scope.sh w/t/t1090-sparse-checkout-scope.sh
index 5b96716235..da9534d222 100755
--- c/t/t1090-sparse-checkout-scope.sh
+++ w/t/t1090-sparse-checkout-scope.sh
@@ -54,6 +54,40 @@ test_expect_success 'return to full checkout of main' '
test "$(cat b)" = "modified"
'
+test_expect_success 'sparse-checkout set command line parsing' '
+ # baseline
+ git sparse-checkout disable &&
+ git sparse-checkout set --no-cone "a*" &&
+ echo "a*" >expect &&
+ test_cmp .git/info/sparse-checkout expect &&
+
+ # unknown string that looks like a dashed option is
+ # taken as a mere pattern
+ git sparse-checkout disable &&
+ git sparse-checkout set --no-cone --foo "a*" &&
+ test_write_lines "--foo" "a*" >expect &&
+ test_cmp .git/info/sparse-checkout expect &&
+
+ # --end-of-options can be used to protect parameters that
+ # potentially begin with dashes
+ set x --cone "a*" && shift &&
+ git sparse-checkout disable &&
+ git sparse-checkout set --no-cone --end-of-options "$@" &&
+ test_write_lines "$@" >expect &&
+ test_cmp .git/info/sparse-checkout expect &&
+
+ # but writing --end-of-options after what the command does not
+ # recognize is too late; it becomes one of the patterns, so
+ # that the end-user input that happens to be "--end-of-options"
+ # can be passed through. To be absoutely sure, you should write
+ # --end-of-options yourself before taking "$@" in.
+ set x --foo --end-of-options "a*" && shift &&
+ git sparse-checkout disable &&
+ git sparse-checkout set --no-cone "$@" &&
+ test_write_lines "$@" >expect &&
+ test_cmp .git/info/sparse-checkout expect
+'
+
test_expect_success 'skip-worktree on files outside sparse patterns' '
git sparse-checkout disable &&
git sparse-checkout set --no-cone "a*" &&
^ permalink raw reply related
* [PATCH v2 9/9] SubmittingPatches: hyphenate non-ASCII
From: Josh Soref via GitGitGadget @ 2023-12-21 16:41 UTC (permalink / raw)
To: git
Cc: Elijah Newren, René Scharfe, Phillip Wood, Josh Soref,
Josh Soref, Josh Soref
In-Reply-To: <pull.1623.v2.git.1703176865.gitgitgadget@gmail.com>
From: Josh Soref <jsoref@gmail.com>
Git documentation does this with the exception of ancient release notes.
Signed-off-by: Josh Soref <jsoref@gmail.com>
---
Documentation/SubmittingPatches | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index cb0dcce6a17..9283eb0ef71 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -699,7 +699,7 @@ message to an external program, and this is a handy way to drive
`git am`. However, if the message is MIME encoded, what is
piped into the program is the representation you see in your
`*Article*` buffer after unwrapping MIME. This is often not what
-you would want for two reasons. It tends to screw up non ASCII
+you would want for two reasons. It tends to screw up non-ASCII
characters (most notably in people's names), and also
whitespaces (fatal in patches). Running "C-u g" to display the
message in raw form before using "|" to run the pipe can work
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 8/9] SubmittingPatches: clarify GitHub artifact format
From: Josh Soref via GitGitGadget @ 2023-12-21 16:41 UTC (permalink / raw)
To: git
Cc: Elijah Newren, René Scharfe, Phillip Wood, Josh Soref,
Josh Soref, Josh Soref
In-Reply-To: <pull.1623.v2.git.1703176865.gitgitgadget@gmail.com>
From: Josh Soref <jsoref@gmail.com>
GitHub wraps artifacts generated by workflows in a .zip file.
Internally, workflows can package anything they like in them.
A recently generated failure artifact had the form:
windows-artifacts.zip
Length Date Time Name
--------- ---------- ----- ----
76001695 12-19-2023 01:35 artifacts.tar.gz
11005650 12-19-2023 01:35 tracked.tar.gz
--------- -------
87007345 2 files
Signed-off-by: Josh Soref <jsoref@gmail.com>
---
Documentation/SubmittingPatches | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 8f79253c5cb..cb0dcce6a17 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -605,7 +605,8 @@ branches here: `https://github.com/<Your GitHub handle>/git/actions/workflows/ma
If a branch did not pass all test cases then it is marked with a red
+x+. In that case you can click on the failing job and navigate to
"ci/run-build-and-tests.sh" and/or "ci/print-test-failures.sh". You
-can also download "Artifacts" which are tarred (or zipped) archives
+can also download "Artifacts" which are zip archives containing
+tarred (or zipped) archives
with test data relevant for debugging.
Then fix the problem and push your fix to your GitHub fork. This will
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 7/9] SubmittingPatches: clarify GitHub visual
From: Josh Soref via GitGitGadget @ 2023-12-21 16:41 UTC (permalink / raw)
To: git
Cc: Elijah Newren, René Scharfe, Phillip Wood, Josh Soref,
Josh Soref, Josh Soref
In-Reply-To: <pull.1623.v2.git.1703176865.gitgitgadget@gmail.com>
From: Josh Soref <jsoref@gmail.com>
GitHub has two general forms for its states, sometimes they're a simple
colored object (e.g. green check or red x), and sometimes there's also a
colored container (e.g. green box or red circle) with containing that
object (e.g. check or x).
That's a lot of words to try to describe things, but in general, the key
for a failure is that it's recognized as an `x` and that it's associated
with the color red -- the color of course is problematic for people who
are red-green color-blind, but that's why they are paired with distinct
shapes.
Using the term `cross` doesn't really help.
Signed-off-by: Josh Soref <jsoref@gmail.com>
---
Documentation/SubmittingPatches | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 4476b52a50f..8f79253c5cb 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -603,7 +603,7 @@ to your fork of Git on GitHub. You can monitor the test state of all your
branches here: `https://github.com/<Your GitHub handle>/git/actions/workflows/main.yml`
If a branch did not pass all test cases then it is marked with a red
-cross. In that case you can click on the failing job and navigate to
++x+. In that case you can click on the failing job and navigate to
"ci/run-build-and-tests.sh" and/or "ci/print-test-failures.sh". You
can also download "Artifacts" which are tarred (or zipped) archives
with test data relevant for debugging.
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 6/9] SubmittingPatches: improve extra tags advice
From: Josh Soref via GitGitGadget @ 2023-12-21 16:41 UTC (permalink / raw)
To: git
Cc: Elijah Newren, René Scharfe, Phillip Wood, Josh Soref,
Josh Soref, Josh Soref
In-Reply-To: <pull.1623.v2.git.1703176865.gitgitgadget@gmail.com>
From: Josh Soref <jsoref@gmail.com>
Current statistics show a strong preference to only capitalize the first
letter in a hyphenated tag, but that some guidance would be helpful:
git log |
perl -ne 'next unless /^\s+(?:Signed-[oO]ff|Acked)-[bB]y:/;
s/^\s+//;s/:.*/:/;print'|
sort|uniq -c|sort -n
2 Signed-off-By:
4 Signed-Off-by:
22 Acked-By:
47 Signed-Off-By:
2202 Acked-by:
95315 Signed-off-by:
Signed-off-by: Josh Soref <jsoref@gmail.com>
---
Documentation/SubmittingPatches | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 31878cb70b7..4476b52a50f 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -368,6 +368,9 @@ While you can also create your own trailer if the situation warrants it, we
encourage you to instead use one of the common trailers in this project
highlighted above.
+Extra tags should only capitalize the very first letter, i.e. favor
+"Signed-off-by" over "Signed-Off-By" and "Acked-by:" over "Acked-By".
+
[[git-tools]]
=== Generate your patch using Git tools out of your commits.
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 5/9] SubmittingPatches: update extra tags list
From: Josh Soref via GitGitGadget @ 2023-12-21 16:41 UTC (permalink / raw)
To: git
Cc: Elijah Newren, René Scharfe, Phillip Wood, Josh Soref,
Josh Soref, Josh Soref
In-Reply-To: <pull.1623.v2.git.1703176865.gitgitgadget@gmail.com>
From: Josh Soref <jsoref@gmail.com>
Add items with at least 100 uses in the past three years:
- Co-authored-by
- Helped-by
- Mentored-by
- Suggested-by
git log --since=3.years|
perl -ne 'next unless /^\s+[A-Z][a-z]+-\S+:/;s/^\s+//;s/:.*/:/;print'|
sort|uniq -c|sort -n|grep '[0-9][0-9] '
14 Based-on-patch-by:
14 Original-patch-by:
17 Tested-by:
100 Suggested-by:
121 Co-authored-by:
163 Mentored-by:
274 Reported-by:
290 Acked-by:
450 Helped-by:
602 Reviewed-by:
14111 Signed-off-by:
Signed-off-by: Josh Soref <jsoref@gmail.com>
---
Documentation/SubmittingPatches | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 58dfe405049..31878cb70b7 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -355,6 +355,14 @@ If you like, you can put extra tags at the end:
patch after a detailed analysis.
. `Tested-by:` is used to indicate that the person applied the patch
and found it to have the desired effect.
+. `Co-authored-by:` is used to indicate that people exchanged drafts
+ of a patch before submitting it.
+. `Helped-by:` is used to credit someone who suggested ideas for
+ changes without providing the precise changes in patch form.
+. `Mentored-by:` is used to credit someone with helping develop a
+ patch as part of a mentorship program (e.g., GSoC or Outreachy).
+. `Suggested-by:` is used to credit someone with suggesting the idea
+ for a patch.
While you can also create your own trailer if the situation warrants it, we
encourage you to instead use one of the common trailers in this project
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 4/9] SubmittingPatches: discourage new trailers
From: Josh Soref via GitGitGadget @ 2023-12-21 16:41 UTC (permalink / raw)
To: git
Cc: Elijah Newren, René Scharfe, Phillip Wood, Josh Soref,
Josh Soref, Josh Soref
In-Reply-To: <pull.1623.v2.git.1703176865.gitgitgadget@gmail.com>
From: Josh Soref <jsoref@gmail.com>
There seems to be consensus amongst the core Git community on a working
set of common trailers, and there are non-trivial costs to people
inventing new trailers (research to discover what they mean/how they
differ from existing trailers) such that inventing new ones is generally
unwarranted and not something to be recommended to new contributors.
Suggested-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Josh Soref <jsoref@gmail.com>
---
Documentation/SubmittingPatches | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 32e90238777..58dfe405049 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -356,8 +356,9 @@ If you like, you can put extra tags at the end:
. `Tested-by:` is used to indicate that the person applied the patch
and found it to have the desired effect.
-You can also create your own tag or use one that's in common usage
-such as "Thanks-to:", "Based-on-patch-by:", or "Mentored-by:".
+While you can also create your own trailer if the situation warrants it, we
+encourage you to instead use one of the common trailers in this project
+highlighted above.
[[git-tools]]
=== Generate your patch using Git tools out of your commits.
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 3/9] SubmittingPatches: drop ref to "What's in git.git"
From: Josh Soref via GitGitGadget @ 2023-12-21 16:40 UTC (permalink / raw)
To: git
Cc: Elijah Newren, René Scharfe, Phillip Wood, Josh Soref,
Josh Soref, Josh Soref
In-Reply-To: <pull.1623.v2.git.1703176865.gitgitgadget@gmail.com>
From: Josh Soref <jsoref@gmail.com>
"What's in git.git" was last seen in 2010:
https://lore.kernel.org/git/?q=%22what%27s+in+git.git%22
https://lore.kernel.org/git/7vaavikg72.fsf@alter.siamese.dyndns.org/
Signed-off-by: Josh Soref <jsoref@gmail.com>
---
Documentation/SubmittingPatches | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index bce7f97815c..32e90238777 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -570,7 +570,7 @@ their trees themselves.
master).
* Read the Git mailing list, the maintainer regularly posts messages
- entitled "What's cooking in git.git" and "What's in git.git" giving
+ entitled "What's cooking in git.git" giving
the status of various proposed changes.
== GitHub CI[[GHCI]]
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 2/9] CodingGuidelines: write punctuation marks
From: Josh Soref via GitGitGadget @ 2023-12-21 16:40 UTC (permalink / raw)
To: git
Cc: Elijah Newren, René Scharfe, Phillip Wood, Josh Soref,
Josh Soref, Josh Soref
In-Reply-To: <pull.1623.v2.git.1703176865.gitgitgadget@gmail.com>
From: Josh Soref <jsoref@gmail.com>
- Match style in Release Notes
Signed-off-by: Josh Soref <jsoref@gmail.com>
---
Documentation/CodingGuidelines | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index af94ed3a75d..578587a4715 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -578,7 +578,7 @@ Externally Visible Names
. The variable name describes the effect of tweaking this knob.
The section and variable names that consist of multiple words are
- formed by concatenating the words without punctuations (e.g. `-`),
+ formed by concatenating the words without punctuation marks (e.g. `-`),
and are broken using bumpyCaps in documentation as a hint to the
reader.
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 1/9] CodingGuidelines: move period inside parentheses
From: Josh Soref via GitGitGadget @ 2023-12-21 16:40 UTC (permalink / raw)
To: git
Cc: Elijah Newren, René Scharfe, Phillip Wood, Josh Soref,
Josh Soref, Josh Soref
In-Reply-To: <pull.1623.v2.git.1703176865.gitgitgadget@gmail.com>
From: Josh Soref <jsoref@gmail.com>
The contents within parenthesis should be omittable without resulting
in broken text.
Eliding the parenthesis left a period to end a run without any content.
Signed-off-by: Josh Soref <jsoref@gmail.com>
---
Documentation/CodingGuidelines | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 8ed517a5ca0..af94ed3a75d 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -450,7 +450,7 @@ For C programs:
one of the approved headers that includes it first for you. (The
approved headers currently include "builtin.h",
"t/helper/test-tool.h", "xdiff/xinclude.h", or
- "reftable/system.h"). You do not have to include more than one of
+ "reftable/system.h".) You do not have to include more than one of
these.
- A C file must directly include the header files that declare the
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 0/9] Minor improvements to CodingGuidelines and SubmittingPatches
From: Josh Soref via GitGitGadget @ 2023-12-21 16:40 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, René Scharfe, Phillip Wood, Josh Soref,
Josh Soref
In-Reply-To: <pull.1623.git.1702975319.gitgitgadget@gmail.com>
These are a bunch of things I've run into over my past couple of attempts to
contribute to Git.
* Incremental punctuation/grammatical improvements
* Update extra tags suggestions based on common usage
* drop reference to an article that was discontinued over a decade ago
* update GitHub references
* harmonize non-ASCII while I'm here
Note that I'm trying to do things "in the neighborhood". It'll be slower
than me replacing things topically, but hopefully easier for others to
digest. My current estimate is a decade or two :).
Josh Soref (9):
CodingGuidelines: move period inside parentheses
CodingGuidelines: write punctuation marks
SubmittingPatches: drop ref to "What's in git.git"
SubmittingPatches: discourage new trailers
SubmittingPatches: update extra tags list
SubmittingPatches: improve extra tags advice
SubmittingPatches: clarify GitHub visual
SubmittingPatches: clarify GitHub artifact format
SubmittingPatches: hyphenate non-ASCII
Documentation/CodingGuidelines | 4 ++--
Documentation/SubmittingPatches | 27 ++++++++++++++++++++-------
2 files changed, 22 insertions(+), 9 deletions(-)
base-commit: 624eb90fa8f65a79396615f3c2842ac5a3743350
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1623%2Fjsoref%2Fdocumentation-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1623/jsoref/documentation-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1623
Range-diff vs v1:
1: b9a8eb6aa4e = 1: b9a8eb6aa4e CodingGuidelines: move period inside parentheses
2: c0db8336e51 = 2: c0db8336e51 CodingGuidelines: write punctuation marks
3: 22d66c5b78a = 3: 22d66c5b78a SubmittingPatches: drop ref to "What's in git.git"
4: e5c7f29af43 ! 4: eac2211332f SubmittingPatches: update extra tags list
@@ Metadata
Author: Josh Soref <jsoref@gmail.com>
## Commit message ##
- SubmittingPatches: update extra tags list
+ SubmittingPatches: discourage new trailers
- Add items with at least 100 uses:
- - Co-authored-by
- - Helped-by
- - Mentored-by
- - Suggested-by
-
- Updating the create suggestion to something less commonly used.
-
- git log |
- perl -ne 'next unless /^\s+[A-Z][a-z]+-\S+:/;s/^\s+//;s/:.*/:/;print'|
- sort|uniq -c|sort -n|grep '[0-9][0-9] '
- 11 Helped-By:
- 13 Message-ID:
- 14 Reported-By:
- 22 Acked-By:
- 27 Inspired-by:
- 29 Requested-by:
- 35 Original-patch-by:
- 43 Contributions-by:
- 47 Signed-Off-By:
- 65 Based-on-patch-by:
- 68 Thanks-to:
- 88 Improved-by:
- 145 Co-authored-by:
- 171 Noticed-by:
- 182 Tested-by:
- 361 Suggested-by:
- 469 Mentored-by:
- 1196 Reported-by:
- 1727 Helped-by:
- 2177 Reviewed-by:
- 2202 Acked-by:
- 95313 Signed-off-by:
+ There seems to be consensus amongst the core Git community on a working
+ set of common trailers, and there are non-trivial costs to people
+ inventing new trailers (research to discover what they mean/how they
+ differ from existing trailers) such that inventing new ones is generally
+ unwarranted and not something to be recommended to new contributors.
+ Suggested-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Josh Soref <jsoref@gmail.com>
## Documentation/SubmittingPatches ##
@@ Documentation/SubmittingPatches: If you like, you can put extra tags at the end:
-
- . `Reported-by:` is used to credit someone who found the bug that
- the patch attempts to fix.
-+. `Noticed-by:` liked `Reported-by:` indicates someone who noticed
-+ the item being fixed.
- . `Acked-by:` says that the person who is more familiar with the area
- the patch attempts to modify liked the patch.
- . `Reviewed-by:`, unlike the other tags, can only be offered by the
-@@ Documentation/SubmittingPatches: If you like, you can put extra tags at the end:
- patch after a detailed analysis.
. `Tested-by:` is used to indicate that the person applied the patch
and found it to have the desired effect.
-+. `Co-authored-by:` is used to indicate that multiple people
-+ contributed to the work of a patch.
-+. `Helped-by:` is used to credit someone with helping develop a
-+ patch.
-+. `Mentored-by:` is used to credit someone with helping develop a
-+ patch.
-+. `Suggested-by:` is used to credit someone with suggesting the idea
-+ for a patch.
- You can also create your own tag or use one that's in common usage
+-You can also create your own tag or use one that's in common usage
-such as "Thanks-to:", "Based-on-patch-by:", or "Mentored-by:".
-+such as "Thanks-to:", "Based-on-patch-by:", or "Improved-by:".
++While you can also create your own trailer if the situation warrants it, we
++encourage you to instead use one of the common trailers in this project
++highlighted above.
[[git-tools]]
=== Generate your patch using Git tools out of your commits.
-: ----------- > 5: 8848572fe2c SubmittingPatches: update extra tags list
5: 11688e4360c ! 6: 8f16c7caa73 SubmittingPatches: improve extra tags advice
@@ Commit message
Signed-off-by: Josh Soref <jsoref@gmail.com>
## Documentation/SubmittingPatches ##
-@@ Documentation/SubmittingPatches: If you like, you can put extra tags at the end:
- You can also create your own tag or use one that's in common usage
- such as "Thanks-to:", "Based-on-patch-by:", or "Improved-by:".
+@@ Documentation/SubmittingPatches: While you can also create your own trailer if the situation warrants it, we
+ encourage you to instead use one of the common trailers in this project
+ highlighted above.
+Extra tags should only capitalize the very first letter, i.e. favor
+"Signed-off-by" over "Signed-Off-By" and "Acked-by:" over "Acked-By".
6: 043d2a24202 ! 7: cdb5fd0957f SubmittingPatches: clarify GitHub visual
@@ Metadata
## Commit message ##
SubmittingPatches: clarify GitHub visual
- Some people would expect a cross to be upright, and potentially have
- unequal lengths...
+ GitHub has two general forms for its states, sometimes they're a simple
+ colored object (e.g. green check or red x), and sometimes there's also a
+ colored container (e.g. green box or red circle) with containing that
+ object (e.g. check or x).
- GitHub uses a white x overlaying a solid red circle to indicate failure.
+ That's a lot of words to try to describe things, but in general, the key
+ for a failure is that it's recognized as an `x` and that it's associated
+ with the color red -- the color of course is problematic for people who
+ are red-green color-blind, but that's why they are paired with distinct
+ shapes.
+
+ Using the term `cross` doesn't really help.
Signed-off-by: Josh Soref <jsoref@gmail.com>
7: cdab65a4259 ! 8: 77576327df8 SubmittingPatches: clarify GitHub artifact format
@@ Commit message
GitHub wraps artifacts generated by workflows in a .zip file.
- Internally workflows can package anything they like in them.
+ Internally, workflows can package anything they like in them.
A recently generated failure artifact had the form:
8: 92469324813 = 9: a4878f58fe4 SubmittingPatches: hyphenate non-ASCII
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH 0/1] revert/cherry-pick: add --show-current-patch option
From: phillip.wood123 @ 2023-12-21 16:32 UTC (permalink / raw)
To: Michael Lohmann; +Cc: git, mi.al.lohmann, newren, phillip.wood
In-Reply-To: <20231220065141.7599-1-mi.al.lohmann@gmail.com>
Hi Michael
On 20/12/2023 06:51, Michael Lohmann wrote:
> Hi Phillip
>
> On 18/12/2023 16:42, Phillip Wood wrote:
>> Thanks for bringing this up I agree it can be very helpful to look at
>> the original commit when resolving cherry-pick and revert conflicts.
As an aside I find it useful is to do a kind of range-diff before
committing the conflict resolution. Unfortunately one cannot use "git
range-diff" because the conflict resolution is not yet committed.
Instead I use
diff <(git diff CHERRY_PICK_HEAD^-) <(git diff HEAD)
in practice it is helpful to pipe the diffs through sed to delete the
"index" lines and normalize the hunk headers.
>> I'm in two minds about this change though - I wonder if it'd be better
>> to improve the documentation for CHERRY_PICK_HEAD and REVERT_HEAD and
>> tell users to run "git show CHERRY_PICK_HEAD" instead. I think the
>> main reason we have a "--show-current-patch" option for "rebase" is
>> that there are two different implementations of that command and the
>> patched-based one of them does not support REBASE_HEAD. That reasoning
>> does not apply to "cherry-pick" and "revert" and
>> "--show-current-patch" suggests a patch-based implementation which is
>> also not the case for these commands.
>
> I appreciate the urge of limiting the interface to the minimum needed
> and not to duplicate functionality that already exists. On the other
> hand, this would
> a) grant the user the same experience, not having to wonder about
> implementation details such as different backends for rebase, but not
> for revert/cherry-pick and
> b) (I know it is more indicative of me, but:) when I am looking for a
> feature in software and I look into the respective man page I tend to
> focus first on the synopsis instead of reading the whole page (or
> sometimes I even just rely on the shell autocompletion for
> discoverability).
>
> So yes, mentioning REVERT_HEAD and CHERRY_PICK_HEAD in the respective
> docs would technically be sufficient, but I don't think it is as
> discoverable to an average user (who does not know about the details of
> all the existing pseudo refs) as a toplevel action would be. But an
> assessment of the pros and cons is not on me to decide.
To make the psuedo refs discoverable we should certainly be mentioning
them in the section about resolving conflicts. I haven't checked what
the docs say at the moment but a worked example showing how to inspect
the conflicts and the original changes would be helpful I think. That
does assume that the user actually reads the section about resolving
conflicts rather than just scanning the available command line options
though.
> I have to be honest: I have troubles distinguishing a "patch" and a
> "diff", the latter of which `git show <commit>` shows according to the
> documentation ("For commits it shows the log message and textual
> diff."), though my understanding was that a patch is a diff + context
> lines, which is what `git show` actually shows... I think this is
> probably why I don't feel so strong about the potential loose usage of
> the word here.
I think for the purposes of this discussion "patch" and "diff" are
largely interchangeable (a "patch" is essentially a "diff" with a commit
message). Maybe I'm overthinking it but the reason I'm not very keen on
"--show-current-patch" (in addition to the "duplicate functionality"
argument you mention above) is that cherry-pick and revert do not work
by applying patches (or diffs) but use a 3-way merge instead. I think
--show-current-patch first appeared as an option to "git am" which makes
sense as that command is all about applying patches.
I'd be interested to hear what other people think about whether it makes
"--show-current-patch" make sense for other commands.
> Also the documentation of cherry-pick already uses the word "patch" in a
> (according to my understanding from a technical perspective) sloppy (but
> from a layman's point of view probably nevertheless helpful) way:
>
>> The following sequence attempts to backport a patch, bails out because
>> the code the patch applies to has changed too much, and then tries
>> again, this time exercising more care about matching up context lines.
>>
>> ------------
>> $ git cherry-pick topic^ <1>
>> $ git diff <2>
>> $ git cherry-pick --abort <3>
>> $ git cherry-pick -Xpatience topic^ <4>
>> ------------
>> <1> apply the change that would be shown by `git show topic^`.
>> In this example, the patch does not apply cleanly, so
>> information about the conflict is written to the index and
>> working tree and no new commit results.
>
> Should that also be rephrased?
It would certainly be more accurate for the first paragraph to say
something like
The following sequence tries to backport a commit. It bails out
because the code modified by the commit has conflicting changes in
the current branch.
The bit about exercising more care about matching up context lines is
moot these days as the default merge strategy is "ort" which uses the
histogram diff algorithm to do just that so commands <3> & <4> should
not be needed.
>
> Out of curiosity: The following from the rebase docs seems to imply that
> the apply backend will probably be removed in the future:
>> --apply
>> Use applying strategies to rebase (calling git-am
>> internally). This option may become a no-op in the future
>> once the merge backend handles everything the apply one
>> does.
>
> But I would expect the `rebase --show-current-patch` still to be
> working. Would that only be a legacy compatibility flag and instead also
> for rebases the recommended option would be to run
> `git show REBASE_HEAD`?
The long term goal is to remove the apply backend but I don't think
anyone is actively working on it at the moment. We'd certainly need to
keep the --show-current-patch option for backwards compatibility.
I'll be off the list for the next couple of weeks but I'll be sure to
catch up with this thread in the New Year
Best Wishes
Phillip
^ permalink raw reply
* Re: [PATCH 4/8] SubmittingPatches: update extra tags list
From: phillip.wood123 @ 2023-12-21 15:09 UTC (permalink / raw)
To: Josh Soref, Elijah Newren; +Cc: git, phillip.wood
In-Reply-To: <CACZqfqCar=tay5diocU7jVWBwPUFqewNYfPLHkYvvR1fSBSdPA@mail.gmail.com>
Hi Josh
On 20/12/2023 17:42, Josh Soref wrote:
> SubmittingPatches: discourage new trailers
>
> There seems to be consensus amongst the core Git community on a working
> set of common trailers, and there are non-trivial costs to people
> inventing new trailers (research to discover what they mean/how they
> differ from existing trailers) such that inventing new ones is generally
> unwarranted and not something to be recommended to new contributors.
>
> Suggested-by: Elijah Newren <newren@gmail.com>
> Signed-off-by: Josh Soref <jsoref@gmail.com>
>
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index 32e9023877..58dfe40504 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -358,4 +358,5 @@ If you like, you can put extra tags at the end:
>
> -You can also create your own tag or use one that's in common usage
> -such as "Thanks-to:", "Based-on-patch-by:", or "Mentored-by:".
> +While you can also create your own trailer if the situation warrants it, we
> +encourage you to instead use one of the common trailers in this project
> +highlighted above.
Thanks for this, it looks good and would be a useful addition to v2 of
your patch series.
Best Wishes
Phillip
^ permalink raw reply
* Re: [PATCH] Use ^=1 to toggle between 0 and 1
From: phillip.wood123 @ 2023-12-21 15:06 UTC (permalink / raw)
To: Jeff King, phillip.wood
Cc: René Scharfe, AtariDreams via GitGitGadget, git, Seija Kijin
In-Reply-To: <20231221095606.GB570888@coredump.intra.peff.net>
Hi Peff
On 21/12/2023 09:56, Jeff King wrote:
> On Fri, Dec 15, 2023 at 02:46:36PM +0000, Phillip Wood wrote:
>
>>> Yeah. b2() is wrong for passing "2" to a bool.
>>
>> I think it depends what you mean by "wrong" §6.3.1.2 of standard is quite
>> clear that when any non-zero scalar value is converted to _Bool the result
>> is "1"
>
> Yeah, sorry, I was being quite sloppy with my wording. I meant "wrong"
> as in "I would ideally flag this in review for being weird and
> confusing".
That makes sense, it certainly is confusing
> Of course there are many reasonable cases where you might pass an
> integer "foo" rather than explicitly booleanizing it with "!!foo". So I
> do agree it's a real potential problem (and I'm sufficiently convinced
> that we should avoid an "int" fallback if we can).
I had a look at gnulib the other day and the list of limitations in the
documentation of their <stdbool.h> fallback makes it look quite
unattractive. They helpfully list some compilers where _Bool is not
implemented (IRIX, Tru64) or does not work correctly (HP-UX, AIX). As
far as I can see all the bug reports cited are from 2003-2006 on
obsolete compiler versions, hopefully _Bool is better supported these days.
Best Wishes
Phillip
> -Peff
^ permalink raw reply
* Re: git bisect stuck - --force flag required for checkout
From: brian m. carlson @ 2023-12-21 12:22 UTC (permalink / raw)
To: Devste Devste; +Cc: git
In-Reply-To: <CANM0SV3SEF28QJ2V0Q9ydp8yDbL8TDc1m871oxOB=UtwF1TtxQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2016 bytes --]
On 2023-12-21 at 10:47:57, Devste Devste wrote:
> Thank you for filling out a Git bug report!
> Please answer the following questions to help us understand your issue.
>
> What did you do before the bug happened? (Steps to reproduce your issue)
> add file Foo.txt to .git and commit
> add some commits with any changes to other files, as this is needed
> for reproduction
> run: git config core.ignorecase false
`core.ignorecase` is specifically designed for this case. It's set
internally by Git when the repository is created, and it's not supposed
to be changed by the user.
If you want a repository where there's no case sensitivity, then I'd
recommend WSL. It's also possible to make some directories case
sensitive in Windows 10 and newer and allegedly that works recursively,
so you could use `fsutil` to do that, then run `git init`, then add
data.
> rename Foo.txt to foo.txt and commit
> add some commits with any changes to other files, as this is needed
> for reproduction
> run: git bisect start && git bisect bad
> eventually, when running "git bisect good" (or bad) you will get an error:
> >error: The following untracked working tree files would be overwritten by checkout:
> >Foo.php
>
> Anything else you want to add:
> git bisect good/bad needs to have support for a "--force" flag, which
> is passed to the git checkout it runs internally
> At the moment git bisect cannot be used on Windows, as there is no way
> to continue the bisect from here.
> Changing the "git config core.ignorecase true" temporarily is not an
> option, as this will introduce a variety of other bugs,
> which, on Windows, eventually will require you to completely delete
> and reclone the repo, as Windows file paths are case-insensitive
Could you share what those problems are? `core.ignorecase` is
specifically designed to deal with case-insensitive file systems, and
that's why Git sets it to true.
--
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox