* Re: [PATCH v2 0/3] contrib/subtree: reduce recursion during split
From: Ian Jackson @ 2026-06-02 9:06 UTC (permalink / raw)
To: Junio C Hamano
Cc: Colin Stagner, git, Christian Heusel, george, Christian Hesse,
Phillip Wood
In-Reply-To: <xmqqv7c13o5l.fsf@gitster.g>
Junio C Hamano writes ("Re: [PATCH v2 0/3] contrib/subtree: reduce recursion during split"):
> So after this message the thread went dark (except for a side
> discussion about rewriting subtree in Rust, which I do think it is a
> good direction to go in the longer term).
I'm indeed still working on this. Given other things on my plate it
will be months rather than weeks before I have anything anyone one
might want to use.
> While I do agree that avoiding bash-isms in the main part of Git and
> sticking to vanilla POSIX has merit, this particular one seems more
> like an artificial limit imposed by dash than sticking to the POSIX
> as the common denoninator, at least to me.
I would be in favour of switching to bash, making bash a dependency
for this script. We could use the env trick to support platforms that
don't have it in /bin.
Ian.
--
Ian Jackson <ijackson@chiark.greenend.org.uk> These opinions are my own.
Pronouns: they/he. If I emailed you from @fyvzl.net or @evade.org.uk,
that is a private address which bypasses my fierce spamfilter.
^ permalink raw reply
* Re: [PATCH 3/4] t/lib-git-p4: silence output when killing p4d and its watchdog
From: Junio C Hamano @ 2026-06-02 9:32 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <20260602-pks-t7527-fix-tap-output-v1-3-db3da2a1b137@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> stop_p4d_and_watchdog () {
> kill -9 $p4d_pid $watchdog_pid
> + wait $p4d $watchdog_pid 2>/dev/null
> }
Shoudln't we be waiting on $p4d_pid (not $p4d)...
> @@ -175,7 +176,7 @@ retry_until_success () {
>
> stop_and_cleanup_p4d () {
> kill -9 $p4d_pid $watchdog_pid
> - wait $p4d_pid
> + wait $p4d_pid $watchdog_pid 2>/dev/null
> rm -rf "$db" "$cli" "$pidfile"
> }
... like we do here?
^ permalink raw reply
* Re: [PATCH v2] doc: document and test `@` prefix for raw timestamps
From: Junio C Hamano @ 2026-06-02 9:20 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Luna Schwalbe, git
In-Reply-To: <ah6X0X9EQdL6hn53@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> You can use a tool like b4, which can nowadays be configured exactly
> like this with `git config set b4.send-same-thread shallow`. b4 overall
> makes all of the mailing list wrangling a ton easier. Makes me wonder
> whether we should maybe highlight this tool more in our docs.
It would be a very much welcome change.
And "b4" also helps the receiving end, not just the sender. With
"b4" finding the latest round automatically, getting updated round
once a topic is in my tree is a breeze, thanks to the amlog notes.
Essentially, I detach the HEAD at the previous base commit (often a
bit older 'master', but a few prerequisite topis merged into it),
give "b4 am -o-" a message-id of one of the patches I have queued to
download the latest and pipe that to "git am -s". That way, both
"git range-diff @{-1}..." and "git diff @{-1}" become very effective
ways to see what changes were made to the topic.
^ permalink raw reply
* Re: [PATCH v2] doc: document and test `@` prefix for raw timestamps
From: Junio C Hamano @ 2026-06-02 9:14 UTC (permalink / raw)
To: Luna Schwalbe; +Cc: git
In-Reply-To: <20260602081924.673763-2-dev@luna.gl>
Luna Schwalbe <dev@luna.gl> writes:
> The Git internal date format `<unix-timestamp> <time-zone-offset>`
> fails to parse when the timestamp is less than 100,000,000 (fewer than
> 9 digits). This happens to avoid potential ambiguity with other date
> formats such as `YYYYMMDD`, especially when used with approxidate.
>
> To force the parser to interpret the value as a raw timestamp, it must
> be prefixed with `@` (e.g., `@0 +0000`). This behavior was introduced
> in 2c733fb24c10a9d7aacc51f956bf9b7881980870 (parse_date(): '@' prefix
> forces git-timestamp, 2012-02-02) but was never documented.
>
> Document the `@` prefix in `Documentation/date-formats.adoc` to make
> this behavior explicit. Also add test cases to `t/t0006-date.sh` to
> verify and demonstrate the difference between prefixed and unprefixed
> small timestamps (e.g., `@2000` vs `2000`).
>
> Signed-off-by: Luna Schwalbe <dev@luna.gl>
> Co-authored-by: Junio C Hamano <gitster@pobox.com>
> ---
> Fixed the asciidoc formatting, removed parens around YYYYMMDD example.
Looks good.
Next time, when sending a [v2] patch of a singleton topic,
please make it a reply e-mail message to the [v1] message. That
would make it easier to see how patches evolved in mailing list
archive. E.g., https://lore.kernel.org/git/$message_id would show
both rounds (including the review comments they received) on the
same page.
Will queue. Thanks.
> Documentation/date-formats.adoc | 5 +++++
> t/t0006-date.sh | 11 +++++++++++
> 2 files changed, 16 insertions(+)
>
> diff --git a/Documentation/date-formats.adoc b/Documentation/date-formats.adoc
> index e24517c49..330424b2b 100644
> --- a/Documentation/date-formats.adoc
> +++ b/Documentation/date-formats.adoc
> @@ -9,6 +9,11 @@ Git internal format::
> `<unix-timestamp>` is the number of seconds since the UNIX epoch.
> `<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`.
>
> RFC 2822::
> The standard date format as described by RFC 2822, for example
> diff --git a/t/t0006-date.sh b/t/t0006-date.sh
> index 53ced36df..8b4e1870b 100755
> --- a/t/t0006-date.sh
> +++ b/t/t0006-date.sh
> @@ -138,6 +138,13 @@ check_parse '1969-12-31 23:59:59 Z' bad
> check_parse '1969-12-31 23:59:59 +11' bad
> check_parse '1969-12-31 23:59:59 -11' bad
>
> +# 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
> +check_parse '@100000000 +0000' '1973-03-03 09:46:40 +0000'
> +check_parse '100000000 +0000' '1973-03-03 09:46:40 +0000'
> +
> REQUIRE_64BIT_TIME=HAVE_64BIT_TIME
> check_parse '2099-12-31 23:59:59' '2099-12-31 23:59:59 +0000'
> check_parse '2099-12-31 23:59:59 +00' '2099-12-31 23:59:59 +0000'
> @@ -195,6 +202,10 @@ check_approxidate '6AM, June 7, 2009' '2009-06-07 06:00:00'
> check_approxidate '2008-12-01' '2008-12-01 19:20:00'
> check_approxidate '2009-12-01' '2009-12-01 19:20:00'
>
> +# ambiguous raw timestamp
> +check_approxidate '2000 +0000' '2000-08-30 19:20:00'
> +check_approxidate '@2000 +0000' '1970-01-01 00:33:20'
> +
> check_date_format_human() {
> t=$(($GIT_TEST_DATE_NOW - $1))
> echo "$t -> $2" >expect
^ permalink raw reply
* Re: [PATCH] doc: document and test `@` prefix for raw timestamps
From: Junio C Hamano @ 2026-06-02 9:10 UTC (permalink / raw)
To: Luna Schwalbe; +Cc: git
In-Reply-To: <2194e42a-5c41-44e1-ba36-1599cbd41415@luna.gl>
Luna Schwalbe <dev@luna.gl> writes:
> > 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?
>
> Terribly sorry, you're right of course, I somehow forgot to actually
> build and check the docs. Will send an updated patch right away.
No need to be sorry---we all make mistakes.
> As far as I can tell the rule is technically not necessary at all (apart
> from some unusual approxidate interpretations like the `2000 +0000`
> example, which I honestly think are more confusing than useful), seeing
> that YYYYMMDD isn't a supported format anywhere.
Sounds good. In any case, that is totally outside the scope of this
patch.
Thanks.
^ permalink raw reply
* [PATCH 2/2] SubmittingPatches: describe cover letter
From: Junio C Hamano @ 2026-06-02 9:08 UTC (permalink / raw)
To: git
In-Reply-To: <20260602090808.87837-1-gitster@pobox.com>
We talk about how a commit log message should look like, but do not
give advice on writing the cover letter to sell a series to widest
possible audience.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/SubmittingPatches | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index dec8aea4cb..8ff1792b9b 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -472,6 +472,25 @@ highlighted above.
Only capitalize the very first letter of the trailer, i.e. favor
"Signed-off-by" over "Signed-Off-By" and "Acked-by:" over "Acked-By".
+[[cover-letter]]
+=== Cover Letter
+
+The purpose of your cover letter is to sell your changes, explain what
+they are about, and get your target audience interested enough to read
+the patches.
+
+. Make sure your target audience can understand what the patches are
+ about and why they are needed without prior context.
+
+. For a second or subsequent iteration of the same topic, make sure
+ people who missed the earlier discussion can still understand what
+ the patches are about, so they can judge if the topic is worth their
+ time to read and comment on.
+
+. To help those who are familiar with earlier iterations, give a
+ summary of changes since the previous rounds.
+
+
[[ai]]
=== Use of Artificial Intelligence (AI)
--
2.54.0-567-gf25c749695
^ permalink raw reply related
* [PATCH 1/2] SubmittingPatches: separate typofixes section
From: Junio C Hamano @ 2026-06-02 9:08 UTC (permalink / raw)
To: git
In-Reply-To: <20260602090808.87837-1-gitster@pobox.com>
The existing text said something about tests (with [[tests]] to make
it easier to refer to it from elsewhere) and then flowed into a
different topic of typofixes, but it was unclear where the latter
started. Add a similar [[typofies]] marker to the document.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/SubmittingPatches | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index d570184ec8..dec8aea4cb 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -237,6 +237,7 @@ Do not forget to update the documentation to describe the updated
behavior and make sure that the resulting documentation set formats
well (try the Documentation/doc-diff script).
+[[typofixes]]
We currently have a liberal mixture of US and UK English norms for
spelling and grammar, which is somewhat unfortunate. A huge patch that
touches the files all over the place only to correct the inconsistency
--
2.54.0-567-gf25c749695
^ permalink raw reply related
* [PATCH 0/2] Small updates to SubmittingPatches
From: Junio C Hamano @ 2026-06-02 9:08 UTC (permalink / raw)
To: git
Recently I gave some advice on how a cover letter should
try to sell the idea to widest possible audience, and then
I realized that we do not seem to teach how in our guides.
Here is a small series to do so.
1/2: SubmittingPatches: separate typofixes section
2/2: SubmittingPatches: describe cover letter
Documentation/SubmittingPatches | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
--
2.54.0-567-gf25c749695
^ permalink raw reply
* [PATCH 4/4] t: let prove fail when parsing invalid TAP output
From: Patrick Steinhardt @ 2026-06-02 8:54 UTC (permalink / raw)
To: git
In-Reply-To: <20260602-pks-t7527-fix-tap-output-v1-0-db3da2a1b137@pks.im>
To make the result of our tests accessible we use the TAP protocol. This
protocol is parsed by either prove or by Meson. Unfortunately, these two
tools differ when it comes to their strictness when parsing the
protocol:
- Prove by default happily accepts lines not specified by the
protocol.
- Meson will also accept such lines, but prints a big and ugly warning
message.
We have fixed our test suite in the past to not print invalid TAP lines
anymore via b1dc2e796e (Merge branch 'ps/meson-tap-parse', 2025-06-17).
But as none of our tools perform a strict check it's still possible for
broken tests to sneak back in, like for example in 362f69547f (Merge
branch 'ps/t1006-tap-fix', 2025-07-16). This doesn't hurt at all when
using prove, but it's quite annoying when using Meson due to the
generated warnings.
Unfortunately, there doesn't seem to be a portable way to make all tools
complain about violations of the TAP format. The TAP 14 specification
has added pragmas to the protocol that would allow us to say `pragma
+strict`, and the effect of that would be to treat invalid TAP lines as
a test failure. But the release of TAP 14 is still rather recent, and
Test-Harness for example only gained support for it in version 3.48,
which was released in 2023.
In fact though, this pragma was already introduced as an inofficial
extension of the TAP protocol with Test-Harness 3.10, released in 2008.
So while not all tools understand the pragma, at least prove does for a
long time.
Unconditionally enable the pragma when using prove so that we'll detect
tests that emit broken TAP output right away. This would have detected
the issues fixed in preceding commits:
$ prove t7527-builtin-fsmonitor.sh
t7527-builtin-fsmonitor.sh .. All 69 subtests passed
(less 6 skipped subtests: 63 okay)
Test Summary Report
-------------------
t7527-builtin-fsmonitor.sh (Wstat: 0 Tests: 69 Failed: 0)
Parse errors: Unknown TAP token: "Initialized empty Git repository in /tmp/git/test_fsmonitor_smoke/.git/"
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/test-lib.sh | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index d1d24c4124..ceefb99bff 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1532,6 +1532,12 @@ then
BAIL_OUT 'You need to build test-tool; Run "make t/helper/test-tool" in the source (toplevel) directory'
fi
+if test -n "$HARNESS_ACTIVE"
+then
+ say "TAP version 13"
+ say "pragma +strict"
+fi
+
# Are we running this test at all?
remove_trash=
this_test=${0##*/}
--
2.54.0.1064.gd145956f57.dirty
^ permalink raw reply related
* [PATCH 3/4] t/lib-git-p4: silence output when killing p4d and its watchdog
From: Patrick Steinhardt @ 2026-06-02 8:54 UTC (permalink / raw)
To: git
In-Reply-To: <20260602-pks-t7527-fix-tap-output-v1-0-db3da2a1b137@pks.im>
When stopping the p4d watchdog process via "kill -9", the shell may
print a job-control notification like:
./test-lib.sh: line 1269: 57960 Killed: 9 while true; do
if test $nr_tries_left -eq 0; then
kill -9 $p4d_pid; exit 1;
fi; sleep 1; nr_tries_left=$(($nr_tries_left - 1));
done 2> /dev/null 4>&2 (wd: ~)
This message is printed asynchronously by the shell when it reaps the
process. While harmless right now, this will cause breakage once we
enable strict parsing of the TAP protocol in a subsequent commit.
Fix this by using `wait` so that we can synchronously reap the watchdog
process and swallow the diagnostic.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/lib-git-p4.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index d22e9c684a..0afa5111ab 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -65,6 +65,7 @@ pidfile="$TRASH_DIRECTORY/p4d.pid"
stop_p4d_and_watchdog () {
kill -9 $p4d_pid $watchdog_pid
+ wait $p4d $watchdog_pid 2>/dev/null
}
# git p4 submit generates a temp file, which will
@@ -175,7 +176,7 @@ retry_until_success () {
stop_and_cleanup_p4d () {
kill -9 $p4d_pid $watchdog_pid
- wait $p4d_pid
+ wait $p4d_pid $watchdog_pid 2>/dev/null
rm -rf "$db" "$cli" "$pidfile"
}
--
2.54.0.1064.gd145956f57.dirty
^ permalink raw reply related
* [PATCH 2/4] t/test-lib: silence EBUSY errors on Windows during test cleanup
From: Patrick Steinhardt @ 2026-06-02 8:54 UTC (permalink / raw)
To: git
In-Reply-To: <20260602-pks-t7527-fix-tap-output-v1-0-db3da2a1b137@pks.im>
When tests have finished we clean up the trash directory via `rm -rf`.
On Windows this can fail with EBUSY in cases where a process still holds
some of the files open, for example when we have spawned a daemonized
process that wasn't properly terminated. We thus retry several times,
but every failure will result in error messages being printed, and that
in turn breaks the TAP output format.
One such case where this is causing issues is in t921x, which contains
tests related to Scalar. Some tests spawn the fsmonitor daemon, and we
never properly terminate it.
The obvious fix would be to ensure that we never leak any processes, but
that gets ugly fast. Instead, let's work around the issue by silencing
error messages printed by the `rm -rf` calls. We already know to print
an error when the retry loop fails, so we don't loose much.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/test-lib.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 4a7357b547..d1d24c4124 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1299,10 +1299,10 @@ test_done () {
error "Tests passed but trash directory already removed before test cleanup; aborting"
cd "$TRASH_DIRECTORY/.." &&
- rm -fr "$TRASH_DIRECTORY" || {
+ rm -fr "$TRASH_DIRECTORY" 2>/dev/null || {
# try again in a bit
sleep 5;
- rm -fr "$TRASH_DIRECTORY"
+ rm -fr "$TRASH_DIRECTORY" 2>/dev/null
} ||
error "Tests passed but test cleanup failed; aborting"
fi
--
2.54.0.1064.gd145956f57.dirty
^ permalink raw reply related
* [PATCH 1/4] t7527: fix broken TAP output
From: Patrick Steinhardt @ 2026-06-02 8:54 UTC (permalink / raw)
To: git
In-Reply-To: <20260602-pks-t7527-fix-tap-output-v1-0-db3da2a1b137@pks.im>
Before running the tests in t7527 we first verify whether the fsmonitor
even works, which seems to depend on the actual filesystem that is in
use. The verification executes outside of any prerequisite or test body,
so its stdout/stderr is not being redirected.
The consequence of this is that any command that prints to stdout/stderr
may break the TAP specification by printing invalid lines. And in fact
we already do that, as git-init(1) prints the path to the created Git
repository by default.
Fix this issue by moving the logic into a lazy prerequisite.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/t7527-builtin-fsmonitor.sh | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh
index b63c162f9b..d881e27466 100755
--- a/t/t7527-builtin-fsmonitor.sh
+++ b/t/t7527-builtin-fsmonitor.sh
@@ -25,7 +25,8 @@ maybe_timeout () {
"$@"
fi
}
-verify_fsmonitor_works () {
+
+test_lazy_prereq FSMONITOR_WORKS '
git init test_fsmonitor_smoke || return 1
GIT_TRACE_FSMONITOR="$PWD/smoke.trace" &&
@@ -50,9 +51,9 @@ verify_fsmonitor_works () {
ret=$?
rm -rf test_fsmonitor_smoke smoke.trace
return $ret
-}
+'
-if ! verify_fsmonitor_works
+if ! test_have_prereq FSMONITOR_WORKS
then
skip_all="filesystem does not deliver fsmonitor events (container/overlayfs?)"
test_done
--
2.54.0.1064.gd145956f57.dirty
^ permalink raw reply related
* [PATCH 0/4] t: fix broken TAP output
From: Patrick Steinhardt @ 2026-06-02 8:54 UTC (permalink / raw)
To: git
Hi,
this small patch series fixes another instance of broken TAP output that
has landed via 4d11b9c218 (Merge branch 'pt/fsmonitor-linux', 2026-05-31).
As this has happened multiple times by now I decided to have a look at
whether we can fix this class of issues a bit more holistically. So this
series also contains a change that makes prove bail out when it sees
invalid TAP output, which uncovers a small set of preexisting issues in
our test suite.
Thanks!
Patrick
---
Patrick Steinhardt (4):
t7527: fix broken TAP output
t/test-lib: silence EBUSY errors on Windows during test cleanup
t/lib-git-p4: silence output when killing p4d and its watchdog
t: let prove fail when parsing invalid TAP output
t/lib-git-p4.sh | 3 ++-
t/t7527-builtin-fsmonitor.sh | 7 ++++---
t/test-lib.sh | 10 ++++++++--
3 files changed, 14 insertions(+), 6 deletions(-)
---
base-commit: 1666c1265231b0bc5f613fbbf3f0a9896cdef76e
change-id: 20260601-pks-t7527-fix-tap-output-105da1d73df0
^ permalink raw reply
* Re: [PATCH v2] doc: document and test `@` prefix for raw timestamps
From: Patrick Steinhardt @ 2026-06-02 8:44 UTC (permalink / raw)
To: Luna Schwalbe; +Cc: git, Junio C Hamano
In-Reply-To: <20260602081924.673763-2-dev@luna.gl>
On Tue, Jun 02, 2026 at 10:17:36AM +0200, Luna Schwalbe wrote:
> The Git internal date format `<unix-timestamp> <time-zone-offset>`
> fails to parse when the timestamp is less than 100,000,000 (fewer than
> 9 digits). This happens to avoid potential ambiguity with other date
> formats such as `YYYYMMDD`, especially when used with approxidate.
>
> To force the parser to interpret the value as a raw timestamp, it must
> be prefixed with `@` (e.g., `@0 +0000`). This behavior was introduced
> in 2c733fb24c10a9d7aacc51f956bf9b7881980870 (parse_date(): '@' prefix
> forces git-timestamp, 2012-02-02) but was never documented.
>
> Document the `@` prefix in `Documentation/date-formats.adoc` to make
> this behavior explicit. Also add test cases to `t/t0006-date.sh` to
> verify and demonstrate the difference between prefixed and unprefixed
> small timestamps (e.g., `@2000` vs `2000`).
>
> Signed-off-by: Luna Schwalbe <dev@luna.gl>
> Co-authored-by: Junio C Hamano <gitster@pobox.com>
One nit: the order of trailers is wrong, as your Signed-off-by trailer
should always be the last line of the commit message.
It would also be great to send the new version of a series as a reply to
the previous version, so that it becomes easier for reviewers to connect
the two series.
You can use a tool like b4, which can nowadays be configured exactly
like this with `git config set b4.send-same-thread shallow`. b4 overall
makes all of the mailing list wrangling a ton easier. Makes me wonder
whether we should maybe highlight this tool more in our docs.
The patch itself looks good to me, thanks!
Patrick
^ permalink raw reply
* Re: [PATCH] read_gitfile_gently(): return non-repo path on error
From: Patrick Steinhardt @ 2026-06-02 8:36 UTC (permalink / raw)
To: Jeff King; +Cc: git, Tian Yuchen
In-Reply-To: <20260602061159.GA693928@coredump.intra.peff.net>
On Tue, Jun 02, 2026 at 02:11:59AM -0400, Jeff King wrote:
[snip]
> 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.
I have to agree that the patch is somewhat gross, and I myself don't
really see much of an issue to move to an error message like the above
if it ends up simplifying the logic.
> 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.
Both git-gc(1) and git-maintenance(1) prune orphaned worktrees that are
older than three months by default, which can be configured via
"gc.worktreePruneExpire". That logic has changed in 4dda60c9df (Merge
branch 'ps/maintenance-missing-tasks', 2025-05-15), which would kind of
match your timeline.
But rereading that patch series I cannot really see how it could result
in more aggressive pruning of worktrees. We used `git worktree prune
--expire <expiry>` before that series, and we still use that logic now.
Hum.
> diff --git a/setup.c b/setup.c
> index 075bf89fa9..2df6fbf595 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -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;
> + }
The `if` branch should also gain some curly braces here.
Patrick
^ permalink raw reply
* Re: [PATCH 2/2] builtin/init-db: deprecate alias for git-init(1)
From: Junio C Hamano @ 2026-06-02 8:27 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Kristoffer Haugsbakk, Phillip Wood, git
In-Reply-To: <ah58IJ8DgSZYRjMM@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> 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.
I'd actually find myself annoyed by such a rename when looking for
builtin/init-db.c only to find it gone---much like how a previous
rename made ll-merge difficult to locate.
My point is that while static names may annoy some, renaming them
does not resolve the annoyance; it merely shifts it to someone else.
So, if the primary motivation is just the first patch, I would be
less inclined to support this series.
^ permalink raw reply
* [PATCH v2] doc: document and test `@` prefix for raw timestamps
From: Luna Schwalbe @ 2026-06-02 8:17 UTC (permalink / raw)
To: git; +Cc: Luna Schwalbe, Junio C Hamano
The Git internal date format `<unix-timestamp> <time-zone-offset>`
fails to parse when the timestamp is less than 100,000,000 (fewer than
9 digits). This happens to avoid potential ambiguity with other date
formats such as `YYYYMMDD`, especially when used with approxidate.
To force the parser to interpret the value as a raw timestamp, it must
be prefixed with `@` (e.g., `@0 +0000`). This behavior was introduced
in 2c733fb24c10a9d7aacc51f956bf9b7881980870 (parse_date(): '@' prefix
forces git-timestamp, 2012-02-02) but was never documented.
Document the `@` prefix in `Documentation/date-formats.adoc` to make
this behavior explicit. Also add test cases to `t/t0006-date.sh` to
verify and demonstrate the difference between prefixed and unprefixed
small timestamps (e.g., `@2000` vs `2000`).
Signed-off-by: Luna Schwalbe <dev@luna.gl>
Co-authored-by: Junio C Hamano <gitster@pobox.com>
---
Fixed the asciidoc formatting, removed parens around YYYYMMDD example.
Documentation/date-formats.adoc | 5 +++++
t/t0006-date.sh | 11 +++++++++++
2 files changed, 16 insertions(+)
diff --git a/Documentation/date-formats.adoc b/Documentation/date-formats.adoc
index e24517c49..330424b2b 100644
--- a/Documentation/date-formats.adoc
+++ b/Documentation/date-formats.adoc
@@ -9,6 +9,11 @@ Git internal format::
`<unix-timestamp>` is the number of seconds since the UNIX epoch.
`<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`.
RFC 2822::
The standard date format as described by RFC 2822, for example
diff --git a/t/t0006-date.sh b/t/t0006-date.sh
index 53ced36df..8b4e1870b 100755
--- a/t/t0006-date.sh
+++ b/t/t0006-date.sh
@@ -138,6 +138,13 @@ check_parse '1969-12-31 23:59:59 Z' bad
check_parse '1969-12-31 23:59:59 +11' bad
check_parse '1969-12-31 23:59:59 -11' bad
+# 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
+check_parse '@100000000 +0000' '1973-03-03 09:46:40 +0000'
+check_parse '100000000 +0000' '1973-03-03 09:46:40 +0000'
+
REQUIRE_64BIT_TIME=HAVE_64BIT_TIME
check_parse '2099-12-31 23:59:59' '2099-12-31 23:59:59 +0000'
check_parse '2099-12-31 23:59:59 +00' '2099-12-31 23:59:59 +0000'
@@ -195,6 +202,10 @@ check_approxidate '6AM, June 7, 2009' '2009-06-07 06:00:00'
check_approxidate '2008-12-01' '2008-12-01 19:20:00'
check_approxidate '2009-12-01' '2009-12-01 19:20:00'
+# ambiguous raw timestamp
+check_approxidate '2000 +0000' '2000-08-30 19:20:00'
+check_approxidate '@2000 +0000' '1970-01-01 00:33:20'
+
check_date_format_human() {
t=$(($GIT_TEST_DATE_NOW - $1))
echo "$t -> $2" >expect
--
2.53.0
^ permalink raw reply related
* Re: [PATCH] doc: document and test `@` prefix for raw timestamps
From: Luna Schwalbe @ 2026-06-02 8:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqfr35zt6h.fsf@gitster.g>
> 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?
Terribly sorry, you're right of course, I somehow forgot to actually
build and check the docs. Will send an updated patch right away.
> 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.
I agree there is room for change with this rule, although I'm not sure
how sensible it is to start allowing certain values based on whether
they are also a valid calendar date or not (we'd end up trying to parse
YYYYMMDD first, and only afterwards do the actual timestamp parsing; I
feel like this might just make the system less predictable for users in
practice).
As far as I can tell the rule is technically not necessary at all (apart
from some unusual approxidate interpretations like the `2000 +0000`
example, which I honestly think are more confusing than useful), seeing
that YYYYMMDD isn't a supported format anywhere.
If we want to have it as a safeguard tho, better documentation is
probably the most important aspect. As a user, ideally I'd love to get a
"ambiguous date format, prefix with @ if you intend to specify a raw
timestamp" kind of error message, but I suspect that might be difficult
to implement.
^ permalink raw reply
* Re: [PATCH v4 3/8] environment: move `zlib_compression_level` into `struct repo_config_values`
From: Patrick Steinhardt @ 2026-06-02 8:12 UTC (permalink / raw)
To: Junio C Hamano
Cc: Olamide Caleb Bello, git, phillip.wood123, christian.couder,
usmanakinyemi202, kaartic.sivaraam, me
In-Reply-To: <xmqqpl29ztx7.fsf@gitster.g>
On Tue, Jun 02, 2026 at 09:07:32AM +0900, Junio C Hamano wrote:
> 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?
I think as an intermediate step it's okay-ish to use `the_repository`,
as it doesn't make the status quo any worse. But ideally, we'd have a
follow-up patch series that converts "object-file.c" to drop the
dependency on `the_repository` completely, which will be easier after
this patch series here has landed as there will only be a handful more
config options to migrate:
- `pack_compression_level` and `zlib_compression_level` get migrated
in this series.
- `object_creation_mode` still needs migration.
- `pack_size_limit_cfg` still needs migration.
Other than that we really only need to use the correct repo in a small
set of functions.
Overall, I think it's sensible to always use `the_repository` at the
callsites in a patch series like this so that it's obvious that there is
no change in behaviour. So every patch series that gets rid of global
state in a subsystem X will basically bubble up the global state into
the next-higher level, and it's then the duty of the next patch series
to address that next-higher level.
The only exception of course is subsystems that already got rid of
`the_repository` -- we really shouldn't reintroduce the use there.
Patrick
^ permalink raw reply
* Re: [PATCH] read_gitfile_gently(): return non-repo path on error
From: Jeff King @ 2026-06-02 8:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Tian Yuchen
In-Reply-To: <xmqq4ijlz8vc.fsf@gitster.g>
On Tue, Jun 02, 2026 at 04:42:15PM +0900, Junio C Hamano wrote:
> > 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.
>
> Thanks. While reading the main explanation, it was the first thing
> that came to me.
Here's what that looks like, for reference. It is nice and simple, if we
think the change in error message is acceptable. I hate to change
user-facing error messages because of internal code details, but I
really do wonder if the existing message is the most useful thing to
print in the first place.
diff --git a/setup.c b/setup.c
index 075bf89fa9..ed86671d84 100644
--- a/setup.c
+++ b/setup.c
@@ -920,7 +920,7 @@ int verify_repository_format(const struct repository_format *format,
return 0;
}
-void read_gitfile_error_die(int error_code, const char *path, const char *dir)
+void read_gitfile_error_die(int error_code, const char *path)
{
switch (error_code) {
case READ_GITFILE_ERR_NOT_A_FILE:
@@ -940,7 +940,8 @@ void read_gitfile_error_die(int error_code, const char *path, const char *dir)
case READ_GITFILE_ERR_NO_PATH:
die(_("no path in gitfile: %s"), path);
case READ_GITFILE_ERR_NOT_A_REPO:
- die(_("not a git repository: %s"), dir);
+ die(_("gitfile does not point to a valid repository: %s"),
+ path);
default:
BUG("unknown error code");
}
@@ -1031,7 +1032,7 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)
if (return_error_code)
*return_error_code = error_code;
else if (error_code)
- read_gitfile_error_die(error_code, path, dir);
+ read_gitfile_error_die(error_code, path);
free(buf);
return error_code ? NULL : path;
@@ -1641,7 +1642,7 @@ 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);
+ read_gitfile_error_die(error_code, dir->buf);
else
return GIT_DIR_INVALID_GITFILE;
}
diff --git a/setup.h b/setup.h
index 7878c9d267..436aaa22c1 100644
--- a/setup.h
+++ b/setup.h
@@ -38,7 +38,7 @@ int is_nonbare_repository_dir(struct strbuf *path);
#define READ_GITFILE_ERR_TOO_LARGE 8
#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);
+void read_gitfile_error_die(int error_code, const char *path);
const char *read_gitfile_gently(const char *path, int *return_error_code);
#define read_gitfile(path) read_gitfile_gently((path), NULL)
const char *resolve_gitdir_gently(const char *suspect, int *return_error_code);
diff --git a/submodule.c b/submodule.c
index a939ff5072..c36732ca0b 100644
--- a/submodule.c
+++ b/submodule.c
@@ -2578,7 +2578,7 @@ void absorb_git_dir_into_superproject(const char *path,
if (err_code != READ_GITFILE_ERR_NOT_A_REPO)
/* We don't know what broke here. */
- read_gitfile_error_die(err_code, path, NULL);
+ read_gitfile_error_die(err_code, path);
/*
* Maybe populated, but no git directory was found?
diff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh
index dfbcdddbcc..6356e9ec72 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 "gitfile does not point to a valid repository" .err
'
test_expect_success 'final setup + check rev-parse --git-dir' '
^ permalink raw reply related
* [BUG] t/perf scripts lose GIT_PERF_* when used with --verbose-log
From: Jeff King @ 2026-06-02 7:56 UTC (permalink / raw)
To: git
Imagine I have a perf script like this:
#!/bin/sh
test_description=foo
. ./perf-lib.sh
echo >&2 "large_repo = $GIT_PERF_LARGE_REPO"
test_perf_large_repo
[...some actual tests...]
If I run the command below, I'd expect it to use linux.git as the test
repo (and print to stderr telling me so). And it does:
$ GIT_PERF_LARGE_REPO=/path/to/linux.git ./p1234-foo.sh
large_repo = /path/to/linux.git
[...]
This is courtesy of 32b74b9809 (perf: do allow `GIT_PERF_*` to be
overridden again, 2025-04-04). But that breaks if we use --tee or any
other option which implies it:
$ GIT_PERF_LARGE_REPO=/path/to/linux.git ./p1234-foo.sh --verbose-log
large_repo = /home/peff/compile/git/t/..
[...]
What happens in the happy path is this:
0. The script sources perf-lib.sh.
1. perf-lib.sh stashes GIT_PERF_* in a variable to restore later.
2. perf-lib.sh sources GIT-BUILD-OPTIONS, which overwrites the
environment.
3. perf-lib.sh sources test-lib.sh.
4. test-lib.sh itself sources GIT-BUILD-OPTIONS.
5. Eventually test-lib.sh finishes, returning control to perf-lib.sh.
6. perf-lib.sh restores GIT_PERF_* from the stashed copy. All is well.
But if --tee or --verbose-log is used, then step 5 never happens!
Instead test-lib.sh re-executes a second copy of the script piped to
tee. And that re-executed copy sees the environment we had after step 4,
with all of GIT_PERF_* coming from GIT-BUILD-OPTIONS. So even though it
tries to do the save/restore, its step 1 never sees the original
environment (so it "saves" nothing useful).
This is especially insidious if you use the "./run" program to compare
versions. It reads GIT-BUILD-OPTIONS, too, and also knows how to
preserve GIT_PERF_*, courtesy of 79d301c767 (t/perf/run: preserve
GIT_PERF_* from environment, 2026-01-06). But it reads GIT_TEST_OPTS
from the build-options file and runs each script with it. So while this
may work:
$ GIT_PERF_LARGE_REPO=/path/to/linux.git ./run HEAD p1234-foo.sh
[...]
=== Running 1 tests in /home/peff/compile/git/t/perf/build/1211f0ef99a75931f170bc2a838172a45300ad63/bin-wrappers ===
large_repo = /path/to/linux.git
[...]
you may get spooky action at a distance from whenever you last ran make:
$ make -C ../.. GIT_TEST_OPTS=--verbose-log
$ GIT_PERF_LARGE_REPO=/path/to/linux.git ./run HEAD p1234-foo.sh
[...]
=== Running 1 tests in /home/peff/compile/git/t/perf/build/1211f0ef99a75931f170bc2a838172a45300ad63/bin-wrappers ===
large_repo = /home/peff/compile/git/t/..
Doubly confusing if that GIT_TEST_OPTS is in your config.mak (because
you want normal "make test" to run under prove but still keep logs, and
you put it in the file ages ago).
I don't think this can be fixed by perf-lib.sh. The problem is internal
to test-lib.sh, which is overwriting the environment when it sources
GIT-BUILD-OPTIONS, without any opportunity for perf-lib to act before
getting re-executed. It would require test-lib.sh itself to have some
notion of "here are some stashed variables; restore them via eval".
Which just feels like stacking band-aids upon band-aids. The original
problem started with 4638e8806e (Makefile: use common template for
GIT-BUILD-OPTIONS, 2024-12-06), though one could argue that even before
then the precedence rules were kind of sketchy (it just made things much
worse because now it crops up even if you don't set GIT_PERF_LARGE_REPO
in your config.mak at all).
So I dunno. I couldn't quite bring myself to write a patch, but I
thought I'd at least write a warning to the list in case anybody else is
bit by it.
-Peff
^ permalink raw reply
* Re: [PATCH 2/2] builtin/init-db: deprecate alias for git-init(1)
From: Kristoffer Haugsbakk @ 2026-06-02 7:54 UTC (permalink / raw)
To: Patrick Steinhardt, Junio C Hamano; +Cc: Phillip Wood, git
In-Reply-To: <ah58IJ8DgSZYRjMM@pks.im>
On Tue, Jun 2, 2026, at 08:45, Patrick Steinhardt wrote:
> On Tue, Jun 02, 2026 at 07:22:50AM +0900, Junio C Hamano wrote:
>> "Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:
>>>[snip]
>> 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.
Could it be worthwhile to mark it as soft deprecated? In the sense that
it is a legacy alias that is not planned for removal?
What I think was mistake in topic jc/you-still-use-whatchanged was that
git-whatchanged(1) was not explicitly marked as deprecated before that
series, and then it started failing without a new `--i-still-use-this`
flag. The doc before that said:
New users are encouraged to use git-log(1) instead. The
`whatchanged` command is essentially the same as git-log(1) but
defaults to showing the raw format diff output and skipping merges.
The command is primarily kept for historical reasons; fingers of
many people who learned Git long before `git log` was invented by
reading the Linux kernel mailing list are trained to type it.
Reading between the lines, this looks like a soft deprecation. Then
there were emails that said that there was no prior warning. And then
someone replied to that saying that it had really been deprecated for
over a decade because that was the intent.[1] But IMO just saying
something to the effect of soft deprecated would have been better
(before it got hard deprecated).
Trying to simulate amnesia, I think just the word “init-db” looks
slightly legacy, and the fact that the documentation just links to
git-init(1) solidifies that. On the other hand git-stage(1) was
introduced as a better name for “staging” files and that too just links
to git-add(1). So you have two commands which just link to other
commands, but one is definitely more deprecated than the other.
† 1: But “trained fingers” reading the man page every other year on the
off chance that there are new developments? That’s another
question.
^ permalink raw reply
* Re: [PATCH] read_gitfile_gently(): return non-repo path on error
From: Junio C Hamano @ 2026-06-02 7:42 UTC (permalink / raw)
To: Jeff King; +Cc: git, Tian Yuchen
In-Reply-To: <20260602061159.GA693928@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> 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.
Nice, probably. I do not know what to feel about the first point,
though, as it burdens those who add new callers in the future more.
> 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).
None of thoese are particularly appetizing ;-).
> 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.
Yup. I can agree with this.
> ---
> 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.
Thanks. While reading the main explanation, it was the first thing
that came to me.
The implementation and the test are as expected in patches from you
and matches the intent explained in the log message exactly.
Thanks, will queue.
^ permalink raw reply
* [PATCH v3] config.mak.uname: avoid macOS linker warning on Xcode 16.3+
From: Harald Nordgren via GitGitGadget @ 2026-06-02 7:37 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2313.v2.git.git.1780065163866.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
Building on macOS with Xcode 16.3 or newer emits:
ld: warning: reducing alignment of section __DATA,__common
from 0x8000 to 0x4000 because it exceeds segment maximum
alignment
Pass -fno-common when "ld -v" reports ld-1167 or newer, so tentative
definitions of large arrays go into BSS instead of __DATA,__common.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
fix macOS linker warning
Check for empty LD_MAJOR_VERSION.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2313%2FHaraldNordgren%2Fpkt-line-init-buffer-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2313/HaraldNordgren/pkt-line-init-buffer-v3
Pull-Request: https://github.com/git/git/pull/2313
Range-diff vs v2:
1: 0e660a346e ! 1: f864912c53 config.mak.uname: avoid macOS linker warning on Xcode 16.3+
@@ config.mak.uname: ifeq ($(uname_S),Darwin)
+ # 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')
-+ ifeq ($(shell test "$(LD_MAJOR_VERSION)" -ge 1167 && echo 1),1)
++ ifeq ($(shell test -n "$(LD_MAJOR_VERSION)" && test "$(LD_MAJOR_VERSION)" -ge 1167 && echo 1),1)
+ BASIC_CFLAGS += -fno-common
+ endif
+
config.mak.uname | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/config.mak.uname b/config.mak.uname
index f9a5ad9720..8719e09f66 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -173,6 +173,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')
+ ifeq ($(shell test -n "$(LD_MAJOR_VERSION)" && test "$(LD_MAJOR_VERSION)" -ge 1167 && echo 1),1)
+ 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: 1666c1265231b0bc5f613fbbf3f0a9896cdef76e
--
gitgitgadget
^ permalink raw reply related
* 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
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