Git development
 help / color / mirror / Atom feed
* Re: [PATCH v5 0/5] merge-ort: implement support for packing objects together
From: Junio C Hamano @ 2023-11-06 23:19 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Taylor Blau, git, Elijah Newren, Eric W. Biederman, Jeff King,
	Patrick Steinhardt
In-Reply-To: <0ac32374-7d52-8f0c-8583-110de678291e@gmx.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> What I have in mind is more along Elijah's suggestion at the Contributor
> Summit to use the `tmp_objdir*()` machinery. But instead of discarding the
> temporary object database, the contained objects would be repacked and the
> `.pack`, (maybe `.rev`) and the `.idx` file would then be moved (in that
> order) before discarding the temporary object database.

That may be more involved but does indeed sound like an approach
more generally applicable.  Back when the bulk-checkin machinery was
invented, I envisioned that we would be adding annotations to
various codepaths so that object creation machinery can say "now we
are plugged, in anticipation for creating many objects at once" and
"now the flood of new object creation is done, time to wrap up" for
that kind of optimization.  

The callsites to {begin,end}_odb_transaction() functions haven't
grown beyond the original "add" and "update-index" (because the user
can add the entire working tree worth of files to the object
database), "unpack-objects" (because a fetch can bring in many
objects), and "cache-tree" (because a tree creation can cascade up
to create many objects), but I agree "merge" and "replay" are prime
candidates to benefit from the optimization of the same kind (so is
"fast-import").  They are about creating many objects at once, and
give us an opportunity for such an optimization.

^ permalink raw reply

* Re: Regression: git send-email Message-Id: numbering doesn't start at 1 any more
From: Junio C Hamano @ 2023-11-06 23:06 UTC (permalink / raw)
  To: Uwe Kleine-König, Michael Strawbridge
  Cc: git, Douglas Anderson, entwicklung
In-Reply-To: <20231106153214.s5abourejkuiwk64@pengutronix.de>

Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:

> Hello,
>
> Since commit 3ece9bf0f9e24909b090cf348d89e8920bd4f82f I experience that
> the generated Message-Ids don't start at ....-1-... any more. I have:
>
> $ git send-email w/*
> ...
> Subject: [PATCH 0/5] watchdog: Drop platform_driver_probe() and convert to platform remove callback returning void (part II)
> Date: Mon,  6 Nov 2023 16:10:04 +0100
> Message-ID: <20231106151003.3844134-7-u.kleine-koenig@pengutronix.de>
> ...
>
> So the cover letter is sent with Message-Id: ...-7-...

The above is consistent with the fact that a 5-patch series with a
cover letter consists of 6 messages.  Dry-run uses message numbers
1-6 and forgets to reset the counter, so the next message becomes 7.
As you identified, the fix in 3ece9bf0 (send-email: clear the
$message_id after validation, 2023-05-17) for the fallout from an
even earlier change to process each message twice still had left an
observable side effect subject to the Hyrum's law, it seems.

> +my ($message_id_stamp, $message_id_serial);
>  if ($validate) {
>  	# FIFOs can only be read once, exclude them from validation.
>  	my @real_files = ();
> @@ -821,6 +822,7 @@ sub is_format_patch_arg {
>  	}
>  	delete $ENV{GIT_SENDEMAIL_FILE_COUNTER};
>  	delete $ENV{GIT_SENDEMAIL_FILE_TOTAL};
> +	$message_id_serial = 0;
>  }

This fix looks quite logical to me, but even with this, the side
effects of the earlier "read message twice" persists in end-user
observable form, don't they?  IIRC, when sending out an N message
series, we start from the timestamp as of N seconds ago and give
each message the Date: header that increments by 1 second, which
would mean the validator will see Date: that is different from what
will actually be sent out, and more importantly, the messages sent
out for real will have timestamps from the future, negating the
point of starting from N seconds ago in the first place.  Your
script may not have been paying attention to it and only noticed the
difference in id_serial, but somebody else would complain the
difference coming from calling gen_header more than once for each
messages since a8022c5f (send-email: expose header information to
git-send-email's sendemail-validate hook, 2023-04-19).

So, I dunno.  Michael, what do you think?  It appears to me that a
more fundamental fix to the fallout from a8022c5f might be needed
(e.g., we still let gen_header run while validating, but once
validation is done, save the headers that validator saw and use them
without calling gen_header again when we send the messages out, or
something), if we truly want to be regression free.

By the way, out of curiosity, earlier you said your script looks at
the Message-IDs and counts the number of messages.  How does it do
that?  Does it read the output of send-email and pass the messages
to MTA for sending out for real?

Thanks.

>  @files = handle_backup_files(@files);
> @@ -1181,7 +1183,6 @@ sub validate_address_list {
>  
>  # We'll setup a template for the message id, using the "from" address:
>  
> -my ($message_id_stamp, $message_id_serial);
>  sub make_message_id {
>  	my $uniq;
>  	if (!defined $message_id_stamp) {
>
> But I guess this could be done prettier by someone who is fluent in
> Perl.
>
> Best regards
> Uwe

^ permalink raw reply

* [PATCH 2/2] pack-bitmap: drop --unpacked non-commit objects from results
From: Taylor Blau @ 2023-11-06 22:56 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <cover.1699311386.git.me@ttaylorr.com>

When performing revision queries with `--objects` and
`--use-bitmap-index`, the output may incorrectly contain objects which
are packed, even when the `--unpacked` option is given. This affects
traversals, but also other querying operations, like `--count`,
`--disk-usage`, etc.

Like in the previous commit, the fix is to exclude those objects from
the result set before they are shown to the user (or, in this case,
before the bitmap containing the result of the traversal is enumerated
and its objects listed).

This is performed by a new function in pack-bitmap.c, called
`filter_packed_objects_from_bitmap()`. Note that we do not have to
inspect individual bits in the result bitmap, since we know that the
first N (where N is the number of objects in the bitmap's pack/MIDX)
bits correspond to objects which packed by definition.

In other words, for an object to have a bitmap position (not in the
extended index), it must appear in either the bitmap's pack or one of
the packs in its MIDX.

This presents an appealing optimization to us, which is that we can
simply memset() the corresponding number of `eword_t`'s to zero,
provided that we handle any objects which spill into the next word (but
don't occupy all 64 bits of the word itself).

We only have to handle objects in the bitmap's extended index. These
objects may (or may not) appear in one or more pack(s). Since these
objects are known to not appear in either the bitmap's MIDX or pack,
they may be stored as loose, appear in other pack(s), or both.

Before returning a bitmap containing the result of the traversal back to
the caller, drop any bits from the extended index which appear in one or
more packs. This implements the correct behavior for rev-list operations
which use the bitmap index to compute their result.

Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 pack-bitmap.c                      | 27 +++++++++++++++++++++++++++
 t/t6113-rev-list-bitmap-filters.sh | 13 +++++++++++++
 t/t6115-rev-list-du.sh             |  7 +++++++
 3 files changed, 47 insertions(+)

diff --git a/pack-bitmap.c b/pack-bitmap.c
index ca8319b87c..0260890341 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -1666,6 +1666,30 @@ static int can_filter_bitmap(struct list_objects_filter_options *filter)
 	return !filter_bitmap(NULL, NULL, NULL, filter);
 }
 
+
+static void filter_packed_objects_from_bitmap(struct bitmap_index *bitmap_git,
+					      struct bitmap *result)
+{
+	struct eindex *eindex = &bitmap_git->ext_index;
+	uint32_t objects_nr;
+	size_t i, pos;
+
+	objects_nr = bitmap_num_objects(bitmap_git);
+	pos = objects_nr / BITS_IN_EWORD;
+
+	if (pos > result->word_alloc)
+		pos = result->word_alloc;
+
+	memset(result->words, 0x00, sizeof(eword_t) * pos);
+	for (i = pos * BITS_IN_EWORD; i < objects_nr; i++)
+		bitmap_unset(result, i);
+
+	for (i = 0; i < eindex->count; ++i) {
+		if (has_object_pack(&eindex->objects[i]->oid))
+			bitmap_unset(result, objects_nr + i);
+	}
+}
+
 struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
 					 int filter_provided_objects)
 {
@@ -1788,6 +1812,9 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
 		      wants_bitmap,
 		      &revs->filter);
 
+	if (revs->unpacked)
+		filter_packed_objects_from_bitmap(bitmap_git, wants_bitmap);
+
 	bitmap_git->result = wants_bitmap;
 	bitmap_git->haves = haves_bitmap;
 
diff --git a/t/t6113-rev-list-bitmap-filters.sh b/t/t6113-rev-list-bitmap-filters.sh
index 4d8e09167e..86c70521f1 100755
--- a/t/t6113-rev-list-bitmap-filters.sh
+++ b/t/t6113-rev-list-bitmap-filters.sh
@@ -141,4 +141,17 @@ test_expect_success 'combine filter with --filter-provided-objects' '
 	done <objects
 '
 
+test_expect_success 'bitmap traversal with --unpacked' '
+	git repack -adb &&
+	test_commit unpacked &&
+
+	git rev-list --objects --no-object-names unpacked^.. >expect.raw &&
+	sort expect.raw >expect &&
+
+	git rev-list --use-bitmap-index --objects --all --unpacked >actual.raw &&
+	sort actual.raw >actual &&
+
+	test_cmp expect actual
+'
+
 test_done
diff --git a/t/t6115-rev-list-du.sh b/t/t6115-rev-list-du.sh
index d59111dede..c0cfda62fa 100755
--- a/t/t6115-rev-list-du.sh
+++ b/t/t6115-rev-list-du.sh
@@ -48,6 +48,13 @@ check_du HEAD
 check_du --objects HEAD
 check_du --objects HEAD^..HEAD
 
+test_expect_success 'setup for --unpacked tests' '
+	git repack -adb &&
+	test_commit unpacked
+'
+
+check_du --all --objects --unpacked
+
 # As mentioned above, don't use hardcode sizes as actual size, but use the
 # output from git cat-file.
 test_expect_success 'rev-list --disk-usage=human' '
-- 
2.43.0.rc0.2.gef6b2154a3

^ permalink raw reply related

* [PATCH 1/2] list-objects: drop --unpacked non-commit objects from results
From: Taylor Blau @ 2023-11-06 22:56 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <cover.1699311386.git.me@ttaylorr.com>

In git-rev-list(1), we describe the `--unpacked` option as:

    Only useful with `--objects`; print the object IDs that are not in
    packs.

This is true of commits, which we discard via get_commit_action(), but
not of the objects they reach. So if we ask for an --objects traversal
with --unpacked, we may get arbitrarily many objects which are indeed
packed.

I am nearly certain this behavior dates back to the introduction of
`--unpacked` via 12d2a18780 ("git rev-list --unpacked" shows only
unpacked commits, 2005-07-03), but I couldn't get that revision of Git
to compile for me. At least as early as v2.0.0 this has been subtly
broken:

    $ git.compile --version
    git version 2.0.0

    $ git.compile rev-list --objects --all --unpacked
    72791fe96c93f9ec5c311b8bc966ab349b3b5bbe
    05713d991c18bbeef7e154f99660005311b5004d v1.0
    153ed8b7719c6f5a68ce7ffc43133e95a6ac0fdb
    8e4020bb5a8d8c873b25de15933e75cc0fc275df one
    9200b628cf9dc883a85a7abc8d6e6730baee589c two
    3e6b46e1b7e3b91acce99f6a823104c28aae0b58 unpacked.t

There, only the first, third, and sixth entries are loose, with the
remaining set of objects belonging to at least one pack.

The implications for this are relatively benign: bare 'git repack'
invocations which invoke pack-objects with --unpacked are impacted, and
at worst we'll store a few extra objects that should have been excluded.

Arguably changing this behavior is a backwards-incompatible change,
since it alters the set of objects emitted from rev-list queries with
`--objects` and `--unpacked`. But I argue that this change is still
sensible, since the existing implementation deviates from
clearly-written documentation.

The fix here is straightforward: avoid showing any non-commit objects
which are contained in packs by discarding them within list-objects.c,
before they are shown to the user. Note that similar treatment for
`list-objects.c::show_commit()` is not needed, since that case is
already handled by `revision.c::get_commit_action()`.

Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 list-objects.c           |  3 +++
 t/t6000-rev-list-misc.sh | 13 +++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/list-objects.c b/list-objects.c
index c25c72b32c..c8a5fb998e 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -39,6 +39,9 @@ static void show_object(struct traversal_context *ctx,
 {
 	if (!ctx->show_object)
 		return;
+	if (ctx->revs->unpacked && has_object_pack(&object->oid))
+		return;
+
 	ctx->show_object(object, name, ctx->show_data);
 }
 
diff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh
index 12def7bcbf..6289a2e8b0 100755
--- a/t/t6000-rev-list-misc.sh
+++ b/t/t6000-rev-list-misc.sh
@@ -169,4 +169,17 @@ test_expect_success 'rev-list --count --objects' '
 	test_line_count = $count actual
 '
 
+test_expect_success 'rev-list --unpacked' '
+	git repack -ad &&
+	test_commit unpacked &&
+
+	git rev-list --objects --no-object-names unpacked^.. >expect.raw &&
+	sort expect.raw >expect &&
+
+	git rev-list --all --objects --unpacked --no-object-names >actual.raw &&
+	sort actual.raw >actual &&
+
+	test_cmp expect actual
+'
+
 test_done
-- 
2.43.0.rc0.2.gef6b2154a3


^ permalink raw reply related

* [PATCH 0/2] revision: exclude all packed objects with `--unpacked`
From: Taylor Blau @ 2023-11-06 22:56 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano

While working on my longer series to enable verbatim pack reuse across
multiple packs[^1], I noticed a couple of oddities with the `--unpacked`
rev-walk flag.

While it does exclude packed commits, it does not exclude (all) packed
trees/blobs/annotated tags. This problem exists in the pack-bitmap
machinery, too, which will over-count queries like:

    $ git rev-list --use-bitmap-index --all --unpacked --objects

, etc.

The fix is relatively straightforward, split across two patches that
Peff and I worked on together earlier today.

This is technically a backwards-incompatible change, but the existing
behavior is broken and does not match the documented behavior, so I
think in this case we are OK to change --unpacked to faithfully
implement its documentation.

[^1]: Which, I'm very excited to say, is working :-).

Taylor Blau (2):
  list-objects: drop --unpacked non-commit objects from results
  pack-bitmap: drop --unpacked non-commit objects from results

 list-objects.c                     |  3 +++
 pack-bitmap.c                      | 27 +++++++++++++++++++++++++++
 t/t6000-rev-list-misc.sh           | 13 +++++++++++++
 t/t6113-rev-list-bitmap-filters.sh | 13 +++++++++++++
 t/t6115-rev-list-du.sh             |  7 +++++++
 5 files changed, 63 insertions(+)


base-commit: bc5204569f7db44d22477485afd52ea410d83743
-- 
2.43.0.rc0.2.gef6b2154a3

^ permalink raw reply

* Re: RE: first-class conflicts?
From: Sandra Snan @ 2023-11-06 22:45 UTC (permalink / raw)
  To: git, Dragan Simic, rsbecker
In-Reply-To: <002901da1101$7d39a420$77acec60$@nexbridge.com>

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

Randall, thank you for that.

I did mean of the first type, pure content conflicts (just like the examples 
on that jj page).

I just have sometimes wish git could be a little more aware of them beyond 
just storing them with ASCII art in the files themselves (and alerting / 
warning when they happen but I often can't properly see those warnings flash 
by so I end up having to search for the conflict markers manually). So if 
conflicts are a thing that *can* happen, it'd be better if vc could know 
about them which would make some of the rebases simpler as in jj. That doesn't 
mean we wanna adopt the jj workflow of deliberately checking in conflicts 
(not even locally), just be able to deal with them better if it does happen.

I dunno… and I've really appreciated the naysayers so far, helps me sort 
out my thoughts in this. I personally really prefer the vanilla "explicit 
staging" workflow (with magit) over jj, got, gitless etc. I'm more scared 
of overcommitting by mistake than undercommitting. But this one feature 
seemed to me that it might be really good: just having the vc be aware of 
the conflicts it has created.

^ permalink raw reply

* RE: first-class conflicts?
From: rsbecker @ 2023-11-06 22:34 UTC (permalink / raw)
  To: 'Dragan Simic', 'Sandra Snan'; +Cc: git
In-Reply-To: <ef30a484525157579c64249a396f10ae@manjaro.org>

On November 6, 2023 5:01 PM, Dragan Simic wrote:
>On 2023-11-06 22:17, Sandra Snan wrote:
>> Is this feature from jj also a good idea for git?
>> https://martinvonz.github.io/jj/v0.11.0/conflicts/
>
>Hmm, that's quite interesting, but frankly it makes little sense to me.
>See, the source code in a repository should always be in a compileable or
runnable
>state, in each and every commit, so going against that rule wouldn't make
much
>sense.  Just think about various CI/CD tools that also expect the same.

It seems to me, perhaps naively, that the longer a conflict persists in a
repository, the greater the potential for chaotic results. There are,
notably, at least two fundamental types of conflicts:

1. Content conflict, where a point in a file is modified in two (or n)
branches being combined, is what git tries to ensure never happens. The
longer such a conflict exists in a file, the greater the variance from a
buildable or consistent state will persist and will likely be increasingly
harder to resolve.

2. Semantic conflicts, where unrelated modification points cause
incompatibilities are much harder to resolve and quantify - many are, in
fact, undetectable from a computational standpoint (as in detecting general
semantic conflicts is an uncomputable problem). The longer those persist,
partly when they are missed by pull requests/code reviews, the more
persistent a defect can become.

3. I am avoiding matters such as code optimization conflicts which are
outside the scope of the proposal.

In either case, storing conflicts in the integration branches of a
repository is, in my view, a bad thing that eventually can make the
repository unsustainable. I will concede that keeping conflicts around in
non-integration branches may have intellectual value for recording research
and development progress.

This is just my opinion.
Randall

--
Brief whoami: NonStop&UNIX developer since approximately
UNIX(421664400)
NonStop(211288444200000000)
-- In real life, I talk too much.




^ permalink raw reply

* Re: first-class conflicts?
From: Sandra Snan @ 2023-11-06 22:34 UTC (permalink / raw)
  To: git
In-Reply-To: <ef30a484525157579c64249a396f10ae@manjaro.org>

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

I've sometimes merged stuff in and almost not notice that I had a conflict 
in there and in those cases the code wasn't compilable even though I was 
using vanilla git.

^ permalink raw reply

* Re: first-class conflicts?
From: Dragan Simic @ 2023-11-06 22:01 UTC (permalink / raw)
  To: Sandra Snan; +Cc: git
In-Reply-To: <87cywmintp.fsf@ellen.idiomdrottning.org>

On 2023-11-06 22:17, Sandra Snan wrote:
> Is this feature from jj also a good idea for git?
> https://martinvonz.github.io/jj/v0.11.0/conflicts/

Hmm, that's quite interesting, but frankly it makes little sense to me.  
See, the source code in a repository should always be in a compileable 
or runnable state, in each and every commit, so going against that rule 
wouldn't make much sense.  Just think about various CI/CD tools that 
also expect the same.

^ permalink raw reply

* first-class conflicts?
From: Sandra Snan @ 2023-11-06 21:17 UTC (permalink / raw)
  To: git

Is this feature from jj also a good idea for git?
https://martinvonz.github.io/jj/v0.11.0/conflicts/

^ permalink raw reply

* git refusing to merge branches when pulling using a refspec
From: Santiago García Pimentel @ 2023-11-06 17:36 UTC (permalink / raw)
  To: git

Hello,

I'm dealing with a small automation (CI) to synchronise some specific branches between two git repositories.

I need to sync a branch with other using a user-given refspec. e.g.

$ git pull origin "refs/heads/branchOrigin:refs/heads/branchDestination”.

(I’ll have a list of refspecs, but so far Im trying to make it work with one)

When the branch can be fast-forwarded there is no problem, but I cannot manage to make it work when it cant.

I just. get the message "[rejected] branchOrigin -> branchDestination (non-fast forward)."

I've tried  the following:
- adding a git config with pull.rebase=false  (I also tried true)
- adding --no-rebase to the pull command  (I also tried —rebase)

In any cases the result is the same

The issue is, It does work if I just pass the branch names. e.g.

git pull origin branchOrigin --no-edit

merges the branch just fine

Im not sure why git refuses to merge the branches when I pass a full refspec. is this intentional? is there some other incantation I need to use to make this work?

Thank you!

Santiago García Pimentel.
santiago@garciapimentel.com

^ permalink raw reply

* Re: [PATCH 0/4] Memory leak fixes
From: Jeff King @ 2023-11-06 17:32 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git
In-Reply-To: <cover.1699267422.git.ps@pks.im>

On Mon, Nov 06, 2023 at 11:45:48AM +0100, Patrick Steinhardt wrote:

> this patch series fixes some memory leaks. All of these leaks have been
> found while working on the reftable backend.

All four look good to me (and the refactoring in 3/4 is very cleanly
done).

-Peff

^ permalink raw reply

* Re: Bug: magic-less pathspecs that start with ":" not processed as expected.
From: Jeff King @ 2023-11-06 17:31 UTC (permalink / raw)
  To: Joanna Wang; +Cc: git
In-Reply-To: <CAMmZTi-JJ6=Uw_+r50hKMYf34D0NtqXhaA=f+11+wAQBjqA7_g@mail.gmail.com>

On Mon, Nov 06, 2023 at 10:54:03AM -0500, Joanna Wang wrote:

> What did you do before the bug happened? (Steps to reproduce your issue)
> 
> What did you expect to happen? (Expected behavior)
> `git stash push -- :file` where `:` is part of the filename, should search
> for files named ":file"
> 
> What happened instead? (Actual behavior)
> The match string used to find files is "file" without the ":".
> 
> What's different between what you expected and what actually happened?
> file named ":file" is not found.
> 
> Anything else you want to add:
> I believe this is due to parse_short_magic() not handling the case where
> ":" is part of the file name rather than a prefix for pathspec magic.
> 
> I could not find any documentation that says ":" is a reserved
> character to indicate pathspec magic symbols MUST follow or that
> ":file" is not a valid file name.

I think this is the correct behavior according to the documentation.
From "git help glossary", the entry on "pathspec" says:

  A pathspec that begins with a colon : has special meaning. In the
  short form, the leading colon : is followed by zero or more "magic
  signature" letters (which optionally is terminated by another colon
  :), and the remainder is the pattern to match against the path.

So ":file" just has zero magic signature letters. I think you want:

  :::file

which of course is rather ugly, but then so is your filename. ;)
Although longer, probably this is more readable:

  :(literal):file

And if you're working with a lot of such files (or you are scripting and
know you have a set of filenames), then your best bet is probably
turning on literal mode, like:

  git --literal-pathspecs ls-files -- :file

-Peff

PS It took me a while to figure out where we document pathspec syntax. I
   wonder if a "gitpathspecs" manpage would make sense, like we have
   "gitrevisions".

^ permalink raw reply

* Re: [PATCH 2/4] shallow: fix memory leak when registering shallow roots
From: Jeff King @ 2023-11-06 17:21 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git
In-Reply-To: <137c150e358a8248200e0c7174a79358adb92e45.1699267422.git.ps@pks.im>

On Mon, Nov 06, 2023 at 11:45:57AM +0100, Patrick Steinhardt wrote:

> --- a/shallow.c
> +++ b/shallow.c
> @@ -38,8 +38,10 @@ int register_shallow(struct repository *r, const struct object_id *oid)
>  
>  	oidcpy(&graft->oid, oid);
>  	graft->nr_parent = -1;
> -	if (commit && commit->object.parsed)
> +	if (commit && commit->object.parsed) {
> +		free_commit_list(commit->parents);
>  		commit->parents = NULL;
> +	}
>  	return register_commit_graft(r, graft, 0);
>  }

Good catch. When I've dealt with leaks around commit_lists in the past,
one gotcha is that sometimes we've saved a pointer to the list
elsewhere. But in this case it looks pretty clear that the parent list
just goes away. So this patch is doing the right thing.

-Peff

^ permalink raw reply

* Re: [PATCH 1/4] test-bloom: stop setting up Git directory twice
From: Jeff King @ 2023-11-06 17:20 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git
In-Reply-To: <fb1cc73ed3395ba3de72aba70d7679dd85ebb034.1699267422.git.ps@pks.im>

On Mon, Nov 06, 2023 at 11:45:53AM +0100, Patrick Steinhardt wrote:

> We're setting up the Git directory twice in the `test-tool bloom`
> helper, once at the beginning of `cmd_bloom()` and once in the local
> subcommand implementation `get_bloom_filter_for_commit()`. This can lead
> to memory leaks as we'll overwrite variables of `the_repository` with
> newly allocated data structures. On top of that it's simply unnecessary.
> 
> Fix this by only setting up the Git directory once.

Makes sense. This situation was created by 094a685cd7 (t: make
test-bloom initialize repository, 2020-07-29), which added the setup
call at the start of the program.

That commit closed the door to running test-bloom outside of a
repository for sub-commands that could handle it (perhaps the murmur3
one, but I didn't test). So there are two possible directions here:

  - drop the call in cmd__bloom() and make sure all of the relevant
    sub-command functions do the setup.

  - drop the one command-specific one (i.e., your patch)

In practice I don't think anybody cares about running this test-helper
outside of a repository, and having to do setup in each sub-command is
an extra maintenance burden. So I think your patch is the best
direction.

-Peff

^ permalink raw reply

* Re: Explanation of `eol` attribute doesn't seem correct for Windows machines
From: Torsten Bögershausen @ 2023-11-06 16:21 UTC (permalink / raw)
  To: Hans Meiser; +Cc: git@vger.kernel.org
In-Reply-To: <AS4P195MB15272FE283D0CF606A8B074EE2AAA@AS4P195MB1527.EURP195.PROD.OUTLOOK.COM>

On Mon, Nov 06, 2023 at 03:11:54PM +0000, Hans Meiser wrote:
> https://git-scm.com/docs/gitattributes#_eol
>
> claims that:
>
> ---
> Set to string value "crlf"
>
>     This setting converts the file’s line endings in the working directory to CRLF when the file is checked out.
> Set to string value "lf"
>
>     This setting uses the same line endings in the working directory as in the index when the file is checked out.
> ---
>
> I don't think this explanation is sufficient for files stored on Windows machines.
> A file might be stored with CRLF in the index.
> What happens then?

  >This setting uses the same line endings in the working directory
  >as in the index when the file is checked out.
  If you have a file commited with CRLF into the index, it will have CRLF in the working tree.
  If the file had been commited with LF into the index, it will have LF in the working tree.
  That is what the documentation tries to say, I think.

However, the above discussion is only valid, when the "text" attribute is defined.
Otherwise the eol attribute has no effect.
So you need something like
* text=auto
and/or
*.txt text
in your .gitattributes file


> And what happens when a file is added to the index with `core.autocrlf=input` on Windows machines?
Do you have a .gitattributes file ? Or not ?
Is the file a new one, or does it exist ?

In any way,
git ls-files --eol
may help to find out, what is in the index and what is in the workingtree.

^ permalink raw reply

* Bug: magic-less pathspecs that start with ":" not processed as expected.
From: Joanna Wang @ 2023-11-06 15:54 UTC (permalink / raw)
  To: git

What did you do before the bug happened? (Steps to reproduce your issue)

What did you expect to happen? (Expected behavior)
`git stash push -- :file` where `:` is part of the filename, should search
for files named ":file"

What happened instead? (Actual behavior)
The match string used to find files is "file" without the ":".

What's different between what you expected and what actually happened?
file named ":file" is not found.

Anything else you want to add:
I believe this is due to parse_short_magic() not handling the case where
":" is part of the file name rather than a prefix for pathspec magic.

I could not find any documentation that says ":" is a reserved
character to indicate pathspec magic symbols MUST follow or that
":file" is not a valid file name.

[System Info]
git version 2.42.0
cpu: x86_64
no commit associated with this build
shell-path: /bin/sh

[Enabled Hooks]
not run from a git repository - no hooks to show

^ permalink raw reply

* Re: [PATCH v5 0/5] merge-ort: implement support for packing objects together
From: Johannes Schindelin @ 2023-11-06 15:46 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Taylor Blau, git, Elijah Newren, Eric W. Biederman, Jeff King,
	Patrick Steinhardt
In-Reply-To: <xmqqo7go7w63.fsf@gitster.g>

Hi,

On Mon, 23 Oct 2023, Junio C Hamano wrote:

> Taylor Blau <me@ttaylorr.com> writes:
>
> > But I think that this approach ended up being less heavy-weight than I
> > had originally imagined, so I think that this version is a worthwhile
> > improvement over v4.
>
> ;-).
>
> This version is a good place to stop, a bit short of going full OO.
> Nicely done.

I wonder whether a more generic approach would be more desirable, an
approach that would work for `git replay`, too, for example (where
streaming objects does not work because they need to be made available
immediately because subsequent `merge_incore_nonrecursive()` might expect
the created objects to be present)?

What I have in mind is more along Elijah's suggestion at the Contributor
Summit to use the `tmp_objdir*()` machinery. But instead of discarding the
temporary object database, the contained objects would be repacked and the
`.pack`, (maybe `.rev`) and the `.idx` file would then be moved (in that
order) before discarding the temporary object database.

This would probably need to be implemented as a new
`tmp_objdir_pack_and_migrate()` function that basically spawns
`pack-objects` and feeds it the list of generated objects, writing
directly into the non-temporary object directory, then discarding the
`tmp_objdir`.

This approach would not only more easily extend to other commands, but it
would also be less intrusive (a `tmp_objdir_*()` call to begin the
transaction before the objects are written, another one to commit the
transaction after the objects are written), and it would potentially allow
for more efficient packs to be generated.

Ciao,
Johannes

^ permalink raw reply

* Regression: git send-email Message-Id: numbering doesn't start at 1 any more
From: Uwe Kleine-König @ 2023-11-06 15:32 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Douglas Anderson, entwicklung

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

Hello,

Since commit 3ece9bf0f9e24909b090cf348d89e8920bd4f82f I experience that
the generated Message-Ids don't start at ....-1-... any more. I have:

$ git send-email w/*
...
Subject: [PATCH 0/5] watchdog: Drop platform_driver_probe() and convert to platform remove callback returning void (part II)
Date: Mon,  6 Nov 2023 16:10:04 +0100
Message-ID: <20231106151003.3844134-7-u.kleine-koenig@pengutronix.de>
...

So the cover letter is sent with Message-Id: ...-7-...

Before above mentioned commit I had:

...
Message-ID: <20231106151003.3844134-1-u.kleine-koenig@pengutronix.de>
...

Similar to my earlier regression report this also only happens in the
presence of a sendemail-validate hook. Passing --no-validate works
around this issue.

While this isn't an issue for git itself, it breaks one of my scripts
that knows how to determine the number of patches in a series from the
last Message-Id:.

The following patch works for me:

diff --git a/git-send-email.perl b/git-send-email.perl
index 9e21b0b3f43a..095a3d9dd720 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -799,6 +799,7 @@ sub is_format_patch_arg {
 
 $time = time - scalar $#files;
 
+my ($message_id_stamp, $message_id_serial);
 if ($validate) {
 	# FIFOs can only be read once, exclude them from validation.
 	my @real_files = ();
@@ -821,6 +822,7 @@ sub is_format_patch_arg {
 	}
 	delete $ENV{GIT_SENDEMAIL_FILE_COUNTER};
 	delete $ENV{GIT_SENDEMAIL_FILE_TOTAL};
+	$message_id_serial = 0;
 }
 
 @files = handle_backup_files(@files);
@@ -1181,7 +1183,6 @@ sub validate_address_list {
 
 # We'll setup a template for the message id, using the "from" address:
 
-my ($message_id_stamp, $message_id_serial);
 sub make_message_id {
 	my $uniq;
 	if (!defined $message_id_stamp) {

But I guess this could be done prettier by someone who is fluent in
Perl.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

^ permalink raw reply related

* Explanation of `eol` attribute doesn't seem correct for Windows machines
From: Hans Meiser @ 2023-11-06 15:11 UTC (permalink / raw)
  To: git@vger.kernel.org

https://git-scm.com/docs/gitattributes#_eol

claims that:

---
Set to string value "crlf"

    This setting converts the file’s line endings in the working directory to CRLF when the file is checked out.
Set to string value "lf"

    This setting uses the same line endings in the working directory as in the index when the file is checked out.
---

I don't think this explanation is sufficient for files stored on Windows machines. A file might be stored with CRLF in the index. What happens then? And what happens when a file is added to the index with `core.autocrlf=input` on Windows machines?

^ permalink raw reply

* Git Rename Detection Bug
From: Jeremy Pridmore @ 2023-11-06 12:00 UTC (permalink / raw)
  To: git@vger.kernel.org; +Cc: Paul Baumgartner

Thank you for filling out a Git bug report!
Please answer the following questions to help us understand your issue.

What did you do before the bug happened? (Steps to reproduce your issue)
I have two GIT repositories (A and B). Both migrated from the same TFS server using git-tfs tool. I migrated code into A and made lots of changes, including moving 50,000+ files from folder "/Landscape" to "/Landscape/src".  B contains the same code but with various other changes made since my original migration from TFS to A.  All the files in B are still in the "/Landscape" folder.  I recently needed to merge my changes from A to B, so I added A as a remote to B and then performed a number of cherry-picks from A to B, but got stuck when trying to cherry-pick the commit containing the results of moving all files into "/Landscape/src".

What did you expect to happen? (Expected behavior)
I expected the git rename detection to match all files in A "/Landscape" to files in B "/Landscape/src".

What happened instead? (Actual behavior)
Although many files were matched successfully, git mismatched over two dozen similarly named files, e.g.

Incorrect path match: Landscape/Services/uiServices/Complaints/Interfaces/IAccountsIntegration.vb -> Landscape/src/Complaints/Rdt.Complaints.UI/Interfaces/IAccountsIntegration.vb
Incorrect path match: Landscape/Services/uiServices/Complaints/Interfaces/IDocumentIntegration.vb -> Landscape/src/Complaints/Rdt.Complaints.UI/Interfaces/IDocumentIntegration.vb
Incorrect path match: Landscape/Deployment/PowershellScripts/pre-req/Rdt.BatchProcessingService.Setup/pre-req.ps1 -> Landscape/src/Deployment/PowershellScripts/pre-req/Landscape.Net/pre-req.ps1
Incorrect path match: Landscape/Deployment/PowershellScripts/pre-req/Workflow/pre-req.ps1 -> Landscape/src/Deployment/PowershellScripts/pre-req/Rdt.BatchProcessingService.Setup/pre-req.ps1
Incorrect name match: Landscape/Documentation/Rdt.Documentation.UI/Properties/licenses.licx -> Landscape/src/Deployment/PowershellScripts/pre-req/Workflow/pre-req.ps1
Incorrect path match: Landscape/Documentation/uiDocumentation/licenses.licx -> Landscape/src/Documentation/Rdt.Documentation.UI/Properties/licenses.licx
Incorrect path match: Landscape/Import/uiImport/My Project/licenses.licx -> Landscape/src/Documentation/uiDocumentation/licenses.licx
Incorrect path match: Landscape/Main/uiMain.Workflow/My Project/licenses.licx -> Landscape/src/Import/uiImport/My Project/licenses.licx
Incorrect path match: Landscape/Main/uiMain/My Project/licenses.licx -> Landscape/src/Main/uiMain.Workflow/My Project/licenses.licx
Incorrect path match: Landscape/LandscapeApiService.Setup/Setup/UIContent/RDT_Logo.ico -> Landscape/src/Main/uiMain.Workflow/Resources/RDT_Logo.ico
Incorrect path match: Landscape/Policy/Rdt.Policy.UI.Templates/Properties/licenses.licx -> Landscape/src/Main/uiMain/My Project/licenses.licx
Incorrect path match: Landscape/Main/uiMain.Workflow/Resources/RDT_Logo.ico -> Landscape/src/Main/uiMain/Resources/RDT_Logo.ico
Incorrect path match: Landscape/Policy/Rdt.Policy.UI/Properties/licenses.licx -> Landscape/src/Policy/Rdt.Policy.UI.Templates/Properties/licenses.licx
Incorrect path match: Landscape/Rates/uiRates/My Project/licenses.licx -> Landscape/src/Policy/Rdt.Policy.UI/Properties/licenses.licx
Incorrect path match: Landscape/Rdt.Claim.UI/Properties/licenses.licx -> Landscape/src/Rates/uiRates/My Project/licenses.licx
Incorrect path match: Landscape/Rdt.Landscape.UI.Templates.Workflow/Properties/licenses.licx -> Landscape/src/Rdt.Claim.UI/Properties/licenses.licx
Incorrect path match: Landscape/Rdt.Landscape.UI.Templates/Properties/licenses.licx -> Landscape/src/Rdt.Landscape.UI.Templates.Workflow/Properties/licenses.licx
Incorrect path match: Landscape/Rdt.Landscape.UI.Workflow/Properties/licenses.licx -> Landscape/src/Rdt.Landscape.UI.Templates/Properties/licenses.licx
Incorrect path match: Landscape/Rdt.Landscape.UI/Properties/licenses.licx -> Landscape/src/Rdt.Landscape.UI.Workflow/Properties/licenses.licx
Incorrect path match: Landscape/StandardLetters/uiStandardLetters/My Project/licenses.licx -> Landscape/src/Rdt.Landscape.UI/Properties/licenses.licx
Incorrect path match: Landscape/Complaints/Rdt.Complaints.UI/Interfaces/IDocumentIntegration.vb -> Landscape/src/Services/uiServices/Complaints/Interfaces/IDocumentIntegration.vb
Incorrect path match: Landscape/SystemEvents/uiSystemEvents/My Project/licenses.licx -> Landscape/src/StandardLetters/uiStandardLetters/My Project/licenses.licx
Incorrect path match: Landscape/Services/busServices/RDT_Logo.ico -> Landscape/src/Startup/uiStartup.Workflow/Resources/RDT_Logo.ico
Incorrect path match: Landscape/Startup/uiStartup.Workflow/Resources/RDT_Logo.ico -> Landscape/src/Startup/uiStartup/Resources/RDT_Logo.ico
Incorrect path match: Landscape/Startup/uiStartup/Resources/RDT_Logo.ico -> Landscape/src/Startup/uiStartup32/RDT_Logo.ico
Incorrect path match: Landscape/Startup/uiStartup/Resources/newrdlogogradiant48shad.ico -> Landscape/src/Startup/uiStartup32/newrdlogogradiant48shad.ico
Incorrect path match: Landscape/Templates/uiTemplates.Workflow/My Project/licenses.licx -> Landscape/src/SystemEvents/uiSystemEvents/My Project/licenses.licx
Incorrect path match: Landscape/Utils/Rdt.Utils.UI/Properties/licenses.licx -> Landscape/src/Templates/uiTemplates.Workflow/My Project/licenses.licx
Incorrect path match: Landscape/Utils/uiUtils/My Project/licenses.licx -> Landscape/src/Utils/Rdt.Utils.UI/Properties/licenses.licx
Incorrect name match: Landscape/WebServices/ServiceFabric/Policy/Rdt.Policy.Repository.Service.Fabric.Host/PackageRoot/Data/Swagger/Examples/POST_UKSTasks_Response.json -> Landscape/src/Utils/uiUtils/My Project/licenses.licx


What's different between what you expected and what actually happened?

As you can see, although the filenames (and content) are the same, there are a few files that have been incorrectly matched to files with teh same name, but in a different sub folder.  In some cases, it seems that the catalyst has been git thinking that a file from B has been deleted from A, when in fact it has not actually been deleted at all.
For example, the file Landscape/Deployment/PowershellScripts/pre-req/Landscape.Net/pre-req.ps1 has not been deleted in A or B, therefore git should not have attempted to rename Landscape/Deployment/PowershellScripts/pre-req/Rdt.BatchProcessingService.Setup/pre-req.ps1 to Landscape/Deployment/PowershellScripts/pre-req/Landscape.Net/pre-req.ps1, especially as it then attempts to rename Landscape/Deployment/PowershellScripts/pre-req/Workflow/pre-req.ps1 to Landscape/src/Deployment/PowershellScripts/pre-req/Rdt.BatchProcessingService.Setup/pre-req.ps1 and so on.

Git status contains, for example:
        deleted by them: Landscape/Deployment/PowershellScripts/pre-req/Landscape.Net/pre-req.ps1
        renamed:    Landscape/Deployment/PowershellScripts/pre-req/Rdt.BatchProcessingService.Setup/pre-req.ps1 -> Landscape/src/Deployment/PowershellScripts/pre-req/Landscape.Net/pre-req.ps1

The correct renames should have been:
git mv "Landscape/Complaints/Rdt.Complaints.UI/Interfaces/IAccountsIntegration.vb" "Landscape/src/Complaints/Rdt.Complaints.UI/Interfaces/IAccountsIntegration.vb"
git mv "Landscape/Complaints/Rdt.Complaints.UI/Interfaces/IDocumentIntegration.vb" "Landscape/src/Complaints/Rdt.Complaints.UI/Interfaces/IDocumentIntegration.vb"
git mv "Landscape/Deployment/PowershellScripts/pre-req/Rdt.BatchProcessingService.Setup/pre-req.ps1" "Landscape/src/Deployment/PowershellScripts/pre-req/Rdt.BatchProcessingService.Setup/pre-req.ps1"
git mv "Landscape/Deployment/PowershellScripts/pre-req/Workflow/pre-req.ps1" "Landscape/src/Deployment/PowershellScripts/pre-req/Workflow/pre-req.ps1"
git mv "Landscape/Documentation/Rdt.Documentation.UI/Properties/licenses.licx" "Landscape/src/Documentation/Rdt.Documentation.UI/Properties/licenses.licx"
git mv "Landscape/Documentation/uiDocumentation/licenses.licx" "Landscape/src/Documentation/uiDocumentation/licenses.licx"
git mv "Landscape/Import/uiImport/My Project/licenses.licx" "Landscape/src/Import/uiImport/My Project/licenses.licx"
git mv "Landscape/Main/uiMain.Workflow/My Project/licenses.licx" "Landscape/src/Main/uiMain.Workflow/My Project/licenses.licx"
git mv "Landscape/Main/uiMain.Workflow/Resources/RDT_Logo.ico" "Landscape/src/Main/uiMain.Workflow/Resources/RDT_Logo.ico"
git mv "Landscape/Main/uiMain/My Project/licenses.licx" "Landscape/src/Main/uiMain/My Project/licenses.licx"
git mv "Landscape/Main/uiMain/Resources/RDT_Logo.ico" "Landscape/src/Main/uiMain/Resources/RDT_Logo.ico"
git mv "Landscape/Policy/Rdt.Policy.UI.Templates/Properties/licenses.licx" "Landscape/src/Policy/Rdt.Policy.UI.Templates/Properties/licenses.licx"
git mv "Landscape/Policy/Rdt.Policy.UI/Properties/licenses.licx" "Landscape/src/Policy/Rdt.Policy.UI/Properties/licenses.licx"
git mv "Landscape/Rdt.Claim.UI/Properties/licenses.licx" "Landscape/src/Rdt.Claim.UI/Properties/licenses.licx"
git mv "Landscape/Rdt.Landscape.UI.Templates.Workflow/Properties/licenses.licx" "Landscape/src/Rdt.Landscape.UI.Templates.Workflow/Properties/licenses.licx"
git mv "Landscape/Rdt.Landscape.UI.Templates/Properties/licenses.licx" "Landscape/src/Rdt.Landscape.UI.Templates/Properties/licenses.licx"
git mv "Landscape/Rdt.Landscape.UI.Workflow/Properties/licenses.licx" "Landscape/src/Rdt.Landscape.UI.Workflow/Properties/licenses.licx"
git mv "Landscape/Rdt.Landscape.UI/Properties/licenses.licx" "Landscape/src/Rdt.Landscape.UI/Properties/licenses.licx"
git mv "Landscape/Services/busServices/RDT_Logo.ico" "Landscape/src/Services/busServices/RDT_Logo.ico"
git mv "Landscape/Services/uiServices/Complaints/Interfaces/IAccountsIntegration.vb" "Landscape/src/Services/uiServices/Complaints/Interfaces/IAccountsIntegration.vb"
git mv "Landscape/Services/uiServices/Complaints/Interfaces/IDocumentIntegration.vb" "Landscape/src/Services/uiServices/Complaints/Interfaces/IDocumentIntegration.vb"
git mv "Landscape/StandardLetters/uiStandardLetters/My Project/licenses.licx" "Landscape/src/StandardLetters/uiStandardLetters/My Project/licenses.licx"
git mv "Landscape/Startup/uiStartup.Workflow/Resources/RDT_Logo.ico" "Landscape/src/Startup/uiStartup.Workflow/Resources/RDT_Logo.ico"
git mv "Landscape/Startup/uiStartup/Resources/RDT_Logo.ico" "Landscape/src/Startup/uiStartup/Resources/RDT_Logo.ico"
git mv "Landscape/SystemEvents/uiSystemEvents/My Project/licenses.licx" "Landscape/src/SystemEvents/uiSystemEvents/My Project/licenses.licx"
git mv "Landscape/Templates/uiTemplates.Workflow/My Project/licenses.licx" "Landscape/src/Templates/uiTemplates.Workflow/My Project/licenses.licx"
git mv "Landscape/Utils/Rdt.Utils.UI/Properties/licenses.licx" "Landscape/src/Utils/Rdt.Utils.UI/Properties/licenses.licx"
git mv "Landscape/Utils/uiUtils/My Project/licenses.licx" "Landscape/src/Utils/uiUtils/My Project/licenses.licx"


Anything else you want to add:
I can't help but think that this is related to changes made by Palantir:
https://blog.palantir.com/optimizing-gits-merge-machinery-1-127ceb0ef2a1

I have tried to unstage these renames using "git restore --staged <file_name>" so I can then apply the correct "git mv" commands, but bizzarely, this then results in "git status" reporting a different, smaller set of mismatched names:

Incorrect name match: Landscape/Services/busServices/Service References/DataCollectorService/busServices.DataCollectorService.GetDataResponse.datasource -> Landscape/src/Deployment/PowershellScripts/pre-req/Landscape.Net/pre-req.ps1
Incorrect name match: Landscape/Services/busServices/Service References/GlobalGatewayService/busServices.GlobalGatewayService.OrderResponse.datasource -> Landscape/src/Deployment/PowershellScripts/pre-req/Rdt.BatchProcessingService.Setup/pre-req.ps1
Incorrect name match: Landscape/WebServices/ServiceFabric/Policy/Rdt.Policy.Repository.Service.Fabric.Host/PackageRoot/Data/Swagger/Examples/POST_UKSTasks_Response.json -> Landscape/src/Deployment/PowershellScripts/pre-req/Workflow/pre-req.ps1
Incorrect path match: Landscape/LandscapeApiService.Setup/Setup/UIContent/RDT_Logo.ico -> Landscape/src/Main/uiMain.Workflow/Resources/RDT_Logo.ico
Incorrect path match: Landscape/WebServices/WCFServices/Landscape WCF Service/wsWcfLandscapeServices/RDT_Logo.ico -> Landscape/src/Main/uiMain/Resources/RDT_Logo.ico
Incorrect path match: Landscape/WindowsServices/Rdt.BatchProcessingService/Rdt.BatchProcessingService.Integration/RDT_Logo.ico -> Landscape/src/Services/busServices/RDT_Logo.ico
Incorrect path match: Landscape/WindowsServices/Rdt.BatchProcessingService/Rdt.BatchProcessingService.Setup/UIContent/RDT_Logo.ico -> Landscape/src/Startup/uiStartup.Workflow/Resources/RDT_Logo.ico
Incorrect path match: Landscape/WindowsServices/Rdt.BatchProcessingService/Rdt.BatchProcessingService/RDT_Logo.ico -> Landscape/src/Startup/uiStartup/Resources/RDT_Logo.ico
Incorrect path match: Landscape/_Tests/Rdt.BatchProcessingService.Tests/RDT_Logo.ico -> Landscape/src/Startup/uiStartup32/RDT_Logo.ico
Incorrect path match: Landscape/Startup/uiStartup/Resources/newrdlogogradiant48shad.ico -> Landscape/src/Startup/uiStartup32/newrdlogogradiant48shad.ico
Incorrect path match: Landscape/uiStartup.Setup/Setup/UIContent/RDT_Logo.ico -> Landscape/src/WebServices/WCFServices/Landscape WCF Service/wsWcfLandscapeServices/RDT_Logo.ico
Incorrect path match: Landscape/wisWorkflow.Setup/Setup/UIContent/RDT_Logo.ico -> Landscape/src/WindowsServices/Rdt.BatchProcessingService/Rdt.BatchProcessingService.Integration/RDT_Logo.ico
Incorrect path match: Landscape/wsXmlLandscapeServices.Setup/Setup/Binary/RDT_Logo.ico -> Landscape/src/WindowsServices/Rdt.BatchProcessingService/Rdt.BatchProcessingService.Setup/UIContent/RDT_Logo.ico

Notice how "busServices.DataCollectorService.GetDataResponse.datasource" is now renamed to "Landscape.Net/pre-req.ps1", whereas the datasource file wasn't even listed previously?

Please help.  I've been on this for a couple of weeks now and I'm running out of ideas.

Please review the rest of the bug report below.
You can delete any lines you don't wish to share.


[System Info]
git version:
git version 2.42.0.windows.2
cpu: x86_64
built from commit: 2f819d1670fff9a1818f63b6722e9959405378e3
sizeof-long: 4
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon
uname: Windows 10.0 19044
compiler info: gnuc: 13.2
libc info: no libc information available
$SHELL (typically, interactive shell): <unset>


[Enabled Hooks]
<none>


Regards,

Jeremy Pridmore
Lead Solution Architect

________________________________

DISCLAIMER This email is confidential. It should only be read by those persons to whom it is addressed. RDT Ltd accept no liability for the consequences of any person acting, or refraining from acting, on any information contained within this e-mail or any attached documents prior to the receipt by those persons of subsequent written confirmation of that information. If you think this e-mail may not be intended for you, do not use, pass on or copy the transmission in any way. While all reasonable precautions are taken to minimise the risk of transmitting software viruses we advise you to carry out your own virus checks on any attachment to this message. We cannot accept liability for any loss or damage caused by software viruses.

^ permalink raw reply

* Re: [PATCH] revision: Don't queue uninteresting commits
From: Øystein Walle @ 2023-11-06 11:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, derrickstolee
In-Reply-To: <xmqqpm1lnme4.fsf@gitster.g>

Hi, Junio, and sorry for the late response.

On Wed, 11 Oct 2023 at 18:40, Junio C Hamano <gitster@pobox.com> wrote:

> What is not described here is what benefit we are expecting to gain
> by making this change.  Is anything leaking?  Are we showing wrong
> output?  Is the effect something we can demonstrate, and more
> importantly we can protect from future breakages, with a test or
> two?

As far as I know there is no significant benefit to this change. The
only one I can think of is a case such as this:

    git rev-list some-rev ^a ^very ^large ^amount ^of ^negative ^revs ^here

but even then I would assume the work done by the algorithm in total is
so large that the work saved by this change is insignificant.

I was just a bit happy after grokking a piece of this code and let the
excitement get the best of me :-) I suggest we just drop it.

Øsse

^ permalink raw reply

* Re: [PATCH v3 2/2] docs: rewrite rebase --(no-)autosquash description
From: Phillip Wood @ 2023-11-06 11:07 UTC (permalink / raw)
  To: Andy Koppe, git; +Cc: gitster, newren
In-Reply-To: <20231105000808.10171-2-andy.koppe@gmail.com>

Hi Andy

On 05/11/2023 00:08, Andy Koppe wrote:
> Rewrite the description of the rebase --(no-)autosquash options to try
> to make it a bit clearer. Don't use "the '...'" to refer to part of a
> commit message,

Thanks for doing that, it is a definite improvement, I'm a bit concerned 
about the loss of detail in other areas though - see below.

> mention how --interactive can be used to review the
> todo list, and add a bit more detail on commit --squash/amend.
> 
> Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
> ---
>   Documentation/git-rebase.txt | 32 ++++++++++++++++++--------------
>   1 file changed, 18 insertions(+), 14 deletions(-)
> 
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 102ff91493..594158fcbc 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -589,21 +589,25 @@ See also INCOMPATIBLE OPTIONS below.
>   
>   --autosquash::
>   --no-autosquash::
> -	When the commit log message begins with "squash! ..." or "fixup! ..."
> -	or "amend! ...", and there is already a commit in the todo list that
> -	matches the same `...`, automatically modify the todo list of
> -	`rebase`, so that the commit marked for squashing comes right after
> -	the commit to be modified, and change the action of the moved commit
> -	from `pick` to `squash` or `fixup` or `fixup -C` respectively. A commit
> -	matches the `...` if the commit subject matches, or if the `...` refers
> -	to the commit's hash. As a fall-back, partial matches of the commit
> -	subject work, too. The recommended way to create fixup/amend/squash
> -	commits is by using the `--fixup`, `--fixup=amend:` or `--fixup=reword:`
> -	and `--squash` options respectively of linkgit:git-commit[1].
> +	Automatically squash commits with specially formatted messages into
> +	previous commits.  If a commit message starts with "squash! ",
> +	"fixup! " or "amend! ", the remainder of the subject line is taken
> +	as a commit specifier,

I like this change as I think saying "remainder of the subject line" is 
clearer than using `...`

> which matches a previous commit if it matches
> +	the start of the subject line or the hash of that commit.

This simplification glosses over some details, in particular it is no 
longer clear we prefer an exact subject match to a prefix match so that

	pick 123 foo
	pick 456 foo bar
	pick 789 fixup! foo

will be re-arranged to
	
	pick 123 foo
	fixup 789 fixup! foo
	pick 456 foo bar

and not

	pick 123 foo
	pick 456 foo bar
	fixup 789 fixup! foo

>   +
> -If the `--autosquash` option is enabled by default using the
> -configuration variable `rebase.autoSquash`, this option can be
> -used to override and disable this setting.
> +In the rebase todo list, commits marked for squashing are moved right after
> +the commits they modify, and their action is changed from `pick` to `squash`,
> +`fixup` or `fixup -C`, depending on the squash marker.

Here we have lost the documentation that told users which marker 
corresponds to which action which is unfortunate.

Best Wishes

Phillip

>  The `--interactive`
> +option can be used to review and edit the todo list before proceeding.
> ++
> +The recommended way to create commits with squash markers is by using the
> +`--squash`, `--fixup`, `--fixup=amend:` or `--fixup=reword:` options of
> +linkgit:git-commit[1], which take the target commit as an argument and
> +automatically fill in the subject line of the new commit from that.
> ++
> +The configuration variable `rebase.autoSquash` can be used to enable
> +`--autosquash` by default.  See the CONFIGURATION section below for details.
> +The `--no-autosquash` option overrides that setting.
>   +
>   See also INCOMPATIBLE OPTIONS below.
>   

^ permalink raw reply

* Re: [PATCH v3 1/2] rebase: support non-interactive autosquash
From: Phillip Wood @ 2023-11-06 11:06 UTC (permalink / raw)
  To: Andy Koppe, git; +Cc: gitster, newren
In-Reply-To: <20231105000808.10171-1-andy.koppe@gmail.com>

Hi Andy

On 05/11/2023 00:08, Andy Koppe wrote:
> So far, the rebase --autosquash option and rebase.autoSquash=true
> config setting are quietly ignored when used without --interactive,

Thanks for working on this, I agree that "--autosquash" being ignored 
without "--interactive" is something that we should address. I think 
there are several possible solutions

1 - make "--autosquash" imply "--interactive". This has the advantage
     that the user gets to check the commits are going to be reordered as
     they expect when they edit the todo list. It is hard to see how to
     accommodate the config setting though - I don't think we want
     "rebase.autosquash=true" to imply "--interactive".

2 - make "--autosquash" without "--interactive" an error. This would
     prevent the user being surprised that their commits are not squashed
     by a non-interactive rebase. Users who have set
     "rebase.autosquash=true" would have to pass "--no-autosquash" to
     perform any form of non-interactive rebase. This is similar to the
     current behavior where the user has to pass "--no-autosquash" if
     they want to use the apply backend with "rebase.autosquash=true".

3 - make "--autosquash" rearrange and squash commits without
     "--interactive". This is convenient but there is a risk in that the
     user does not get a chance to check the todo list before the commits
     are reordered and squashed. I think that risk is fairly small with
     an explicit "--autosquash" on the commandline. This is the approach
     taken by this patch. I do have some concerns about extending the
     config setting to non-interactive rebases though. If the user has
     commits that look like

	fixup! foo (HEAD)
	foo bar
	foo

     and runs "git -c rebase.autosquash=non-interactive rebase HEAD~2"
     then we'll silently squash the fixup into the wrong commit due to a
     prefix subject match.

> except that they prevent fast-forward and that they trigger conflicts
> with --apply and relatives, which is less than helpful particularly for
> the config setting.

The behavior to make the config setting incompatible with the apply 
backend was implemented to avoid users being surprised that their 
commits are not squashed by that backend even when they have set 
"rebase.autosquash=true"[1]. I think one could consider "--autosquash" 
being silently ignored without "--interactive" to be an oversight in 
796abac7e1 (rebase: add coverage of other incompatible options, 
2023-01-25) that introduced that change.

[1] 
https://lore.kernel.org/git/pull.1466.v5.git.1674619434.gitgitgadget@gmail.com/

> Since the "merge" backend used for interactive rebase also is the
> default for non-interactive rebase, there doesn't appear to be a
> reason not to do --autosquash without --interactive, so support that.

I think making "--autosquash" on the commandline work for 
non-interactive rebases is reasonable but I would be open to the 
argument that it would be better to make it an error and require 
"--interactive" to allow the user to check that the commits are going to 
be reordered as they expect.

> Turn rebase.autoSquash into a comma-separated list of flags, with
> "interactive" or "i" enabling auto-squashing with --interactive, and
> "no-interactive" or "no-i" enabling it without. Make boolean true mean
> "interactive" for backward compatibility.

Please, please, please don't introduce abbreviated config settings, it 
just makes the interface more complicated. The user only has to set this 
once so I think the short names just add confusion. I also think 
"non-interactive" would be a better name for the config setting 
corresponding to non-interactive rebases. Does this mean the user can 
request that commits are only rearranged when the do not pass 
"--interactive"?

As I said above I do have some concerns that the 
"rebase.autosquash=non-interactive" setting will catch people out. 
Having said that ignoring "rebase.autosquash=true" without 
"--interactive" as we do now is inconsistent with the behavior of 
"rebase.autosquash=true" with "--apply". One possibility would be to 
introduce "rebase.autosquash=interactive" which would not cause an error 
with "--apply" and always require an explicit "--autosquash" on the 
commandline to squash fixups without "--interactive"

> Don't prevent fast-forwards or report conflicts with --apply options
> when auto-squashing is not active.

I think this change deserves to be in a separate commit (which probably 
means separating out the config changes into that commit) as it is not 
directly related to fixing "--autosquash" without "--interactive" on the 
commandline. It would revert 796abac7e1 (rebase: add coverage of other 
incompatible options, 2023-01-25) and part of eddfcd8ece (rebase: 
provide better error message for apply options vs. merge config, 
2023-01-25) and so the commit message would need to explain why that is 
desirable (and why it is desirable to revert only the changes related to 
"rebase.autosquash" and not those related to "rebase.updateRefs")

> Change the git-rebase and config/rebase documentation accordingly, and
> extend t3415-rebase-autosquash.sh to test the new rebase.autosquash
> values and combinations with and without --interactive.

Thanks for adding tests for the new behavior.

In summary I like "--autosquash" working without "--interactive" but I'm 
unsure about the config changes.

Best Wishes

Phillip

> Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
> ---
>   Documentation/config/rebase.txt        | 11 +++-
>   Documentation/git-rebase.txt           |  2 +-
>   builtin/rebase.c                       | 66 +++++++++++++++-----
>   t/t3415-rebase-autosquash.sh           | 83 +++++++++++++++++++++-----
>   t/t3422-rebase-incompatible-options.sh |  2 +-
>   5 files changed, 132 insertions(+), 32 deletions(-)
> 
> diff --git a/Documentation/config/rebase.txt b/Documentation/config/rebase.txt
> index 9c248accec..68191e5673 100644
> --- a/Documentation/config/rebase.txt
> +++ b/Documentation/config/rebase.txt
> @@ -9,7 +9,16 @@ rebase.stat::
>   	rebase. False by default.
>   
>   rebase.autoSquash::
> -	If set to true enable `--autosquash` option by default.
> +	A comma-separated list of flags for when to enable auto-squashing.
> +	Specifying `interactive` or `i` enables auto-squashing for rebasing with
> +	`--interactive`, whereas `no-interactive` or `no-i` enables it for
> +	rebasing without that option. For example, setting this to `i,no-i`
> +	enables auto-squashing for both types. Setting it to true is equivalent
> +	to setting it to `interactive`.
> +
> +	The `--autosquash` and `--no-autosquash` options of
> +	linkgit:git-rebase[1] override the setting here.
> +	Auto-squashing is disabled by default.
>   
>   rebase.autoStash::
>   	When set to true, automatically create a temporary stash entry
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index e7b39ad244..102ff91493 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -592,7 +592,7 @@ See also INCOMPATIBLE OPTIONS below.
>   	When the commit log message begins with "squash! ..." or "fixup! ..."
>   	or "amend! ...", and there is already a commit in the todo list that
>   	matches the same `...`, automatically modify the todo list of
> -	`rebase -i`, so that the commit marked for squashing comes right after
> +	`rebase`, so that the commit marked for squashing comes right after
>   	the commit to be modified, and change the action of the moved commit
>   	from `pick` to `squash` or `fixup` or `fixup -C` respectively. A commit
>   	matches the `...` if the commit subject matches, or if the `...` refers
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 261a9a61fc..26c3e5dcb4 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -131,7 +131,10 @@ struct rebase_options {
>   	int reapply_cherry_picks;
>   	int fork_point;
>   	int update_refs;
> -	int config_autosquash;
> +	enum {
> +		AUTOSQUASH_INTERACTIVE = 1 << 0,
> +		AUTOSQUASH_NO_INTERACTIVE = 1 << 1,
> +	} config_autosquash;
>   	int config_rebase_merges;
>   	int config_update_refs;
>   };
> @@ -149,7 +152,6 @@ struct rebase_options {
>   		.reapply_cherry_picks = -1,             \
>   		.allow_empty_message = 1,               \
>   		.autosquash = -1,                       \
> -		.config_autosquash = -1,                \
>   		.rebase_merges = -1,                    \
>   		.config_rebase_merges = -1,             \
>   		.update_refs = -1,                      \
> @@ -711,10 +713,8 @@ static int run_specific_rebase(struct rebase_options *opts)
>   	if (opts->type == REBASE_MERGE) {
>   		/* Run sequencer-based rebase */
>   		setenv("GIT_CHERRY_PICK_HELP", resolvemsg, 1);
> -		if (!(opts->flags & REBASE_INTERACTIVE_EXPLICIT)) {
> +		if (!(opts->flags & REBASE_INTERACTIVE_EXPLICIT))
>   			setenv("GIT_SEQUENCE_EDITOR", ":", 1);
> -			opts->autosquash = 0;
> -		}
>   		if (opts->gpg_sign_opt) {
>   			/* remove the leading "-S" */
>   			char *tmp = xstrdup(opts->gpg_sign_opt + 2);
> @@ -748,6 +748,30 @@ static int run_specific_rebase(struct rebase_options *opts)
>   	return status ? -1 : 0;
>   }
>   
> +static void parse_rebase_autosquash_value(struct rebase_options *opts,
> +					  const char *var, const char *value)
> +{
> +	struct string_list tokens = STRING_LIST_INIT_NODUP;
> +	char *buf = xstrdup(value);
> +
> +	opts->config_autosquash = 0;
> +	string_list_split_in_place(&tokens, buf, ",", -1);
> +
> +	for (int i = 0; i < tokens.nr; i++) {
> +		const char *s = tokens.items[i].string;
> +
> +		if (!strcmp(s, "i") || !strcmp(s, "interactive"))
> +			opts->config_autosquash |= AUTOSQUASH_INTERACTIVE;
> +		else if (!strcmp(s, "no-i") || !strcmp(s, "no-interactive"))
> +			opts->config_autosquash |= AUTOSQUASH_NO_INTERACTIVE;
> +		else
> +			die(_("invalid value for '%s': '%s'"), var, s);
> +	}
> +
> +	string_list_clear(&tokens, 0);
> +	free(buf);
> +}
> +
>   static void parse_rebase_merges_value(struct rebase_options *options, const char *value)
>   {
>   	if (!strcmp("no-rebase-cousins", value))
> @@ -772,8 +796,14 @@ static int rebase_config(const char *var, const char *value,
>   	}
>   
>   	if (!strcmp(var, "rebase.autosquash")) {
> -		opts->config_autosquash = git_config_bool(var, value);
> -		return 0;
> +		int b = git_parse_maybe_bool(value);
> +
> +		if (b < 0)
> +			parse_rebase_autosquash_value(opts, var, value);
> +		else if (b)
> +			opts->config_autosquash = AUTOSQUASH_INTERACTIVE;
> +		else
> +			opts->config_autosquash = 0;
>   	}
>   
>   	if (!strcmp(var, "commit.gpgsign")) {
> @@ -1402,13 +1432,23 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>   		    state_dir_base, cmd_live_rebase, buf.buf);
>   	}
>   
> +	if (options.autosquash == 1) {
> +		imply_merge(&options, "--autosquash");
> +	} else if (options.autosquash == -1) {
> +		int conf = options.config_autosquash;
> +		options.autosquash =
> +			(options.flags & REBASE_INTERACTIVE_EXPLICIT)
> +			? !!(conf & AUTOSQUASH_INTERACTIVE)
> +			: !!(conf & AUTOSQUASH_NO_INTERACTIVE);
> +	}
> +
>   	if ((options.flags & REBASE_INTERACTIVE_EXPLICIT) ||
>   	    (options.action != ACTION_NONE) ||
>   	    (options.exec.nr > 0) ||
> -	    (options.autosquash == -1 && options.config_autosquash == 1) ||
> -	    options.autosquash == 1) {
> +	    options.autosquash) {
>   		allow_preemptive_ff = 0;
>   	}
> +
>   	if (options.committer_date_is_author_date || options.ignore_date)
>   		options.flags |= REBASE_FORCE;
>   
> @@ -1508,7 +1548,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>   			if (is_merge(&options))
>   				die(_("apply options and merge options "
>   					  "cannot be used together"));
> -			else if (options.autosquash == -1 && options.config_autosquash == 1)
> +			else if (options.autosquash && options.config_autosquash)
>   				die(_("apply options are incompatible with rebase.autoSquash.  Consider adding --no-autosquash"));
>   			else if (options.rebase_merges == -1 && options.config_rebase_merges == 1)
>   				die(_("apply options are incompatible with rebase.rebaseMerges.  Consider adding --no-rebase-merges"));
> @@ -1529,11 +1569,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>   	options.rebase_merges = (options.rebase_merges >= 0) ? options.rebase_merges :
>   				((options.config_rebase_merges >= 0) ? options.config_rebase_merges : 0);
>   
> -	if (options.autosquash == 1)
> -		imply_merge(&options, "--autosquash");
> -	options.autosquash = (options.autosquash >= 0) ? options.autosquash :
> -			     ((options.config_autosquash >= 0) ? options.config_autosquash : 0);
> -
>   	if (options.type == REBASE_UNSPECIFIED) {
>   		if (!strcmp(options.default_backend, "merge"))
>   			options.type = REBASE_MERGE;
> @@ -1858,3 +1893,4 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>   	free(keep_base_onto_name);
>   	return !!ret;
>   }
> +
> diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
> index a364530d76..1386eb6394 100755
> --- a/t/t3415-rebase-autosquash.sh
> +++ b/t/t3415-rebase-autosquash.sh
> @@ -43,7 +43,7 @@ test_auto_fixup () {
>   
>   	git tag $1 &&
>   	test_tick &&
> -	git rebase $2 -i HEAD^^^ &&
> +	git rebase $2 HEAD^^^ &&
>   	git log --oneline >actual &&
>   	if test -n "$no_squash"
>   	then
> @@ -61,15 +61,43 @@ test_auto_fixup () {
>   }
>   
>   test_expect_success 'auto fixup (option)' '
> -	test_auto_fixup final-fixup-option --autosquash
> +	test_auto_fixup fixup-option --autosquash &&
> +	test_auto_fixup fixup-option-i "-i --autosquash"
>   '
>   
> -test_expect_success 'auto fixup (config)' '
> -	git config rebase.autosquash true &&
> -	test_auto_fixup final-fixup-config-true &&
> -	test_auto_fixup ! fixup-config-true-no --no-autosquash &&
> +test_expect_success 'auto fixup (config false)' '
>   	git config rebase.autosquash false &&
> -	test_auto_fixup ! final-fixup-config-false
> +	test_auto_fixup ! fixup-config-false &&
> +	test_auto_fixup ! fixup-config-false-i -i
> +'
> +
> +test_expect_success 'auto fixup (config true)' '
> +	git config rebase.autosquash true &&
> +	test_auto_fixup ! fixup-config-true &&
> +	test_auto_fixup fixup-config-true-i -i &&
> +	test_auto_fixup ! fixup-config-true-i-no "-i --no-autosquash"
> +'
> +
> +test_expect_success 'auto fixup (config interactive)' '
> +	git config rebase.autosquash interactive &&
> +	test_auto_fixup ! fixup-config-interactive &&
> +	test_auto_fixup fixup-config-interactive-i -i &&
> +	test_auto_fixup ! fixup-config-interactive-i-no "-i --no-autosquash"
> +'
> +
> +test_expect_success 'auto fixup (config no-interactive)' '
> +	git config rebase.autosquash no-interactive &&
> +	test_auto_fixup fixup-config-no-interactive &&
> +	test_auto_fixup ! fixup-config-no-interactive-i -i &&
> +	test_auto_fixup ! fixup-config-no-interactive-no "--no-autosquash"
> +'
> +
> +test_expect_success 'auto fixup (config always)' '
> +	git config rebase.autosquash i,no-i &&
> +	test_auto_fixup fixup-config-always &&
> +	test_auto_fixup fixup-config-always-i -i &&
> +	test_auto_fixup ! fixup-config-always-no --no-autosquash &&
> +	test_auto_fixup ! fixup-config-always-i-no "-i --no-autosquash"
>   '
>   
>   test_auto_squash () {
> @@ -87,7 +115,7 @@ test_auto_squash () {
>   	git commit -m "squash! first" -m "extra para for first" &&
>   	git tag $1 &&
>   	test_tick &&
> -	git rebase $2 -i HEAD^^^ &&
> +	git rebase $2 HEAD^^^ &&
>   	git log --oneline >actual &&
>   	if test -n "$no_squash"
>   	then
> @@ -105,15 +133,42 @@ test_auto_squash () {
>   }
>   
>   test_expect_success 'auto squash (option)' '
> -	test_auto_squash final-squash --autosquash
> +	test_auto_squash squash-option --autosquash &&
> +	test_auto_squash squash-option-i "-i --autosquash"
>   '
>   
> -test_expect_success 'auto squash (config)' '
> -	git config rebase.autosquash true &&
> -	test_auto_squash final-squash-config-true &&
> -	test_auto_squash ! squash-config-true-no --no-autosquash &&
> +test_expect_success 'auto squash (config false)' '
>   	git config rebase.autosquash false &&
> -	test_auto_squash ! final-squash-config-false
> +	test_auto_squash ! squash-config-false &&
> +	test_auto_squash ! squash-config-false-i -i
> +'
> +
> +test_expect_success 'auto squash (config true)' '
> +	git config rebase.autosquash true &&
> +	test_auto_squash ! squash-config-true &&
> +	test_auto_squash squash-config-true-i -i &&
> +	test_auto_squash ! squash-config-true-i-no "-i --no-autosquash"
> +'
> +
> +test_expect_success 'auto squash (config interactive)' '
> +	git config rebase.autosquash i &&
> +	test_auto_squash ! squash-config-interactive &&
> +	test_auto_squash squash-config-interactive-i -i &&
> +	test_auto_squash ! squash-config-interactive-i-no "-i --no-autosquash"
> +'
> +
> +test_expect_success 'auto squash (config no-interactive)' '
> +	git config rebase.autosquash no-i &&
> +	test_auto_squash squash-config-no-interactive &&
> +	test_auto_squash ! squash-config-no-interactive-i -i &&
> +	test_auto_squash ! squash-config-no-interactive-no "--no-autosquash"
> +'
> +test_expect_success 'auto squash (config always)' '
> +	git config rebase.autosquash interactive,no-interactive &&
> +	test_auto_squash squash-config-always &&
> +	test_auto_squash squash-config-always-i -i &&
> +	test_auto_squash ! squash-config-always-no --no-autosquash &&
> +	test_auto_squash ! squash-config-always-i-no "-i --no-autosquash"
>   '
>   
>   test_expect_success 'misspelled auto squash' '
> diff --git a/t/t3422-rebase-incompatible-options.sh b/t/t3422-rebase-incompatible-options.sh
> index 2eba00bdf5..e5119e7371 100755
> --- a/t/t3422-rebase-incompatible-options.sh
> +++ b/t/t3422-rebase-incompatible-options.sh
> @@ -102,7 +102,7 @@ test_rebase_am_only () {
>   
>   	test_expect_success "$opt incompatible with rebase.autosquash" "
>   		git checkout B^0 &&
> -		test_must_fail git -c rebase.autosquash=true rebase $opt A 2>err &&
> +		test_must_fail git -c rebase.autosquash=no-i rebase $opt A 2>err &&
>   		grep -e --no-autosquash err
>   	"
>   

^ permalink raw reply

* [PATCH 4/4] setup: fix leaking repository format
From: Patrick Steinhardt @ 2023-11-06 10:46 UTC (permalink / raw)
  To: git
In-Reply-To: <cover.1699267422.git.ps@pks.im>

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

While populating the `repository_format` structure may cause us to
allocate memory, we do not call `clear_repository_format()` in some
places and thus potentially leak memory. Fix this.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 setup.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/setup.c b/setup.c
index b9474b163a0..fc592dc6dd5 100644
--- a/setup.c
+++ b/setup.c
@@ -722,6 +722,7 @@ int upgrade_repository_format(int target_version)
 	ret = 1;
 
 out:
+	clear_repository_format(&repo_fmt);
 	strbuf_release(&repo_version);
 	strbuf_release(&err);
 	return ret;
@@ -2199,6 +2200,7 @@ int init_db(const char *git_dir, const char *real_git_dir,
 			       git_dir, len && git_dir[len-1] != '/' ? "/" : "");
 	}
 
+	clear_repository_format(&repo_fmt);
 	free(original_git_dir);
 	return 0;
 }
-- 
2.42.0


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

^ permalink raw reply related


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