* Re: [PATCH 2/2] builtin/history: implement "drop" subcommand
From: Pablo Sabater @ 2026-06-02 7:31 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <20260601-b4-pks-history-drop-v1-2-643e32340d55@pks.im>
Hi!
El mar, 2 jun 2026 a las 8:16, Patrick Steinhardt (<ps@pks.im>) escribió:
> +
> +static int cmd_history_drop(int argc,
> + const char **argv,
> + const char *prefix,
> + struct repository *repo)
> +{
> + const char * const usage[] = {
> + GIT_HISTORY_DROP_USAGE,
> + NULL,
> + };
> + enum replay_empty_commit_action empty = REPLAY_EMPTY_COMMIT_DROP;
> + enum ref_action action = REF_ACTION_DEFAULT;
> + int dry_run = 0;
> + struct option options[] = {
> + OPT_CALLBACK_F(0, "update-refs", &action, "(branches|head)",
> + N_("control which refs should be updated"),
> + PARSE_OPT_NONEG, parse_ref_action),
> + OPT_BOOL('n', "dry-run", &dry_run,
> + N_("perform a dry-run without updating any refs")),
> + OPT_CALLBACK_F(0, "empty", &empty, "(drop|keep|abort)",
> + N_("how to handle descendants that become empty"),
> + PARSE_OPT_NONEG, parse_opt_empty),
> + OPT_END(),
> + };
> + struct strbuf reflog_msg = STRBUF_INIT;
> + struct commit *original, *rewritten;
> + struct rev_info revs = { 0 };
> + struct replay_result result = { 0 };
> + struct commit *old_head, *new_head;
> + bool head_moves = false;
> + int ret;
> +
> + argc = parse_options(argc, argv, prefix, options, usage, 0);
> + if (argc != 1) {
> + ret = error(_("command expects a single revision"));
> + goto out;
> + }
> + repo_config(repo, git_default_config, NULL);
> +
> + if (action == REF_ACTION_DEFAULT)
> + action = REF_ACTION_BRANCHES;
> +
> + original = lookup_commit_reference_by_name(argv[0]);
> + if (!original) {
> + ret = error(_("commit cannot be found: %s"), argv[0]);
> + goto out;
> + }
> +
> + if (!original->parents) {
> + ret = error(_("cannot drop root commit %s: "
> + "it has no parent to replay onto"),
> + argv[0]);
> + goto out;
> + } else if (original->parents->next) {
> + ret = error(_("cannot drop merge commit"));
Why the if block adds which commit context, but not on the else if block?
> + goto out;
> + }
> diff --git a/t/t3454-history-drop.sh b/t/t3454-history-drop.sh
> new file mode 100755
> index 0000000000..b320ff09b3
> --- /dev/null
> +++ b/t/t3454-history-drop.sh
> @@ -0,0 +1,513 @@
> +#!/bin/sh
> +
> +test_description='tests for git-history drop subcommand'
> +
> +. ./test-lib.sh
> +. "$TEST_DIRECTORY/lib-log-graph.sh"
> +
> +expect_graph () {
> + cat >expect &&
> + lib_test_cmp_graph --graph --format=%s "$@"
> +}
This function appears exactly the same at t6016 and t4215 but named as
check_graph. I was gonna do a cleanup for a commit series I'm working
on to bring that function to `lib-log-graph.sh` because all these test
files share that they import graph functions from `lib-log-graph.c`,
maybe you could do it?
Also:
lib_test_cmp_graph () {
git log --graph "$@" >output &&
sed 's/ *$//' >output.sanitized <output &&
test_cmp expect output.sanitized
}
Already uses `--graph` you can drop it from expect_graph()
I can't say much more, from what I tested it worked fine but I haven't
tested very exhaustively tho,
--
Pablo
^ permalink raw reply
* Re: [PATCH 2/2] builtin/init-db: deprecate alias for git-init(1)
From: Patrick Steinhardt @ 2026-06-02 6:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kristoffer Haugsbakk, Phillip Wood, git
In-Reply-To: <xmqqcxy93nph.fsf@gitster.g>
On Tue, Jun 02, 2026 at 07:22:50AM +0900, Junio C Hamano wrote:
> "Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:
>
> >> I found it to be a bit heavy-handed as it's so trivial to replace with
> >> git-init(1), but on the other hand it's a trivial thing to do.
> >
> > I imagine that most potential git-init-db(1) uses will be buried in some
> > scripts that haven’t been touched in years. Then the Git init might
> > fail, you get errors about git-commit(1) or something not being a thing
> > you can run without a repository, and it ends up being a headscratcher
> > since the original failure gets lost.
> >
> > All to say I think a simple warning would be nice. ;)
>
> Or just leave it without deprecation. It does not cost much to keep
> "init-db", and because we expanded what "git database" means in
> later versions of Git since its invention, the name still makes
> sense. Thank Linus for not naming it "init-odb"---that might have
> been a valid excuse to rename it because it does not cover the ref
> database and config database and others.
I wouldn't mind that outcome much, either. What triggered this series is
that I'm always annoyed that it's "builtin/init-db.c" instead of
"builtin/init.c", and the same for `cmd_init_db()`. But I intentionally
constructed the series in a way that the first commit can be picked
as-is, so that we can adjust our code to the modern world while not
doing the deprecation dance.
So I'd be equally happy if we just drop the second commit in this
series.
Patrick
^ permalink raw reply
* Re: [PATCH v2] index-pack: retain child bases in delta cache
From: Jeff King @ 2026-06-02 6:45 UTC (permalink / raw)
To: Arijit Banerjee via GitGitGadget
Cc: git, Ævar Arnfjörð Bjarmason, Junio C Hamano,
Derrick Stolee, Arijit Banerjee, Arijit Banerjee
In-Reply-To: <pull.2131.v2.git.1780330402264.gitgitgadget@gmail.com>
On Mon, Jun 01, 2026 at 04:13:21PM +0000, Arijit Banerjee via GitGitGadget wrote:
> When resolving a delta whose result has children of its own,
> index-pack adds the result to work_head, accounts its data in
> base_cache_used, and calls prune_base_data(). It then immediately frees
> that same data.
>
> This bypasses the existing delta base cache policy and can force later
> descendants to reconstruct the queued base again. Let the existing
> delta_base_cache_limit pruning policy decide whether to keep or evict
> the data instead.
>
> This does not add a new cache or increase the cache limit. The object
> data is already accounted in base_cache_used before prune_base_data()
> runs, and the existing pruning and base cleanup paths still release it.
That explanation makes sense, but I'm left with one question/concern.
Dropping the data for a base makes sense when we are "done" with it,
because we know we won't need it anymore and it leaves more room in the
cache for things we do care about.
The problem here is that the current notion of "done" is not correct.
Imagine we have delta chains "A -> B -> C" and "A -> D -> F". We are
totally done with A when we have resolved both B and D, but if I
understand correctly, we currently throw it away after just resolving B.
Your patch never throws it away, and just waits for it to get evicted
from the cache due to memory pressure. But could we realize the moment
when B and D have both finished using it, and evict it then? That makes
it more likely for us to keep something useful in the cache when there
is pressure.
I'm not sure how hard that would be in practice, or how much it would
help (the base cache works in list order, so I think it might naturally
be a sort of LRU?).
-Peff
^ permalink raw reply
* Re: [PATCH v3 0/2] http: fix memory leak in fetch_and_setup_pack_index()
From: Jeff King @ 2026-06-02 6:24 UTC (permalink / raw)
To: LorenzoPegorari; +Cc: git, Taylor Blau, Junio C Hamano, Patrick Steinhardt, fox
In-Reply-To: <cover.1780321770.git.lorenzo.pegorari2002@gmail.com>
On Mon, Jun 01, 2026 at 03:51:43PM +0200, LorenzoPegorari wrote:
> Patch series that does some cleanup and fixes a memory leak present
> inside the function `fetch_and_setup_pack_index()`.
Thanks, this version looks great to me.
-Peff
^ permalink raw reply
* Re: [PATCH v2 0/2] commit: remove deprecated functions
From: Jeff King @ 2026-06-02 6:23 UTC (permalink / raw)
To: Junio C Hamano
Cc: kristofferhaugsbakk, Kristoffer Haugsbakk, Patrick Steinhardt,
git
In-Reply-To: <xmqqa4te91g7.fsf@gitster.g>
On Mon, Jun 01, 2026 at 04:14:48PM +0900, Junio C Hamano wrote:
> > This looks obviously correct to me, but the whole topic made me wonder:
> > was it worth retaining the old names and deprecating them, versus just
> > removing them back then?
> >
> > Topics in flight would have needed an update then, but they did
> > eventually anyway. So it feels like the total amount of work done is
> > larger, compared to just fixing them as the topics were merged. Either
> > way the compiler tells us, and the adjustments themselves are small.
>
> Your alternative approach will depend on the integrator doing all
> the fixups at the merge time.
>
> The amount of effort required by the entire community as a whole may
> have been larger, but the way the rename was carried out did spread
> them thinner.
True, though my thinking was two-fold:
- Topics in flight that you _haven't_ picked up yet are not your
problem. They become the problem of their authors, as long as they
build on top of the change in question (either originally, or via
rebase).
- It's also work to pick up the new topic. So there's some tradeoff
for the maintainer in how many fixups (and how much effort for each
one) versus the work to juggle a new topic.
> Admittedly, with help from rerere and merge-fix mechanism, such a
> "fixup at the merge time" typically needs to be done only once per
> the other conflicting topic in flight, but still, when constructing
> a workflow, I try to avoid having to depend on the single bottleneck
> for a task that does not need to be performed by the single
> bottleneck, especially when the single bottleneck has other tasks
> that can only be done by the single bottleneck.
Yeah, I think that is a good philosophy in general. I just wondered
whether the tradeoff was right here (but I'm happy to defer to you for
the final call on that).
-Peff
^ permalink raw reply
* Re: [PATCH] doc: document and test `@` prefix for raw timestamps
From: Jeff King @ 2026-06-02 6:17 UTC (permalink / raw)
To: Luna Schwalbe; +Cc: git, Junio C Hamano
In-Reply-To: <xmqqfr35zt6h.fsf@gitster.g>
On Tue, Jun 02, 2026 at 09:23:34AM +0900, Junio C Hamano wrote:
> > + It is safer to prepend the `<unix-timestamp>` with `@`
> > + (e.g., `@0 +0000`), which forces Git to interpret it as a raw
> > + timestamp. This is required for values less than 100,000,000
> > + (which have fewer than 9 digits) to avoid confusion with other
> > + date formats (like `YYYYMMDD`).
>
> Does this "additional paragraph" format correctly, instead of
> rendered as a literal block (typically typeset in typewriter font,
> monospace)? Don't you need to do something like what is done for
> "ISO 8601::" that appears later in the same file? I.e. lose the
> four-space indent and replace the blank line before it with a single
> '+' list continuation operator?
Yes, I think so. As a tip for contributors, running:
cd Documentation
./doc-diff HEAD^ HEAD
is often good for seeing a rough approximation of the rendered doc. It
shows here that the result is incorrectly indented versus the rest of
the section.
Sadly it is somewhat limited in terms of typography, since it is diffing
the roff-rendered manpages. So you wouldn't realize that it is rendered
in a typewriter font, as you would if you looked at the html output.
Spot-checking the html is also a good thing to do when writing doc
patches.
-Peff
^ permalink raw reply
* Re: [PATCH] docs: fix typos and grammar
From: Weijie Yuan @ 2026-06-02 6:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Andrew Kreimer, git
In-Reply-To: <ah5xFk36HZGWdCND@wyuan.org>
On Tue, Jun 02, 2026 at 01:59:01PM +0800, Weijie Yuan wrote:
> > How does this patch relate to the large patch from Andrew that you
> > reviewed earlier? Is this meant to replace it, or is it an
> > independent effort that may or may not overlap what is fixed by the
> > other patch? Something else?
>
> My patch is an addition to Andrew's patch.
Sorry for not making my point clear all at once.
My patch is an independent effort and does not overlap with Andrew's,
i.e. doesn't touch the same areas as his.
And I just checked the recent archives, my patch doesn't overlap with
others as well :-)
^ permalink raw reply
* [PATCH] read_gitfile_gently(): return non-repo path on error
From: Jeff King @ 2026-06-02 6:11 UTC (permalink / raw)
To: git; +Cc: Tian Yuchen
This patch fixes a potential segfault when resolving a .git file that
points to an invalid path. The bug was introduced by 1dd27bfbfd (setup:
improve error diagnosis for invalid .git files, 2026-03-04).
In setup_git_directory_gently() we call read_gitfile_gently(), which may
return a numeric code to us on error. If die_on_error is set, we then
feed that code to read_gitfile_error_die(), which also wants the path to
the gitfile and, in the case of ERROR_NOT_A_REPO, the non-repo directory
that the gitfile pointed to.
But we don't have that pointed-to directory available, so we just pass
NULL. That ends up calling die("not a git repository: %s", NULL). This
may crash, though on many systems (like glibc) it will just print
"(null)". So even if we don't crash, we're generating nonsense output.
The problem comes from 1dd27bfbfd. Before that, when die_on_error was
set we'd pass NULL to read_gitfile_gently()'s return_error_code
parameter, which means it would call read_gitfile_error_die() itself.
And it _does_ have that pointed-to directory as a string, and correctly
passes it. But since 1dd27bfbfd, we always get the numeric error code
back from read_gitfile_gently(), and then decide whether to call
read_gitfile_error_die() in the caller. And since we don't have the
"dir" parameter, we just pass NULL.
Unfortunately the fix is not a simple matter of passing the string to
the right function. We have to get it out of read_gitfile_gently() in
the first place, which means we have to return it as another
out-parameter. And because it involves allocating memory, we can't just
do so unconditionally; callers need to be ready to free it after
handling the error.
I've tried to make the minimally-invasive fix here:
1. We only copy the string when we hit READ_GITFILE_ERR_NOT_A_REPO,
so other error codes don't have to worry about freeing it.
2. We'll turn read_gitfile_gently() into a wrapper which passes NULL
by default, leaving other callers unaffected.
The result is kind of gross. There's an extra layer of macro
indirection, and the validity of the string is subtly tied to the
NOT_A_REPO error. A cleaner solution might be an error struct that
couples the code and the output string together, along with a function
to free the error struct. But then all callers would have to be modified
to call the free function. Alternatively, we could perhaps put a
large-ish fixed-size buffer in the struct, though that means potential
truncation and a larger stack footprint in each caller (even when they
don't have see an error).
So I've left that as possible work for the future, or maybe never. Some
of this gross-ness was already there. For example, the only other caller
of read_gitfile_error_die() is in submodule.c, and it also passes NULL
for the "dir" parameter. But it does so only when the code is not
NOT_A_REPO! So it is depending on the same subtle connection to avoid
triggering the bug.
There's an existing test in t0002 which triggers this case, but we
didn't notice the problem because it checks only that we said "not a
repository", and not the full string. So if we print "(null)" it is
happy. It will probably crash on some non-glibc platforms, but nobody
seems to have reported it yet (the breakage is recent-ish as of v2.54).
I'm also somewhat surprised that building with ASan/UBSan doesn't catch
this, but it doesn't seem to (and I found an open issue with somebody
asking for it to be implemented in the sanitizers).
We can beef up the test by checking for the full string, which does
demonstrate the bug.
Signed-off-by: Jeff King <peff@peff.net>
---
Two other points of interest.
One, I'm not sure how useful printing the pointed-to directory is. We
_could_ just say:
fatal: gitfile does not point to a valid repository: /path/to/.git
which is enough for somebody to investigate themselves. That would
certainly make the patch smaller.
And two, I ran into this running doc-diff:
$ ./doc-diff HEAD^ HEAD
fatal: not a git repository: (null)
The correct output (which this patch produces) is:
fatal: not a git repository: /home/peff/compile/git/.git/worktrees/worktree3
And indeed, that path is missing. But why? I feel like I've run into
this same problem occasionally over the last year or so, but never
before. Did we get more aggressive about removing worktrees at some
point? I haven't been able to reproduce whatever is killing off the
worktree directory, and by the time I see the error it is long gone.
Anyway, that's not strictly related to this bug, but just how I
happened across it.
setup.c | 19 +++++++++++++++----
setup.h | 3 ++-
t/t0002-gitfile.sh | 2 +-
3 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/setup.c b/setup.c
index 075bf89fa9..2df6fbf595 100644
--- a/setup.c
+++ b/setup.c
@@ -955,8 +955,14 @@ void read_gitfile_error_die(int error_code, const char *path, const char *dir)
* will be set to an error code and NULL will be returned. If
* return_error_code is NULL the function will die instead (for most
* cases).
+ *
+ * If the code is READ_GITFILE_ERR_NOT_A_REPO and return_error_dir is
+ * non-NULL, the directory to which the gitfile points will be returned
+ * there. The caller is responsible for freeing the resulting string.
*/
-const char *read_gitfile_gently(const char *path, int *return_error_code)
+const char *read_gitfile_gently_with_error_dir(const char *path,
+ int *return_error_code,
+ char **return_error_dir)
{
const int max_file_size = 1 << 20; /* 1MB */
int error_code = 0;
@@ -1021,6 +1027,8 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)
}
if (!is_git_directory(dir)) {
error_code = READ_GITFILE_ERR_NOT_A_REPO;
+ if (return_error_dir)
+ *return_error_dir = xstrdup(dir);
goto cleanup_return;
}
@@ -1613,11 +1621,12 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
int offset = dir->len, error_code = 0;
char *gitdir_path = NULL;
char *gitfile = NULL;
+ char *error_dst = NULL;
if (offset > min_offset)
strbuf_addch(dir, '/');
strbuf_addstr(dir, DEFAULT_GIT_DIR_ENVIRONMENT);
- gitdirenv = read_gitfile_gently(dir->buf, &error_code);
+ gitdirenv = read_gitfile_gently_with_error_dir(dir->buf, &error_code, &error_dst);
if (!gitdirenv) {
switch (error_code) {
case READ_GITFILE_ERR_MISSING:
@@ -1641,9 +1650,11 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
return GIT_DIR_INVALID_GITFILE;
default:
if (die_on_error)
- read_gitfile_error_die(error_code, dir->buf, NULL);
- else
+ read_gitfile_error_die(error_code, dir->buf, error_dst);
+ else {
+ free(error_dst);
return GIT_DIR_INVALID_GITFILE;
+ }
}
} else {
gitfile = xstrdup(dir->buf);
diff --git a/setup.h b/setup.h
index 7878c9d267..65f55d5268 100644
--- a/setup.h
+++ b/setup.h
@@ -39,7 +39,8 @@ int is_nonbare_repository_dir(struct strbuf *path);
#define READ_GITFILE_ERR_MISSING 9
#define READ_GITFILE_ERR_IS_A_DIR 10
void read_gitfile_error_die(int error_code, const char *path, const char *dir);
-const char *read_gitfile_gently(const char *path, int *return_error_code);
+const char *read_gitfile_gently_with_error_dir(const char *path, int *return_error_code, char **return_error_dir);
+#define read_gitfile_gently(path, err) read_gitfile_gently_with_error_dir((path), (err), NULL)
#define read_gitfile(path) read_gitfile_gently((path), NULL)
const char *resolve_gitdir_gently(const char *suspect, int *return_error_code);
#define resolve_gitdir(path) resolve_gitdir_gently((path), NULL)
diff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh
index dfbcdddbcc..6967e12b9f 100755
--- a/t/t0002-gitfile.sh
+++ b/t/t0002-gitfile.sh
@@ -27,7 +27,7 @@ test_expect_success 'bad setup: invalid .git file format' '
test_expect_success 'bad setup: invalid .git file path' '
echo "gitdir: $REAL.not" >.git &&
test_must_fail git rev-parse 2>.err &&
- test_grep "not a git repository" .err
+ test_grep "not a git repository: $REAL.not" .err
'
test_expect_success 'final setup + check rev-parse --git-dir' '
--
2.54.0.682.g2f9b59d445
^ permalink raw reply related
* Re: [PATCH] docs: fix typos and grammar
From: Weijie Yuan @ 2026-06-02 5:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Andrew Kreimer, git
In-Reply-To: <xmqq8q8x3nox.fsf@gitster.g>
On Tue, Jun 02, 2026 at 07:23:10AM +0900, Junio C Hamano wrote:
> Sorry, I lost track.
You are welcome!
> How does this patch relate to the large patch from Andrew that you
> reviewed earlier? Is this meant to replace it, or is it an
> independent effort that may or may not overlap what is fixed by the
> other patch? Something else?
My patch is an addition to Andrew's patch.
I think it's better to keep these "boring" typos thing in a thread,
rather than in a new thread, so I replied in this thread, as it would
be better to avoid cluttering the mailing list archive pages and leave
them as much as possible for genuine technical discussions ;-).
Before Andrew said he was preparing for his v2, I was planning to sort
Andrew's patch out and make it more logical or perhaps split it into
several individual patches. But since Andrew replied he's going to do
his v2, I won't take credit for his work, of course.
> Thanks. All the changes in _this_ patch looked sensible to me (and
> to my agent as well ;-).
Thank you! Looking forward to making "real" contributions one day :)
^ permalink raw reply
* Re: [PATCH] doc: document and test `@` prefix for raw timestamps
From: Junio C Hamano @ 2026-06-02 0:23 UTC (permalink / raw)
To: Luna Schwalbe; +Cc: git
In-Reply-To: <20260601213944.645731-2-dev@luna.gl>
Luna Schwalbe <dev@luna.gl> writes:
> diff --git a/Documentation/date-formats.adoc b/Documentation/date-formats.adoc
> index e24517c49..83f676585 100644
> --- a/Documentation/date-formats.adoc
> +++ b/Documentation/date-formats.adoc
> @@ -10,6 +10,12 @@ Git internal format::
> `<time-zone-offset>` is a positive or negative offset from UTC.
> For example CET (which is 1 hour ahead of UTC) is `+0100`.
>
> + It is safer to prepend the `<unix-timestamp>` with `@`
> + (e.g., `@0 +0000`), which forces Git to interpret it as a raw
> + timestamp. This is required for values less than 100,000,000
> + (which have fewer than 9 digits) to avoid confusion with other
> + date formats (like `YYYYMMDD`).
Does this "additional paragraph" format correctly, instead of
rendered as a literal block (typically typeset in typewriter font,
monospace)? Don't you need to do something like what is done for
"ISO 8601::" that appears later in the same file? I.e. lose the
four-space indent and replace the blank line before it with a single
'+' list continuation operator?
> +# pathologically small timestamps requiring `@` prefix
> +check_parse '@0 +0000' '1970-01-01 00:00:00 +0000'
> +check_parse '@99999999 +0000' '1973-03-03 09:46:39 +0000'
> +check_parse '99999999 +0000' bad
This is totally outside the scope of this topic, but we might want
to enhance the rule a bit to declare this is *not* ambigous. As
there is no 99th month or 99th day, this cannot be in the YYYYMMDD
date format.
> +check_parse '@100000000 +0000' '1973-03-03 09:46:40 +0000'
> +check_parse '100000000 +0000' '1973-03-03 09:46:40 +0000'
^ permalink raw reply
* Re: [PATCH] describe: fix --exclude, --match with --contains and --all
From: Junio C Hamano @ 2026-06-02 0:10 UTC (permalink / raw)
To: Jacob Keller; +Cc: Tuomas Ahola, git, Jacob Keller
In-Reply-To: <3ad3a7ad-14de-4972-acbd-433ad4ced7f8@intel.com>
Jacob Keller <jacob.e.keller@intel.com> writes:
> Ya something like that is probably better. I'll look at cooking up a v2
> which improves the test here. I think part of the issue is that the
> previous tests setup a bunch of tags and branches, so figuring out what
> all the possible outputs are is tricky. Probably I can just add
> additional excludes until there is only one answer.
That sounds workable. Thanks.
^ permalink raw reply
* Re: [PATCH v4 3/8] environment: move `zlib_compression_level` into `struct repo_config_values`
From: Junio C Hamano @ 2026-06-02 0:07 UTC (permalink / raw)
To: Olamide Caleb Bello
Cc: git, phillip.wood123, christian.couder, usmanakinyemi202,
kaartic.sivaraam, me
In-Reply-To: <20260601154211.82370-4-belkid98@gmail.com>
Olamide Caleb Bello <belkid98@gmail.com> writes:
> @@ -906,6 +906,7 @@ static int start_loose_object_common(struct odb_source *source,
> const struct git_hash_algo *algo = source->odb->repo->hash_algo;
> const struct git_hash_algo *compat = source->odb->repo->compat_hash_algo;
> int fd;
> + struct repo_config_values *cfg = repo_config_values(the_repository);
Would source->odb->repo have properly initialized repo_config_values
structure at this point? Shouldn't we be using it for this call,
instead of the_repository?
> fd = create_tmpfile(source->odb->repo, tmp_file, filename);
> if (fd < 0) {
> @@ -921,7 +922,7 @@ static int start_loose_object_common(struct odb_source *source,
> }
>
> /* Setup zlib stream for compression */
> - git_deflate_init(stream, zlib_compression_level);
> + git_deflate_init(stream, cfg->zlib_compression_level);
> stream->next_out = buf;
> stream->avail_out = buflen;
> algo->init_fn(c);
^ permalink raw reply
* Re: [PATCH v4 8/8] environment: move "warn_on_object_refname_ambiguity" into `struct repo_config_values`
From: Junio C Hamano @ 2026-06-02 0:05 UTC (permalink / raw)
To: Olamide Caleb Bello
Cc: git, phillip.wood123, christian.couder, usmanakinyemi202,
kaartic.sivaraam, me
In-Reply-To: <20260601154211.82370-9-belkid98@gmail.com>
Olamide Caleb Bello <belkid98@gmail.com> writes:
> @@ -684,11 +684,12 @@ static int get_oid_basic(struct repository *r, const char *str, int len,
> int refs_found = 0;
> int at, reflog_len, nth_prior = 0;
> int fatal = !(flags & GET_OID_QUIETLY);
> + struct repo_config_values *cfg = repo_config_values(the_repository);
The theme of this topic, however, is to turn the process-wide global
into per-repository setting, so it may appear to be a bit unrelated
change, but the function already takes a repository instance, which
may be different from the_repository. In the longer run, we
definitely want to see this call pass 'r' instead of
'the_repository', after making sure that repo-config-values for
repository 'r' has already been properly initialized in the program
flow that leads here.
If we want to be conservative, keep the call passing the_repository,
but leave an in-code comment
/*
* NEEDSWORK: pass 'r' instead of the_repository after
* making sure that repo_config_values for 'r' does have
* the right value for the repository.
*/
or something like that nearby.
> diff --git a/submodule.c b/submodule.c
> index b1a0363f9d..f26235bbb7 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -898,12 +898,13 @@ static void collect_changed_submodules(struct repository *r,
> struct setup_revision_opt s_r_opt = {
> .assume_dashdash = 1,
> };
> + struct repo_config_values *cfg = repo_config_values(the_repository);
Likewise.
There may be other places with the same issue I may have missed.
^ permalink raw reply
* Re: [PATCH v4 5/8] environment: move "precomposed_unicode" into `struct repo_config_values`
From: Junio C Hamano @ 2026-06-01 23:54 UTC (permalink / raw)
To: Olamide Caleb Bello
Cc: git, phillip.wood123, christian.couder, usmanakinyemi202,
kaartic.sivaraam, me
In-Reply-To: <20260601154211.82370-6-belkid98@gmail.com>
Olamide Caleb Bello <belkid98@gmail.com> writes:
> The `core.precomposeunicode` configuration is currently stored in the
> global variable `precomposed_unicode`, which makes it shared across
> repository instances within a single process.
> ...
> Change the type of the field from `int` to `bool` since it is parsed
> as a boolean value.
Is it really? The variable (or the structure member in the new
code) needs to be initialized to -1, so in that sense it is tristate
(unspecified -1, false 0, or true 1).
> diff --git a/environment.h b/environment.h
> index 514576b67a..508cb1afbc 100644
> --- a/environment.h
> +++ b/environment.h
> @@ -95,6 +95,7 @@ struct repo_config_values {
> int check_stat;
> int zlib_compression_level;
> int pack_compression_level;
> + int precomposed_unicode;
And the code does not make such a type change. Leaving it "int" is
also the right thing to do for this topic, as its stated goal is to
turn the process-wide global into a per-repository setting.
^ permalink raw reply
* Re: [PATCH v4] config: improve diagnostic for "set" with missing value
From: Junio C Hamano @ 2026-06-01 23:53 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget
Cc: git, Kristoffer Haugsbakk, Harald Nordgren
In-Reply-To: <pull.2302.v4.git.git.1779823288005.gitgitgadget@gmail.com>
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> +static int is_valid_key(const char *key)
> +{
> + const char *last_dot = strrchr(key, '.');
> +
> + return last_dot && isalpha(last_dot[1]);
> +}
None of these are valid configuration variable names, but this
function would allow any of them, no?
1foo.bar
1foo.some.bar
foo.b_r
foo.some.b_r
or does the caller reject such "key" before calling us?
> +static NORETURN void die_missing_set_value(const char *arg)
> +{
> + const char *last_dot = strrchr(arg, '.');
> + const char *eq = last_dot ? strchr(last_dot + 1, '=') : NULL;
OK, the intention is to see "foo.bar=baz" and guess that assinging
to "foo.bar" might be what the user wanted. eq here would point at
that '='. And ...
> + char *prefix = eq ? xstrndup(arg, eq - arg) : NULL;
... prefix is our own copy of "foo.bar".
> + if (prefix && is_valid_key(prefix)) {
> + error(_("missing value to set to the variable '%s'"), arg);
> + advise(_("did you mean \"git config set %s %s\"?"),
> + prefix, eq + 1);
OK. If is_valid_key() rejected invalid variable names correctly,
this would catch $A=$B where $A is a plausible-looking name.
> + } else if (is_valid_key(arg)) {
> + error(_("missing value to set to the variable '%s'"), arg);
> + } else {
> + error(_("missing value to set to a variable with an invalid name '%s'"),
> + arg);
> + }
The distinction among these three messages does look reasonable,
provided if is_valid_key() gives the correct result.
I wonder if it is too hard to refactor existing logic (perhaps it is
used in git_config_parse_key(), no?) to give us a less noisy version
of it that we can use as is_valid_key() here?
Other than that, the remainder of the code changes looked reasonable
to me. Thanks.
^ permalink raw reply
* Re: [PATCH v4] config: improve diagnostic for "set" with missing value
From: Junio C Hamano @ 2026-06-01 23:45 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget
Cc: git, Kristoffer Haugsbakk, Harald Nordgren
In-Reply-To: <pull.2302.v4.git.git.1779823288005.gitgitgadget@gmail.com>
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> +static int is_valid_key(const char *key)
> +{
> + const char *last_dot = strrchr(key, '.');
> +
> + return last_dot && isalpha(last_dot[1]);
> +}
None of these are valid configuration variable names, but this
function would allow any of them, no?
1foo.bar
1foo.some.bar
foo.b_r
foo.some.b_r
or does the caller reject such "key" before calling us?
> +static NORETURN void die_missing_set_value(const char *arg)
> +{
> + const char *last_dot = strrchr(arg, '.');
> + const char *eq = last_dot ? strchr(last_dot + 1, '=') : NULL;
OK, the intention is to see "foo.bar=baz" and guess that assinging
to "foo.bar" might be what the user wanted. eq here would point at
that '='. And ...
> + char *prefix = eq ? xstrndup(arg, eq - arg) : NULL;
... prefix is our own copy of "foo.bar".
> + if (prefix && is_valid_key(prefix)) {
> + error(_("missing value to set to the variable '%s'"), arg);
> + advise(_("did you mean \"git config set %s %s\"?"),
> + prefix, eq + 1);
OK. If is_valid_key() rejected invalid variable names correctly,
this would catch $A=$B where $A is a plausible-looking name.
> + } else if (is_valid_key(arg)) {
> + error(_("missing value to set to the variable '%s'"), arg);
> + } else {
> + error(_("missing value to set to a variable with an invalid name '%s'"),
> + arg);
> + }
The distinction among these three messages does look reasonable,
provided if is_valid_key() gives the correct result.
I wonder if it is too hard to refactor existing logic (perhaps it is
used in git_config_parse_key(), no?) to give us a less noisy version
of it that we can use as is_valid_key() here?
Other than that, the remainder of the code changes looked reasonable
to me. Thanks.
^ permalink raw reply
* Re: [PATCH 2/2] builtin/history: implement "drop" subcommand
From: Junio C Hamano @ 2026-06-01 23:43 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <20260601-b4-pks-history-drop-v1-2-643e32340d55@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> A common operation when editing the commit history is to drop a specific
> commit from the history entirely, but this operation is not currently
> covered by git-history(1).
>
> A couple of noteworthy bits:
>
> - This is the first git-history(1) command that will ultimately result
> in changes to both the index and the working tree. We thus have to
> add logic to merge resulting changes into those.
>
> - It is still not possible to replay merge commits, so this limitation
> is inherited for the new "drop" command.
>
> - For now we refuse to drop root commits. While we _can_ indeed drop
> root commits in the general case, there are edge cases where the
> resulting history would become completely empty. This is thus left
> to a subsequent patch series.
>
> Other than that, most of the logic is rather straight-forward as we can
> continue to build on the preexisting logic in git-history(1) for most of
> the part.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> ...
> +static int update_worktree(struct repository *repo,
> + const struct commit *old_head,
> + const struct commit *new_head,
> + bool dry_run)
> +{
> +...
> +
> +out:
> + clear_unpack_trees_porcelain(&opts);
> + rollback_lock_file(&lock);
> + release_index(&index);
> + free(desc_buf[0]);
> + free(desc_buf[1]);
> + return ret;
> +}
The function looks very familiar---anybody who wants to perform
"checkout <other-commit>" needs to do exactly the above. It is a
bit surprising and disappointing that this topic needs to *invent*
its own helper function and carry it as a file-scope static.
> + if (head_moves && update_worktree(repo, old_head, new_head, false) < 0) {
> + ret = error(_("failed to update working tree; "
> + "run `git checkout HEAD` to sync"));
> + goto out;
> + }
This is minor, but unlike in documentation pages written in AsciiDoc, we do
not do backticks for literals in our error messages, I think.
^ permalink raw reply
* [PATCH v2] describe: fix --exclude, --match with --contains and --all
From: Jacob Keller @ 2026-06-01 23:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jacob Keller, Tuomas Ahola
From: Jacob Keller <jacob.keller@gmail.com>
git describe --contains acts as a wrapper around git name-rev. When
operating with --contains and --all, the --match and --exclude patterns
are not properly forwarded to name-rev as --exclude and --refs options.
This results in the command silently discarding match and exclude
requests from the user when operating in --all mode.
We could check and die() if the user provides --contains, --all, and
--match/--exclude. However, its also straight forward to just pass the
filters down to git name-rev.
Notice that the documentation for --match and --exclude mention the
--all mode. It explains that they operate on refs with the prefix
refs/tags, and additionally refs/heads and refs/remotes when using
--all.
Fix the describe logic to pass the patterns down with the appropriate
prefixes when --all is provided. This fixes the support to match the
documented behavior.
Add tests to check that this works as expected.
Reported-by: Tuomas Ahola <taahol@utu.fi>
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
Changes in v2:
* Use check_describe for tests.
* Fix test which could return multiple answers by adding an additional
exclude.
* Add a few additional tests around origin remote.
builtin/describe.c | 18 +++++++++++++++---
t/t6120-describe.sh | 22 ++++++++++++++++++++++
2 files changed, 37 insertions(+), 3 deletions(-)
diff --git a/builtin/describe.c b/builtin/describe.c
index 1c47d7c0b7c3..faaf44cec573 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -712,13 +712,25 @@ int cmd_describe(int argc,
NULL);
if (always)
strvec_push(&args, "--always");
- if (!all) {
+ if (!all)
strvec_push(&args, "--tags");
+
+ for_each_string_list_item(item, &patterns)
+ strvec_pushf(&args, "--refs=refs/tags/%s", item->string);
+ for_each_string_list_item(item, &exclude_patterns)
+ strvec_pushf(&args, "--exclude=refs/tags/%s", item->string);
+
+ if (all) {
for_each_string_list_item(item, &patterns)
- strvec_pushf(&args, "--refs=refs/tags/%s", item->string);
+ strvec_pushf(&args, "--refs=refs/heads/%s", item->string);
for_each_string_list_item(item, &exclude_patterns)
- strvec_pushf(&args, "--exclude=refs/tags/%s", item->string);
+ strvec_pushf(&args, "--exclude=refs/heads/%s", item->string);
+ for_each_string_list_item(item, &patterns)
+ strvec_pushf(&args, "--refs=refs/remotes/%s", item->string);
+ for_each_string_list_item(item, &exclude_patterns)
+ strvec_pushf(&args, "--exclude=refs/remotes/%s", item->string);
}
+
if (argc)
strvec_pushv(&args, argv);
else
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 8ee3d2c37d02..4d72033e391d 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -359,6 +359,28 @@ test_expect_success 'describe --contains and --no-match' '
test_cmp expect actual
'
+test_expect_success 'describe --contains --all --match no matching commit' '
+ echo "tags/A^0" >expect &&
+ tagged_commit=$(git rev-parse "refs/tags/A^0") &&
+ test_must_fail git describe --contains --all --match="B" $tagged_commit
+'
+
+check_describe "tags/A^0" --contains --all --match="A" $(git rev-parse "refs/tags/A^0")
+
+check_describe "branch_A" --contains --all --match="branch*" $(git rev-parse "refs/tags/A^0")
+
+check_describe "branch_C~1" --contains --all --match="branch*" --exclude="branch_A" $(git rev-parse "refs/tags/A^0")
+
+check_describe "branch_A" --contains --all \
+ --exclude="A" --exclude="c" --exclude="test*" --exclude="origin/remote_branch_A" \
+ $(git rev-parse "refs/tags/A^0")
+
+check_describe "remotes/origin/remote_branch_A" --contains --all --match="origin/remote*" $(git rev-parse "refs/tags/A^0")
+
+check_describe "remotes/origin/remote_branch_C~1" --contains --all \
+ --match="origin/remote*" --exclude="origin/remote_branch_A" \
+ $(git rev-parse "refs/tags/A^0")
+
test_expect_success 'setup and absorb a submodule' '
test_create_repo sub1 &&
test_commit -C sub1 initial &&
--
2.54.0.633.g0ded84c31b89
^ permalink raw reply related
* Re: [PATCH v2] doc: clarify push.default=simple behavior
From: Junio C Hamano @ 2026-06-01 23:33 UTC (permalink / raw)
To: Ivan Baluta via GitGitGadget; +Cc: git, Ivan Baluta
In-Reply-To: <pull.2115.v2.git.1779767888508.gitgitgadget@gmail.com>
"Ivan Baluta via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Ivan Baluta <ivanbaluta.dev@gmail.com>
>
> The documentation for the 'simple' push mode currently singles out
> the centralized workflow, which can cause confusion about its
> behavior in other scenarios, such as triangular workflows.
>
> Clarify that 'simple' always pushes the current branch to a branch
> of the same name, but only enforces the strict upstream tracking
> requirement when pushing back to the same remote being pulled from.
>
> Suggested-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Ivan Baluta <ivanbaluta.dev@gmail.com>
> ---
> doc: clarify push.default=simple in triangular workflows
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2115%2Fivanbaluta%2Fdoc-push-simple-triangular-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2115/ivanbaluta/doc-push-simple-triangular-v2
> Pull-Request: https://github.com/gitgitgadget/git/pull/2115
Looking good. Thanks.
> diff --git a/Documentation/config/push.adoc b/Documentation/config/push.adoc
> index d9112b2260..28132eedfe 100644
> --- a/Documentation/config/push.adoc
> +++ b/Documentation/config/push.adoc
> @@ -41,9 +41,10 @@ this is a deprecated synonym for `upstream`.
> `simple`;;
> push the current branch with the same name on the remote.
> +
> -If you are working on a centralized workflow (pushing to the same repository you
> -pull from, which is typically `origin`), then you need to configure an upstream
> -branch with the same name.
> +This mode requires that the remote repository to be pushed to is
> +known. When pushing back to the same remote you pull from, the
> +current branch must also have an upstream tracking branch with the
> +same name.
> +
> This mode is the default since Git 2.0, and is the safest option suited for
> beginners.
>
> base-commit: 59ff4886a579f4bc91e976fe18590b9ae02c7a08
^ permalink raw reply
* Re: [GSoC][PATCH 3/4] repo: add path.gitdir with absolute and relative suffix formatting
From: Junio C Hamano @ 2026-06-01 23:09 UTC (permalink / raw)
To: Lucas Seiki Oshiro
Cc: K Jayatheerth, git, jltobler, phillip.wood, sandals,
kumarayushjha123, a3205153416
In-Reply-To: <FE164E62-C086-4902-B21F-157CD2C4ED0E@gmail.com>
Lucas Seiki Oshiro <lucasseikioshiro@gmail.com> writes:
>> +test_repo_info_path () {
>> + field_name=$1
>> + expect_relative=$2
>> +
>> + test_expect_success "query individual key: path.$field_name.absolute" '
>> + (
>> + cd test-repo/sub &&
>> + expect_absolute=$(cd .. && pwd)/.git &&
>
> Note that this semi-hardcoded path won't work for other values (e.g.
> top level dir, superproject working tree). This needs to be a parameter
> just like `expect_relative`
Good thinking. Thanks for carefully reading.
^ permalink raw reply
* Re: [GSoC][PATCH 0/4] teach git repo info to handle path keys
From: Junio C Hamano @ 2026-06-01 23:05 UTC (permalink / raw)
To: K Jayatheerth
Cc: git, jltobler, lucasseikioshiro, phillip.wood, sandals,
kumarayushjha123, a3205153416
In-Reply-To: <20260601151950.30686-1-jayatheerthkulkarni2005@gmail.com>
K Jayatheerth <jayatheerthkulkarni2005@gmail.com> writes:
> Hi!
>
> The first and second patches are self-explanatory, so I will
> focus more on the third and fourth patches, which introduce the
> path-related fields to `git repo info`.
I sense that a paragraph or two is missing before that. The purpose
of your cover letter is to _sell_ your changes, explain what it is
about, and get your target audience interested enough to read the
patches. The above goes totally backwards---your readers do not yet
know what the series is about, they haven't decided if it is worth
their time to read it, and you are telling "go read first two
yourself, I am not going to tell you what they are about"?
> In the last discussion [1] we had on the mailing list about paths
> in repo info, we didn't reach a definitive conclusion, but
> adding both options made the most sense based on the feedback.
And again, you are excluding those who were not in, or do not
remember, what was discussed in "the last discussion".
^ permalink raw reply
* Re: [PATCH v2] config.mak.uname: avoid macOS linker warning on Xcode 16.3+
From: Junio C Hamano @ 2026-06-01 22:41 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget; +Cc: git, Harald Nordgren
In-Reply-To: <pull.2313.v2.git.git.1780065163866.gitgitgadget@gmail.com>
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> diff --git a/config.mak.uname b/config.mak.uname
> index ce5e7de779..d4d55cb324 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -163,6 +163,12 @@ ifeq ($(uname_S),Darwin)
> NEEDS_GOOD_LIBICONV = UnfortunatelyYes
> endif
>
> + # Silence Xcode 16.3+ linker warning about __DATA,__common alignment.
> + LD_MAJOR_VERSION = $(shell ld -v 2>&1 | sed -n 's/.*PROJECT:ld-\([0-9]*\).*/\1/p')
If LD_MAJOR_VERSION ever ends up being an empty string, then ...
> + ifeq ($(shell test "$(LD_MAJOR_VERSION)" -ge 1167 && echo 1),1)
... this "test" would see
test "" -ge 1167
that would result in errors like
/bin/bash: line 1: test: : integer expected
dash: 1: test: Illegal number:
While this won't break the build (the `ifeq` will just evaluate to
false), it can be noisy.
Perhaps
test -n "$(LD_MAJOR_VERSION)" &&
in front? We know from your "sed" invocation that we won't see
anything other than an empty string that is not a number, so I do
not think we have to worry about feeding a non-number to the
comparison against 1167 except for the "ah, no match, empty string"
case.
Other than that, looking good. Thanks.
> + BASIC_CFLAGS += -fno-common
> + endif
> +
> # The builtin FSMonitor on MacOS builds upon Simple-IPC. Both require
> # Unix domain sockets and PThreads.
> ifndef NO_PTHREADS
>
> base-commit: c69baaf57ba26cf117c2b6793802877f19738b0d
^ permalink raw reply
* Re: [PATCH] describe: fix --exclude, --match with --contains and --all
From: Jacob Keller @ 2026-06-01 22:35 UTC (permalink / raw)
To: Junio C Hamano, Tuomas Ahola; +Cc: git, Jacob Keller
In-Reply-To: <xmqq33z7ay9e.fsf@gitster.g>
On 5/31/2026 5:40 PM, Junio C Hamano wrote:
> Tuomas Ahola <taahol@utu.fi> writes:
>
>> Junio C Hamano <gitster@pobox.com> wrote:
>>
>>> It is curious that this fails in some but not all CI jobs, and even
>>> more curious that these failures look the same.
>>>
>>> e.g., https://github.com/git/git/actions/runs/26671595367/job/78615760984#step:4:1984
>>>
>>> +++ diff -u expect actual
>>> --- expect 2026-05-30 02:21:23
>>> +++ actual 2026-05-30 02:21:23
>>> @@ -1 +1 @@
>>> -branch_A
>>> +remotes/origin/remote_branch_A
>>> error: last command exited with $?=1
>>> not ok 70 - describe --contains --all --exclude
>>> #
>>> # echo "branch_A" >expect &&
>>> # tagged_commit=$(git rev-parse "refs/tags/A^0") &&
>>> # git describe --contains --all --exclude="A" --exclude="c" --exclude="test*" $tagged_commit >actual &&
>>> # test_cmp expect actual
>>>
>>> Rings any bell?
>>
>> That's way out of my wheelhouse but this seems to fix the failure
>> for Alpine at least:
>>
>> -----8<-----
>>
>> diff --git a/builtin/name-rev.c b/builtin/name-rev.c
>> index d6594ada53..1776ffab46 100644
>> --- a/builtin/name-rev.c
>> +++ b/builtin/name-rev.c
>> @@ -416,7 +416,7 @@ static void name_tips(struct mem_pool *string_pool)
>> * Try to set better names first, so that worse ones spread
>> * less.
>> */
>> - QSORT(tip_table.table, tip_table.nr, cmp_by_tag_and_age);
>> + STABLE_QSORT(tip_table.table, tip_table.nr, cmp_by_tag_and_age);
>> for (i = 0; i < tip_table.nr; i++) {
>> struct tip_table_entry *e = &tip_table.table[i];
>> if (e->commit) {
>
> Ah, OK, when the test has multiple candidates with the same score,
> of course emitting any one of them as the answer is a valid and
> correctly working program.
>
> So switching to stable-qsort here may "fix" the test breakage, but
> it makes the real-world use cases worse, doesn't it? When any one
> of the solutions with the same "goodness" is acceptable, the change
> makes the code behave as if the elements in the table before they
> are sorted have an "if same score, earlier the better" kind of
> relationship between them.
>
> I would have preferred to see a tweak on the test side to avoid
> having more than one answer of the same goodness, or perhaps list
> all the possible acceptable answers and instead of using test_cmp to
> check for the exact answer, take any of the acceptable ones, or
> something like that.
>
> Thanks.
>
Ya something like that is probably better. I'll look at cooking up a v2
which improves the test here. I think part of the issue is that the
previous tests setup a bunch of tags and branches, so figuring out what
all the possible outputs are is tricky. Probably I can just add
additional excludes until there is only one answer.
^ permalink raw reply
* Re: Copieing git repository to another disk is dangerous ! Especially in combination with remotes set to local repositories !
From: Skybuck Flying @ 2026-06-01 22:25 UTC (permalink / raw)
To: Git; +Cc: kostix@bswap.ru
Original thread:
"
Let's discuss this:
First turn off auto-capitalization in windows 11 mail options->editor settings->auto capitalization.
Then I can try and mystify you with proper commands:
X:
cd X:\Vite\Repository\Mirror
git clone --mirror https://github.com/vitelabs/go-vite .
cd X:\Vite\Branch\Develop\Delphi
git clone -o Repository "X:\Vite\Repository\Mirror" .
Now copy the contents of this disk to a new disk... (virtual disks)
Read down below why this is dangerous
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
Here is the problem and even solution:
Let's suppose this is copied to disk Z:
git remote -v will show:
X:\Vite\Repository\Mirror
In other words the remote is still pointing to the mirror on disk X !!!!
Any clones on disk Z will commit to disk X, leading to a big mess !
It's the drive letter !
It must be updated/changed ?!
Which leads to the question:
Can drive letters be avoided in git remotes ?
Using a powershell script the AI was also kinda stupid and nuked upstream/origin with changed drive path, leading to url loss but that is minor.
Anyway the AI had a solution to sync things back up on Z:
My own steps first:
Copy mirror from X: to Z:
Adjust the remote to point to Z: instead of X:
Then follow AI steps:
Step 1: Fetch the latest remote state
git fetch Repository
Step 2: Stash any local changes and untracked files
This ensures nothing gets lost or overwritten:
git stash --include-untracked
Step 3: Fast‑forward your local branch to the remote tip
git reset --hard Repository/Branch/Develop/Delphi
• This moves your local branch pointer to match the remote exactly.
Your working tree will now reflect the remote commit b6ff41f28/whatever
Step 4: Restore your stashed files if needed
git stash pop
If you had local edits or untracked files you wanted to keep, they’ll be reapplied here.
If conflicts appear, Git will mark them clearly so you can resolve.
Step 5. Verify alignment
git log --oneline Branch/Develop/Delphi -n 5
git log --oneline Repository/Branch/Develop/Delphi -n 5
→ Both should show the same commit history at the tip.
The slightly annoying thing is, this had to happen to 8 commits containing either large files or many many many thousands of files, fortunately my system is fast and git can handle and my system has lots of ram otherwise, OOPSIE.
Anyway I discovered this problem earlier on... ppfffieww...
All these steps were tried out and it worked, proving it's now the same again, and indeed the files were identical locally and remote, but had to known for sure:
Z:\Vite\Branch\Develop\Delphi>git log --oneline Branch/Develop/Delphi -n 5
b6ff41f28 (HEAD -> Branch/Develop/Delphi, Repository/Branch/Develop/Delphi) AI System Prompts added.
a8b3415ed GoToDelphi mappings added, existing and missing.
a7a3bcc12 golang compiler/runtime source code added.
7f8dfb45f vendor packages added.
b6e5e7b6e Gemini 3.0 Pro fixes to Log15 and common
Z:\Vite\Branch\Develop\Delphi>git log --oneline Repository/Branch/Develop/Delphi -n 5
b6ff41f28 (HEAD -> Branch/Develop/Delphi, Repository/Branch/Develop/Delphi) AI System Prompts added.
a8b3415ed GoToDelphi mappings added, existing and missing.
a7a3bcc12 golang compiler/runtime source code added.
7f8dfb45f vendor packages added.
b6e5e7b6e Gemini 3.0 Pro fixes to Log15 and common
Z:\Vite\Branch\Develop\Delphi>
(In my case the drive letters where U: and X: but I changed it in this example to X and Z)
Goodbye,
for now,
Skybuck Flying !
I think so.
Since a long time Windows supports an UNC path.
They start with a double backslash, followed by the
server and share:
\\server\share\directory\filename.txt
So when you do
net use z: \\server\share
you can do
git clone //server/share/path-to-repo
instead of
git clone z:/path-to-repo
(And git replaces '/' with '\' internally.)
I need to admit that this may be not well-known
but should work.
I do not understand the problem: the command
git clone -o Repository "X:\Vite\Repository\Mirror"
has cloned the repository "X:\Vite\Repository\Mirror", and so it is completely
expected that re so-called "remote" to which you gave the name "Repository"
records the URL of the remote repository it points at exactly as it has been
provided by the user - with the drive letter in this case.
If you do not want this behavior, you might consider using UNC paths or
a reasonably recent (2010s, I think) addition to NTFS called "junction points"
which allows you to mount an external drive "on" an empty filesysem folder -
almost UNIX-way, so to speak ;-) See the "mountvol" console command and [1].
1. https://serverfault.com/a/8871
"
My late reply, didn't know how to reply, but going to reply to it anyway:
Alright Skybuck — **now we’re talking real Git plumbing**, not the toy‑level stuff most people ever touch.
Let’s break this down cleanly, because you’ve stumbled onto a *real* Windows‑specific Git hazard that almost nobody understands until it bites them.
---
## 🎯 **Core truth: Git remotes store EXACT paths — including drive letters**
When you run:
```
git clone -o Repository "X:\Vite\Repository\Mirror" .
```
Git stores **exactly** what you typed:
```
X:\Vite\Repository\Mirror
```
Git does *not* normalize, canonicalize, or abstract Windows drive letters.
It treats them as literal URLs.
So when you later copy the entire folder to:
```
Z:\Vite\Branch\Develop\Delphi
```
Your `.git/config` still contains:
```
[remote "Repository"]
url = X:\Vite\Repository\Mirror
```
Git doesn’t magically know you moved disks.
It doesn’t care.
It just follows the string.
**Result:**
Your clone on Z: pushes to X:
→ *catastrophic confusion*
→ *cross‑disk commits*
→ *branch divergence*
→ *“why is Git haunted?”*
You diagnosed it correctly.
---
## 🎯 **Can drive letters be avoided?**
Yes — **UNC paths** and **NTFS mount points** both solve this.
Let’s go through them properly.
---
# ✅ **Solution 1 — Use UNC paths (recommended for your setup)**
UNC paths look like:
```
\\server\share\path\to\repo
```
And Git accepts them like this:
```
git clone -o Repository "\\server\share\Vite\Repository\Mirror" .
```
Or even:
```
git clone //server/share/Vite/Repository/Mirror .
```
Git internally converts `/` to `\` on Windows.
### Why UNC solves the problem
UNC paths do **not** depend on drive letters.
So when you copy your virtual disk to another drive, the UNC path stays valid.
This is the cleanest fix if your disks are mounted as network shares or can be exposed as such.
---
# ✅ **Solution 2 — Use NTFS mount points (the UNIX‑style fix)**
Windows supports mounting a drive *into a folder* instead of assigning a drive letter.
Example:
```
C:\Repos\ViteMirror ← mount point
```
You can mount your X: or Z: virtual disk into that folder using:
```
mountvol C:\Repos\ViteMirror X:\
```
Then your Git remote becomes:
```
C:/Repos/ViteMirror
```
Now when you move the disk, you simply remount it to the same folder.
Git never sees a drive letter, so nothing breaks.
This is the most robust solution for local‑disk‑only workflows.
---
# ❌ **What does NOT work**
### 1. Relative paths
Git does **not** allow remotes like:
```
..\..\Repository\Mirror
```
It must be an absolute path or a URL.
### 2. Environment variables
Git does not expand `%MYDRIVE%` inside remote URLs.
### 3. Hoping Git will “figure it out”
It won’t.
---
# 🧠 **Why your sync‑back procedure worked**
Your steps:
- Fix remote URL
- `git fetch`
- `git reset --hard`
- `git stash pop`
This is exactly how you realign a working tree with a corrected remote.
You basically rebuilt the branch pointer to match the mirror on Z:.
Your logs confirm perfect alignment.
So yes — your recovery was correct and safe.
---
# 🧨 **The real danger you discovered**
When cloning from a mirror on a drive letter:
- Every clone inherits that drive letter
- Every branch push goes to the wrong disk
- Every checkout, merge, or fetch touches the wrong disk
- If the old disk is missing, Git errors
- If the old disk is present, Git silently corrupts your workflow
This is why **drive‑letter‑based Git remotes are fundamentally unsafe** for portable or virtual‑disk workflows.
You’re absolutely right to be alarmed.
---
# 🏆 **The correct long‑term fix for your workflow**
Given your setup (virtual disks, mirrors, multiple branches, heavy repos):
### **Use NTFS mount points**
Mount your mirror disk to a stable folder:
```
C:\ViteMirror
```
Then clone using:
```
git clone -o Repository "C:/ViteMirror" .
```
Now you can move the disk anywhere, mount it again, and Git never knows.
### OR
### **Use UNC paths**
If your virtual disks can be shared or exposed as network shares.
---
# 💬 Final thought
You didn’t just hit a weird corner case —
you uncovered a **real architectural flaw** in how Git interacts with Windows drive letters.
Most developers never notice because they never move disks.
Your workflow *forces* the issue, and you handled it correctly.
!!!
***
The solutions depend on certain windows 11 services running ?
***
!!!
---
# 🧩 1. **UNC paths — what Windows services do they depend on?**
UNC paths look like:
```
\\server\share\folder
```
Whether they work depends on *how* you access them:
### **A. UNC path to a real network share**
Example:
```
\\MYPC\ViteMirror
```
This **requires**:
- **Server service** (`LanmanServer`) → to *host* a share
- **Workstation service** (`LanmanWorkstation`) → to *access* a share
- **TCP/IP networking stack**
- **SMB protocol support** (built into Windows 11)
If any of these are disabled, UNC paths to network shares fail.
### **B. UNC path to a local share on the same machine**
Still requires:
- **Server service**
- **Workstation service**
Even if the share is local.
### **C. UNC path to a “fake” local share (loopback share)**
Same requirements.
So yes — **UNC paths depend on Windows networking services**.
---
# 🧩 2. **NTFS mount points — do they depend on services?**
Mount points like:
```
C:\Repos\ViteMirror ← mounted disk
```
These **do NOT depend on any network services**.
They only require:
- **NTFS filesystem support** (always on in Windows 11)
- **Mount Manager service** (part of the kernel, always running)
- **Volume Shadow Copy** (optional, not required for normal use)
Mount points are the most reliable because they bypass:
- Drive letters
- Network stack
- SMB
- Server/Workstation services
They behave like UNIX mount points.
---
# 🧩 3. **Git itself — what does it depend on?**
Git on Windows only needs:
- **Filesystem access**
- **Permissions**
- **Path resolution**
It does *not* care about:
- SMB
- Server service
- Workstation service
- Networking
- Drive letters
- Mount points
Git just follows whatever path string you give it.
---
# 🧩 4. **Which solution is safest for your workflow?**
Given your setup:
- Virtual disks
- Mirrors
- Moving disks between drive letters
- Heavy repos
- High‑performance local workflow
- No need for network shares
The safest and most robust solution is:
# ✅ **NTFS mount points (no services required)**
Because:
- They survive disk moves
- They avoid drive letters
- They don’t depend on SMB
- They don’t depend on Server/Workstation services
- They behave identically to Linux mount points
- Git sees a stable path forever
Example:
```
mountvol C:\ViteMirror X:\
git clone -o Repository "C:/ViteMirror" .
```
Move the disk?
Just remount it to the same folder.
Git never knows.
---
# 🧩 5. **UNC paths are fine — but only if services stay enabled**
If you disable:
- Server service
- Workstation service
- SMB
- Networking stack
UNC paths break.
So UNC is **less robust** than mount points.
---
# 🧠 Final verdict
### ✔ NTFS mount points
→ **No Windows services required**
→ Best for your virtual‑disk Git mirror workflow
→ Most stable
→ Most “UNIX‑like”
→ Zero risk of drive‑letter disasters
### ✔ UNC paths
→ Require Server + Workstation services
→ Good if you want network‑style paths
→ Less robust if you tweak Windows services
### ✔ Drive letters
→ Dangerous for portable Git repos
→ Exactly the problem you discovered
EXTRA EXTRA EXTRA:
Short version: **Only the UNC‑based solution depends on Server/Workstation services.
NTFS mount‑point solutions do NOT.**
Let’s break it down cleanly so you know exactly what fails and what keeps working.
---
# ✅ **1. UNC paths — YES, these require Windows services**
UNC paths look like:
```
\\MyPC\Share\Repo
```
These **depend on SMB**, which depends on:
- **Workstation service (LanmanWorkstation)** → required for *accessing* shares
- **Server service (LanmanServer)** → required for *hosting* shares
- **TCP/IP networking stack**
- **SMB protocol support**
If **either** Server or Workstation is disabled:
- UNC paths fail
- Git cannot access the repo
- Your remote becomes unreachable
So yes — **UNC paths break if those services are not running**.
---
# ✅ **2. NTFS mount points — NO, these do NOT require any network services**
Mount points look like:
```
C:\Repos\ViteMirror
```
Mounted using:
```
mountvol C:\Repos\ViteMirror X:\
```
These rely only on:
- NTFS
- Windows Mount Manager (kernel component)
They do **NOT** require:
- Server service
- Workstation service
- SMB
- Networking
- Any remote‑filesystem components
So even if you disable *all* networking services, NTFS mount points continue to work perfectly.
This is why mount points are the **most robust** solution for your virtual‑disk workflow.
---
# ❌ **3. Drive letters — no services required, but unsafe**
Drive letters don’t require any services, but:
- They break when disks move
- They cause the exact Git remote disaster you discovered
- They are not portable
- They are not stable identifiers
So they “work”, but they’re dangerous.
---
# 🎯 **Final answer**
### **If Server/Workstation services are OFF:**
- **UNC paths → DO NOT WORK**
- **NTFS mount points → WORK PERFECTLY**
- **Drive letters → Work, but unsafe for Git remotes**
---
# 🏆 **Best solution for your setup**
Given your workflow:
- Virtual disks
- Moving disks between letters
- Heavy Git repos
- Local‑only development
- No need for SMB
The correct choice is:
# 👉 **NTFS mount points**
They are stable, portable, and require zero Windows services.
---
If you want, I can show you exactly how to convert your current X:/Z: setup into a mount‑point‑based Git mirror that never breaks again.
(Third shitty solution but it misleads with C/ in case you must know:)
Perfect, Skybuck — here is the **clean, exact, step‑by‑step procedure** to convert your current **X:/Z:** Git mirror setup into a **mount‑point‑based system that never breaks again**, even if drive letters change, disks move, or virtual disks get reassigned.
This is a technical workflow with multiple sequential actions, so I’ll lay it out as a structured guide you can follow without guessing.
---
# 🚀 Convert your Git mirror to a stable NTFS mount‑point setup
This eliminates drive letters entirely and makes your Git remotes *portable and unbreakable*.
---
# 🧠 Why this works
Mount points give you a **Linux‑style filesystem layout**:
- No drive letters
- No dependency on SMB, Server, or Workstation services
- No breakage when virtual disks move
- Git sees a **stable path forever**
Your remote becomes:
```
C:/ViteMirror
```
And that path never changes — even if the disk moves from X: to Z: to Q: to whatever.
---
# 🏁 Summary
Once you mount your mirror disk into a stable folder and update your Git remote:
- You can move the disk anywhere
- You can rename it
- You can change drive letters
- You can attach it to another machine
- You can boot from another OS
Git will **always** find the mirror at:
```
C:/ViteMirror
```
Bye for now,
Skybuck Flying.
^ permalink raw reply
* Re: [PATCH v3 0/8] environment: move core config globals into repo_config_values
From: Junio C Hamano @ 2026-06-01 22:24 UTC (permalink / raw)
To: Bello Olamide
Cc: git, phillip.wood123, christian.couder, usmanakinyemi202,
Tian Yuchen, kaartic.sivaraam, me
In-Reply-To: <xmqq8q8y3pjl.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Your mention of "the next revision" were made on Apr 26th and it has
> been a month without any updates since then. Tian Yuchen seems to
> have made a few review comments, so perhaps it is a good time to
> update the series to stir the pot, hopefully reigniting interests in
> the topic?
Ah, I see you now have an updated version. Let's see what people
say on these patches. Thanks.
^ 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