Git development
 help / color / mirror / Atom feed
* Re: [PATCH 5/8] reftable/stack: perform auto-compaction with transactional interface
From: Patrick Steinhardt @ 2023-12-21 10:45 UTC (permalink / raw)
  To: Han-Wen Nienhuys; +Cc: git, Jonathan Nieder, psteinhardt
In-Reply-To: <CAOw_e7Yfdt_Wqm-9XDJknaN-iH=haP0R4K-S4c_E3EFDzvG5aA@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 930 bytes --]

On Thu, Dec 21, 2023 at 11:29:52AM +0100, Han-Wen Nienhuys wrote:
> On Tue, Nov 21, 2023 at 8:07 AM Patrick Steinhardt <ps@pks.im> wrote:
> > Whenever updating references or reflog entries in the reftable stack, we
> > need to add a new table to the stack, thus growing the stack's length by
> ..
> 
> bug is correctly identified, but can't the fix remove the
> 
>          if (!st->disable_auto_compact)
>                 return reftable_stack_auto_compact(st);
> 
> fragment from reftable_stack_add? reftable_addition_commit eventually
> calls reftable_addition_commit.

"calls reftable_stack_add" you probably mean here. But yes, you're
right. As this patch series has already been merged to `next` I can roll
this cleanup into my second patch series that addresses issues with the
reftable library [1]. Does that work for you?

[1]: http://public-inbox.org/git/xmqqr0jgsn9g.fsf@gitster.g/T/#t

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v2 11/11] reftable/block: reuse buffer to compute record keys
From: Han-Wen Nienhuys @ 2023-12-21 10:43 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Jonathan Nieder
In-Reply-To: <02b11f3a80608ba8748a0d0e2294f432e02464e5.1702047081.git.ps@pks.im>

On Fri, Dec 8, 2023 at 3:53 PM Patrick Steinhardt <ps@pks.im> wrote:

> @@ -84,10 +84,12 @@ struct block_iter {
>
>         /* key for last entry we read. */
>         struct strbuf last_key;
> +       struct strbuf key;
>  };

it's slightly more efficient, but the new field has no essential
meaning. If I encountered this code with the change you make here, I
would probably refactor it in the opposite direction to increase code
clarity.

I suspect that the gains are too small to be measurable, but if you
are after small efficiency gains, you can have
reftable_record_decode() consume the key to avoid copying overhead in
record.c.

-- 
Han-Wen Nienhuys - hanwenn@gmail.com - http://www.xs4all.nl/~hanwen

^ permalink raw reply

* Re: [PATCH 5/8] reftable/stack: perform auto-compaction with transactional interface
From: Han-Wen Nienhuys @ 2023-12-21 10:29 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Jonathan Nieder
In-Reply-To: <25522b042cdc5986972cc7b62e6b88be0569d3cb.1700549493.git.ps@pks.im>

On Tue, Nov 21, 2023 at 8:07 AM Patrick Steinhardt <ps@pks.im> wrote:
> Whenever updating references or reflog entries in the reftable stack, we
> need to add a new table to the stack, thus growing the stack's length by
..

bug is correctly identified, but can't the fix remove the

         if (!st->disable_auto_compact)
                return reftable_stack_auto_compact(st);

fragment from reftable_stack_add? reftable_addition_commit eventually
calls reftable_addition_commit.

-- 
Han-Wen Nienhuys - hanwenn@gmail.com - http://www.xs4all.nl/~hanwen

^ permalink raw reply

* Re: [PATCH v3 0/4] refs: improve handling of special refs
From: Patrick Steinhardt @ 2023-12-21 10:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Taylor Blau, Phillip Wood, Ramsay Jones
In-Reply-To: <xmqqil4ssmfg.fsf@gitster.g>

[-- Attachment #1: Type: text/plain, Size: 857 bytes --]

On Wed, Dec 20, 2023 at 11:28:51AM -0800, Junio C Hamano wrote:
> 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. We (myself or a fellow team member at GitLab) will work on
converting the remaining not-so-special refs once this topic lands on
the `master` branch. Unless of course somebody else picks it up before
we do.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH] git-compat-util: convert skip_{prefix,suffix}{,_mem} to bool
From: Jeff King @ 2023-12-21  9:59 UTC (permalink / raw)
  To: René Scharfe
  Cc: git, AtariDreams via GitGitGadget, Seija Kijin, Junio C Hamano,
	Phillip Wood
In-Reply-To: <2d30dc36-6091-4b47-846f-92d3f4a8b135@web.de>

On Sat, Dec 16, 2023 at 11:47:21AM +0100, René Scharfe wrote:

> Use the data type bool and its values true and false to document the
> binary return value of skip_prefix() and friends more explicitly.
> 
> This first use of stdbool.h, introduced with C99, is meant to check
> whether there are platforms that claim support for C99, as tested by
> 7bc341e21b (git-compat-util: add a test balloon for C99 support,
> 2021-12-01), but still lack that header for some reason.
> 
> A fallback based on a wider type, e.g. int, would have to deal with
> comparisons somehow to emulate that any non-zero value is true:
> 
>    bool b1 = 1;
>    bool b2 = 2;
>    if (b1 == b2) puts("This is true.");
> 
>    int i1 = 1;
>    int i2 = 2;
>    if (i1 == i2) puts("Not printed.");
>    #define BOOLEQ(a, b) (!(a) == !(b))
>    if (BOOLEQ(i1, i2)) puts("This is true.");
> 
> So we'd be better off using bool everywhere without a fallback, if
> possible.  That's why this patch doesn't include any.

Thanks for putting this together. I agree this is the right spot to end
up for now (and that if for whatever reason we find that some platforms
can't handle it, we probably should revert and not try the naive
fallback).

-Peff

^ permalink raw reply

* Re: [PATCH] Use ^=1 to toggle between 0 and 1
From: Jeff King @ 2023-12-21  9:56 UTC (permalink / raw)
  To: phillip.wood
  Cc: René Scharfe, AtariDreams via GitGitGadget, git, Seija Kijin
In-Reply-To: <99b3a727-36fd-4fa5-a6be-60ae6fc5911e@gmail.com>

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".

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).

-Peff

^ permalink raw reply

* [PATCH] t1006: add tests for %(objectsize:disk)
From: Jeff King @ 2023-12-21  9:47 UTC (permalink / raw)
  To: René Scharfe
  Cc: git, Ondrej Pohorelsky, brian m . carlson, Junio C Hamano
In-Reply-To: <6750c93c-78d0-46b5-bfc2-0774156ed2ed@web.de>

On Tue, Dec 19, 2023 at 05:42:39PM +0100, René Scharfe wrote:

> > This adds a packed-object function, but I doubt anybody actually calls
> > it. If we're going to do that, it's probably worth adding some tests for
> > "cat-file --batch-check" or similar.
> 
> Yes, and I was assuming that someone else would be eager to add such
> tests. *ahem*

:P OK, here it is. This can be its own topic, or go on top of the
rs/t6300-compressed-size-fix branch.

> > At which point I wonder if rather than having a function for a single
> > object, we are better off just testing the result of:
> >
> >   git cat-file --batch-all-objects --unordered --batch-check='%(objectsize:disk)'
> >
> > against a single post-processed "show-index" invocation.
> 
> Sure, we might want to optimize for bulk-processing and possibly end up
> only checking the size of single objects in t6300, making new library
> functions unnecessary.

So yeah, I think the approach here makes library functions unnecessary
(and I see you already asked Junio to drop your patch 2).

> When dumping size information of multiple objects, it's probably a good
> idea to include "%(objectname)" as well in the format.

Yep, definitely.

-- >8 --
Subject: [PATCH] t1006: add tests for %(objectsize:disk)

Back when we added this placeholder in a4ac106178 (cat-file: add
%(objectsize:disk) format atom, 2013-07-10), there were no tests,
claiming "[...]the exact numbers returned are volatile and subject to
zlib and packing decisions".

But we can use a little shell hackery to get the expected numbers
ourselves. To a certain degree this is just re-implementing what Git is
doing under the hood, but it is still worth doing. It makes sure we
exercise the %(objectsize:disk) code at all, and having the two
implementations agree gives us more confidence.

Note that our shell code assumes that no object appears twice (either in
two packs, or as both loose and packed), as then the results really are
undefined. That's OK for our purposes, and the test will notice if that
assumption is violated (the shell version would produce duplicate lines
that Git's output does not have).

Helped-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Jeff King <peff@peff.net>
---
I stole a bit of your awk. You can tell because I'd have written it in
perl. ;)

 t/t1006-cat-file.sh | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index 271c5e4fd3..21915be308 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -1100,6 +1100,40 @@ test_expect_success 'cat-file --batch="batman" with --batch-all-objects will wor
 	cmp expect actual
 '
 
+test_expect_success 'cat-file %(objectsize:disk) with --batch-all-objects' '
+	# our state has both loose and packed objects,
+	# so find both for our expected output
+	{
+		find .git/objects/?? -type f |
+		awk -F/ "{ print \$0, \$3\$4 }" |
+		while read path oid
+		do
+			size=$(test_file_size "$path") &&
+			echo "$oid $size" ||
+			return 1
+		done &&
+		rawsz=$(test_oid rawsz) &&
+		find .git/objects/pack -name "*.idx" |
+		while read idx
+		do
+			git show-index <"$idx" >idx.raw &&
+			sort -n <idx.raw >idx.sorted &&
+			packsz=$(test_file_size "${idx%.idx}.pack") &&
+			end=$((packsz - rawsz)) &&
+			awk -v end="$end" "
+			  NR > 1 { print oid, \$1 - start }
+			  { start = \$1; oid = \$2 }
+			  END { print oid, end - start }
+			" idx.sorted ||
+			return 1
+		done
+	} >expect.raw &&
+	sort <expect.raw >expect &&
+	git cat-file --batch-all-objects \
+		--batch-check="%(objectname) %(objectsize:disk)" >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'set up replacement object' '
 	orig=$(git rev-parse HEAD) &&
 	git cat-file commit $orig >orig &&
-- 
2.43.0.430.gaf21263e5d


^ permalink raw reply related

* Re: [RFC/PATCH] archive: "--list" does not take further options
From: Jeff King @ 2023-12-21  8:59 UTC (permalink / raw)
  To: René Scharfe; +Cc: Junio C Hamano, git
In-Reply-To: <296e8d69-c1d7-4ad2-943a-dfc54940abc2@web.de>

On Thu, Dec 21, 2023 at 08:30:36AM +0100, René Scharfe wrote:

> >    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?
> 
> Don't we have one?  It would affect other unsupported options as well,
> and this seems to work just fine, e.g.:
> 
>    $ git archive --remote=. --format=foo HEAD
>    remote: fatal: Unknown archive format 'foo'
>    remote: git upload-archive: archiver died with error
>    fatal: sent error to the client: git upload-archive: archiver died with error

Right. The whole idea of upload-archive is to spawn a separate writer
process and mux the conversation (including errors) back over the wire.
There are a zillion reasons it can die (including bad arguments) and we
catch and report them in the muxing process.

> >  	if (list) {
> > +		if (argc) {
> > +			if (!is_remote)
> > +				die(_("extra command line parameter '%s'"), *argv);
> > +			else
> > +				printf("!ERROR! extra command line parameter '%s'\n",
> > +				       *argv);
> > +		}
> 
> So just call die() here?

Yes, exactly.

-Peff

^ permalink raw reply

* Re: [RFC/PATCH] archive: "--list" does not take further options
From: Jeff King @ 2023-12-21  8:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, René Scharfe
In-Reply-To: <xmqqttocp98r.fsf@gitster.g>

On Wed, Dec 20, 2023 at 06:41:56PM -0800, Junio C Hamano wrote:

> ---
>  * 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.

Right. Not only is it handled by the second parser, but _not_ keeping it
would be a bug, because that second parser would have no idea that we
saw end-of-options (and so "git archive --end-of-options --foo" would
try to treat "--foo" as an option).

The recent commit 9385174627 did fix a case like this for fast-export,
but git-archive was not changed. It passed KEEP_DASHDASH along with
KEEP_UNKNOWN_OPT, so it already retained and passed along
--end-of-options.

>    - 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().

Right, and this is just the same case but with a lot of complicated
network-ferrying in between. :) We must retain --end-of-options so that
the next parser knows to stop treating them as arguments. And because it
doesn't use KEEP_UNKNOWN_OPT, the ferried "--end-of-options" is removed
then.

> 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);
> +		}

This general direction seems reasonable to me, since we're letting the
user know that their extra argument was ignored (though note that if it
was a misspelled option, like "--otuput", we would complain about that).
It's largely orthogonal to end-of-options, but I see how you got here by
wondering about it. :)

-Peff

^ permalink raw reply

* Re: [PATCH/RFC] sparse-checkout: take care of "--end-of-options" in set/add
From: Jeff King @ 2023-12-21  8:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Josh Steadmon, git
In-Reply-To: <xmqqplz0p90k.fsf@gitster.g>

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.

-Peff

^ permalink raw reply

* Re: [PATCH/RFC] sparse-checkout: take care of "--end-of-options" in set/add
From: Jeff King @ 2023-12-21  8:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Derrick Stolee, git, Josh Steadmon
In-Reply-To: <xmqqbkakqx6s.fsf@gitster.g>

On Wed, Dec 20, 2023 at 03:19:23PM -0800, 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.

Thanks for finding this. Though I only half-agree with the
"unfortunately" part. ;)

As argued in 93851746, any caller passing KEEP_UNKNOWN_OPT but _not_
handling the now-returned end-of-options is rather suspicious. Because
if they are planning to parse those options further, they will have no
idea that we saw --end-of-options, and will err in the unsafe direction
of still treating any elements with dashes as options.

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.

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.

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.

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).

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. 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.

-Peff

^ permalink raw reply

* Re: [RFC/PATCH] archive: "--list" does not take further options
From: René Scharfe @ 2023-12-21  7:30 UTC (permalink / raw)
  To: Junio C Hamano, git; +Cc: Jeff King
In-Reply-To: <xmqqttocp98r.fsf@gitster.g>

Am 21.12.23 um 03:41 schrieb Junio C Hamano:
> "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?

Don't we have one?  It would affect other unsupported options as well,
and this seems to work just fine, e.g.:

   $ git archive --remote=. --format=foo HEAD
   remote: fatal: Unknown archive format 'foo'
   remote: git upload-archive: archiver died with error
   fatal: sent error to the client: git upload-archive: archiver died with error

>
>  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);
> +		}

So just call die() here?

>  		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

... and use test_must_fail in both cases?

> +'
> +
> +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

* [PATCH 1/3] sparse-checkout: take care of "--end-of-options" in set/add/check-rules
From: Junio C Hamano @ 2023-12-21  6:59 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Josh Steadmon
In-Reply-To: <20231221065925.3234048-1-gitster@pobox.com>

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.

A breakage that results from the same cause exists in the check-rules
subcommand, but check-rules has a few other problems that need to be
corrected before it can fully work with --end-of-options safely,
which will be addressed later.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/sparse-checkout.c          | 3 +++
 parse-options.c                    | 8 ++++++++
 parse-options.h                    | 2 ++
 t/t1090-sparse-checkout-scope.sh   | 8 ++++++++
 t/t1091-sparse-checkout-builtin.sh | 2 +-
 5 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 5c8ffb1f75..8f55127202 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -779,6 +779,7 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
 			     builtin_sparse_checkout_add_options,
 			     builtin_sparse_checkout_add_usage,
 			     PARSE_OPT_KEEP_UNKNOWN_OPT);
+	parse_opt_skip_end_of_options(&argc, &argv);
 
 	sanitize_paths(argc, argv, prefix, add_opts.skip_checks);
 
@@ -826,6 +827,7 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
 			     builtin_sparse_checkout_set_options,
 			     builtin_sparse_checkout_set_usage,
 			     PARSE_OPT_KEEP_UNKNOWN_OPT);
+	parse_opt_skip_end_of_options(&argc, &argv);
 
 	if (update_modes(&set_opts.cone_mode, &set_opts.sparse_index))
 		return 1;
@@ -998,6 +1000,7 @@ static int sparse_checkout_check_rules(int argc, const char **argv, const char *
 			     builtin_sparse_checkout_check_rules_options,
 			     builtin_sparse_checkout_check_rules_usage,
 			     PARSE_OPT_KEEP_UNKNOWN_OPT);
+	parse_opt_skip_end_of_options(&argc, &argv);
 
 	if (check_rules_opts.rules_file && check_rules_opts.cone_mode < 0)
 		check_rules_opts.cone_mode = 1;
diff --git a/parse-options.c b/parse-options.c
index d50962062e..fe265bbf68 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -1321,3 +1321,11 @@ void die_for_incompatible_opt4(int opt1, const char *opt1_name,
 		break;
 	}
 }
+
+void parse_opt_skip_end_of_options(int *argc, const char ***argv)
+{
+	if (*argc && !strcmp(**argv, "--end-of-options")) {
+		(*argc)--;
+		(*argv)++;
+	}
+}
diff --git a/parse-options.h b/parse-options.h
index bd62e20268..0d3354d4a8 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -498,6 +498,8 @@ int parse_opt_passthru_argv(const struct option *, const char *, int);
 /* value is enum branch_track* */
 int parse_opt_tracking_mode(const struct option *, const char *, int);
 
+void parse_opt_skip_end_of_options(int *argc, const char ***argv);
+
 #define OPT__VERBOSE(var, h)  OPT_COUNTUP('v', "verbose", (var), (h))
 #define OPT__QUIET(var, h)    OPT_COUNTUP('q', "quiet",   (var), (h))
 #define OPT__VERBOSITY(var) { \
diff --git a/t/t1090-sparse-checkout-scope.sh b/t/t1090-sparse-checkout-scope.sh
index 3a14218b24..5b96716235 100755
--- a/t/t1090-sparse-checkout-scope.sh
+++ b/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 a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
index f67611da28..e33a6ed1b4 100755
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/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 &&
 	/*
 	!/*/
-- 
2.43.0-174-g055bb6e996


^ permalink raw reply related

* [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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox