* Re: Does extending `--empty` to git-cherry-pick make sense?
From: Brian Lyles @ 2024-01-05 3:20 UTC (permalink / raw)
To: Elijah Newren; +Cc: Taylor Blau, git
In-Reply-To: <CABPp-BGJfvBhO_zEX8nLoa8WNsjmwvtZ2qOjmYm9iPoZg4SwPw@mail.gmail.com>
On Thu, Jan 4, 2024 at 8:28 PM Elijah Newren <newren@gmail.com> wrote:
>
> I was indeed focused on simply getting the multiple rebase backends to
> have consistent behavior (we had like 4-5 backends at the time,
> right?) and just didn't consider cherry-pick at the time. Now that
> those are more consistent (though there's still work to be done on
> that end too), cherry-pick and rebase really ought to share a lot more
> between each other, from command line flags, to temporary control
> files written, to more shared code paths. Adding an --empty flag to
> cherry-pick as a step along the way makes sense.
I appreciate the insight from both of you.
It sounds like I'll be diving into some C code for the first time in
over a decade, then! I'll plan to take a crack at this soon.
-Brian Lyles
^ permalink raw reply
* Re: rebase invoking pre-commit
From: Elijah Newren @ 2024-01-05 4:59 UTC (permalink / raw)
To: Sean Allred; +Cc: Git List, Junio C Hamano
In-Reply-To: <m0sf3itvpy.fsf@epic96565.epic.com>
Hi,
On Sun, Dec 31, 2023 at 4:14 AM Sean Allred <allred.sean@gmail.com> wrote:
>
> Elijah Newren <newren@gmail.com> writes:
>
> > On Thu, Dec 21, 2023 at 12:59 PM Sean Allred <allred.sean@gmail.com> wrote:
> >> Is there a current reason why pre-commit shouldn't be invoked during
> >> rebase, or is this just waiting for a reviewable patch?
> >>
> >> This was brought up before at [1] in 2015, but that thread so old at
> >> this point that it seemed prudent to double-check before investing time
> >> in a developing and testing a patch.
> >>
> >> [1]: https://lore.kernel.org/git/1m55i3m.1fum4zo1fpnhncM%25lists@haller-berlin.de/
> >
> > I'm very opinionated here. I'm just one person, so definitely take
> > this with a grain of salt, but in my view...
> >
> > Personally, I think implementing any per-commit hook in rebase by
> > default is a mistake. It enforces a must-be-in-a-worktree-and-the-
> > worktree-must-be-updated-with-every-replayed-commit mindset, which I
> > find highly problematic[2], even if that's "what we always used to
> > do".
> >
> > [2] https://lore.kernel.org/git/20231124111044.3426007-1-christian.couder@gmail.com/
>
> I'm not hip with what most pre-commit hooks do, but I'll point out that
> a hook like pre-commit assuming there is a worktree is the fault of the
> hook implementation, not of the infrastructure that invokes the hook. I
> imagine most folks on this list are aware that a worktree is not needed
> to create a commit and update a branch to point at it.
Well, since git-commit requires a worktree (cmd_commit in cmd_struct
in git.c has NEED_WORK_TREE in its options), and "pre-commit" is
likely to be inferred to be a "git-commit" thing (in fact, my reading
of the current text of the pre-commit hook in the githooks(1) manpage
seems to suggest that this hook is tied exclusively to git-commit),
assuming a working tree for "pre-commit" doesn't seem like a very far
leap at all. In fact, I can't see why that assumption is broken given
our current documentation.
Perhaps we'd like to change the documentation and try to avoid such an
assumption...but even in cases where we've made explicit claims in the
past about various assumptions not being reliable, users have gone
ahead and made those assumptions anyway (Hyrum's Law), and then we
have sometimes been unable to make changes that broke those
assumptions.
So, I'm still concerned, especially if this is applied to every commit.
Granted, hooks are already pretty messed up. We invoke the
post-commit hook -- IF we're using the merge backend (and don't invoke
it if we're using the apply backend), meaning it's not clear to users
whether the post-commit hook will be invoked or not in a rebase.
(Especially since while the backend can be specified directly, it's
often just selected based on other options that are only implemented
by one of the two backends). I've put a lot of work into making the
rebase backends more consistent and would like them to be even more
so, but the history here is slightly troubling. The interactive
backend was "fixed" once, accidentally, and then the accident was
realized and was treated as a bug and reverted
(https://lore.kernel.org/git/67a711754efce038914e8ec15c5dec4a5983566d.1571135132.git.gitgitgadget@gmail.com/),
leaving us again inconsistent. At least we've documented the
inconsistency and the desire to change it
(https://lore.kernel.org/git/pull.749.v3.git.git.1586044818132.gitgitgadget@gmail.com/).
> FWIW, I would also find such a mindset to be highly problematic :-) I'll
> take a moment here to thank you, Christian, and everyone else in that
> effort for your interest in and work on git-replay; I've been trying to
> watch its activity on-list closely in the hopes that we can adopt it
> into our system once it's ready.
I'm glad others are interested in git-replay. Sadly, the work on
pushing it forward stopped about a year and a half ago. All work
since then was limited to pulling out the bits that were ready to be
upstreamed and cleaning those up.
> > Because of that, I would prefer to see this at most be a command line
> > flag. However, we've already got a command line flag that while not
> > identical, is essentially equivalent: "--exec $MY_SCRIPT" (it's not
> > the same because it's a post-commit check, but you get notification of
> > any problematic commits, and an immediate stop of the rebase for you
> > to fix up the problematic commit; fixing up the commit shouldn't be
> > problematic since you are, after all, already rebasing).
>
> Indeed, and an
>
> --exec 'git hook run pre-commit || git reset --soft HEAD~'
>
> would probably get you farther. I can certainly see an argument for
> this, but from the perspective of designing a system for other
> developers to use, such a rebase would have to be triggered
> automatically (perhaps on pre-push).
Well, there is a pre-push hook... ;-)
But yeah, it does defer the discovery of the issue for the developer,
which is kind of counter-productive.
> > I see Phillip already responded and suggested not running the
> > pre-commit hook with every commit, but only upon the first commit
> > after a "git rebase --continue". That seems far more reasonable to me
> > than running on every commit...though even that worries me in regards
> > to what assumptions that entails about what is present in the working
> > tree.
>
> It's worth noting the context here is to prevent developers from
> committing conflict markers, so this would actually be exactly
> sufficient.
Ah, that makes sense what you want to do. Totally fair.
> Invoking pre-commit at this time would also be consistent with the
> behaviors of prepare-commit-msg, commit-msg, and post-commit -- at least
> when I reword a commit during a rebase.
>
> However, post-commit is executed after each picked commit during a
> rebase
Only if using the merge backend, and even in that case I personally
don't think it should be executed; see the "Hooks" subsection of the
"Behavioral Differences" section of the git-rebase manpage.
> , so pre-commit there would also be consistent.
Or maybe consistently inconsistent (i.e. being consistent with the
inconsistency that exists with the post-commit hook between backends),
and diverging even further from the aspirational goal in the "Hooks"
documentation in the git-rebase manpage.
> > (For example, what about folks with large repositories, so large that
> > a branch switch or full checkout is extremely costly, and which could
> > benefit from resolving conflicts in a separate sparse-checkout
> > worktree, potentially much more sparse than their main checkout?
>
> As it happens, a single checkout of our source runs upwards of 2GB, so
> I'm exactly in the population you're describing :-) The main reason
> we're moving to Git from SVN is that an SVN checkout can take upwards of
> an hour for us today -- even with some real shenanigans to make them go
> faster. On the Git side, we've also looked into (though I don't recall
> if we had much success with) narrowing the sparsity patterns to just the
> conflicts for conflict resolution workflows -- particularly when moving
> feature code between separate trunks. So I guess I'm also glad we
> weren't too far off in left field on that one! (As I recall, one of the
> main challenges we faced there was ensuring there was enough stuff
> 'still around' so that both binary and project references could resolve
> and folks could use that information to help resolve conflicts.
> Hopefully git-replay can be smart enough to allow some customization on
> that front. We found some success with feeding the list of conflicted
> files into some arbitrary logic that spat out the sparsity pattern to
> use.)
An hour? For a mere 2 GB? I mean, I know 2 GB isn't exactly tiny and
it's a pain to deal with...but an hour? Do you have some directories
with an extraordinary number of files directly within them (i.e. not
multiple subdirectories deep, but a directory immediately containing a
huge number of files)? Or some kind of network filesystem? Or some
really slow hooks?
Anyway, I'm glad others are concerned with slow checkouts and branch
switches too. And yeah, the idea was just that we make an initial
sparse checkout limited to the conflicted files, but users can use the
sparse-checkout command to widen as needed. The bigger piece was
making sure the git code didn't defeat this by unnecessarily expanding
the index (currently with a sparse index, any conflicted files causes
the sparse-index to be expanded to a completely full index).
> > And what if people like that really fast rebase resolution (namely,
> > done in a separate very sparse checkout which also has the advantage
> > of not polluting your current working tree) so much that they use it
> > on smaller repositories as well? Can I not even experiment with this
> > idea because of the historical per-commit-at-least-as-full-as-main
> > -worktree-checkout assumptions we've baked into rebase?)
>
> I'd be interested in reading more about this baked-in assumption. Are
> these mostly laid out in replay-design-notes.txt[3]?
merge-recursive, our default merge backend for about 15 years (and
reused in rebase, cherry-pick, etc. too) only worked in a worktree and
muddied it as it went. The other merge algorithms also assumed a
worktree was present. When I wrote merge-ort and removed the
requirement for having a worktree, I naturally wanted to fix rebase
(and cherry-pick) to stop updating the working tree with every single
commit being rebased; it was such a needless waste of resources. But
that assumption was all over the code and hard to remove. And, I ran
into several items where it was unclear if "fixing" the code would be
deemed a backward compatibility break. Hooks were one of the issues.
It's been quite a while since I've looked at it, but yes, I suspect
some of the issues are documented (explicitly or implicitly) in the
replay-design-notes.txt file, and you may see some as well in the
"Behavioral differences" section of the git-rebase manpage.
But there may well be additional parts that aren't documented, since I
never went through the full effort to de-worktree-ify rebase, and only
got git-replay doing some basic things. So, at least part of the
issue is what other unknown assumptions are in the code which people
may have knowingly or unknowingly depended upon.
> > While at it, I should also mention that I'm not a fan of the broken
> > pre-rebase hook; its design ties us to doing a single branch at a
> > time. Maybe that hook is not quite as bad, though, since we already
> > broke that hook and no one seemed to care (in particular, when
> > --update-refs was implemented). But if no one seems to care about
> > broken hooks, I think the proper answer is to either get rid of the
> > hook or fix it.
>
> If I were to guess, this likely stems either from an inexact definition
> of the hook in documentation (ultimately resulting in incomplete tests)
> or folks incorrectly assuming what each hook should do based purely on
> its name.
No, neither is true. The definition is very exact; the problem is
that the definition is excessively rigid. It says, "The second
parameter is *the* branch being rebased" (emphasis added). There is
no facility in the hook for rebasing more than one branch or
reference, and if we attempt to pass additional parameters, we're
potentially breaking all the existing pre-rebase hooks (which won't be
prepared to handle the extra parameters and thus may fail to reject
the rebase for those other branches that are being rebased, and thus
defeat the point of the hook). Therefore, allowing rebase to operate
on multiple branches is a backward compatibility break...though one
that we overlooked/ignored when we added --update-refs.
And I want something more general than rebase's --update-refs; I want
the ability to replay multiple branches that aren't entirely contained
within each other, and perhaps only have a little common history (or
maybe even none) since the base point(s) we're replaying from.
> Which leads to an interesting point: pre-commit specifically states that
> it is invoked by git-commit -- not that it's invoked whenever a commit
> is created. So perhaps the correct thing to do here (if a hook is in
> fact needed) would be to define a new hook -- but I worry about doing
> that in the current state where there doesn't *seem* to be very rigid
> coordination of when client hooks are invoked in terms of plumbing
> rather than porcelain.
Yeah, as you're discovering, hooks are a mess. Made all the more
messy by the fact that higher level commands traditionally started as
scripts that invoked other lower-level commands (git-rebase.sh
literally called git-cherry-pick and git-commit, or git-format-patch
and git-am, or git-merge-recursive and git-commit, or ...), and now
that we've discovered inconsistencies between backends, and
performance problems, and whatnot, it is not clear what we should or
even can do. Also, even when we rewrote the scripts into C code,
we've often done so by reimplementing shell in C -- we just fork
various subprocesses repeatedly (yes, really), and then maybe come
along later and clean it up a little.
> > Anyway, as I mentioned, I'm quite opinionated here. To the point that
> > I deemed git-rebase in its current form to possibly be unfixable
> > (after first putting a lot of work into improving it over the past
> > years) and eventually introduced a new "git-replay" command which I
> > hope to make into a competent replacement for it. Given that I do
> > have another command to do my experiments, others on the list may
> > think it's fine to send rebase further down this route, but I was
> > hoping to avoid further drift so that there might be some way of
> > re-implementing rebase on top of my "git-replay" ideas/design.
>
> I appreciate your perspective; you've certainly thought a lot about this
> space -- and I definitely share your goal of consolidating
> implementations for obvious reasons.
>
> So I suppose that leaves me with four possible paths forward:
>
> 1. Pursue invoking pre-commit before each commit in `git rebase` (likely
> generic in the sequencer) to be consistent with post-commit.
>
> It sounds like this isn't a popular option, but I'm curious to folks'
> thoughts on the noted behavior of post-commit here.
I've already said quite a bit about this above, but as a quick
reminder: (1) I'm not sure it makes sense to make something consistent
with something that is inconsistent, and (2) we have a documented
desire to stop calling post-commit from rebase even in the situations
where it is currently called.
> 2. Pursue invoking pre-commit on `git rebase --continue` (likely on any
> --continue in the sequencer). This has the benefit of using existing
> configuration on developer machines to purportedly 'do the right
> thing' when its likely humans are touching code during conflict
> resolution. It's worth noting this isn't the only reason you might
> --continue, though, since the naive interpretation of this approach
> completely ignores sequencer commands like 'break', though it could
> probably just do what commit-msg does.
In the case of `break`, though, the commit was already transplanted,
so a `--continue` doesn't need to create one before moving on. Thus,
we could say that only when rebase --continue has an unfinished commit
that needs to be created (i.e. we have a conflicted commit to resolve)
that the `pre-commit` hook gets called.
The idea of having an 'unfinished' commit state is also useful because
interactive rebases make it easy to forget if you're in the middle of
a break/edit or trying to resolve a conflict elsewhere. That seems to
happen to me far too many times, and I really want `git commit
--amend` to throw an error (unless overridden with e.g. --force) when
you have an unfinished commit that you are working on by resolving
conflicts. In fact, this state is probably already recorded
somewhere, we just need to make better use of it...but I'm going off
on a tangent now.
> 3. Define and implement a new hook that is called whenever a new commit
> is about to be (or has been?) written. Such a hook could be
> specifically designed to discourage assuming there's a working copy,
> though we're kidding nobody by thinking it won't be used downstream
> with that assumption. At least we could be explicit about
> expectations, though.
>
> This is *probably* a lot more design work than this little paragraph
> lets on, but I've not personally watched the introduction of a new
> hook so I don't have context for what to expect.
Yeah, and at least as worded, this suggestion would run afoul of the
logic for the distinction between pre-commit and pre-merge-commit.
And it seems to have all the same problems as #1, unless we restrict
it to just when running "rebase --continue", in which case it's not
clear what it buys us over #2.
> 4. Trigger a rebase --exec in our pre-push. This is certainly the least
> work in git.git (i.e., no work at all), but it comes with the
> distinct disadvantage of playing whiplash with the developer's focus.
> During conflict resolution, they're thinking about conflicts. When
> you're ready to push, its likely that you're no longer thinking about
> conflicts.
Yeah, simple to implement but much less helpful.
> Does the behavior of post-commit here change any minds?
No...but although I'm slightly worried about solution #2 (as opposed
to very worried with #1), your usecase for pre-commit seems quite
reasonable. So, I can get behind #2, though it'd be nice if we could
update the description of the `pre-commit` hook (so that it's clear
that it's not just "git-commit"), and while we're at it perhaps try to
be more explicit about what can and can't be assumed by the hook.
^ permalink raw reply
* [PATCH] commit-graph: retain commit slab when closing NULL commit_graph
From: Jeff King @ 2024-01-05 5:41 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Scott Leggett, Taylor Blau
This fixes a regression introduced in ac6d45d11f (commit-graph: move
slab-clearing to close_commit_graph(), 2023-10-03), in which running:
git -c fetch.writeCommitGraph=true fetch --recurse-submodules
multiple times in a freshly cloned repository causes a segfault. What
happens in the second (and subsequent) runs is this:
1. We make a "struct commit" for any ref tips which we're storing
(even if we already have them, they still go into FETCH_HEAD).
Because the first run will have created a commit graph, we'll find
those commits in the graph.
The commit struct is therefore created with a NULL "maybe_tree"
entry, because we can load its oid from the graph later. But to do
that we need to remember that we got the commit from the graph,
which is recorded in a global commit_graph_data_slab object.
2. Because we're using --recurse-submodules, we'll try to fetch each
of the possible submodules. That implies creating a separate
"struct repository" in-process for each submodule, which will
require a later call to repo_clear().
The call to repo_clear() calls raw_object_store_clear(), which in
turn calls close_object_store(), which in turn calls
close_commit_graph(). And the latter frees the commit graph data
slab.
3. Later, when trying to write out a new commit graph, we'll ask for
their tree oid via get_commit_tree_oid(), which will see that the
object is parsed but with a NULL maybe_tree field. We'd then
usually pull it from the graph file, but because the slab was
cleared, we don't realize that we can do so! We end up returning
NULL and segfaulting.
(It seems questionable that we'd write a graph entry for such a
commit anyway, since we know we already have one. I didn't
double-check, but that may simply be another side effect of having
cleared the slab).
The bug is in step (2) above. We should not be clearing the slab when
cleaning up the submodule repository structs. Prior to ac6d45d11f, we
did not do so because it was done inside a helper function that returned
early when it saw NULL. So the behavior change from that commit is that
we'll now _always_ clear the slab via repo_clear(), even if the
repository being closed did not have a commit graph (and thus would have
a NULL commit_graph struct).
The most immediate fix is to add in a NULL check in close_commit_graph(),
making it a true noop when passed in an object_store with a NULL
commit_graph (it's OK to just return early, since the rest of its code
is already a noop when passed NULL). That restores the pre-ac6d45d11f
behavior. And that's what this patch does, along with a test that
exercises it (we already have a test that uses submodules along with
fetch.writeCommitGraph, but the bug only triggers when there is a
subsequent fetch and when that fetch uses --recurse-submodules).
So that fixes the regression in the least-risky way possible.
I do think there's some fragility here that we might want to follow up
on. We have a global commit_graph_data_slab that contains graph
positions, and our global commit structs depend on the that slab
remaining valid. But close_commit_graph() is just about closing _one_
object store's graph. So it's dangerous to call that function and clear
the slab without also throwing away any "struct commit" we might have
parsed that depends on it.
Which at first glance seems like a bug we could already trigger. In the
situation described here, there is no commit graph in the submodule
repository, so our commit graph is NULL (in fact, in our test script
there is no submodule repo at all, so we immediately return from
repo_init() and call repo_clear() only to free up memory). But what
would happen if there was one? Wouldn't we see a non-NULL commit_graph
entry, and then clear the global slab anyway?
The answer is "no", but for very bizarre reasons. Remember that
repo_clear() calls raw_object_store_clear(), which then calls
close_object_store() and thus close_commit_graph(). But before it does
so, raw_object_store_clear() does something else: it frees the commit
graph and sets it to NULL! So by this code path we'll _never_ see a
non-NULL commit_graph struct, and thus never clear the slab.
So it happens to work out. But it still seems questionable to me that we
would clear a global slab (which might still be in use) when closing the
commit graph. This clearing comes from 957ba814bf (commit-graph: when
closing the graph, also release the slab, 2021-09-08), and was fixing a
case where we really did need it to be closed (and in that case we
presumably call close_object_store() more directly).
So I suspect there may still be a bug waiting to happen there, as any
object loaded before the call to close_object_store() may be stranded
with a bogus maybe_tree entry (and thus looking at it after the call
might cause an error). But I'm not sure how to trigger it, nor what the
fix should look like (you probably would need to "unparse" any objects
pulled from the graph). And so this patch punts on that for now in favor
of fixing the recent regression in the most direct way, which should not
have any other fallouts.
Signed-off-by: Jeff King <peff@peff.net>
---
I prepared this on top of jk/commit-graph-leak-fixes, which is the
branch in v2.43.0 that introduced the regression.
commit-graph.c | 3 +++
t/t5510-fetch.sh | 3 ++-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/commit-graph.c b/commit-graph.c
index e4d09da090..f26503295a 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -727,6 +727,9 @@ struct bloom_filter_settings *get_bloom_filter_settings(struct repository *r)
void close_commit_graph(struct raw_object_store *o)
{
+ if (!o->commit_graph)
+ return;
+
clear_commit_graph_data_slab(&commit_graph_data_slab);
free_commit_graph(o->commit_graph);
o->commit_graph = NULL;
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 19c36b57f4..bcf524d549 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -802,7 +802,8 @@ test_expect_success 'fetch.writeCommitGraph with submodules' '
cd super-clone &&
rm -rf .git/objects/info &&
git -c fetch.writeCommitGraph=true fetch origin &&
- test_path_is_file .git/objects/info/commit-graphs/commit-graph-chain
+ test_path_is_file .git/objects/info/commit-graphs/commit-graph-chain &&
+ git -c fetch.writeCommitGraph=true fetch --recurse-submodules origin
)
'
--
2.43.0.514.g7147b80757
^ permalink raw reply related
* [PATCH] index-pack: spawn threads atomically
From: Jeff King @ 2024-01-05 8:50 UTC (permalink / raw)
To: git; +Cc: Taylor Blau
The t5309 script triggers a racy false positive with SANITIZE=leak on a
multi-core system. Running with "--stress --run=6" usually fails within
10 seconds or so for me, complaining with something like:
+ git index-pack --fix-thin --stdin
fatal: REF_DELTA at offset 46 already resolved (duplicate base 01d7713666f4de822776c7622c10f1b07de280dc?)
=================================================================
==3904583==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 32 byte(s) in 1 object(s) allocated from:
#0 0x7fa790d01986 in __interceptor_realloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:98
#1 0x7fa790add769 in __pthread_getattr_np nptl/pthread_getattr_np.c:180
#2 0x7fa790d117c5 in __sanitizer::GetThreadStackTopAndBottom(bool, unsigned long*, unsigned long*) ../../../../src/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cpp:150
#3 0x7fa790d11957 in __sanitizer::GetThreadStackAndTls(bool, unsigned long*, unsigned long*, unsigned long*, unsigned long*) ../../../../src/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cpp:598
#4 0x7fa790d03fe8 in __lsan::ThreadStart(unsigned int, unsigned long long, __sanitizer::ThreadType) ../../../../src/libsanitizer/lsan/lsan_posix.cpp:51
#5 0x7fa790d013fd in __lsan_thread_start_func ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:440
#6 0x7fa790adc3eb in start_thread nptl/pthread_create.c:444
#7 0x7fa790b5ca5b in clone3 ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
SUMMARY: LeakSanitizer: 32 byte(s) leaked in 1 allocation(s).
Aborted
What happens is this:
0. We construct a bogus pack with a duplicate object in it and trigger
index-pack.
1. We spawn a bunch of worker threads to resolve deltas (on my system
it is 16 threads).
2. One of the threads sees the duplicate object and bails by calling
exit(), taking down all of the threads. This is expected and is the
point of the test.
3. At the time exit() is called, we may still be spawning threads from
the main process via pthread_create(). LSan hooks thread creation
to update its book-keeping; it has to know where each thread's
stack is (so it can find entry points for reachable memory). So it
calls pthread_getattr_np() to get information about the new thread.
That may allocate memory that must be freed with a matching call to
pthread_attr_destroy(). Probably LSan does that immediately, but
if you're unlucky enough, the exit() will happen while it's between
those two calls, and the allocated pthread_attr_t appears as a
leak.
This isn't a real leak. It's not even in our code, but rather in the
LSan instrumentation code. So we could just ignore it. But the false
positive can cause people to waste time tracking it down.
It's possibly something that LSan could protect against (e.g., cover the
getattr/destroy pair with a mutex, and then in the final post-exit()
check for leaks try to take the same mutex). But I don't know enough
about LSan to say if that's a reasonable approach or not (or if my
analysis is even completely correct).
In the meantime, it's pretty easy to avoid the race by making creation
of the worker threads "atomic". That is, we'll spawn all of them before
letting any of them start to work. That's easy to do because we already
have a work_lock() mutex for handing out that work. If the main process
takes it, then all of the threads will immediately block until we've
finished spawning and released it.
This shouldn't make any practical difference for non-LSan runs. The
thread spawning is quick, and could happen before any worker thread gets
scheduled anyway.
Probably other spots that use threads are subject to the same issues.
But since we have to manually insert locking (and since this really is
kind of a hack), let's not bother with them unless somebody experiences
a similar racy false-positive in practice.
Signed-off-by: Jeff King <peff@peff.net>
---
Rescuing this from:
https://lore.kernel.org/git/20231221105124.GD570888@coredump.intra.peff.net/
where it was buried deep in a thread. I still think it's kind of gross,
but it may be the least-bad thing.
builtin/index-pack.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index dda94a9f46..0e94819216 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1257,13 +1257,15 @@ static void resolve_deltas(void)
base_cache_limit = delta_base_cache_limit * nr_threads;
if (nr_threads > 1 || getenv("GIT_FORCE_THREADS")) {
init_thread();
+ work_lock();
for (i = 0; i < nr_threads; i++) {
int ret = pthread_create(&thread_data[i].thread, NULL,
threaded_second_pass, thread_data + i);
if (ret)
die(_("unable to create thread: %s"),
strerror(ret));
}
+ work_unlock();
for (i = 0; i < nr_threads; i++)
pthread_join(thread_data[i].thread, NULL);
cleanup_thread();
--
2.43.0.553.g8113c77dd0
^ permalink raw reply related
* Re: What's cooking in git.git (Jan 2024, #01; Tue, 2)
From: Jeff King @ 2024-01-05 8:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: René Scharfe, git
In-Reply-To: <xmqq8r56bcew.fsf@gitster.g>
On Wed, Jan 03, 2024 at 08:37:59AM -0800, Junio C Hamano wrote:
> This is totally unrelated tangent, but the way "show-index" gets
> invoked in the above loop makes readers wonder how the caller found
> out which $idx file to read.
>
> Of course, the above loop sits downstream of a pipe
>
> find .git/objects/pack -type f -name \*.idx
>
> which means that any user of "git show-index" must be intimately
> familiar with how the object database is structured. I wonder if we
> want an extra layer of abstraction, similar to how the reference
> database can have different backend implementation.
I assume you mean a test helper by "extra layer of abstraction".
That could make sense, though I think this is just the tip of the
iceberg. There are a bazillion tests that muck with .git/objects/
directly (especially ones finding and munging loose objects). So it
wouldn't do much good until we know what cases the abstract code would
have to handle. And I don't think we have any concrete alternative yet
to the current object-dir layout.
So I think we should just punt on it for now. Adding one case here is
not making a hypothetical abstraction layer significantly harder to add
later.
-Peff
^ permalink raw reply
* Re: [GSOC][RFC] Heed core.bare from template config file when no command line override given, as a microproject.
From: Junio C Hamano @ 2024-01-05 15:59 UTC (permalink / raw)
To: Elijah Newren
Cc: Christian Couder, Ghanshyam Thakkar, git, johannes.schindelin
In-Reply-To: <CABPp-BH+cPdfsctquE60tw_nD6_LCaWf0JwGusuZ0tvQQuWy4w@mail.gmail.com>
Elijah Newren <newren@gmail.com> writes:
> If you look back at the mailing list discussion on the series that
> introduced this TODO comment you are trying to address, you'll note
> that both Glen and I dug into the code and attempted to explain it to
> each other, and we both got it wrong on our first try.
I think you meant 0f7443bd (init-db: document existing bug with
core.bare in template config, 2023-05-16), where it says:
The comments in create_default_files() talks about reading config from
the config file in the specified `--templates` directory, which leads to
the question of whether core.bare could be set in such a config file and
thus whether the code is doing the right thing.
But I suspect the all of the above comes from a misunderstanding.
The comment the above commit log message talks about is:
/*
* First copy the templates -- we might have the default
* config file there, in which case we would want to read
* from it after installing.
*
* Before reading that config, we also need to clear out any cached
* values (since we've just potentially changed what's available on
* disk).
*/
This primarily comes from my 4f629539 (init-db: check template and
repository format., 2005-11-25), whose focus was to control the way
HEAD symref is created, but care was taken to avoid propagating
values from the configuration variables in the template that do not
make sense for the repository being initialized. The most important
thing being the repository format version, but the intent to avoid
nonsense combination between the characteristic the new repository
has and the configuration values copied from the template was not
limited to the format version.
Specifically, the commit that introduced the comment never wanted to
honor core.bare in the template. I do not think I has core.bare in
mind when I wrote the comment, but I would have described it as the
same category as the repository format version, i.e. something you
would not want to copy, if I were pressed to clarify back then.
Besides, create_default_files() is way too late, even if we wanted
to create a bare repository when the template config file says
core.bare = true, as the caller would already have created before
passing $DIR (when "git --bare init $DIR" was run) or $DIR/.git
(when "git init $DIR" was run) to the function.
If somebody wants to always create a bare repository by having
core.bare=true in their template and if we wanted to honor it (which
I am dubious of the value of, by the way), I would think the right
place to do so would be way before create_default_files() is called.
When running "git init [$DIR]", long before calling init_db(), we
decide if we are about to create a bare repository and either create
$DIR or $DIR/.git. What is in the template, if we really wanted to
do so, should be read before that happens, no?
^ permalink raw reply
* Re: [PATCH v2 2/2] fetch: add cli option --default-only
From: Junio C Hamano @ 2024-01-05 16:13 UTC (permalink / raw)
To: Tamino Bauknecht; +Cc: git
In-Reply-To: <20240104222259.15659-2-dev@tb6.eu>
Tamino Bauknecht <dev@tb6.eu> writes:
> This option can be used to restore the default behavior of "git fetch"
> if the "fetch.all" config option is enabled.
> The flag cannot be used in combination with "--all" or explicit
> remote(s).
There is "--all" option that can be used to alter the behaviour of
the command. This is OPT_BOOL(), and if you have
[alias] f = fetch --all
you can say "git f --no-all" to countermand it from the command
line, as it is equivalent to "git fetch --all --no-all" and the last
one wins.
If you add "fetch.all" that makes the command behave as if it was
given "--all" from the command line even when the user didn't,
passing "--no-all" would be a lot more natural way to countermand
it, no?
"git fetch" (no parameters) are omitting two kinds of stuff, i.e.,
where we fetch from (repository) and what we are fetching from there
(refspec). You created a need to override the configured fetch
source (aka fetch.all), and in order to countermand it, one way is
to tell the command to "fetch from the default repository", but for
that, an option that does not say "repository" anywhere is closing
door for future evolution of the command. What if the next person
(which could be you) invented a way to say what refspec to be used
without specifying it from the command line, and needs to say "no,
no, no, do not use the configured value, but just use the default"?
In other words, "--default" will be met by a reaction "default of
which one???".
Compared to that, I would think "--[no-]all" would be a lot simpler
to understand.
The best part is that you do not have to add a new option. Just
make sure the one specified from the command line, either --all
or --no-all, will cause the command to ignore the configured
fetch.all and you do not need to add anything new to the UI.
^ permalink raw reply
* [Outreachy][PATCH v4] Port helper/test-ctype.c to unit-tests/t-ctype.c
From: Achu Luma @ 2024-01-05 16:14 UTC (permalink / raw)
To: git
Cc: gitster, chriscool, christian.couder, l.s.r, phillip.wood,
steadmon, me, Achu Luma, Phillip Wood
In-Reply-To: <20240101104017.9452-2-ach.lumap@gmail.com>
In the recent codebase update (8bf6fbd00d (Merge branch
'js/doc-unit-tests', 2023-12-09)), a new unit testing framework was
merged, providing a standardized approach for testing C code. Prior to
this update, some unit tests relied on the test helper mechanism,
lacking a dedicated unit testing framework. It's more natural to perform
these unit tests using the new unit test framework.
This commit migrates the unit tests for C character classification
functions (isdigit(), isspace(), etc) from the legacy approach
using the test-tool command `test-tool ctype` in t/helper/test-ctype.c
to the new unit testing framework (t/unit-tests/test-lib.h).
The migration involves refactoring the tests to utilize the testing
macros provided by the framework (TEST() and check_*()).
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Helped-by: René Scharfe <l.s.r@web.de>
Helped-by: Phillip Wood <phillip.wood123@gmail.com>
Helped-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Achu Luma <ach.lumap@gmail.com>
---
The changes between version 3 and version 4 are the following:
- Some duplication has been reduced using a new TEST_CHAR_CLASS() macro.
- A "0x"prefix has been added to avoid confusion between decimal and
hexadecimal codes printed by test_msg().
- The "Mentored-by:..." trailer has been restored.
- "works as expected" has been reduced to just "works" as suggested by Taylor.
- Some "Helped-by: ..." trailers have been added.
- Some whitespace fixes have been made.
Thanks to Junio, René, Phillip and Taylor for commenting on previous versions
of this patch.
Here is a diff between v3 and v4:
@@ -14,15 +14,16 @@ static int is_in(const char *s, int ch)
}
/* Macro to test a character type */
-#define TEST_CTYPE_FUNC(func, string) \
-static void test_ctype_##func(void) \
-{ \
- for (int i = 0; i < 256; i++) { \
- if (!check_int(func(i), ==, is_in(string, i))) \
- test_msg(" i: %02x", i); \
- } \
+#define TEST_CTYPE_FUNC(func, string) \
+static void test_ctype_##func(void) { \
+ for (int i = 0; i < 256; i++) { \
+ if (!check_int(func(i), ==, is_in(string, i))) \
+ test_msg(" i: 0x%02x", i); \
+ } \
}
+#define TEST_CHAR_CLASS(class) TEST(test_ctype_##class(), #class " works")
+
#define DIGIT "0123456789"
#define LOWER "abcdefghijklmnopqrstuvwxyz"
#define UPPER "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
@@ -58,20 +59,20 @@ TEST_CTYPE_FUNC(isprint, LOWER UPPER DIGIT PUNCT " ")
int cmd_main(int argc, const char **argv) {
/* Run all character type tests */
- TEST(test_ctype_isspace(), "isspace() works as we expect");
- TEST(test_ctype_isdigit(), "isdigit() works as we expect");
- TEST(test_ctype_isalpha(), "isalpha() works as we expect");
- TEST(test_ctype_isalnum(), "isalnum() works as we expect");
- TEST(test_ctype_is_glob_special(), "is_glob_special() works as we expect");
- TEST(test_ctype_is_regex_special(), "is_regex_special() works as we expect");
- TEST(test_ctype_is_pathspec_magic(), "is_pathspec_magic() works as we expect");
- TEST(test_ctype_isascii(), "isascii() works as we expect");
- TEST(test_ctype_islower(), "islower() works as we expect");
- TEST(test_ctype_isupper(), "isupper() works as we expect");
- TEST(test_ctype_iscntrl(), "iscntrl() works as we expect");
- TEST(test_ctype_ispunct(), "ispunct() works as we expect");
- TEST(test_ctype_isxdigit(), "isxdigit() works as we expect");
- TEST(test_ctype_isprint(), "isprint() works as we expect");
+ TEST_CHAR_CLASS(isspace);
+ TEST_CHAR_CLASS(isdigit);
+ TEST_CHAR_CLASS(isalpha);
+ TEST_CHAR_CLASS(isalnum);
+ TEST_CHAR_CLASS(is_glob_special);
+ TEST_CHAR_CLASS(is_regex_special);
+ TEST_CHAR_CLASS(is_pathspec_magic);
+ TEST_CHAR_CLASS(isascii);
+ TEST_CHAR_CLASS(islower);
+ TEST_CHAR_CLASS(isupper);
+ TEST_CHAR_CLASS(iscntrl);
+ TEST_CHAR_CLASS(ispunct);
+ TEST_CHAR_CLASS(isxdigit);
+ TEST_CHAR_CLASS(isprint);
Makefile | 2 +-
t/helper/test-ctype.c | 70 -------------------------------------
t/helper/test-tool.c | 1 -
t/helper/test-tool.h | 1 -
t/t0070-fundamental.sh | 4 ---
t/unit-tests/t-ctype.c | 78 ++++++++++++++++++++++++++++++++++++++++++
6 files changed, 79 insertions(+), 77 deletions(-)
delete mode 100644 t/helper/test-ctype.c
create mode 100644 t/unit-tests/t-ctype.c
diff --git a/Makefile b/Makefile
index 88ba7a3c51..a4df48ba65 100644
--- a/Makefile
+++ b/Makefile
@@ -792,7 +792,6 @@ TEST_BUILTINS_OBJS += test-chmtime.o
TEST_BUILTINS_OBJS += test-config.o
TEST_BUILTINS_OBJS += test-crontab.o
TEST_BUILTINS_OBJS += test-csprng.o
-TEST_BUILTINS_OBJS += test-ctype.o
TEST_BUILTINS_OBJS += test-date.o
TEST_BUILTINS_OBJS += test-delta.o
TEST_BUILTINS_OBJS += test-dir-iterator.o
@@ -1341,6 +1340,7 @@ THIRD_PARTY_SOURCES += sha1dc/%
UNIT_TEST_PROGRAMS += t-basic
UNIT_TEST_PROGRAMS += t-strbuf
+UNIT_TEST_PROGRAMS += t-ctype
UNIT_TEST_PROGS = $(patsubst %,$(UNIT_TEST_BIN)/%$X,$(UNIT_TEST_PROGRAMS))
UNIT_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(UNIT_TEST_PROGRAMS))
UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/test-lib.o
diff --git a/t/helper/test-ctype.c b/t/helper/test-ctype.c
deleted file mode 100644
index e5659df40b..0000000000
--- a/t/helper/test-ctype.c
+++ /dev/null
@@ -1,70 +0,0 @@
-#include "test-tool.h"
-
-static int rc;
-
-static void report_error(const char *class, int ch)
-{
- printf("%s classifies char %d (0x%02x) wrongly\n", class, ch, ch);
- rc = 1;
-}
-
-static int is_in(const char *s, int ch)
-{
- /*
- * We can't find NUL using strchr. Accept it as the first
- * character in the spec -- there are no empty classes.
- */
- if (ch == '\0')
- return ch == *s;
- if (*s == '\0')
- s++;
- return !!strchr(s, ch);
-}
-
-#define TEST_CLASS(t,s) { \
- int i; \
- for (i = 0; i < 256; i++) { \
- if (is_in(s, i) != t(i)) \
- report_error(#t, i); \
- } \
- if (t(EOF)) \
- report_error(#t, EOF); \
-}
-
-#define DIGIT "0123456789"
-#define LOWER "abcdefghijklmnopqrstuvwxyz"
-#define UPPER "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
-#define PUNCT "!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~"
-#define ASCII \
- "\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f" \
- "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" \
- "\x20\x21\x22\x23\x24\x25\x26\x27\x28\x29\x2a\x2b\x2c\x2d\x2e\x2f" \
- "\x30\x31\x32\x33\x34\x35\x36\x37\x38\x39\x3a\x3b\x3c\x3d\x3e\x3f" \
- "\x40\x41\x42\x43\x44\x45\x46\x47\x48\x49\x4a\x4b\x4c\x4d\x4e\x4f" \
- "\x50\x51\x52\x53\x54\x55\x56\x57\x58\x59\x5a\x5b\x5c\x5d\x5e\x5f" \
- "\x60\x61\x62\x63\x64\x65\x66\x67\x68\x69\x6a\x6b\x6c\x6d\x6e\x6f" \
- "\x70\x71\x72\x73\x74\x75\x76\x77\x78\x79\x7a\x7b\x7c\x7d\x7e\x7f"
-#define CNTRL \
- "\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f" \
- "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" \
- "\x7f"
-
-int cmd__ctype(int argc UNUSED, const char **argv UNUSED)
-{
- TEST_CLASS(isdigit, DIGIT);
- TEST_CLASS(isspace, " \n\r\t");
- TEST_CLASS(isalpha, LOWER UPPER);
- TEST_CLASS(isalnum, LOWER UPPER DIGIT);
- TEST_CLASS(is_glob_special, "*?[\\");
- TEST_CLASS(is_regex_special, "$()*+.?[\\^{|");
- TEST_CLASS(is_pathspec_magic, "!\"#%&',-/:;<=>@_`~");
- TEST_CLASS(isascii, ASCII);
- TEST_CLASS(islower, LOWER);
- TEST_CLASS(isupper, UPPER);
- TEST_CLASS(iscntrl, CNTRL);
- TEST_CLASS(ispunct, PUNCT);
- TEST_CLASS(isxdigit, DIGIT "abcdefABCDEF");
- TEST_CLASS(isprint, LOWER UPPER DIGIT PUNCT " ");
-
- return rc;
-}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index 37ba996539..33b9501c21 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -19,7 +19,6 @@ static struct test_cmd cmds[] = {
{ "config", cmd__config },
{ "crontab", cmd__crontab },
{ "csprng", cmd__csprng },
- { "ctype", cmd__ctype },
{ "date", cmd__date },
{ "delta", cmd__delta },
{ "dir-iterator", cmd__dir_iterator },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index 8a1a7c63da..b72f07ded9 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -12,7 +12,6 @@ int cmd__chmtime(int argc, const char **argv);
int cmd__config(int argc, const char **argv);
int cmd__crontab(int argc, const char **argv);
int cmd__csprng(int argc, const char **argv);
-int cmd__ctype(int argc, const char **argv);
int cmd__date(int argc, const char **argv);
int cmd__delta(int argc, const char **argv);
int cmd__dir_iterator(int argc, const char **argv);
diff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh
index 487bc8d905..a4756fbab9 100755
--- a/t/t0070-fundamental.sh
+++ b/t/t0070-fundamental.sh
@@ -9,10 +9,6 @@ Verify wrappers and compatibility functions.
TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
-test_expect_success 'character classes (isspace, isalpha etc.)' '
- test-tool ctype
-'
-
test_expect_success 'mktemp to nonexistent directory prints filename' '
test_must_fail test-tool mktemp doesnotexist/testXXXXXX 2>err &&
grep "doesnotexist/test" err
diff --git a/t/unit-tests/t-ctype.c b/t/unit-tests/t-ctype.c
new file mode 100644
index 0000000000..3a338df541
--- /dev/null
+++ b/t/unit-tests/t-ctype.c
@@ -0,0 +1,78 @@
+#include "test-lib.h"
+
+static int is_in(const char *s, int ch)
+{
+ /*
+ * We can't find NUL using strchr. Accept it as the first
+ * character in the spec -- there are no empty classes.
+ */
+ if (ch == '\0')
+ return ch == *s;
+ if (*s == '\0')
+ s++;
+ return !!strchr(s, ch);
+}
+
+/* Macro to test a character type */
+#define TEST_CTYPE_FUNC(func, string) \
+static void test_ctype_##func(void) { \
+ for (int i = 0; i < 256; i++) { \
+ if (!check_int(func(i), ==, is_in(string, i))) \
+ test_msg(" i: 0x%02x", i); \
+ } \
+}
+
+#define TEST_CHAR_CLASS(class) TEST(test_ctype_##class(), #class " works")
+
+#define DIGIT "0123456789"
+#define LOWER "abcdefghijklmnopqrstuvwxyz"
+#define UPPER "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+#define PUNCT "!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~"
+#define ASCII \
+ "\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f" \
+ "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" \
+ "\x20\x21\x22\x23\x24\x25\x26\x27\x28\x29\x2a\x2b\x2c\x2d\x2e\x2f" \
+ "\x30\x31\x32\x33\x34\x35\x36\x37\x38\x39\x3a\x3b\x3c\x3d\x3e\x3f" \
+ "\x40\x41\x42\x43\x44\x45\x46\x47\x48\x49\x4a\x4b\x4c\x4d\x4e\x4f" \
+ "\x50\x51\x52\x53\x54\x55\x56\x57\x58\x59\x5a\x5b\x5c\x5d\x5e\x5f" \
+ "\x60\x61\x62\x63\x64\x65\x66\x67\x68\x69\x6a\x6b\x6c\x6d\x6e\x6f" \
+ "\x70\x71\x72\x73\x74\x75\x76\x77\x78\x79\x7a\x7b\x7c\x7d\x7e\x7f"
+#define CNTRL \
+ "\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f" \
+ "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" \
+ "\x7f"
+
+TEST_CTYPE_FUNC(isdigit, DIGIT)
+TEST_CTYPE_FUNC(isspace, " \n\r\t")
+TEST_CTYPE_FUNC(isalpha, LOWER UPPER)
+TEST_CTYPE_FUNC(isalnum, LOWER UPPER DIGIT)
+TEST_CTYPE_FUNC(is_glob_special, "*?[\\")
+TEST_CTYPE_FUNC(is_regex_special, "$()*+.?[\\^{|")
+TEST_CTYPE_FUNC(is_pathspec_magic, "!\"#%&',-/:;<=>@_`~")
+TEST_CTYPE_FUNC(isascii, ASCII)
+TEST_CTYPE_FUNC(islower, LOWER)
+TEST_CTYPE_FUNC(isupper, UPPER)
+TEST_CTYPE_FUNC(iscntrl, CNTRL)
+TEST_CTYPE_FUNC(ispunct, PUNCT)
+TEST_CTYPE_FUNC(isxdigit, DIGIT "abcdefABCDEF")
+TEST_CTYPE_FUNC(isprint, LOWER UPPER DIGIT PUNCT " ")
+
+int cmd_main(int argc, const char **argv) {
+ /* Run all character type tests */
+ TEST_CHAR_CLASS(isspace);
+ TEST_CHAR_CLASS(isdigit);
+ TEST_CHAR_CLASS(isalpha);
+ TEST_CHAR_CLASS(isalnum);
+ TEST_CHAR_CLASS(is_glob_special);
+ TEST_CHAR_CLASS(is_regex_special);
+ TEST_CHAR_CLASS(is_pathspec_magic);
+ TEST_CHAR_CLASS(isascii);
+ TEST_CHAR_CLASS(islower);
+ TEST_CHAR_CLASS(isupper);
+ TEST_CHAR_CLASS(iscntrl);
+ TEST_CHAR_CLASS(ispunct);
+ TEST_CHAR_CLASS(isxdigit);
+ TEST_CHAR_CLASS(isprint);
+
+ return test_done();
+}
--
2.42.0.windows.2
^ permalink raw reply related
* Re: [PATCH] push: region_leave trace for negotiate_using_fetch
From: Junio C Hamano @ 2024-01-05 16:18 UTC (permalink / raw)
To: Sam Delmerico; +Cc: git, steadmon
In-Reply-To: <CAHVcGPSSJr7L_NyFSKEkEywBap6hUrucga98RLpa6xuZ0k4CzA@mail.gmail.com>
Sam Delmerico <delmerico@google.com> writes:
> * I don't exactly remember how I noticed it. I was doing some
> debugging around the push negotiation code and either 1) saw this
> region get entered twice in the trace output, or 2) I was just reading
> the code around here and saw two enters.
>
> * Perhaps there could be a check before the last git process ends that
> checks that all opened trace regions have been closed? I'm not sure
> how much work this would involve. It's probably also not a very
> proactive way to catch these bugs since it would only get triggered
> when a *user* hits a code path with a trace region that never exits.
>
> * There could also be a test that checks that every region_enter trace
> log has a corresponding region_leave. But I'm not sure how to ensure
> that every code path is checked.
>
> Overall, I'm not sure how much benefit there is from checking for
> this. I'm not sure that it would have a large impact if it were to
> happen again. For example, I think that it could be noticed relatively
> quickly by a person/system looking at metrics like I was (e.g. if the
> time spent in a region is infinite or zero).
>
> FWIW I didn't see any other examples of this when going through logs.
The above matches my intuition. A test that covers this specific
case would likely be of low value as it is unlikely for us to
regress this specific one. A CI job that runs all the tests under
tracing and inspects the log may catch some but its finding would be
limited to the code paths that are covered (but increasing the test
coverage would help here, not just for finding unbalanced region
markers, but for finding bugs in general).
Thanks.
^ permalink raw reply
* Re: rebase invoking pre-commit
From: Junio C Hamano @ 2024-01-05 16:26 UTC (permalink / raw)
To: Sean Allred; +Cc: Git List
In-Reply-To: <m0sf3vi86g.fsf@epic96565.epic.com>
Sean Allred <allred.sean@gmail.com> writes:
> Is there a current reason why pre-commit shouldn't be invoked during
> rebase, or is this just waiting for a reviewable patch?
>
> This was brought up before at [1] in 2015, but that thread so old at
> this point that it seemed prudent to double-check before investing time
> in a developing and testing a patch.
>
> [1]: https://lore.kernel.org/git/1m55i3m.1fum4zo1fpnhncM%25lists@haller-berlin.de/
If you are trying to make it less likely that your developers would
commit conflict markers by mistake, I think an effective way would
be to give "git rebase" an option (or a configuration variable) that
forbids it from making a new commit upon "git rebase --continue",
which AFAIK was added merely to help "lazy" folks to omit the
explicit "git commit" step in the following sequence:
$ git rebase origin/master
... stops with conflicts
$ edit
... now the conflicts are resolved (and hopefully you have
... tested the result)
$ git commit
$ git rebase --continue
^ permalink raw reply
* Re: [PATCH] index-pack: spawn threads atomically
From: Taylor Blau @ 2024-01-05 16:33 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20240105085034.GA3078476@coredump.intra.peff.net>
On Fri, Jan 05, 2024 at 03:50:34AM -0500, Jeff King wrote:
> The t5309 script triggers a racy false positive with SANITIZE=leak on a
> multi-core system. Running with "--stress --run=6" usually fails within
> 10 seconds or so for me, complaining with something like:
>
> + git index-pack --fix-thin --stdin
> fatal: REF_DELTA at offset 46 already resolved (duplicate base 01d7713666f4de822776c7622c10f1b07de280dc?)
>
> =================================================================
> ==3904583==ERROR: LeakSanitizer: detected memory leaks
>
> Direct leak of 32 byte(s) in 1 object(s) allocated from:
> #0 0x7fa790d01986 in __interceptor_realloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:98
> #1 0x7fa790add769 in __pthread_getattr_np nptl/pthread_getattr_np.c:180
> #2 0x7fa790d117c5 in __sanitizer::GetThreadStackTopAndBottom(bool, unsigned long*, unsigned long*) ../../../../src/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cpp:150
> #3 0x7fa790d11957 in __sanitizer::GetThreadStackAndTls(bool, unsigned long*, unsigned long*, unsigned long*, unsigned long*) ../../../../src/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cpp:598
> #4 0x7fa790d03fe8 in __lsan::ThreadStart(unsigned int, unsigned long long, __sanitizer::ThreadType) ../../../../src/libsanitizer/lsan/lsan_posix.cpp:51
> #5 0x7fa790d013fd in __lsan_thread_start_func ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:440
> #6 0x7fa790adc3eb in start_thread nptl/pthread_create.c:444
> #7 0x7fa790b5ca5b in clone3 ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
>
> SUMMARY: LeakSanitizer: 32 byte(s) leaked in 1 allocation(s).
> Aborted
We discussed this in another thread (beginning here [1]), and I would be
fine with this approach. I share your feeling that it is a little gross
to have to work around LSan's implementation by tweaking production
code, but I think that this doesn't have to be the most pristine patch
ever written, either ;-).
Just playing devil's advocate for a moment, I wonder if another approach
might be to disable the threading altogether for the purposes of this
test. The performance difference is negligible, and I don't think we're
exercising any interesting paths in this particular test that have to do
with pack.threads > 1 that aren't covered extensively elsewhere.
So, in other words, I think a reasonable approach would be to do
something like:
--- 8< ---
diff --git a/t/t5309-pack-delta-cycles.sh b/t/t5309-pack-delta-cycles.sh
index 4e910c5b9d..1d132b6324 100755
--- a/t/t5309-pack-delta-cycles.sh
+++ b/t/t5309-pack-delta-cycles.sh
@@ -73,7 +73,7 @@ test_expect_success 'failover to a duplicate object in the same pack' '
pack_obj $A
} >recoverable.pack &&
pack_trailer recoverable.pack &&
- test_must_fail git index-pack --fix-thin --stdin <recoverable.pack
+ test_must_fail git index-pack --threads=1 --fix-thin --stdin <recoverable.pack
'
test_done
--- >8 ---
And call it a day. I built with SANITIZE=leak, and then ran:
$ GIT_TEST_PASSING_SANITIZE_LEAK=true ./t5309-pack-delta-cycles.sh --stress --run=6
For a while and didn't see any failures. That could be luck, of course,
but without the above patch I was seeing failures within a few seconds.
I'm reasonably confident that this would do the trick.
For what it's worth, I'm fine with either approach, mostly to avoid
tying up more of the list's time discussing the options. But I have a
vague preference towards `--threads=1` since it doesn't require us to
touch production code.
> Rescuing this from:
>
> https://lore.kernel.org/git/20231221105124.GD570888@coredump.intra.peff.net/
>
> where it was buried deep in a thread. I still think it's kind of gross,
> but it may be the least-bad thing.
In either case, thanks for digging it back up :-).
Thanks,
Taylor
[1]: https://lore.kernel.org/git/xmqqbkasnxba.fsf@gitster.g/
^ permalink raw reply related
* Re: What's cooking in git.git (Jan 2024, #01; Tue, 2)
From: Junio C Hamano @ 2024-01-05 16:34 UTC (permalink / raw)
To: Jeff King; +Cc: René Scharfe, git
In-Reply-To: <20240105085928.GA3078702@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Jan 03, 2024 at 08:37:59AM -0800, Junio C Hamano wrote:
>
>> This is totally unrelated tangent, but the way "show-index" gets
>> invoked in the above loop makes readers wonder how the caller found
>> out which $idx file to read.
>>
>> Of course, the above loop sits downstream of a pipe
>>
>> find .git/objects/pack -type f -name \*.idx
>>
>> which means that any user of "git show-index" must be intimately
>> familiar with how the object database is structured. I wonder if we
>> want an extra layer of abstraction, similar to how the reference
>> database can have different backend implementation.
>
> I assume you mean a test helper by "extra layer of abstraction".
Well, that might be a good first step, but I was envisioning more
into far future where the object store is reimplemented on top of
high-performance database, at which point we may want subcommands
like "git odb list-packs" that lists packfile names (which may not
name any on-filesystem files) that can be given to "git show-index",
for example. Of course, the abstraction boundary of the object
store might be placed a lot higher and the interface into it may be
limited to "here is an oid, give me its contents" without distinction
between the objects in packfiles and objects in loose object files.
> So I think we should just punt on it for now. Adding one case here is
> not making a hypothetical abstraction layer significantly harder to add
> later.
Totally agree, and that is why I labeled the whole thing as an
unrelated tangent.
Thanks.
^ permalink raw reply
* Re: [PATCH] commit-graph: retain commit slab when closing NULL commit_graph
From: Taylor Blau @ 2024-01-05 17:07 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano, Scott Leggett
In-Reply-To: <20240105054142.GA2035092@coredump.intra.peff.net>
On Fri, Jan 05, 2024 at 12:41:42AM -0500, Jeff King wrote:
> This fixes a regression introduced in ac6d45d11f (commit-graph: move
> slab-clearing to close_commit_graph(), 2023-10-03), in which running:
>
> git -c fetch.writeCommitGraph=true fetch --recurse-submodules
>
> multiple times in a freshly cloned repository causes a segfault. What
> happens in the second (and subsequent) runs is this:
>
> 1. We make a "struct commit" for any ref tips which we're storing
> (even if we already have them, they still go into FETCH_HEAD).
>
> Because the first run will have created a commit graph, we'll find
> those commits in the graph.
>
> The commit struct is therefore created with a NULL "maybe_tree"
> entry, because we can load its oid from the graph later. But to do
> that we need to remember that we got the commit from the graph,
> which is recorded in a global commit_graph_data_slab object.
>
> 2. Because we're using --recurse-submodules, we'll try to fetch each
> of the possible submodules. That implies creating a separate
> "struct repository" in-process for each submodule, which will
> require a later call to repo_clear().
>
> The call to repo_clear() calls raw_object_store_clear(), which in
> turn calls close_object_store(), which in turn calls
> close_commit_graph(). And the latter frees the commit graph data
> slab.
>
> 3. Later, when trying to write out a new commit graph, we'll ask for
> their tree oid via get_commit_tree_oid(), which will see that the
> object is parsed but with a NULL maybe_tree field. We'd then
> usually pull it from the graph file, but because the slab was
> cleared, we don't realize that we can do so! We end up returning
> NULL and segfaulting.
>
> (It seems questionable that we'd write a graph entry for such a
> commit anyway, since we know we already have one. I didn't
> double-check, but that may simply be another side effect of having
> cleared the slab).
Yeah, I agree that we should not be re-writing a commit-graph entry that
already exists in an earlier (non-removed) layer of the commit-graph.
I haven't thought through all of the details here, but that sounds like
a potentially dangerous thing to be doing.
> So that fixes the regression in the least-risky way possible.
Thanks for the detailed explanation. I think that the fix presented here
is a reasonable approach, and is worthwhile to pick up.
> I do think there's some fragility here that we might want to follow up
> on. We have a global commit_graph_data_slab that contains graph
> positions, and our global commit structs depend on the that slab
> remaining valid. But close_commit_graph() is just about closing _one_
> object store's graph. So it's dangerous to call that function and clear
> the slab without also throwing away any "struct commit" we might have
> parsed that depends on it.
I definitely agree that there seems like a high risk of another
potentially-dangerous bug happening as a result of this area.
One thing that sticks out to me is that we have a scope mismatch
between the commit structs, the raw_object_store, and the commit slabs.
Slabs are tied to the lifetime of the raw_object_store, but the commit
objects are tied to the global lifetime of the process. I wonder if it
would make sense to have a slab per object-store, and require that you
pass both the commit *and* the object-store you're looking it up in as
arguments to any slab-related functions.
I dunno. I have not put a ton of thought into that ^^ approach either,
so let me know if it seems obviously bogus to you or if this is worth
looking into further.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH v3] rebase: clarify --reschedule-failed-exec default
From: Taylor Blau @ 2024-01-05 17:11 UTC (permalink / raw)
To: Illia Bobyr; +Cc: git
In-Reply-To: <20240105011424.1443732-2-illia.bobyr@gmail.com>
On Thu, Jan 04, 2024 at 05:14:26PM -0800, Illia Bobyr wrote:
> ---
> Documentation/git-rebase.txt | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
LGTM.
Thanks,
Taylor
^ permalink raw reply
* [BUG] mv: can trigger assertion failure with three parameters (builtin/mv.c:481)
From: Kristoffer Haugsbakk @ 2024-01-05 17:41 UTC (permalink / raw)
To: git
You can trigger an assertion by giving these arguments to `git mv`:
<dir>/file <dir> <other dir>
> 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)
```
git config --global --add safe.directory /tmp
dir=$(mktemp -d)
cd $dir
git init
mkdir a
touch a/a.txt
git add a/a.txt
git commit -m 'init'
mkdir b
# Assertion triggered
git mv a/a.txt a b
# `.git/index.lock` still lingers after this; commands like `git add
# <file>` will fail
```
The output:
```
git: builtin/mv.c:481: cmd_mv: Assertion `pos >= 0' failed.
Aborted (core dumped)
```
Also `.git/index.lock` is still there.
> What did you expect to happen? (Expected behavior)
A normal error message if the command is nonsensical (I don’t know; that’s
not the point). Also `.git/index.lock` to be cleaned up.
> What happened instead? (Actual behavior)
An assertion failed. `.git/index.lock` is not cleaned up.
> What's different between what you expected and what actually happened?
See above.
> Anything else you want to add:
Same behavior on `master` (a26002b628 (The fifth batch, 2024-01-02)).
```
./bin-wrappers/git config --global --add safe.directory /tmp
dir=$(mktemp -d)
./bin-wrappers/git -C $dir init
mkdir $dir/a
touch $dir/a/a.txt
./bin-wrappers/git -C $dir add $dir/a/a.txt
./bin-wrappers/git -C $dir commit -m 'init'
mkdir $dir/b
# Assertion triggered
./bin-wrappers/git -C $dir mv $dir/a/a.txt $dir/a $dir/b
```
> 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.43.0
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
uname: Linux 6.2.0-39-generic #40~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Thu Nov 16 10:53:04 UTC 2 x86_64
compiler info: gnuc: 11.4
libc info: glibc: 2.35
$SHELL (typically, interactive shell): /bin/bash
[Enabled Hooks]
--
Kristoffer Haugsbakk
^ permalink raw reply
* Re: [BUG] mv: can trigger assertion failure with three parameters (builtin/mv.c:481)
From: Junio C Hamano @ 2024-01-05 18:52 UTC (permalink / raw)
To: Kristoffer Haugsbakk; +Cc: git
In-Reply-To: <d1f739fe-b28e-451f-9e01-3d2e24a0fe0d@app.fastmail.com>
"Kristoffer Haugsbakk" <code@khaugsbakk.name> writes:
> You can trigger an assertion by giving these arguments to `git mv`:
>
> <dir>/file <dir> <other dir>
> ...
>> What did you expect to happen? (Expected behavior)
>
> A normal error message if the command is nonsensical (I don’t know; that’s
> not the point). Also `.git/index.lock` to be cleaned up.
Good find.
Not just that, but when the command fails in the middle like this,
it leaves the working tree in a half-updated state, i.e.
> ./bin-wrappers/git -C $dir mv $dir/a/a.txt $dir/a $dir/b
will first move a/a.txt to b/a.txt, then try to move a (actually,
all contents of it, including a/a.txt) to b/a and finds that "the
command is nonsensical" and aborts, and by that time, there is no
a/a.txt (i.e. the working tree has been modified). The failure
should be made atomic, just like "git switch" to another branch may
stop _without_ touching anything in the working tree when it may
have to fail (e.g., due to a file being dirty).
Thanks for reporting, Kristoffer.
Any takers?
$ git shortlog --since=3.years -s -n -e --no-merges v2.43.0 builtin/mv.c
15 Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>
10 Elijah Newren <newren@gmail.com>
5 Ævar Arnfjörð Bjarmason <avarab@gmail.com>
2 Junio C Hamano <gitster@pobox.com>
1 Andrzej Hunt <ajrhunt@google.com>
1 Calvin Wan <calvinwan@google.com>
1 Derrick Stolee <stolee@gmail.com>
1 Sebastian Thiel <sebastian.thiel@icloud.com>
1 Torsten Bögershausen <tboegi@web.de>
^ permalink raw reply
* Re: [BUG] mv: can trigger assertion failure with three parameters (builtin/mv.c:481)
From: Dragan Simic @ 2024-01-05 19:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kristoffer Haugsbakk, git
In-Reply-To: <xmqqil47obnw.fsf@gitster.g>
On 2024-01-05 19:52, Junio C Hamano wrote:
> "Kristoffer Haugsbakk" <code@khaugsbakk.name> writes:
>
>> You can trigger an assertion by giving these arguments to `git mv`:
>>
>> <dir>/file <dir> <other dir>
>> ...
>>> What did you expect to happen? (Expected behavior)
>>
>> A normal error message if the command is nonsensical (I don’t know;
>> that’s
>> not the point). Also `.git/index.lock` to be cleaned up.
>
> Good find.
Yes, thanks to Kristoffer for reporting this issue.
> Not just that, but when the command fails in the middle like this,
> it leaves the working tree in a half-updated state, i.e.
>
>> ./bin-wrappers/git -C $dir mv $dir/a/a.txt $dir/a $dir/b
>
> will first move a/a.txt to b/a.txt, then try to move a (actually,
> all contents of it, including a/a.txt) to b/a and finds that "the
> command is nonsensical" and aborts, and by that time, there is no
> a/a.txt (i.e. the working tree has been modified). The failure
> should be made atomic, just like "git switch" to another branch may
> stop _without_ touching anything in the working tree when it may
> have to fail (e.g., due to a file being dirty).
>
> Thanks for reporting, Kristoffer.
>
> Any takers?
This looks like a rather interesting bugfix to me. :) Though, I've
unfortunately contracted some _nasty_ flu, so I'm still simply unable to
work on pretty much anything in the next 5-6 days or so, at which point
I hope to be operational again.
Thus, unless someone else can get it done faster, I should be able to
start working on it in about a week or so. Hopefully, that is.
> $ git shortlog --since=3.years -s -n -e --no-merges v2.43.0
> builtin/mv.c
> 15 Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>
> 10 Elijah Newren <newren@gmail.com>
> 5 Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> 2 Junio C Hamano <gitster@pobox.com>
> 1 Andrzej Hunt <ajrhunt@google.com>
> 1 Calvin Wan <calvinwan@google.com>
> 1 Derrick Stolee <stolee@gmail.com>
> 1 Sebastian Thiel <sebastian.thiel@icloud.com>
> 1 Torsten Bögershausen <tboegi@web.de>
^ permalink raw reply
* Re: [PATCH] commit-graph: fix memory leak when not writing graph
From: Taylor Blau @ 2024-01-05 19:11 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <0feab5e7d5bc6275e2c7671cd8f6786ea86fd610.1702891190.git.ps@pks.im>
On Mon, Dec 18, 2023 at 11:02:28AM +0100, Patrick Steinhardt wrote:
> When `write_commit_graph()` bails out writing a split commit-graph early
> then it may happen that we have already gathered the set of existing
> commit-graph file names without yet determining the new merged set of
> files. This can result in a memory leak though because we only clear the
> preimage of files when we have collected the postimage.
>
> Fix this issue by dropping the condition altogether so that we always
> try to free both preimage and postimage filenames. As the context
> structure is zero-initialized this simplification is safe to do.
Looks obviously good to me, thanks for finding and fixing.
Thanks,
Taylor
^ permalink raw reply
* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Dragan Simic @ 2024-01-05 19:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jacob Stopak, Oswald Buddenhagen, git
In-Reply-To: <8d45763bb4fa4c7d1e1f69dfaf93e647@manjaro.org>
On 2023-10-24 04:21, Dragan Simic wrote:
> On 2023-10-24 04:03, Junio C Hamano wrote:
>> I think the "use more verbose report format to help relatively
>> inexperienced folks, in exchange for spending more screen real
>> estate" is a good direction to think about this thing.
>>
>> I am not personally interested in adding such support all that much
>> myself, but one piece of advice I can offer those who are interested
>> is not to be too deeply attached to the word "table".
>>
>> ... snip ...
>>
>> So be very careful when choosing what to call this new thing, and
>> avoid naming it after the implementation details (e.g., in what
>> particular shape the data gets presented) that may turn out not to
>> be the most important part of the concept.
>
> Totally agreed, "table" simply sneaked in and remained here as the
> term. Perhaps "<command>.verbose = extra" or something similar would
> be a good choice.
Just a brief update... Some of the associated patches are already ready
to go, and some more are still pending to be implemented. It took me a
while, which I apologize for, and now I've also unfortunately contracted
some really bad flu. I'll be back to the patches as soon as the flu
decides to go away.
>> [Footnote]
>>
>> * FWIW, "git status -s" is a tabular presentation. Maybe we can
>> add a more verbose form of "-s" and be done with it for the
>> command?
>
> That's also an option.
^ permalink raw reply
* Re: [PATCH] commit-graph: retain commit slab when closing NULL commit_graph
From: Junio C Hamano @ 2024-01-05 19:56 UTC (permalink / raw)
To: Jeff King; +Cc: git, Scott Leggett, Taylor Blau
In-Reply-To: <20240105054142.GA2035092@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> This fixes a regression introduced in ac6d45d11f (commit-graph: move
> slab-clearing to close_commit_graph(), 2023-10-03), in which running:
> ...
> So it happens to work out. But it still seems questionable to me that we
> would clear a global slab (which might still be in use) when closing the
> commit graph. This clearing comes from 957ba814bf (commit-graph: when
> closing the graph, also release the slab, 2021-09-08), and was fixing a
> case where we really did need it to be closed (and in that case we
> presumably call close_object_store() more directly).
Wow, that is nasty.
But anyway, thanks for your usual "3 pages of explanation for 2
lines of change". The patch does look the best fix in the meantime.
^ permalink raw reply
* What's cooking in git.git (Jan 2024, #02; Fri, 5)
From: Junio C Hamano @ 2024-01-06 1:13 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).
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/
--------------------------------------------------
[New Topics]
* ms/rebase-insnformat-doc-fix (2024-01-03) 1 commit
(merged to 'next' on 2024-01-04 at d68f2be39b)
+ Documentation: fix statement about rebase.instructionFormat
Docfix.
Will merge to 'master'.
source: <pull.1629.git.git.1704305663254.gitgitgadget@gmail.com>
* cp/git-flush-is-an-env-bool (2024-01-04) 1 commit
(merged to 'next' on 2024-01-04 at b435a96ce8)
+ write-or-die: make GIT_FLUSH a Boolean environment variable
Unlike other environment variables that took the usual
true/false/yes/no as well as 0/1, GIT_FLUSH only understood 0/1,
which has been corrected.
Will merge to 'master'.
source: <pull.1628.v3.git.1704363617842.gitgitgadget@gmail.com>
* sd/negotiate-trace-fix (2024-01-03) 1 commit
- push: region_leave trace for negotiate_using_fetch
Tracing fix.
Waiting for a review response.
cf. <xmqqbka27zu9.fsf@gitster.g>
source: <20240103224054.1940209-1-delmerico@google.com>
* sk/mingw-owner-check-error-message-improvement (2024-01-04) 1 commit
- mingw: give more details about unsafe directory's ownership
In addition to (rather cryptic) Security Identifiers, show username
and domain in the error message when we barf on mismatch between
the Git directory and the current user.
Waiting for a review by the area maintainer.
cf. <xmqqbka07te6.fsf@gitster.g>
source: <20240104192202.2124-2-soekkle@freenet.de>
* ib/rebase-reschedule-doc (2024-01-05) 1 commit
- rebase: clarify --reschedule-failed-exec default
Doc update.
Will merge to 'next'.
source: <20240105011424.1443732-2-illia.bobyr@gmail.com>
* jk/commit-graph-slab-clear-fix (2024-01-05) 1 commit
- commit-graph: retain commit slab when closing NULL commit_graph
Clearing in-core repository (happens during e.g., "git fetch
--recurse-submodules" with commit graph enabled) made in-core
commit object in an inconsistent state by discarding the necessary
data from commit-graph too early, which has been corrected.
Will merge to 'next'.
source: <20240105054142.GA2035092@coredump.intra.peff.net>
* jk/index-pack-lsan-false-positive-fix (2024-01-05) 1 commit
- index-pack: spawn threads atomically
Fix false positive reported by leak sanitizer.
Will merge to 'next'.
source: <20240105085034.GA3078476@coredump.intra.peff.net>
--------------------------------------------------
[Cooking]
* cp/sideband-array-index-comment-fix (2023-12-28) 1 commit
- sideband.c: remove redundant 'NEEDSWORK' tag
In-code comment fix.
Will merge to 'next'.
source: <pull.1625.v4.git.1703750460527.gitgitgadget@gmail.com>
* ps/worktree-refdb-initialization (2023-12-28) 6 commits
- builtin/worktree: create refdb via ref backend
- worktree: expose interface to look up worktree by name
- builtin/worktree: move setup of commondir file earlier
- refs/files: skip creation of "refs/{heads,tags}" for worktrees
- setup: move creation of "refs/" into the files backend
- refs: prepare `refs_init_db()` for initializing worktree refs
Instead of manually creating refs/ hierarchy on disk upon a
creation of a secondary worktree, which is only usable via the
files backend, use the refs API to populate it.
May want to wait until other topics solidify a bit more.
cf. <xmqqedf6gpt8.fsf@gitster.g>
source: <cover.1703754513.git.ps@pks.im>
* ml/doc-merge-updates (2023-12-20) 2 commits
(merged to 'next' on 2023-12-28 at 7a6329a219)
+ Documentation/git-merge.txt: use backticks for command wrapping
+ Documentation/git-merge.txt: fix reference to synopsis
Doc update.
Will merge to 'master'.
source: <20231220195342.17590-1-mi.al.lohmann@gmail.com>
* cp/apply-core-filemode (2023-12-26) 3 commits
- apply: code simplification
- apply: correctly reverse patch's pre- and post-image mode bits
- apply: ignore working tree filemode when !core.filemode
"git apply" on a filesystem without filemode support have learned
to take a hint from what is in the index for the path, even when
not working with the "--index" or "--cached" option, when checking
the executable bit match what is required by the preimage in the
patch.
Needs review.
source: <20231226233218.472054-1-gitster@pobox.com>
* jc/archive-list-with-extra-args (2023-12-21) 1 commit
(merged to 'next' on 2023-12-28 at 2d5c20e67f)
+ archive: "--list" does not take further options
"git archive --list extra garbage" silently ignored excess command
line parameters, which has been corrected.
Will merge to 'master'.
source: <xmqqmsu3mnix.fsf@gitster.g>
* jk/t1006-cat-file-objectsize-disk (2024-01-03) 2 commits
(merged to 'next' on 2024-01-03 at a492c6355c)
+ t1006: prefer shell loop to awk for packed object sizes
(merged to 'next' on 2023-12-28 at d82812e636)
+ t1006: add tests for %(objectsize:disk)
Test update.
Will merge to 'master'.
source: <20231221094722.GA570888@coredump.intra.peff.net>
source: <20240103090152.GB1866508@coredump.intra.peff.net>
* js/contributor-docs-updates (2023-12-27) 9 commits
(merged to 'next' on 2024-01-02 at 0e072117cd)
+ SubmittingPatches: hyphenate non-ASCII
+ SubmittingPatches: clarify GitHub artifact format
+ SubmittingPatches: clarify GitHub visual
+ SubmittingPatches: provide tag naming advice
+ SubmittingPatches: update extra tags list
+ SubmittingPatches: discourage new trailers
+ SubmittingPatches: drop ref to "What's in git.git"
+ CodingGuidelines: write punctuation marks
+ CodingGuidelines: move period inside parentheses
Doc update.
Will merge to 'master'.
source: <pull.1623.v3.git.1703739324.gitgitgadget@gmail.com>
* al/unit-test-ctype (2024-01-05) 1 commit
- unit-tests: rewrite t/helper/test-ctype.c as a unit test
Move test-ctype helper to the unit-test framework.
Will merge to 'next'.
source: <20240105161413.10422-1-ach.lumap@gmail.com>
* bk/bisect-doc-fix (2023-12-27) 1 commit
- doc: use singular form of repeatable path arg
Synopsis fix.
Expecting a reroll.
source: <3d46bca1-96d4-43ba-a912-1f7c76942287@smtp-relay.sendinblue.com>
* en/sparse-checkout-eoo (2023-12-26) 2 commits
(merged to 'next' on 2023-12-28 at 3ddf2ebab6)
+ sparse-checkout: be consistent with end of options markers
+ Merge branch 'jk/end-of-options' into jc/sparse-checkout-set-add-end-of-options
"git sparse-checkout (add|set) --[no-]cone --end-of-options" did
not handle "--end-of-options" correctly after a recent update.
Will merge to 'master'.
source: <pull.1625.v2.git.git.1703619562639.gitgitgadget@gmail.com>
* ja/doc-placeholders-fix (2023-12-26) 2 commits
- doc: enforce placeholders in documentation
- doc: enforce dashes in placeholders
Docfix.
Needs review.
source: <pull.1626.git.1703539287.gitgitgadget@gmail.com>
* jc/sparse-checkout-set-default-fix (2023-12-26) 1 commit
(merged to 'next' on 2023-12-28 at a558eccf8e)
+ sparse-checkout: use default patterns for 'set' only !stdin
"git sparse-checkout set" added default patterns even when the
patterns are being fed from the standard input, which has been
corrected.
Will merge to 'master'.
source: <20231221065925.3234048-3-gitster@pobox.com>
* rs/fast-import-simplify-mempool-allocation (2023-12-26) 1 commit
(merged to 'next' on 2023-12-28 at 16e6dd2a69)
+ fast-import: use mem_pool_calloc()
Code simplification.
Will merge to 'master'.
source: <50c1f410-ca37-4c1c-a28b-3e9fad49f2b4@web.de>
* rs/mem-pool-improvements (2023-12-28) 2 commits
(merged to 'next' on 2023-12-28 at aa03d9441c)
+ mem-pool: simplify alignment calculation
+ mem-pool: fix big allocations
MemPool allocator fixes.
Will merge to 'master'.
source: <fa89d269-1a23-4ed6-bebc-30c0b629f444@web.de>
* ps/refstorage-extension (2024-01-02) 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
- 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
Introduce a new extension "refstorage" so that we can mark a
repository that uses a non-default ref backend, like reftable.
Will merge to 'next'.
source: <cover.1703833818.git.ps@pks.im>
* ps/reftable-fixes-and-optims (2024-01-03) 9 commits
- 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 auto-compact twice in `reftable_stack_add()`
- reftable/stack: do not overwrite errors when compacting
- Merge branch 'ps/reftable-fixes' into ps/reftable-fixes-and-optims
More fixes and optimizations to the reftable backend.
Will merge to 'next'.
source: <cover.1704262787.git.ps@pks.im>
* tb/multi-pack-verbatim-reuse (2023-12-14) 26 commits
(merged to 'next' on 2024-01-04 at 891ac0fa2c)
+ 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 'master'.
cf. <ZXurD1NTZ4TAs7WZ@nand.local>
source: <cover.1702592603.git.me@ttaylorr.com>
* jc/bisect-doc (2023-12-09) 1 commit
- bisect: document "terms" subcommand more fully
Doc update.
Needs review.
source: <xmqqzfyjmk02.fsf@gitster.g>
* en/header-cleanup (2023-12-26) 12 commits
(merged to 'next' on 2023-12-28 at 1ccddc2a10)
+ 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".
Will merge to 'master'.
source: <pull.1617.v2.git.1703351700.gitgitgadget@gmail.com>
* jw/builtin-objectmode-attr (2023-12-28) 1 commit
(merged to 'next' on 2024-01-02 at 4c3784b3a1)
+ 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.
Will merge to 'master'.
cf. <xmqq5y0ssknj.fsf@gitster.g>
source: <20231116054437.2343549-1-jojwang@google.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>
* 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
(merged to 'next' on 2024-01-04 at 1237898a22)
+ 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 'master'.
source: <cover.1702823801.git.zhiyou.jx@alibaba-inc.com>
* 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>
--------------------------------------------------
[Discarded]
* jc/sparse-checkout-eoo (2023-12-21) 5 commits
. sparse-checkout: tighten add_patterns_from_input()
. sparse-checkout: use default patterns for 'set' only !stdin
. SQUASH??? end-of-options test
. sparse-checkout: take care of "--end-of-options" in set/add/check-rules
+ Merge branch 'jk/end-of-options' into jc/sparse-checkout-set-add-end-of-options
"git sparse-checkout (add|set) --[no-]cone --end-of-options" did
not handle "--end-of-options" correctly after a recent update.
Superseded by the en/sparse-checkout-eoo topic.
source: <20231221065925.3234048-1-gitster@pobox.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>
cf. <ZZWOtnP2IHNldcy6@nand.local>
source: <cover.1698101088.git.me@ttaylorr.com>
^ permalink raw reply
* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Jacob Stopak @ 2024-01-06 4:44 UTC (permalink / raw)
To: Dragan Simic; +Cc: Junio C Hamano, Oswald Buddenhagen, git
In-Reply-To: <778c4540924ad076269ac72097cf3789@manjaro.org>
On Fri, Jan 05, 2024 at 08:14:34PM +0100, Dragan Simic wrote:
>
> Just a brief update... Some of the associated patches are already ready to
> go, and some more are still pending to be implemented. It took me a while,
> which I apologize for, and now I've also unfortunately contracted some
> really bad flu. I'll be back to the patches as soon as the flu decides to
> go away.
Hey! No worries - thanks for letting me know. I have been thinking about this
and am looking forward to getting back to it.
Hope you recover quickly. Please include me when those patches are ready. Then
I will try to align my previous patches with them and also I have a bunch of
restructuring/refactoring to do based on the previous feedback from Junio.
Oh and happy new year.
^ permalink raw reply
* Re: [PATCH v3 1/1] bugreport: include +i in outfile suffix as needed
From: Jacob Stopak @ 2024-01-06 4:54 UTC (permalink / raw)
To: Emily Shaffer; +Cc: git
In-Reply-To: <ZTtZ5CbIGETy1ucV.jacob@initialcommit.io>
Hey Emily!
Hope you had a great holiday!
I think this thread might have gotten lost in the shuffle.
I had replied to your feedback a couple months ago with a few questions.
It would be great to get your further input before I continue working on
this one.
Best,
Jack
^ permalink raw reply
* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Dragan Simic @ 2024-01-06 7:06 UTC (permalink / raw)
To: Jacob Stopak; +Cc: Junio C Hamano, Oswald Buddenhagen, git
In-Reply-To: <ZZjar4li8R7Uo0c3.jacob@initialcommit.io>
On 2024-01-06 05:44, Jacob Stopak wrote:
> On Fri, Jan 05, 2024 at 08:14:34PM +0100, Dragan Simic wrote:
>>
>> Just a brief update... Some of the associated patches are already
>> ready to
>> go, and some more are still pending to be implemented. It took me a
>> while,
>> which I apologize for, and now I've also unfortunately contracted some
>> really bad flu. I'll be back to the patches as soon as the flu
>> decides to
>> go away.
>
> Hey! No worries - thanks for letting me know. I have been thinking
> about this
> and am looking forward to getting back to it.
>
> Hope you recover quickly. Please include me when those patches are
> ready. Then
> I will try to align my previous patches with them and also I have a
> bunch of
> restructuring/refactoring to do based on the previous feedback from
> Junio.
I hope to recover soon. It's been already about two miserable weeks
since the flu symptoms started, and it has since moved into my lungs,
making things much worse than usual.
Sure, I'll also include a brief summary of our earlier discussions into
the cover letter, with the links to the mailing list archives, to
provide context for the patches for everyone reviewing them later.
Looking forward to these new git features!
> Oh and happy new year.
Happy New Year to you too, and to everybody on the mailing list! :)
^ permalink raw reply
* [PATCH v6 0/1] mingw: give more details about unsafe directory's ownership
From: Sören Krecker @ 2024-01-06 11:29 UTC (permalink / raw)
To: git; +Cc: sunshine, Sören Krecker
In-Reply-To: <xmqqbka07te6.fsf@gitster.g>
So I change the commit message and the title of this commit.
Thanks for your feedback.
Sören Krecker (1):
mingw: give more details about unsafe directory's ownership
compat/mingw.c | 64 ++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 51 insertions(+), 13 deletions(-)
base-commit: e79552d19784ee7f4bbce278fe25f93fbda196fa
--
2.39.2
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox