* [PATCH 3/3] sparse-checkout: tighten add_patterns_from_input()
From: Junio C Hamano @ 2023-12-21 6:59 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee, William Sprent
In-Reply-To: <20231221065925.3234048-1-gitster@pobox.com>
The add_patterns_from_input() function was introduced at 6fb705ab
(sparse-checkout: extract add_patterns_from_input(), 2020-02-11) and
then modified by 00408ade (builtin/sparse-checkout: add check-rules
command, 2023-03-27). Throughout its life, it either allowed to
read patterns from the file (before 00408ade, it only allowed the
standard input, after 00408ade, an arbitrary FILE *) or from argv[],
but never both. However, when we read from a file, the function
never checked that there is nothing in argv[] (in other words, the
command line arguments have fully been consumed), resulting in
excess arguments silently getting ignored.
This caused commands like "git sparse-checkout set [--stdin]" and
"git sparse-checkout check-rules [--rules-file <file>]" to silently
ignore the rest of the command line arguments without reporting.
The fix finally makes the --end-of-options handling for this
subcommand also work, so add test for it, too.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/sparse-checkout.c | 4 ++++
t/t1091-sparse-checkout-builtin.sh | 11 ++++++++++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 04ae81fce8..1166e1e298 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -555,6 +555,10 @@ static void add_patterns_from_input(struct pattern_list *pl,
FILE *file)
{
int i;
+
+ if (file && argc)
+ die(_("excess command line parameter '%s'"), argv[0]);
+
if (core_sparse_checkout_cone) {
struct strbuf line = STRBUF_INIT;
diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
index e33a6ed1b4..107ed170ac 100755
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh
@@ -937,6 +937,10 @@ test_expect_success 'check-rules cone mode' '
EOF
git -C bare ls-tree -r --name-only HEAD >all-files &&
+
+ test_must_fail git -C bare sparse-checkout check-rules --cone \
+ --rules-file ../rules excess args <all-files &&
+
git -C bare sparse-checkout check-rules --cone \
--rules-file ../rules >check-rules-file <all-files &&
@@ -947,6 +951,7 @@ test_expect_success 'check-rules cone mode' '
git -C repo sparse-checkout check-rules >check-rules-default <all-files &&
test_grep "deep/deeper1/deepest/a" check-rules-file &&
+ test_grep ! "end-of-options" check-rules-file &&
test_grep ! "deep/deeper2" check-rules-file &&
test_cmp check-rules-file ls-files &&
@@ -959,8 +964,12 @@ test_expect_success 'check-rules non-cone mode' '
EOF
git -C bare ls-tree -r --name-only HEAD >all-files &&
+
+ test_must_fail git -C bare sparse-checkout check-rules --no-cone \
+ --rules-file ../rules excess args <all-files &&
+
git -C bare sparse-checkout check-rules --no-cone --rules-file ../rules\
- >check-rules-file <all-files &&
+ --end-of-options >check-rules-file <all-files &&
cat rules | git -C repo sparse-checkout set --no-cone --stdin &&
git -C repo ls-files -t >out &&
--
2.43.0-174-g055bb6e996
^ permalink raw reply related
* [PATCH 0/3] sparse-checkout with --end-of-options
From: Junio C Hamano @ 2023-12-21 6:59 UTC (permalink / raw)
To: git
I only wanted to make sure
git sparse-checkout set --end-of-options path
work fine, but it turns out that there were other sloppiness that
has nothing to do with the end-of-options handling that needs to be
adjusted to the new world order, so I ended up addressing that as
well.
Junio C Hamano (3):
sparse-checkout: take care of "--end-of-options" in set/add
sparse-checkout: allow default patterns for 'set' only !stdin
sparse-checkout: tighten add_patterns_from_input()
builtin/sparse-checkout.c | 9 ++++++++-
parse-options.c | 8 ++++++++
parse-options.h | 2 ++
t/t1090-sparse-checkout-scope.sh | 8 ++++++++
t/t1091-sparse-checkout-builtin.sh | 13 +++++++++++--
5 files changed, 37 insertions(+), 3 deletions(-)
--
2.43.0-174-g055bb6e996
^ permalink raw reply
* [PATCH 2/3] sparse-checkout: use default patterns for 'set' only !stdin
From: Junio C Hamano @ 2023-12-21 6:59 UTC (permalink / raw)
To: git; +Cc: Elijah Newren
In-Reply-To: <20231221065925.3234048-1-gitster@pobox.com>
"git sparse-checkout set ---no-cone" uses default patterns when none
is given from the command line, but it should do so ONLY when
--stdin is not being used. Right now, add_patterns_from_input()
called when reading from the standard input is sloppy and does not
check if there are extra command line parameters that the command
will silently ignore, but that will change soon and not setting this
unnecessary and unused default patterns start to matter when it gets
fixed.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* This came from f2e3a218 (sparse-checkout: enable `set` to
initialize sparse-checkout mode, 2021-12-14) by Elijah.
builtin/sparse-checkout.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 8f55127202..04ae81fce8 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -837,7 +837,7 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
* non-cone mode, if nothing is specified, manually select just the
* top-level directory (much as 'init' would do).
*/
- if (!core_sparse_checkout_cone && argc == 0) {
+ if (!core_sparse_checkout_cone && !set_opts.use_stdin && argc == 0) {
argv = default_patterns;
argc = default_patterns_nr;
} else {
--
2.43.0-174-g055bb6e996
^ permalink raw reply related
* Re: [PATCH/RFC] sparse-checkout: take care of "--end-of-options" in set/add
From: Junio C Hamano @ 2023-12-21 2:46 UTC (permalink / raw)
To: Josh Steadmon; +Cc: Jeff King, git
In-Reply-To: <ZYN-5H-2NNoRRpf-@google.com>
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.
^ permalink raw reply
* [RFC/PATCH] archive: "--list" does not take further options
From: Junio C Hamano @ 2023-12-21 2:41 UTC (permalink / raw)
To: git; +Cc: René Scharfe, Jeff King
In-Reply-To: <xmqqbkakqx6s.fsf@gitster.g>
"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>
---
* This was done to convince myself that even though cmd_archive()
calls parse_options with PARSE_OPT_KEEP_UNKNOWN_OPT and then
uses the resulting argc/argv without apparent filtering of the
"--end-of-options" thing, it is correctly handling it, either
locally or remotely.
- locally, write_archive() uses parse_archive_args(), which calls
parse_options() without KEEP_UNKNOWN_OPT and "--end-of-options"
is handled there just fine.
- remotely, run_remote_archiver() relays the local parameters,
including "--end-of-options" via the "argument" packet. Then
the arguments are assembled into a strvec and used by the
upload-archive running on the other side to spawn an
upload-archive--writer process with.
cmd_upload_archive_writer() then makes the same write_archive()
call; "--end-of-options" would still be in argv[] if the
original "git archive --remote" invocation had one, but it is
consumed the same way as the local case in write_archive() by
calling parse_archive_args().
I do not like the remote error behaviour this one adds at all.
Do we use a more proper mechanism to propagate a remote error
back for other subcommands we can reuse here?
archive.c | 7 +++++++
t/t5000-tar-tree.sh | 14 ++++++++++++++
2 files changed, 21 insertions(+)
diff --git c/archive.c w/archive.c
index 9aeaf2bd87..3244e9f9f2 100644
--- c/archive.c
+++ w/archive.c
@@ -641,6 +641,13 @@ static int parse_archive_args(int argc, const char **argv,
base = "";
if (list) {
+ if (argc) {
+ if (!is_remote)
+ die(_("extra command line parameter '%s'"), *argv);
+ else
+ printf("!ERROR! extra command line parameter '%s'\n",
+ *argv);
+ }
for (i = 0; i < nr_archivers; i++)
if (!is_remote || archivers[i]->flags & ARCHIVER_REMOTE)
printf("%s\n", archivers[i]->name);
diff --git c/t/t5000-tar-tree.sh w/t/t5000-tar-tree.sh
index 918a2fc7c6..04592f45b0 100755
--- c/t/t5000-tar-tree.sh
+++ w/t/t5000-tar-tree.sh
@@ -124,6 +124,20 @@ test_expect_success 'setup' '
EOF
'
+test_expect_success '--list notices extra parameters' '
+ test_must_fail git archive --list blah &&
+ # NEEDSWORK: remote error does not result in non-zero
+ # exit, which we might want to change later.
+ git archive --remote=. --list blah >remote-out &&
+ grep "!ERROR! " remote-out
+'
+
+test_expect_success 'end-of-options is correctly eaten' '
+ git archive --list --end-of-options &&
+ git archive --remote=. --list --end-of-options >remote-out &&
+ ! grep "!ERROR! " remote-out
+'
+
test_expect_success 'populate workdir' '
mkdir a &&
echo simple textfile >a/a &&
^ permalink raw reply related
* Re: [PATCH/RFC] sparse-checkout: take care of "--end-of-options" in set/add
From: Josh Steadmon @ 2023-12-20 23:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <xmqqbkakqx6s.fsf@gitster.g>
On 2023.12.20 15:19, Junio C Hamano wrote:
> 93851746 (parse-options: decouple "--end-of-options" and "--",
> 2023-12-06) updated the world order to make callers of parse-options
> that set PARSE_OPT_KEEP_UNKNOWN_OPT responsible for deciding what to
> do with "--end-of-options" they may see after parse_options() returns.
>
> This unfortunately broke "sparse-checkout set/add", and from this
> invocation,
>
> "git sparse-checkout [add|set] --[no-]cone --end-of-options pattern..."
>
> we now see "--end-of-options" listed in .git/info/sparse-checkout as if
> it is one of the path patterns.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> builtin/sparse-checkout.c | 9 +++++++++
> t/t1090-sparse-checkout-scope.sh | 8 ++++++++
> t/t1091-sparse-checkout-builtin.sh | 2 +-
> 3 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git c/builtin/sparse-checkout.c w/builtin/sparse-checkout.c
> index 80227f3df1..226a458b10 100644
> --- c/builtin/sparse-checkout.c
> +++ w/builtin/sparse-checkout.c
> @@ -776,6 +776,10 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
> builtin_sparse_checkout_add_usage,
> PARSE_OPT_KEEP_UNKNOWN_OPT);
>
> + if (argc && !strcmp(*argv, "--end-of-options")) {
> + argc--;
> + argv++;
> + }
> sanitize_paths(argc, argv, prefix, add_opts.skip_checks);
>
> return modify_pattern_list(argc, argv, add_opts.use_stdin, ADD);
> @@ -823,6 +827,11 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
> builtin_sparse_checkout_set_usage,
> PARSE_OPT_KEEP_UNKNOWN_OPT);
>
> + if (argc && !strcmp(*argv, "--end-of-options")) {
> + argc--;
> + argv++;
> + }
> +
> if (update_modes(&set_opts.cone_mode, &set_opts.sparse_index))
> return 1;
>
> diff --git c/t/t1090-sparse-checkout-scope.sh w/t/t1090-sparse-checkout-scope.sh
> index 3a14218b24..5b96716235 100755
> --- c/t/t1090-sparse-checkout-scope.sh
> +++ w/t/t1090-sparse-checkout-scope.sh
> @@ -57,6 +57,14 @@ test_expect_success 'return to full checkout of main' '
> test_expect_success 'skip-worktree on files outside sparse patterns' '
> git sparse-checkout disable &&
> git sparse-checkout set --no-cone "a*" &&
> + cat .git/info/sparse-checkout >wo-eoo &&
> +
> + git sparse-checkout disable &&
> + git sparse-checkout set --no-cone --end-of-options "a*" &&
> + cat .git/info/sparse-checkout >w-eoo &&
> +
> + test_cmp wo-eoo w-eoo &&
> +
> git checkout-index --all --ignore-skip-worktree-bits &&
>
> git ls-files -t >output &&
> diff --git c/t/t1091-sparse-checkout-builtin.sh w/t/t1091-sparse-checkout-builtin.sh
> index f67611da28..e33a6ed1b4 100755
> --- c/t/t1091-sparse-checkout-builtin.sh
> +++ w/t/t1091-sparse-checkout-builtin.sh
> @@ -334,7 +334,7 @@ test_expect_success 'cone mode: set with nested folders' '
>
> test_expect_success 'cone mode: add independent path' '
> git -C repo sparse-checkout set deep/deeper1 &&
> - git -C repo sparse-checkout add folder1 &&
> + git -C repo sparse-checkout add --end-of-options folder1 &&
> cat >expect <<-\EOF &&
> /*
> !/*/
I can confirm that this fixes an issue noticed by sparse-checkout users
at $DAYJOB. Looks good to me. Thanks!
Reviewed-by: Josh Steadmon <steadmon@google.com>
^ permalink raw reply
* [PATCH/RFC] sparse-checkout: take care of "--end-of-options" in set/add
From: Junio C Hamano @ 2023-12-20 23:19 UTC (permalink / raw)
To: Jeff King; +Cc: git, Josh Steadmon
93851746 (parse-options: decouple "--end-of-options" and "--",
2023-12-06) updated the world order to make callers of parse-options
that set PARSE_OPT_KEEP_UNKNOWN_OPT responsible for deciding what to
do with "--end-of-options" they may see after parse_options() returns.
This unfortunately broke "sparse-checkout set/add", and from this
invocation,
"git sparse-checkout [add|set] --[no-]cone --end-of-options pattern..."
we now see "--end-of-options" listed in .git/info/sparse-checkout as if
it is one of the path patterns.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/sparse-checkout.c | 9 +++++++++
t/t1090-sparse-checkout-scope.sh | 8 ++++++++
t/t1091-sparse-checkout-builtin.sh | 2 +-
3 files changed, 18 insertions(+), 1 deletion(-)
diff --git c/builtin/sparse-checkout.c w/builtin/sparse-checkout.c
index 80227f3df1..226a458b10 100644
--- c/builtin/sparse-checkout.c
+++ w/builtin/sparse-checkout.c
@@ -776,6 +776,10 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
builtin_sparse_checkout_add_usage,
PARSE_OPT_KEEP_UNKNOWN_OPT);
+ if (argc && !strcmp(*argv, "--end-of-options")) {
+ argc--;
+ argv++;
+ }
sanitize_paths(argc, argv, prefix, add_opts.skip_checks);
return modify_pattern_list(argc, argv, add_opts.use_stdin, ADD);
@@ -823,6 +827,11 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
builtin_sparse_checkout_set_usage,
PARSE_OPT_KEEP_UNKNOWN_OPT);
+ if (argc && !strcmp(*argv, "--end-of-options")) {
+ argc--;
+ argv++;
+ }
+
if (update_modes(&set_opts.cone_mode, &set_opts.sparse_index))
return 1;
diff --git c/t/t1090-sparse-checkout-scope.sh w/t/t1090-sparse-checkout-scope.sh
index 3a14218b24..5b96716235 100755
--- c/t/t1090-sparse-checkout-scope.sh
+++ w/t/t1090-sparse-checkout-scope.sh
@@ -57,6 +57,14 @@ test_expect_success 'return to full checkout of main' '
test_expect_success 'skip-worktree on files outside sparse patterns' '
git sparse-checkout disable &&
git sparse-checkout set --no-cone "a*" &&
+ cat .git/info/sparse-checkout >wo-eoo &&
+
+ git sparse-checkout disable &&
+ git sparse-checkout set --no-cone --end-of-options "a*" &&
+ cat .git/info/sparse-checkout >w-eoo &&
+
+ test_cmp wo-eoo w-eoo &&
+
git checkout-index --all --ignore-skip-worktree-bits &&
git ls-files -t >output &&
diff --git c/t/t1091-sparse-checkout-builtin.sh w/t/t1091-sparse-checkout-builtin.sh
index f67611da28..e33a6ed1b4 100755
--- c/t/t1091-sparse-checkout-builtin.sh
+++ w/t/t1091-sparse-checkout-builtin.sh
@@ -334,7 +334,7 @@ test_expect_success 'cone mode: set with nested folders' '
test_expect_success 'cone mode: add independent path' '
git -C repo sparse-checkout set deep/deeper1 &&
- git -C repo sparse-checkout add folder1 &&
+ git -C repo sparse-checkout add --end-of-options folder1 &&
cat >expect <<-\EOF &&
/*
!/*/
^ permalink raw reply related
* Re: [PATCH v3 1/2] Documentation/git-merge.txt: fix reference to synopsis
From: Michael Lohmann @ 2023-12-20 22:09 UTC (permalink / raw)
To: gitster; +Cc: git, l.s.r, mi.al.lohmann, mial.lohmann, newren
In-Reply-To: <xmqqmsu4r1t2.fsf@gitster.g>
> Ah, sorry about the misunderstanding. Will apply. Thanks.
No need to be sorry - my wording was ambiguous.
Thank you for your patience with me! I hope it will get smoother for all
of you the more experience I get...
^ permalink raw reply
* What's cooking in git.git (Dec 2023, #04; Wed, 20)
From: Junio C Hamano @ 2023-12-20 22:07 UTC (permalink / raw)
To: git
Here are the topics that have been cooking in my tree. Commits
prefixed with '+' are in 'next' (being in 'next' is a sign that a
topic is stable enough to be used and are candidate to be in a
future release). Commits prefixed with '-' are only in 'seen', and
aren't considered "accepted" at all and may be annotated with an URL
to a message that raises issues but they are no means exhaustive. A
topic without enough support may be discarded after a long period of
no activity (of course they can be resubmit when new interests
arise).
The 'maint' branch now points at the maintenance track of Git 2.43,
which was released earlier in the month, and the tip of 'next' has
been rewound and rebuilt on top of Git 2.43. I am planning to start
ejecting topics that have been in the "stalled" state for too long.
The RelNotes symbolic link says we are now working towards Git 2.44.
It may not be a bad idea to reflect on what technical debt and UI
warts we have accumulated so far to see if we have enough of them to
start planning for a breaking Git 3.0 release (or, of course, keep
incrementally improve the system, which is much more preferrable---
continuity and stability is good).
Copies of the source code to Git live in many repositories, and the
following is a list of the ones I push into or their mirrors. Some
repositories have only a subset of branches.
With maint, master, next, seen, todo:
git://git.kernel.org/pub/scm/git/git.git/
git://repo.or.cz/alt-git.git/
https://kernel.googlesource.com/pub/scm/git/git/
https://github.com/git/git/
https://gitlab.com/git-vcs/git/
With all the integration branches and topics broken out:
https://github.com/gitster/git/
Even though the preformatted documentation in HTML and man format
are not sources, they are published in these repositories for
convenience (replace "htmldocs" with "manpages" for the manual
pages):
git://git.kernel.org/pub/scm/git/git-htmldocs.git/
https://github.com/gitster/git-htmldocs.git/
Release tarballs are available at:
https://www.kernel.org/pub/software/scm/git/
--------------------------------------------------
[Graduated to 'master']
* en/complete-sparse-checkout (2023-12-03) 4 commits
(merged to 'next' on 2023-12-12 at 3de75bd6af)
+ completion: avoid user confusion in non-cone mode
+ completion: avoid misleading completions in cone mode
+ completion: fix logic for determining whether cone mode is active
+ completion: squelch stray errors in sparse-checkout completion
Command line completion (in contrib/) learned to complete path
arguments to the "add/set" subcommands of "git sparse-checkout"
better.
source: <pull.1349.v3.git.1701583024.gitgitgadget@gmail.com>
* jc/revision-parse-int (2023-12-09) 1 commit
(merged to 'next' on 2023-12-12 at 6209b4c97c)
+ revision: parse integer arguments to --max-count, --skip, etc., more carefully
The command line parser for the "log" family of commands was too
loose when parsing certain numbers, e.g., silently ignoring the
extra 'q' in "git log -n 1q" without complaining, which has been
tightened up.
source: <xmqq5y181fx0.fsf_-_@gitster.g>
* jk/bisect-reset-fix (2023-12-09) 1 commit
(merged to 'next' on 2023-12-12 at 8f946eafb6)
+ bisect: always clean on reset
"git bisect reset" has been taught to clean up state files and refs
even when BISECT_START file is gone.
source: <20231207065341.GA778781@coredump.intra.peff.net>
* jk/config-cleanup (2023-12-09) 9 commits
(merged to 'next' on 2023-12-12 at 44ee006c25)
+ sequencer: simplify away extra git_config_string() call
+ gpg-interface: drop pointless config_error_nonbool() checks
+ push: drop confusing configset/callback redundancy
+ config: use git_config_string() for core.checkRoundTripEncoding
+ diff: give more detailed messages for bogus diff.* config
+ config: use config_error_nonbool() instead of custom messages
+ imap-send: don't use git_die_config() inside callback
+ git_xmerge_config(): prefer error() to die()
+ config: reject bogus values for core.checkstat
(this branch uses jk/implicit-true.)
Code clean-up around use of configuration variables.
source: <20231207071030.GA1275835@coredump.intra.peff.net>
source: <20231207072338.GA1277727@coredump.intra.peff.net>
* jk/end-of-options (2023-12-09) 1 commit
(merged to 'next' on 2023-12-12 at 4ae454b26d)
+ parse-options: decouple "--end-of-options" and "--"
"git $cmd --end-of-options --rev -- --path" for some $cmd failed
to interpret "--rev" as a rev, and "--path" as a path. This was
fixed for many programs like "reset" and "checkout".
source: <20231206222145.GA136253@coredump.intra.peff.net>
* jk/implicit-true (2023-12-09) 7 commits
(merged to 'next' on 2023-12-12 at 2a42fdc998)
+ fsck: handle NULL value when parsing message config
+ trailer: handle NULL value when parsing trailer-specific config
+ submodule: handle NULL value when parsing submodule.*.branch
+ help: handle NULL value for alias.* config
+ trace2: handle NULL values in tr2_sysenv config callback
+ setup: handle NULL value when parsing extensions
+ config: handle NULL value when parsing non-bools
(this branch is used by jk/config-cleanup.)
Some codepaths did not correctly parse configuration variables
specified with valueless "true", which has been corrected.
source: <20231207071030.GA1275835@coredump.intra.peff.net>
* jp/use-diff-index-in-pre-commit-sample (2023-12-03) 1 commit
(merged to 'next' on 2023-12-12 at 4771ea61b9)
+ hooks--pre-commit: detect non-ASCII when renaming
The sample pre-commit hook that tries to catch introduction of new
paths that use potentially non-portable characters did not notice
an existing path getting renamed to such a problematic path, when
rename detection was enabled.
source: <pull.1291.v2.git.git.1701360836307.gitgitgadget@gmail.com>
* mk/doc-gitfile-more (2023-12-03) 1 commit
(merged to 'next' on 2023-12-12 at 7990e4a163)
+ doc: make the gitfile syntax easier to discover
Doc update.
source: <20231128065558.1061206-1-mk+copyleft@pimpmybyte.de>
* ps/ref-tests-update-more (2023-12-03) 10 commits
(merged to 'next' on 2023-12-12 at 3d4004fe3b)
+ t6301: write invalid object ID via `test-tool ref-store`
+ t5551: stop writing packed-refs directly
+ t5401: speed up creation of many branches
+ t4013: simplify magic parsing and drop "failure"
+ t3310: stop checking for reference existence via `test -f`
+ t1417: make `reflog --updateref` tests backend agnostic
+ t1410: use test-tool to create empty reflog
+ t1401: stop treating FETCH_HEAD as real reference
+ t1400: split up generic reflog tests from the reffile-specific ones
+ t0410: mark tests to require the reffiles backend
Tests update.
source: <cover.1701242407.git.ps@pks.im>
* rs/incompatible-options-messages (2023-12-09) 7 commits
(merged to 'next' on 2023-12-12 at a13847a7f6)
+ worktree: simplify incompatibility message for --orphan and commit-ish
+ worktree: standardize incompatibility messages
+ clean: factorize incompatibility message
+ revision, rev-parse: factorize incompatibility messages about - -exclude-hidden
+ revision: use die_for_incompatible_opt3() for - -graph/--reverse/--walk-reflogs
+ repack: use die_for_incompatible_opt3() for -A/-k/--cruft
+ push: use die_for_incompatible_opt4() for - -delete/--tags/--all/--mirror
Clean-up code that handles combinations of incompatible options.
source: <20231206115215.94467-1-l.s.r@web.de>
--------------------------------------------------
[New Topics]
* jc/retire-cas-opt-name-constant (2023-12-19) 1 commit
- remote.h: retire CAS_OPT_NAME
Code clean-up.
Will merge to 'next'.
source: <xmqq5y0uc7tq.fsf@gitster.g>
* rs/rebase-use-strvec-pushf (2023-12-20) 1 commit
(merged to 'next' on 2023-12-20 at ecb190973c)
+ rebase: use strvec_pushf() for format-patch revisions
Code clean-up.
Will merge to 'master'.
source: <4ab7431c-6c1b-448c-b4d2-e8b9be0e4eef@web.de>
* ps/refstorage-extension (2023-12-20) 13 commits
- t9500: write "extensions.refstorage" into config
- builtin/clone: introduce `--ref-format=` value flag
- builtin/init: introduce `--ref-format=` value flag
- builtin/rev-parse: introduce `--show-ref-format` flag
- t: introduce GIT_TEST_DEFAULT_REF_FORMAT envvar
- setup: introduce GIT_DEFAULT_REF_FORMAT envvar
- setup: introduce "extensions.refStorage" extension
- setup: set repository's formats on init
- setup: start tracking ref storage format when
- refs: refactor logic to look up storage backends
- worktree: skip reading HEAD when repairing worktrees
- t: introduce DEFAULT_REPO_FORMAT prereq
- Merge branch 'ps/clone-into-reftable-repository' into ps/refstorage-extension
(this branch uses ps/clone-into-reftable-repository.)
Introduce a new extension "refstorage" so that we can mark a
repository that uses a non-default ref backend, like reftable.
Needs review.
source: <cover.1703067989.git.ps@pks.im>
* ps/reftable-fixes-and-optims (2023-12-20) 9 commits
- SQUASH??? make "make hdr-check" pass
- reftable/merged: transfer ownership of records when iterating
- reftable/merged: really reuse buffers to compute record keys
- reftable/record: store "val2" hashes as static arrays
- reftable/record: store "val1" hashes as static arrays
- reftable/record: constify some parts of the interface
- reftable/writer: fix index corruption when writing multiple indices
- reftable/stack: do not overwrite errors when compacting
- Merge branch 'ps/reftable-fixes' into ps/reftable-fixes-and-optims
(this branch uses ps/reftable-fixes.)
More fixes and optimizations to the reftable backend.
Needs review.
source: <cover.1703063544.git.ps@pks.im>
--------------------------------------------------
[Cooking]
* jk/mailinfo-oob-read-fix (2023-12-12) 1 commit
(merged to 'next' on 2023-12-14 at 0dcfcb0d02)
+ mailinfo: fix out-of-bounds memory reads in unquote_quoted_pair()
(this branch is used by jk/mailinfo-iterative-unquote-comment.)
OOB read fix.
Will merge to 'master'.
source: <20231212221243.GA1656116@coredump.intra.peff.net>
* ps/pseudo-refs (2023-12-14) 4 commits
- bisect: consistently write BISECT_EXPECTED_REV via the refdb
- refs: complete list of special refs
- refs: propagate errno when reading special refs fails
- wt-status: read HEAD and ORIG_HEAD via the refdb
Assorted changes around pseudoref handling.
Will merge to 'next'.
source: <cover.1702560829.git.ps@pks.im>
* rs/t6300-compressed-size-fix (2023-12-12) 1 commit
(merged to 'next' on 2023-12-19 at 37ed09549c)
+ t6300: avoid hard-coding object sizes
Test fix.
Will merge to 'master'.
source: <9feeb6cf-aabf-4002-917f-3f6c27547bc8@web.de>
* es/add-doc-list-short-form-of-all-in-synopsis (2023-12-15) 1 commit
(merged to 'next' on 2023-12-18 at a4f20da2bf)
+ git-add.txt: add missing short option -A to synopsis
Doc update.
Will merge to 'master'.
source: <20231215204333.1253-1-ericsunshine@charter.net>
* jc/doc-misspelt-refs-fix (2023-12-18) 1 commit
(merged to 'next' on 2023-12-18 at e7799fd5c9)
+ doc: format.notes specify a ref under refs/notes/ hierarchy
Doc update.
Will merge to 'master'.
source: <xmqqjzpfje33.fsf_-_@gitster.g>
* jc/doc-most-refs-are-not-that-special (2023-12-15) 5 commits
(merged to 'next' on 2023-12-18 at aead30fcc8)
+ docs: MERGE_AUTOSTASH is not that special
+ docs: AUTO_MERGE is not that special
+ refs.h: HEAD is not that special
+ git-bisect.txt: BISECT_HEAD is not that special
+ git.txt: HEAD is not that special
Doc updates.
Will merge to 'master'.
source: <20231215203245.3622299-1-gitster@pobox.com>
* jk/mailinfo-iterative-unquote-comment (2023-12-14) 2 commits
(merged to 'next' on 2023-12-18 at 92363605fd)
+ mailinfo: avoid recursion when unquoting From headers
+ t5100: make rfc822 comment test more careful
(this branch uses jk/mailinfo-oob-read-fix.)
The code to parse the From e-mail header has been updated to avoid
recursion.
Will merge to 'master'.
source: <20231214214444.GB2297853@coredump.intra.peff.net>
* ps/chainlint-self-check-update (2023-12-15) 1 commit
(merged to 'next' on 2023-12-18 at 0de2e1807f)
+ tests: adjust whitespace in chainlint expectations
Test framework update.
Will merge to 'master'.
source: <fb312f559de7b99244e4c86a995250599cd9be06.1702622508.git.ps@pks.im>
* tb/multi-pack-verbatim-reuse (2023-12-14) 26 commits
- t/perf: add performance tests for multi-pack reuse
- pack-bitmap: enable reuse from all bitmapped packs
- pack-objects: allow setting `pack.allowPackReuse` to "single"
- t/test-lib-functions.sh: implement `test_trace2_data` helper
- pack-objects: add tracing for various packfile metrics
- pack-bitmap: prepare to mark objects from multiple packs for reuse
- pack-revindex: implement `midx_pair_to_pack_pos()`
- pack-revindex: factor out `midx_key_to_pack_pos()` helper
- midx: implement `midx_preferred_pack()`
- git-compat-util.h: implement checked size_t to uint32_t conversion
- pack-objects: include number of packs reused in output
- pack-objects: prepare `write_reused_pack_verbatim()` for multi-pack reuse
- pack-objects: prepare `write_reused_pack()` for multi-pack reuse
- pack-objects: pass `bitmapped_pack`'s to pack-reuse functions
- pack-objects: keep track of `pack_start` for each reuse pack
- pack-objects: parameterize pack-reuse routines over a single pack
- pack-bitmap: return multiple packs via `reuse_partial_packfile_from_bitmap()`
- pack-bitmap: simplify `reuse_partial_packfile_from_bitmap()` signature
- ewah: implement `bitmap_is_empty()`
- pack-bitmap: pass `bitmapped_pack` struct to pack-reuse functions
- midx: implement `midx_locate_pack()`
- midx: implement `BTMP` chunk
- midx: factor out `fill_pack_info()`
- pack-bitmap: plug leak in find_objects()
- pack-bitmap-write: deep-clear the `bb_commit` slab
- pack-objects: free packing_data in more places
Streaming spans of packfile data used to be done only from a
single, primary, pack in a repository with multiple packfiles. It
has been extended to allow reuse from other packfiles, too.
Will merge to 'next'?
cf. <ZXurD1NTZ4TAs7WZ@nand.local>
source: <cover.1702592603.git.me@ttaylorr.com>
* rs/c99-stdbool-test-balloon (2023-12-18) 1 commit
(merged to 'next' on 2023-12-18 at 5a62aaa127)
+ git-compat-util: convert skip_{prefix,suffix}{,_mem} to bool
Test balloon to use C99 "bool" type from <stdbool.h>.
Will merge to 'master'.
source: <2d30dc36-6091-4b47-846f-92d3f4a8b135@web.de>
* sp/test-i18ngrep (2023-12-18) 1 commit
(merged to 'next' on 2023-12-18 at d54442693a)
+ test-lib-functions.sh: fix test_grep fail message wording
Error message fix in the test framework.
Will merge to 'master'.
source: <20231203171956.771-1-shreyanshpaliwalcmsmn@gmail.com>
* jx/fetch-atomic-error-message-fix (2023-12-18) 2 commits
(merged to 'next' on 2023-12-18 at a1988b00e5)
+ fetch: no redundant error message for atomic fetch
+ t5574: test porcelain output of atomic fetch
"git fetch --atomic" issued an unnecessary empty error message,
which has been corrected.
Will merge to 'master'.
cf. <ZX__e7VjyLXIl-uV@tanuki>
source: <cover.1702821462.git.zhiyou.jx@alibaba-inc.com>
* jc/bisect-doc (2023-12-09) 1 commit
- bisect: document "terms" subcommand more fully
Doc update.
Needs review.
source: <xmqqzfyjmk02.fsf@gitster.g>
* rs/show-ref-incompatible-options (2023-12-11) 1 commit
(merged to 'next' on 2023-12-18 at 5a092285f7)
+ show-ref: use die_for_incompatible_opt3()
Code clean-up for sanity checking of command line options for "git
show-ref".
Will merge to 'master'.
source: <e5304253-3347-4900-bbf2-d3c6ee3fb976@web.de>
* sh/completion-with-reftable (2023-12-19) 2 commits
(merged to 'next' on 2023-12-20 at 7957d4aa5b)
+ completion: support pseudoref existence checks for reftables
+ completion: refactor existence checks for pseudorefs
Command line completion script (in contrib/) learned to work better
with the reftable backend.
Will merge to 'master'.
source: <cover.1703022850.git.stanhu@gmail.com>
* en/header-cleanup (2023-12-03) 12 commits
- treewide: remove unnecessary includes in source files
- treewide: add direct includes currently only pulled in transitively
- trace2/tr2_tls.h: remove unnecessary include
- submodule-config.h: remove unnecessary include
- pkt-line.h: remove unnecessary include
- line-log.h: remove unnecessary include
- http.h: remove unnecessary include
- fsmonitor--daemon.h: remove unnecessary includes
- blame.h: remove unnecessary includes
- archive.h: remove unnecessary include
- treewide: remove unnecessary includes in source files
- treewide: remove unnecessary includes from header files
Remove unused header "#include".
Has a few interactions with topics in flight.
source: <pull.1617.git.1701585682.gitgitgadget@gmail.com>
* ps/clone-into-reftable-repository (2023-12-12) 7 commits
(merged to 'next' on 2023-12-19 at adf7eb1f84)
+ builtin/clone: create the refdb with the correct object format
+ builtin/clone: skip reading HEAD when retrieving remote
+ builtin/clone: set up sparse checkout later
+ builtin/clone: fix bundle URIs with mismatching object formats
+ remote-curl: rediscover repository when fetching refs
+ setup: allow skipping creation of the refdb
+ setup: extract function to create the refdb
(this branch is used by ps/refstorage-extension.)
"git clone" has been prepared to allow cloning a repository with
non-default hash function into a repository that uses the reftable
backend.
Will merge to 'master'.
source: <cover.1702361370.git.ps@pks.im>
* jc/checkout-B-branch-in-use (2023-12-13) 2 commits
(merged to 'next' on 2023-12-14 at 0a3998619e)
+ checkout: forbid "-B <branch>" from touching a branch used elsewhere
+ checkout: refactor die_if_checked_out() caller
"git checkout -B <branch> [<start-point>]" allowed a branch that is
in use in another worktree to be updated and checked out, which
might be a bit unexpected. The rule has been tightened, which is a
breaking change. "--ignore-other-worktrees" option is required to
unbreak you, if you are used to the current behaviour that "-B"
overrides the safety.
Will merge to 'master'.
source: <xmqqjzq9cl70.fsf@gitster.g>
* ps/reftable-fixes (2023-12-11) 11 commits
(merged to 'next' on 2023-12-15 at ebba966016)
+ reftable/block: reuse buffer to compute record keys
+ reftable/block: introduce macro to initialize `struct block_iter`
+ reftable/merged: reuse buffer to compute record keys
+ reftable/stack: fix use of unseeded randomness
+ reftable/stack: fix stale lock when dying
+ reftable/stack: reuse buffers when reloading stack
+ reftable/stack: perform auto-compaction with transactional interface
+ reftable/stack: verify that `reftable_stack_add()` uses auto-compaction
+ reftable: handle interrupted writes
+ reftable: handle interrupted reads
+ reftable: wrap EXPECT macros in do/while
(this branch is used by ps/reftable-fixes-and-optims.)
Bunch of small fix-ups to the reftable code.
Will merge to 'master'.
source: <cover.1702285387.git.ps@pks.im>
* jc/orphan-unborn (2023-11-24) 2 commits
- orphan/unborn: fix use of 'orphan' in end-user facing messages
- orphan/unborn: add to the glossary and use them consistently
Doc updates to clarify what an "unborn branch" means.
Will merge to 'next'.
source: <xmqq4jhb977x.fsf@gitster.g>
* jw/builtin-objectmode-attr (2023-12-12) 2 commits
- SQUASH??? - leakfix
- attr: add builtin objectmode values support
The builtin_objectmode attribute is populated for each path
without adding anything in .gitattributes files, which would be
useful in magic pathspec, e.g., ":(attr:builtin_objectmode=100755)"
to limit to executables.
Needs to get leakfix reviewed.
source: <20231116054437.2343549-1-jojwang@google.com>
* tb/merge-tree-write-pack (2023-10-23) 5 commits
- builtin/merge-tree.c: implement support for `--write-pack`
- bulk-checkin: introduce `index_tree_bulk_checkin_incore()`
- bulk-checkin: introduce `index_blob_bulk_checkin_incore()`
- bulk-checkin: generify `stream_blob_to_pack()` for arbitrary types
- bulk-checkin: extract abstract `bulk_checkin_source`
"git merge-tree" learned "--write-pack" to record its result
without creating loose objects.
Broken when an object created during a merge is needed to continue merge
cf. <CABPp-BEfy9VOvimP9==ry_rZXu=metOQ8s=_-XiG_Pdx9c06Ww@mail.gmail.com>
source: <cover.1698101088.git.me@ttaylorr.com>
* tb/pair-chunk-expect (2023-11-10) 8 commits
- midx: read `OOFF` chunk with `pair_chunk_expect()`
- midx: read `OIDL` chunk with `pair_chunk_expect()`
- commit-graph: read `BIDX` chunk with `pair_chunk_expect()`
- commit-graph: read `GDAT` chunk with `pair_chunk_expect()`
- commit-graph: read `CDAT` chunk with `pair_chunk_expect()`
- commit-graph: read `OIDL` chunk with `pair_chunk_expect()`
- chunk-format: introduce `pair_chunk_expect()` helper
- Merge branch 'jk/chunk-bounds-more' into HEAD
Further code clean-up.
Needs review.
source: <cover.1699569246.git.me@ttaylorr.com>
* tb/path-filter-fix (2023-10-18) 17 commits
- bloom: introduce `deinit_bloom_filters()`
- commit-graph: reuse existing Bloom filters where possible
- object.h: fix mis-aligned flag bits table
- commit-graph: drop unnecessary `graph_read_bloom_data_context`
- commit-graph.c: unconditionally load Bloom filters
- bloom: prepare to discard incompatible Bloom filters
- bloom: annotate filters with hash version
- commit-graph: new filter ver. that fixes murmur3
- repo-settings: introduce commitgraph.changedPathsVersion
- t4216: test changed path filters with high bit paths
- t/helper/test-read-graph: implement `bloom-filters` mode
- bloom.h: make `load_bloom_filter_from_graph()` public
- t/helper/test-read-graph.c: extract `dump_graph_info()`
- gitformat-commit-graph: describe version 2 of BDAT
- commit-graph: ensure Bloom filters are read with consistent settings
- revision.c: consult Bloom filters for root commits
- t/t4216-log-bloom.sh: harden `test_bloom_filters_not_used()`
The Bloom filter used for path limited history traversal was broken
on systems whose "char" is unsigned; update the implementation and
bump the format version to 2.
Expecting a reroll.
cf. <20231023202212.GA5470@szeder.dev>
source: <cover.1697653929.git.me@ttaylorr.com>
* ak/color-decorate-symbols (2023-10-23) 7 commits
- log: add color.decorate.pseudoref config variable
- refs: exempt pseudorefs from pattern prefixing
- refs: add pseudorefs array and iteration functions
- log: add color.decorate.ref config variable
- log: add color.decorate.symbol config variable
- log: use designated inits for decoration_colors
- config: restructure color.decorate documentation
A new config for coloring.
Needs review.
source: <20231023221143.72489-1-andy.koppe@gmail.com>
* la/trailer-cleanups (2023-12-20) 3 commits
- trailer: use offsets for trailer_start/trailer_end
- trailer: find the end of the log message
- commit: ignore_non_trailer computes number of bytes to ignore
Code clean-up.
Will merge to 'next'.
source: <pull.1563.v5.git.1697828495.gitgitgadget@gmail.com>
* eb/hash-transition (2023-10-02) 30 commits
- t1016-compatObjectFormat: add tests to verify the conversion between objects
- t1006: test oid compatibility with cat-file
- t1006: rename sha1 to oid
- test-lib: compute the compatibility hash so tests may use it
- builtin/ls-tree: let the oid determine the output algorithm
- object-file: handle compat objects in check_object_signature
- tree-walk: init_tree_desc take an oid to get the hash algorithm
- builtin/cat-file: let the oid determine the output algorithm
- rev-parse: add an --output-object-format parameter
- repository: implement extensions.compatObjectFormat
- object-file: update object_info_extended to reencode objects
- object-file-convert: convert commits that embed signed tags
- object-file-convert: convert commit objects when writing
- object-file-convert: don't leak when converting tag objects
- object-file-convert: convert tag objects when writing
- object-file-convert: add a function to convert trees between algorithms
- object: factor out parse_mode out of fast-import and tree-walk into in object.h
- cache: add a function to read an OID of a specific algorithm
- tag: sign both hashes
- commit: export add_header_signature to support handling signatures on tags
- commit: convert mergetag before computing the signature of a commit
- commit: write commits for both hashes
- object-file: add a compat_oid_in parameter to write_object_file_flags
- object-file: update the loose object map when writing loose objects
- loose: compatibilty short name support
- loose: add a mapping between SHA-1 and SHA-256 for loose objects
- repository: add a compatibility hash algorithm
- object-names: support input of oids in any supported hash
- oid-array: teach oid-array to handle multiple kinds of oids
- object-file-convert: stubs for converting from one object format to another
Teach a repository to work with both SHA-1 and SHA-256 hash algorithms.
Needs review.
source: <878r8l929e.fsf@gmail.froward.int.ebiederm.org>
* jx/remote-archive-over-smart-http (2023-12-14) 4 commits
- archive: support remote archive from stateless transport
- transport-helper: call do_take_over() in connect_helper
- transport-helper: call do_take_over() in process_connect
- transport-helper: no connection restriction in connect_helper
"git archive --remote=<remote>" learned to talk over the smart
http (aka stateless) transport.
Needs review.
source: <cover.1702562879.git.zhiyou.jx@alibaba-inc.com>
* jx/sideband-chomp-newline-fix (2023-12-18) 3 commits
- pkt-line: do not chomp newlines for sideband messages
- pkt-line: memorize sideband fragment in reader
- test-pkt-line: add option parser for unpack-sideband
Sideband demultiplexer fixes.
Will merge to 'next'?
source: <cover.1702823801.git.zhiyou.jx@alibaba-inc.com>
* jc/fake-lstat (2023-09-15) 1 commit
(merged to 'next' on 2023-12-15 at 48e34cc0b4)
+ cache: add fake_lstat()
(this branch is used by jc/diff-cached-fsmonitor-fix.)
A new helper to let us pretend that we called lstat() when we know
our cache_entry is up-to-date via fsmonitor.
Will merge to 'master'.
cf. <e5295dbe-94d2-3186-5663-2466eba4bdde@jeffhostetler.com>
source: <xmqqcyykig1l.fsf@gitster.g>
* jc/rerere-cleanup (2023-08-25) 4 commits
- rerere: modernize use of empty strbuf
- rerere: try_merge() should use LL_MERGE_ERROR when it means an error
- rerere: fix comment on handle_file() helper
- rerere: simplify check_one_conflict() helper function
Code clean-up.
Not ready to be reviewed yet.
source: <20230824205456.1231371-1-gitster@pobox.com>
* rj/status-bisect-while-rebase (2023-10-16) 1 commit
- status: fix branch shown when not only bisecting
"git status" is taught to show both the branch being bisected and
being rebased when both are in effect at the same time.
Will merge to 'next'.
cf. <xmqqil76kyov.fsf@gitster.g>
source: <2e24ca9b-9c5f-f4df-b9f8-6574a714dfb2@gmail.com>
* jc/diff-cached-fsmonitor-fix (2023-09-15) 3 commits
(merged to 'next' on 2023-12-15 at 4aa7596593)
+ diff-lib: fix check_removed() when fsmonitor is active
+ Merge branch 'jc/fake-lstat' into jc/diff-cached-fsmonitor-fix
+ Merge branch 'js/diff-cached-fsmonitor-fix' into jc/diff-cached-fsmonitor-fix
(this branch uses jc/fake-lstat.)
The optimization based on fsmonitor in the "diff --cached"
codepath is resurrected with the "fake-lstat" introduced earlier.
Will merge to 'master'.
cf. <e5295dbe-94d2-3186-5663-2466eba4bdde@jeffhostetler.com>
source: <xmqqr0n0h0tw.fsf@gitster.g>
^ permalink raw reply
* Re: [PATCH v3 1/2] Documentation/git-merge.txt: fix reference to synopsis
From: Junio C Hamano @ 2023-12-20 21:39 UTC (permalink / raw)
To: Michael Lohmann; +Cc: Michael Lohmann, git, l.s.r, newren
In-Reply-To: <20231220213534.18947-1-mi.al.lohmann@gmail.com>
Michael Lohmann <mial.lohmann@gmail.com> writes:
> 437591a9d738 combined the synopsis of "The second syntax" (meaning `git
> merge --abort`) and "The third syntax" (for `git merge --continue`) into
> this single line:
>
> git merge (--continue | --abort | --quit)
>
> but it was still referred to when describing the preconditions that have
> to be fulfilled to run the respective actions. In other words:
> References by number are no longer valid after a merge of some of the
> synopses.
>
> Also the previous version of the documentation did not acknowledge that
> `--no-commit` would result in the precondition being fulfilled (thanks
> to Elijah Newren and Junio C Hamano for pointing that out).
>
> This change also groups `--abort` and `--continue` together when
> explaining the prerequisites in order to avoid duplication.
>
> Helped-by: René Scharfe <l.s.r@web.de>
> Signed-off-by: Michael Lohmann <mi.al.lohmann@gmail.com>
> ---
>
> @Junio My sentence was ambiguous. I wanted to refer to the upstream
> version of the docs, since that also has the faulty prerequisites, so I
> changed it to "the previous version of the documentation" for
> clarification. If you think that this paragraph is not needed
> nevertheless I am perfectly happy to remove it.
Ah, sorry about the misunderstanding. Will apply. Thanks.
^ permalink raw reply
* [PATCH v3 1/2] Documentation/git-merge.txt: fix reference to synopsis
From: Michael Lohmann @ 2023-12-20 21:35 UTC (permalink / raw)
To: gitster; +Cc: Michael Lohmann, git, l.s.r, mial.lohmann, newren
In-Reply-To: <xmqqy1dor3t5.fsf@gitster.g>
437591a9d738 combined the synopsis of "The second syntax" (meaning `git
merge --abort`) and "The third syntax" (for `git merge --continue`) into
this single line:
git merge (--continue | --abort | --quit)
but it was still referred to when describing the preconditions that have
to be fulfilled to run the respective actions. In other words:
References by number are no longer valid after a merge of some of the
synopses.
Also the previous version of the documentation did not acknowledge that
`--no-commit` would result in the precondition being fulfilled (thanks
to Elijah Newren and Junio C Hamano for pointing that out).
This change also groups `--abort` and `--continue` together when
explaining the prerequisites in order to avoid duplication.
Helped-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Michael Lohmann <mi.al.lohmann@gmail.com>
---
@Junio My sentence was ambiguous. I wanted to refer to the upstream
version of the docs, since that also has the faulty prerequisites, so I
changed it to "the previous version of the documentation" for
clarification. If you think that this paragraph is not needed
nevertheless I am perfectly happy to remove it.
@Elijah Thanks!
Documentation/git-merge.txt | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index e8ab340319..33ec5c6b19 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -46,21 +46,21 @@ a log message from the user describing the changes. Before the operation,
D---E---F---G---H master
------------
-The second syntax ("`git merge --abort`") can only be run after the
-merge has resulted in conflicts. 'git merge --abort' will abort the
-merge process and try to reconstruct the pre-merge state. However,
-if there were uncommitted changes when the merge started (and
-especially if those changes were further modified after the merge
-was started), 'git merge --abort' will in some cases be unable to
-reconstruct the original (pre-merge) changes. Therefore:
+A merge stops if there's a conflict that cannot be resolved
+automatically or if `--no-commit` was provided when initiating the
+merge. At that point you can run `git merge --abort` or `git merge
+--continue`.
+
+`git merge --abort` will abort the merge process and try to reconstruct
+the pre-merge state. However, if there were uncommitted changes when the
+merge started (and especially if those changes were further modified
+after the merge was started), `git merge --abort` will in some cases be
+unable to reconstruct the original (pre-merge) changes. Therefore:
*Warning*: Running 'git merge' with non-trivial uncommitted changes is
discouraged: while possible, it may leave you in a state that is hard to
back out of in the case of a conflict.
-The third syntax ("`git merge --continue`") can only be run after the
-merge has resulted in conflicts.
-
OPTIONS
-------
:git-merge: 1
--
2.39.3 (Apple Git-145)
^ permalink raw reply related
* Re: [PATCH 2/2] orphan/unborn: fix use of 'orphan' in end-user facing messages
From: Rubén Justo @ 2023-12-20 21:21 UTC (permalink / raw)
To: Junio C Hamano, git
In-Reply-To: <xmqqa5q4skwg.fsf@gitster.g>
On 20-dic-2023 12:01:51, Junio C Hamano wrote:
> The two-patch series, whose second part is the message I am
> responding to, did not see much reaction, but since it came
> during an end-user puzzlement and was written to make the docs less
> puzzling, I am tempted to merge it down to 'next'.
>
> Thanks.
>
If you need some positive feedback; I've read your series and it looks
good to me and going in a good direction.
I think "unborn branch" is a more accurate term than "orphaned branch".
It's not perfect, but I don't have a better one to offer.
A nit in 1/2, which of course is not worth a re-roll, is a double blank
line near the end; which I suspect is unintentional.
Just in case anyone else is looking for the thread where the puzzlement
was reported:
https://lore.kernel.org/git/FE2AD666-88DE-4F70-8D6D-3A426689EB41@me.com/
Thank you.
^ permalink raw reply
* Re: [PATCH 1/2] Documentation/git-merge.txt: fix reference to synopsis
From: Junio C Hamano @ 2023-12-20 20:56 UTC (permalink / raw)
To: Michael Lohmann; +Cc: l.s.r, Elijah Newren, Michael Lohmann, git
In-Reply-To: <20231220195342.17590-2-mi.al.lohmann@gmail.com>
Michael Lohmann <mial.lohmann@gmail.com> writes:
> Also the previous version did not acknowledge that `--no-merge` would
> result in the precondition being fulfilled (thanks to Elijah Newren and
> Junio C Hamano for pointing that out).
This does not belong to the log message. Please write for those who
only read "git log" output after the work is merged and nothing
else. To them, errors in the previous attempt that was pointed out
by reviewers and corrected in this version do not exist.
It is perfectly fine to write something like the above after the
three-dash line. That is the place to clue reviewers about the
context of this round, reminding what happend in the previous
iteration and what the differences this round has, etc.
Thanks.
^ permalink raw reply
* Re: [PATCH 1/2] Documentation/git-merge.txt: fix reference to synopsis
From: Elijah Newren @ 2023-12-20 20:45 UTC (permalink / raw)
To: Michael Lohmann; +Cc: l.s.r, gitster, Michael Lohmann, git
In-Reply-To: <20231220195342.17590-2-mi.al.lohmann@gmail.com>
Hi,
On Wed, Dec 20, 2023 at 11:54 AM Michael Lohmann <mial.lohmann@gmail.com> wrote:
>
> 437591a9d738 combined the synopsis of "The second syntax" (meaning `git
> merge --abort`) and "The third syntax" (for `git merge --continue`) into
> this single line:
>
> git merge (--continue | --abort | --quit)
>
> but it was still referred to when describing the preconditions that have
> to be fulfilled to run the respective actions. In other words:
> References by number are no longer valid after a merge of some of the
> synopses.
>
> Also the previous version did not acknowledge that `--no-merge` would
`--no-commit` rather than `--no-merge`.
> result in the precondition being fulfilled (thanks to Elijah Newren and
> Junio C Hamano for pointing that out).
>
> This change also groups `--abort` and `--continue` together when
> explaining the prerequisites in order to avoid duplication.
>
> Helped-by: René Scharfe <l.s.r@web.de>
> Signed-off-by: Michael Lohmann <mi.al.lohmann@gmail.com>
> ---
> Documentation/git-merge.txt | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
> index e8ab340319..33ec5c6b19 100644
> --- a/Documentation/git-merge.txt
> +++ b/Documentation/git-merge.txt
> @@ -46,21 +46,21 @@ a log message from the user describing the changes. Before the operation,
> D---E---F---G---H master
> ------------
>
> -The second syntax ("`git merge --abort`") can only be run after the
> -merge has resulted in conflicts. 'git merge --abort' will abort the
> -merge process and try to reconstruct the pre-merge state. However,
> -if there were uncommitted changes when the merge started (and
> -especially if those changes were further modified after the merge
> -was started), 'git merge --abort' will in some cases be unable to
> -reconstruct the original (pre-merge) changes. Therefore:
> +A merge stops if there's a conflict that cannot be resolved
> +automatically or if `--no-commit` was provided when initiating the
> +merge. At that point you can run `git merge --abort` or `git merge
> +--continue`.
> +
> +`git merge --abort` will abort the merge process and try to reconstruct
> +the pre-merge state. However, if there were uncommitted changes when the
> +merge started (and especially if those changes were further modified
> +after the merge was started), `git merge --abort` will in some cases be
> +unable to reconstruct the original (pre-merge) changes. Therefore:
>
> *Warning*: Running 'git merge' with non-trivial uncommitted changes is
> discouraged: while possible, it may leave you in a state that is hard to
> back out of in the case of a conflict.
>
> -The third syntax ("`git merge --continue`") can only be run after the
> -merge has resulted in conflicts.
> -
> OPTIONS
> -------
> :git-merge: 1
> --
> 2.39.3 (Apple Git-145)
Otherwise, looks good.
^ permalink raw reply
* Re: [PATCH 02/12] treewide: remove unnecessary includes in source files
From: Elijah Newren @ 2023-12-20 20:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Elijah Newren via GitGitGadget, git
In-Reply-To: <xmqqedfgsm6u.fsf@gitster.g>
On Wed, Dec 20, 2023 at 11:34 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> >
> >> diff --git a/trace2.c b/trace2.c
> >> index 6dc74dff4c7..d4220af9ae1 100644
> >> --- a/trace2.c
> >> ...
> > An in-flight topic seem to want to see git_env_bool() that is
> > declared in parse.h that is pulled in via inclusion of config.h
> > hence this hunk breaks 'seen'.
> >
> >> diff --git a/t/helper/test-trace2.c b/t/helper/test-trace2.c
> >> index d5ca0046c89..a0032ee3964 100644
> >> --- a/t/helper/test-trace2.c
> >> ...
> > An in-flight topic starts using "struct key_value_info" that is
> > available via the inclusion of "config.h", hence this hunk breaks
> > the build of 'seen'.
>
> It seems that we have gained another topic in flight that gets
> broken by this change. I can keep piling merge-fixes on top, but it
> does not look like a strategy that would scale well.
>
> Can we get this series thoroughly reviewed quickly to merge it down
> via 'next' to 'master' soonish, so that other topics can be rebased
> on the result, or is that too much to ask during the Winter lull?
>
> Thanks.
The Winter lull is my winter surge, so I can certainly quickly make
whatever changes are required (well, assuming I can shake this
fever...). But that doesn't help much with reviewing, since that
should be done by someone other than the author. However, these
particular type of changes are pretty innocuous; there's really not
anything clever going on, it's just a lot of gruntwork, and
does-it-compile is most of the review.
Anyway, I'll reroll, dropping or holding back any changes that
conflict with next or seen, and see if that encourages anyone to chime
in.
^ permalink raw reply
* Re: [PATCH 1/1] attr: add builtin objectmode values support
From: Junio C Hamano @ 2023-12-20 20:07 UTC (permalink / raw)
To: git; +Cc: Joanna Wang, sunshine, tboegi
In-Reply-To: <xmqqedfrovsb.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> The attached is one possible way to plug the leak; I am not sure if
> it is the best one, though. One thing I like about the solution is
> that the approach makes sure that the mode attributes we would ever
> return are very tightly controlled and does not allow a buggy code
> to come up with "mode" to be passed to this new helper function to
> pass random and unsupported mode bits without triggering the BUG().
>
> attr.c | 30 +++++++++++++++++++++++++++---
> 1 file changed, 27 insertions(+), 3 deletions(-)
Anybody who want to propose a better leakfix (we cannot afford to do
with UNLEAK() as the number of leaked mode string will be unbounded)?
Otherwise, I'll squash it in to Jonanna's patch and merge it down to
'next'.
Thanks.
> diff --git c/attr.c w/attr.c
> index b03c20f768..679e42258c 100644
> --- c/attr.c
> +++ w/attr.c
> @@ -1250,10 +1250,34 @@ static struct object_id *default_attr_source(void)
> return &attr_source;
> }
>
> +static const char *interned_mode_string(unsigned int mode)
> +{
> + static struct {
> + unsigned int val;
> + char str[7];
> + } mode_string[] = {
> + { .val = 0040000 },
> + { .val = 0100644 },
> + { .val = 0100755 },
> + { .val = 0120000 },
> + { .val = 0160000 },
> + };
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(mode_string); i++) {
> + if (mode_string[i].val != mode)
> + continue;
> + if (!*mode_string[i].str)
> + snprintf(mode_string[i].str, sizeof(mode_string[i].str),
> + "%06o", mode);
> + return mode_string[i].str;
> + }
> + BUG("Unsupported mode 0%o", mode);
> +}
> +
> static const char *builtin_object_mode_attr(struct index_state *istate, const char *path)
> {
> unsigned int mode;
> - struct strbuf sb = STRBUF_INIT;
>
> if (direction == GIT_ATTR_CHECKIN) {
> struct object_id oid;
> @@ -1287,8 +1311,8 @@ static const char *builtin_object_mode_attr(struct index_state *istate, const ch
> else
> return ATTR__UNSET;
> }
> - strbuf_addf(&sb, "%06o", mode);
> - return strbuf_detach(&sb, NULL);
> +
> + return interned_mode_string(mode);
> }
>
>
^ permalink raw reply
* Re: [PATCH 2/2] orphan/unborn: fix use of 'orphan' in end-user facing messages
From: Junio C Hamano @ 2023-12-20 20:01 UTC (permalink / raw)
To: git
In-Reply-To: <xmqq4jhb977x.fsf@gitster.g>
The two-patch series, whose second part is the message I am
responding to, did not see much reaction, but since it came
during an end-user puzzlement and was written to make the docs less
puzzling, I am tempted to merge it down to 'next'.
Thanks.
^ permalink raw reply
* [PATCH 2/2] Documentation/git-merge.txt: use backticks for command wrapping
From: Michael Lohmann @ 2023-12-20 19:53 UTC (permalink / raw)
To: l.s.r, Elijah Newren, gitster; +Cc: Michael Lohmann, git
In-Reply-To: <c6814a39-b4f9-4b1e-b81b-45ffe4aa7466@web.de>
As René found in the guidance from CodingGuidelines:
Literal examples (e.g. use of command-line options, command names,
branch names, URLs, pathnames (files and directories), configuration
and environment variables) must be typeset in monospace (i.e. wrapped
with backticks)
So all instances of single and double quotes for wraping said examples
were replaced with simple backticks.
Suggested-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Michael Lohmann <mi.al.lohmann@gmail.com>
---
Documentation/git-merge.txt | 50 ++++++++++++++++++-------------------
1 file changed, 25 insertions(+), 25 deletions(-)
diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 33ec5c6b19..7332f53f2f 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -20,12 +20,12 @@ DESCRIPTION
-----------
Incorporates changes from the named commits (since the time their
histories diverged from the current branch) into the current
-branch. This command is used by 'git pull' to incorporate changes
+branch. This command is used by `git pull` to incorporate changes
from another repository and can be used by hand to merge changes
from one branch into another.
Assume the following history exists and the current branch is
-"`master`":
+`master`:
------------
A---B---C topic
@@ -33,7 +33,7 @@ Assume the following history exists and the current branch is
D---E---F---G master
------------
-Then "`git merge topic`" will replay the changes made on the
+Then `git merge topic` will replay the changes made on the
`topic` branch since it diverged from `master` (i.e., `E`) until
its current commit (`C`) on top of `master`, and record the result
in a new commit along with the names of the two parent commits and
@@ -57,7 +57,7 @@ merge started (and especially if those changes were further modified
after the merge was started), `git merge --abort` will in some cases be
unable to reconstruct the original (pre-merge) changes. Therefore:
-*Warning*: Running 'git merge' with non-trivial uncommitted changes is
+*Warning*: Running `git merge` with non-trivial uncommitted changes is
discouraged: while possible, it may leave you in a state that is hard to
back out of in the case of a conflict.
@@ -74,8 +74,8 @@ include::merge-options.txt[]
If `--log` is specified, a shortlog of the commits being merged
will be appended to the specified message.
+
-The 'git fmt-merge-msg' command can be
-used to give a good default for automated 'git merge'
+The `git fmt-merge-msg` command can be
+used to give a good default for automated `git merge`
invocations. The automated message can include the branch description.
--into-name <branch>::
@@ -104,14 +104,14 @@ include::rerere-options.txt[]
present, apply it to the worktree.
+
If there were uncommitted worktree changes present when the merge
-started, 'git merge --abort' will in some cases be unable to
+started, `git merge --abort` will in some cases be unable to
reconstruct these changes. It is therefore recommended to always
-commit or stash your changes before running 'git merge'.
+commit or stash your changes before running `git merge`.
+
-'git merge --abort' is equivalent to 'git reset --merge' when
+`git merge --abort` is equivalent to `git reset --merge` when
`MERGE_HEAD` is present unless `MERGE_AUTOSTASH` is also present in
-which case 'git merge --abort' applies the stash entry to the worktree
-whereas 'git reset --merge' will save the stashed changes in the stash
+which case `git merge --abort` applies the stash entry to the worktree
+whereas `git reset --merge` will save the stashed changes in the stash
list.
--quit::
@@ -120,8 +120,8 @@ list.
stash entry will be saved to the stash list.
--continue::
- After a 'git merge' stops due to conflicts you can conclude the
- merge by running 'git merge --continue' (see "HOW TO RESOLVE
+ After a `git merge` stops due to conflicts you can conclude the
+ merge by running `git merge --continue` (see "HOW TO RESOLVE
CONFLICTS" section below).
<commit>...::
@@ -144,25 +144,25 @@ PRE-MERGE CHECKS
Before applying outside changes, you should get your own work in
good shape and committed locally, so it will not be clobbered if
there are conflicts. See also linkgit:git-stash[1].
-'git pull' and 'git merge' will stop without doing anything when
-local uncommitted changes overlap with files that 'git pull'/'git
-merge' may need to update.
+`git pull` and `git merge` will stop without doing anything when
+local uncommitted changes overlap with files that `git pull`/`git
+merge` may need to update.
To avoid recording unrelated changes in the merge commit,
-'git pull' and 'git merge' will also abort if there are any changes
+`git pull` and `git merge` will also abort if there are any changes
registered in the index relative to the `HEAD` commit. (Special
narrow exceptions to this rule may exist depending on which merge
strategy is in use, but generally, the index must match HEAD.)
-If all named commits are already ancestors of `HEAD`, 'git merge'
+If all named commits are already ancestors of `HEAD`, `git merge`
will exit early with the message "Already up to date."
FAST-FORWARD MERGE
------------------
Often the current branch head is an ancestor of the named commit.
-This is the most common case especially when invoked from 'git
-pull': you are tracking an upstream repository, you have committed
+This is the most common case especially when invoked from `git
+pull`: you are tracking an upstream repository, you have committed
no local changes, and now you want to update to a newer upstream
revision. In this case, a new commit is not needed to store the
combined history; instead, the `HEAD` (along with the index) is
@@ -269,7 +269,7 @@ Barbie's remark on your side. The only thing you can tell is that your
side wants to say it is hard and you'd prefer to go shopping, while the
other side wants to claim it is easy.
-An alternative style can be used by setting the "merge.conflictStyle"
+An alternative style can be used by setting the `merge.conflictStyle`
configuration variable to either "diff3" or "zdiff3". In "diff3"
style, the above conflict may look like this:
@@ -328,10 +328,10 @@ After seeing a conflict, you can do two things:
* Resolve the conflicts. Git will mark the conflicts in
the working tree. Edit the files into shape and
- 'git add' them to the index. Use 'git commit' or
- 'git merge --continue' to seal the deal. The latter command
+ `git add` them to the index. Use `git commit` or
+ `git merge --continue` to seal the deal. The latter command
checks whether there is a (interrupted) merge in progress
- before calling 'git commit'.
+ before calling `git commit`.
You can work through the conflict with a number of tools:
@@ -392,7 +392,7 @@ CONFIGURATION
branch.<name>.mergeOptions::
Sets default options for merging into branch <name>. The syntax and
- supported options are the same as those of 'git merge', but option
+ supported options are the same as those of `git merge`, but option
values containing whitespace characters are currently not supported.
include::includes/cmd-config-section-rest.txt[]
--
2.39.3 (Apple Git-145)
^ permalink raw reply related
* [PATCH 1/2] Documentation/git-merge.txt: fix reference to synopsis
From: Michael Lohmann @ 2023-12-20 19:53 UTC (permalink / raw)
To: l.s.r, Elijah Newren, gitster; +Cc: Michael Lohmann, git
In-Reply-To: <c6814a39-b4f9-4b1e-b81b-45ffe4aa7466@web.de>
437591a9d738 combined the synopsis of "The second syntax" (meaning `git
merge --abort`) and "The third syntax" (for `git merge --continue`) into
this single line:
git merge (--continue | --abort | --quit)
but it was still referred to when describing the preconditions that have
to be fulfilled to run the respective actions. In other words:
References by number are no longer valid after a merge of some of the
synopses.
Also the previous version did not acknowledge that `--no-merge` would
result in the precondition being fulfilled (thanks to Elijah Newren and
Junio C Hamano for pointing that out).
This change also groups `--abort` and `--continue` together when
explaining the prerequisites in order to avoid duplication.
Helped-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Michael Lohmann <mi.al.lohmann@gmail.com>
---
Documentation/git-merge.txt | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index e8ab340319..33ec5c6b19 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -46,21 +46,21 @@ a log message from the user describing the changes. Before the operation,
D---E---F---G---H master
------------
-The second syntax ("`git merge --abort`") can only be run after the
-merge has resulted in conflicts. 'git merge --abort' will abort the
-merge process and try to reconstruct the pre-merge state. However,
-if there were uncommitted changes when the merge started (and
-especially if those changes were further modified after the merge
-was started), 'git merge --abort' will in some cases be unable to
-reconstruct the original (pre-merge) changes. Therefore:
+A merge stops if there's a conflict that cannot be resolved
+automatically or if `--no-commit` was provided when initiating the
+merge. At that point you can run `git merge --abort` or `git merge
+--continue`.
+
+`git merge --abort` will abort the merge process and try to reconstruct
+the pre-merge state. However, if there were uncommitted changes when the
+merge started (and especially if those changes were further modified
+after the merge was started), `git merge --abort` will in some cases be
+unable to reconstruct the original (pre-merge) changes. Therefore:
*Warning*: Running 'git merge' with non-trivial uncommitted changes is
discouraged: while possible, it may leave you in a state that is hard to
back out of in the case of a conflict.
-The third syntax ("`git merge --continue`") can only be run after the
-merge has resulted in conflicts.
-
OPTIONS
-------
:git-merge: 1
--
2.39.3 (Apple Git-145)
^ permalink raw reply related
* [PATCH 0/2] Documentation/git-merge.txt: fix reference to synopsis
From: Michael Lohmann @ 2023-12-20 19:53 UTC (permalink / raw)
To: l.s.r, Elijah Newren, gitster; +Cc: Michael Lohmann, git
In-Reply-To: <c6814a39-b4f9-4b1e-b81b-45ffe4aa7466@web.de>
Hey!
Thank you all for your great reviews!
> Thank you for this patch and sorry for the nitpicking below!
No need to be sorry - this is what I am here for since it is the only
way to learn ;-) (also I am not a native speaker, so I know I am
constantly making mistakes)
> "Synopsys" is a software company. A "synopsis" is a brief outline.
Oh yeah... Now I need a face-palm emoji :D This is what you get from
trusting the spell check for words you just tried to copy far too early
in the morning... And another thing to learn for me: don't submit
patches while you are not yet fully awake... Sorry for everyone who had
to read the first draft! That was indeed not very professional from
me...
> Had to think a moment before I understood that "enumeration" refers to
> "The second syntax" and "The third syntax", which have been combined
> into this line:
>
> git merge (--continue | --abort | --quit)
>
> And it does make sense that we can no longer say "second syntax" and
> only refer to "git merge --abort", or "third syntax" and mean "git
> merge --continue". In other words: References by number are no longer
> valid after a merge of some of the synopses.
That sums it up a lot better. I wasn't happy with my first draft, but
couldn't come up with something better - now I used your explanation
with a slight variation.
> > The connection between these two sentences feels weak to me.
>
> This sentence is a bit more problematic than that: Even when there are
> no conflicts, "git merge --no-commit" will also stop a merge, and one
> can then use either --abort or --continue. So the assertion made by
> this sentence that you're reviewing is not accurate.
Oh! Another thing I learned today! Thanks a lot for pointing that out!
I have to confess: I copied that sentence 1:1 from git-rebase - that is
also why it did not fit in with the next sentence... But Renés
suggestion just avoids this (and the "--continue") problem altogether,
so I went with a slight variation of it instead.
> I like this alternative wording
Same! I fumbled around with it just a bit to include your hint
> Possibly. This looks like a case of me making a mistake while
> criticizing someone else's grammar, though. Which happens almost
> every time. o_O
We all make mistakes (and my own grammar is horrific...), but the more I
appreciate it when people suggest/ask things because that always gives
me the opportunity to learn as well. And you are totally right in that
this sentence does not "feel quite right" anyways, so I understand your
unease with it and why you wanted to discuss it ;-)
> Just being nitpicky and curious, but does the abort/continue also
> apply when you run "git merge --no-commit"?
Yes, indeed that seems to be the case (also pointed out simultaneously
by Elijah Newren). I extended Renés suggestion to mention it as well.
> > The only guidance I found is this paragraph from CodingGuidelines:
> >
> > Literal examples (e.g. use of command-line options, command names,
> > branch names, URLs, pathnames (files and directories), configuration and
> > environment variables) must be typeset in monospace (i.e. wrapped with
> > backticks)
> >
> > So shouldn't we wrap all commands in backticks and nothing more?
> > Probably worth a separate patch.
>
> Thanks for a good review.
Indeed! That was very nice!
And I also added the suggested changes as a second patch. It applies
just fine to master without the first one, though that obviously would
leave the changed paragraphs from the first commit with the mixed
syntax, but that would just be a minor inconsistency until the first
patch (or a future version of it) is applied.
Cheers
Michael
Michael Lohmann (2):
Documentation/git-merge.txt: fix reference to synopsis
Documentation/git-merge.txt: use backticks for command wrapping
Documentation/git-merge.txt | 70 ++++++++++++++++++-------------------
1 file changed, 35 insertions(+), 35 deletions(-)
--
2.39.3 (Apple Git-145)
^ permalink raw reply
* Re: [PATCH 02/12] treewide: remove unnecessary includes in source files
From: Junio C Hamano @ 2023-12-20 19:34 UTC (permalink / raw)
To: Elijah Newren via GitGitGadget; +Cc: git, Elijah Newren
In-Reply-To: <xmqq1qc35sx2.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> diff --git a/trace2.c b/trace2.c
>> index 6dc74dff4c7..d4220af9ae1 100644
>> --- a/trace2.c
>> ...
> An in-flight topic seem to want to see git_env_bool() that is
> declared in parse.h that is pulled in via inclusion of config.h
> hence this hunk breaks 'seen'.
>
>> diff --git a/t/helper/test-trace2.c b/t/helper/test-trace2.c
>> index d5ca0046c89..a0032ee3964 100644
>> --- a/t/helper/test-trace2.c
>> ...
> An in-flight topic starts using "struct key_value_info" that is
> available via the inclusion of "config.h", hence this hunk breaks
> the build of 'seen'.
It seems that we have gained another topic in flight that gets
broken by this change. I can keep piling merge-fixes on top, but it
does not look like a strategy that would scale well.
Can we get this series thoroughly reviewed quickly to merge it down
via 'next' to 'master' soonish, so that other topics can be rebased
on the result, or is that too much to ask during the Winter lull?
Thanks.
^ permalink raw reply
* Re: [PATCH v3 0/4] refs: improve handling of special refs
From: Junio C Hamano @ 2023-12-20 19:28 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Taylor Blau, Phillip Wood, Ramsay Jones
In-Reply-To: <cover.1702560829.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> Patrick Steinhardt (4):
> wt-status: read HEAD and ORIG_HEAD via the refdb
> refs: propagate errno when reading special refs fails
> refs: complete list of special refs
> bisect: consistently write BISECT_EXPECTED_REV via the refdb
With the clear understanding that we plan to make those other than
FETCH_HEAD and MERGE_HEAD in the is_special_ref().special_refs[]
eventually not special at all, this round looked quite sensible to
me.
Let's merge it down to 'next'.
Thanks.
^ permalink raw reply
* Re: [PATCH 0/7] reftable: fixes and optimizations (pt.2)
From: Junio C Hamano @ 2023-12-20 19:10 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <cover.1703063544.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> Patrick Steinhardt (7):
> reftable/stack: do not overwrite errors when compacting
> reftable/writer: fix index corruption when writing multiple indices
> reftable/record: constify some parts of the interface
> reftable/record: store "val1" hashes as static arrays
> reftable/record: store "val2" hashes as static arrays
> reftable/merged: really reuse buffers to compute record keys
> reftable/merged: transfer ownership of records when iterating
Something like this need to be split and sprinkled into relevant
steps in v2 in order to pass "make hdr-check", it seems.
reftable/reftable-record.h | 1 +
reftable/reftable-stack.h | 1 +
2 files changed, 2 insertions(+)
diff --git c/reftable/reftable-record.h w/reftable/reftable-record.h
index 83d252ec2c..fd1160615c 100644
--- c/reftable/reftable-record.h
+++ w/reftable/reftable-record.h
@@ -9,6 +9,7 @@ license that can be found in the LICENSE file or at
#ifndef REFTABLE_RECORD_H
#define REFTABLE_RECORD_H
+#include <hash-ll.h>
#include <stdint.h>
/*
diff --git c/reftable/reftable-stack.h w/reftable/reftable-stack.h
index 1b602dda58..50b1a4f4dd 100644
--- c/reftable/reftable-stack.h
+++ w/reftable/reftable-stack.h
@@ -9,6 +9,7 @@ license that can be found in the LICENSE file or at
#ifndef REFTABLE_STACK_H
#define REFTABLE_STACK_H
+#include <hash-ll.h>
#include "reftable-writer.h"
/*
^ permalink raw reply related
* Re: reftable: How to represent deleted referees of symrefs in the reflog?
From: Han-Wen Nienhuys @ 2023-12-20 19:03 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Jonathan Nieder, Terry Parker, Luca Milanesio
In-Reply-To: <ZVy0zKcmc8tjmgzs@tanuki>
On Tue, Nov 21, 2023 at 2:46 PM Patrick Steinhardt <ps@pks.im> wrote:
> To me it seems like deletions in this case only delete a particular log
> entry instead of the complete log for a particular reference. And some
> older discussion [1] seems to confirm my hunch that a complete reflog is
> deleted not with `log_type = 0x0`, but instead by writing the null
> object ID as new ID.
No, writing a null OID (more precisely a transition from $SHA1 =>
$ZERO) means that a branch was at $SHA1, and then was deleted. The
reflog continues to exist, and new entries may be added by reviving
the branch. That would add a $ZERO => $NEWSHA transition, but the
history of the branch prior to its deletion is retained.
> # This behaviour is a bit more on the weird side. We delete the
> # referee, and that causes the files backend to claim that the reflog
> # for HEAD is gone, as well. The reflog still exists though, as
> # demonstrated in the next command.
> $ git update-ref -m delete-main -d refs/heads/main
> $ git reflog show HEAD
> fatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree.
This looks wrong to me. HEAD has a history, and that history didn't go
away because the current branch happened to be deleted.
> It kind of feels like the second step in the files backend where the
> reflog is claimed to not exist is buggy -- I'd have expected to still
right, I agree.
> see the reflog, as the HEAD reference exists quite alright and has never
> stopped to exist. And in the third step, I'd have expected to see three
> reflog entries, including the deletion that is indeed still present in
> the on-disk logfile.
>
> But with the reftable backend the problem becomes worse: we cannot even
> represent the fact that the reflog still exists, but that the deletion
> of the referee has caused the HEAD to point to the null OID, because the
> null OID indicates complete deletion of the reflog.
This doesn't match my recollection. See
https://github.com/git/git/pull/1215, or more precisely
https://github.com/git/git/blob/3b2c5ddb28fa42a8c944373bea2ca756b1723908/refs/reftable-backend.c#L1582
Removing the entire reflog means removing all the individual entries
using tombstones.
> Consequentially, if
> we wrote the null OID, we'd only be able to see the last log entry here.
>
> It may totally be that I'm missing something obvious here. But if not,
> it leaves us with a couple of options for how to fix it:
>
> 1. Disregard this edge case and accept that the reftable backend
> does not do exactly the same thing as the files backend in very
> specific situations like this.
I remember discussing with Jun that it would be acceptable to have
slight semantic differences if unavoidable for the reflogs, and there
should be a record of this in the list. I think there will always be
some differences: for example, dropping reflogs because of a dir/file
conflict seems like a bug rather than a feature.
> 2. Change the reftable format so that it can discern these cases,
> e.g. by indicating deletion via a new log type.
This will be a bit messy, because it means that every read of the
reflog has to special case the "deletion marker" to make sure it is
absent, and on every reflog write, you have to create a tombstone for
the deletion marker, to make sure the reflog exists. Also, what if you
have something that looks like:
refs/main@1 : 0000... => 345....
refs/main@0xfffffff: DELETION
the reflog doesn't exists ("DELETION"), and now it is recreated, ie.
the following is added:
refs/main@0xfffffff: TOMBSTONE DELETION (make sure reflog exists)
refs/main@2 : 0000... => abc.... (the entry we want to write)
the result is that the reflog would also surface the first entry (@1 ,
000... => 345... ) again. To prevent that from happening, you have to
write tombstones for all entries if you "delete the reflog", but then
do you really need a separate existence marker?
> None of these alternatives feel particularly great to me. In my opinion
> (2) is likely the best option, but would require us to change the format
> format that's already in use by Google in the context of multiple
> projects. So I'm not quite sure how thrilled you folks at Google and
> other users of the reftable library are with this prospect.
Google uses reftable for datacenter-local serving. The reflog is
stored in a database, because it's never involved in serving traffic.
When I implemented reftable-on-filesystem, I found a few bugs in the
reflog code because it was never exercised at Google. Terry/Jonathan
may correct me if my knowledge is outdated.
Luca manages large Gerrit installations. He might know if anyone has
been using reftable in the wild.
HTH.
--
Han-Wen Nienhuys - hanwenn@gmail.com - http://www.xs4all.nl/~hanwen
^ permalink raw reply
* Re: [PATCH 04/12] setup: start tracking ref storage format when
From: Junio C Hamano @ 2023-12-20 18:30 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <6564659d403de098799ddb8101b74c2803a655d4.1703067989.git.ps@pks.im>
There is a topic in-flight that introduces the .compat_hash_algo
member to the repo_fmt structure. Seeing a conflict resolution like
the attached (there are many others that are similar in spirit), I
have to wonder if we want to add repo_set_ref_storage_format()
helper function. There are many assignments to .ref_storage_format
member after this series is applied.
Note that I haven't read the series in full, so take the above with
a grain of salt---it might turn out to be that direct assignment is
more desirable, I dunno.
Thanks.
diff --cc setup.c
index 2f4571c12a,5038e78cdd..0000000000
--- i/setup.c
+++ w/setup.c
@@@ ...
@@@ -1584,8 -1577,8 +1595,10 @@@ const char *setup_git_directory_gently(
}
if (startup_info->have_repository) {
repo_set_hash_algo(the_repository, repo_fmt.hash_algo);
+ repo_set_compat_hash_algo(the_repository,
+ repo_fmt.compat_hash_algo);
+ the_repository->ref_storage_format =
+ repo_fmt.ref_storage_format;
the_repository->repository_format_worktree_config =
repo_fmt.worktree_config;
/* take ownership of repo_fmt.partial_clone */
^ 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