Junio C Hamano [Mon, 14 Jul 2025 18:19:26 +0000 (11:19 -0700)]
Merge branch 'kn/clang-format-updates'
Update ".clang-format" and ".editorconfig" to match our style guide
a bit better.
* kn/clang-format-updates:
meson: add rule to run 'git clang-format'
clang-format: add 'RemoveBracesLLVM' to the main config
clang-format: set 'ColumnLimit' to 0
Junio C Hamano [Mon, 14 Jul 2025 18:19:25 +0000 (11:19 -0700)]
Merge branch 'kh/doc-config-subcommands'
Documentation updates.
* kh/doc-config-subcommands:
config: mention --url in the synopsis
config: use --value instead of value-pattern
config: document --[no-]value
config: use --value=<pattern> consistently
config: document --[no-]show-names
Junio C Hamano [Mon, 14 Jul 2025 18:19:25 +0000 (11:19 -0700)]
Merge branch 'mc/netrc-service-names'
"netrc" credential helper has been improved to understand textual
service names (like smtp) in addition to the numeric port numbers
(like 25).
* mc/netrc-service-names:
contrib: better support symbolic port names in git-credential-netrc
contrib: warn for invalid netrc file ports in git-credential-netrc
contrib: use a more portable shebang for git-credential-netrc
Junio C Hamano [Mon, 14 Jul 2025 18:19:24 +0000 (11:19 -0700)]
Merge branch 'ps/use-reftable-as-default-in-3.0'
The reftable ref backend has matured enough; Git 3.0 will make it
the default format in a newly created repositories by default.
* ps/use-reftable-as-default-in-3.0:
setup: use "reftable" format when experimental features are enabled
BreakingChanges: announce switch to "reftable" format
Junio C Hamano [Mon, 14 Jul 2025 18:19:23 +0000 (11:19 -0700)]
Merge branch 'ac/prune-wo-the-repository'
Some code paths in the "git prune" used to ignore passed in
repository object and used the_repository singleton instance
instead, which has been corrected.
* ac/prune-wo-the-repository:
builtin/prune: stop depending on 'the_repository'
repository: move 'repository_format_precious_objects' to repo scope
Junio C Hamano [Tue, 8 Jul 2025 22:49:19 +0000 (15:49 -0700)]
Merge branch 'kn/fetch-push-bulk-ref-update'
"git push" and "git fetch" are taught to update refs in batches to
gain performance.
* kn/fetch-push-bulk-ref-update:
receive-pack: handle reference deletions separately
refs/files: skip updates with errors in batched updates
receive-pack: use batched reference updates
send-pack: fix memory leak around duplicate refs
fetch: use batched reference updates
refs: add function to translate errors to strings
In a recent security release, 05e9cd64ee (config: quote values
containing CR character, 2025-05-19) added calls to `git config get`,
`git config set`, and `git config unset` which are not present on the
maint-2.43 branch.
These subcommands were added in the following commits, released in
git-2.46.0:
Taylor Blau [Tue, 8 Jul 2025 18:47:50 +0000 (14:47 -0400)]
Documentation/RelNotes: use .adoc extension for new security releases
When preparing the latest round of security fixes, we wrote release
notes in v2.43.7, and then successively merged those up through to the
various 'maint' branches.
However, the 2.49 release series is the first to have commit 1f010d6bdf
(doc: use .adoc extension for AsciiDoc files, 2025-01-20). This means
that we should have renamed the new-but-historical release notes from
*.txt to *.adoc during the merge into the 'maint-2.49' branch, but
neglected to do so.
Rename them accordingly to match the convention introduced by 1f010d6bdf. Since the release materials in question here were prepared
before v2.50.0 was tagged, the 'maint' track for that release series is
OK as is.
Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Mon, 7 Jul 2025 21:12:55 +0000 (14:12 -0700)]
Merge branch 'jk/submodule-remote-lookup-cleanup'
Updating submodules from the upstream did not work well when
submodule's HEAD is detached, which has been improved.
* jk/submodule-remote-lookup-cleanup:
submodule: look up remotes by URL first
submodule: move get_default_remote_submodule()
submodule--helper: improve logic for fallback remote name
remote: remove the_repository from some functions
dir: move starts_with_dot(_dot)_slash to dir.h
remote: fix tear down of struct remote
remote: remove branch->merge_name and fix branch_release()
builtin/gc: correct total_ram calculation with HAVE_BSD_SYSCTL
The calls to sysctl() assume a 64-bit memory size for the variable
holding the value, but the actual size depends on the key name and
platform, at least for HW_PHYSMEM.
Detect any mismatched reads, and retry with a shorter variable
when needed.
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Refactor builtin/prune.c to remove the dependency on the global
'the_repository'. Replace all the occurrences of 'the_repository' with
repo and thus remove the definition '#define
USE_THE_REPOSITORY_VARIABLE'. Also, add a test to make sure that 'git
prune -h' can be called when the repository is `NULL`.
Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com> Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
repository: move 'repository_format_precious_objects' to repo scope
The 'extensions.preciousObjects' setting when set true, prevents
operations that might drop objects from the object storage. This setting
is populated in the global variable
'repository_format_precious_objects'.
Move this global variable to repo scope by adding it to 'struct
repository and also refactor all the occurences accordingly.
This change is part of an ongoing effort to eliminate global variables,
improve modularity and help libify the codebase.
Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com> Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Han Young [Thu, 3 Jul 2025 07:45:02 +0000 (15:45 +0800)]
read-cache: report lock error when refreshing index
In the repo_refresh_and_write_index of read-cache.c, we return -1 to
indicate that writing the index to disk failed.
However, callers do not use this information. Commands such as stash print
"could not write index"
and then exit, which does not help to discover the exact problem.
We can let repo_hold_locked_index print the error message if the locking
failed.
Signed-off-by: Han Young <hanyang.tony@bytedance.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Thu, 3 Jul 2025 22:44:28 +0000 (18:44 -0400)]
setup_revisions(): turn on diffs for all-negative diff filter
When the user gives us a diff filter like --diff-filter=D, we need to do
a tree diff even if we're not planning to show the diff result itself,
in order to decide whether to show the commit at all. So there's an
explicit check of revs->diffopt.filter in setup_revisions(), and we set
revs->diff if any bits are set.
Originally that "filter" field covered both positive capital-letter
filters (like "D") and also negative lowercase filters (like "d"), so it
was sufficient for both cases. But later, 75408ca949 (diff-filter: be
more careful when looking for negative bits, 2022-01-28) split the
negative bits out into a "filter_not" field.
We eventually fold those into "filter", but not until diff_setup_done()
is called, which happens after our explicit check. As a result, a purely
negative filter like:
git log --diff-filter=d
failed to turn on diffs at all. But rather than fail to filter by diff,
because the filter variable is eventually set, we mistakenly show no
commits at all, thinking that the empty diffs were cases where nothing
passed through the filter.
The smallest fix here is to just have our check look for any bits in
either "filter" or "filter_not". I suspect it would also be OK to
reorder the function a bit to call diff_setup_done() earlier, but that
risks violating some other subtle ordering dependency. So I went with
the simple and safe solution here.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
setup: use "reftable" format when experimental features are enabled
With the preceding commit we have announced the switch to the "reftable"
format in Git 3.0 for newly created repositories. The format is being
battle tested by GitLab and a couple of other developers, and except for
a small handful of issues exposed early after it has been merged it has
been rock solid. Regardless of that though the test user base is still
comparatively small, which increases the risk that we miss critical
bugs.
Address this by enabling the reftable format when experimental features
are enabled. This should increase the test user base by some margin and
thus give us more input before making the format the default.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
BreakingChanges: announce switch to "reftable" format
The "reftable" format has come a long way and has matured nicely since
it has been merged into git via 57db2a094d5 (refs: introduce reftable
backend, 2024-02-07). It fixes longstanding issues that cannot be fixed
with the "files" format in a backwards-compatible way and performs
significantly better in many use cases.
Announce that we will switch to the "reftable" format in Git 3.0 for
newly created repositories and wire up the change, hidden behind the
WITH_BREAKING_CHANGES preprocessor define.
This switch is dependent on support in the larger Git ecosystem. Most
importantly, libraries like JGit, libgit2 and Gitoxide should support
the reftable backend so that we don't break all applications and tools
built on top of those libraries.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Wed, 2 Jul 2025 19:08:04 +0000 (12:08 -0700)]
Merge branch 'ag/imap-send-resurrection'
"git imap-send" has been broken for a long time, which has been
resurrected and then taught to talk OAuth2.0 etc.
* ag/imap-send-resurrection:
imap-send: fix minor mistakes in the logs
imap-send: display the destination mailbox when sending a message
imap-send: display port alongwith host when git credential is invoked
imap-send: add ability to list the available folders
imap-send: enable specifying the folder using the command line
imap-send: add PLAIN authentication method to OpenSSL
imap-send: add support for OAuth2.0 authentication
imap-send: gracefully fail if CRAM-MD5 authentication is requested without OpenSSL
imap-send: fix memory leak in case auth_cram_md5 fails
imap-send: fix bug causing cfg->folder being set to NULL
config.mak.uname: set NO_MEMMEM only for functional version
FreeBSD 6 introduced memmem(), but the implementation diverged
from what was standard everywhere else (including our "compat"
fallback).
FreeBSD 10.4 (went EOL in 2018) corrected the functionality bugs
but kept a suboptimal implementation until FreeBSD 11.4 (the last
version of FreeBSD 11, that went EOL in September 2021).
Let's draw the line to require FreeBSD 12 or newer, which allows us
to drop the special casing of FreeBSD 4.x and rely on the platform
implementation of memmem() unconditionally for all versions that are
still being supported.
Suggested-by: Brad Smith <brad@comstyle.com> Helped-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The Makefile has a 'style' rule to run 'git clang-format'. While Meson
intrinsically supports a 'clang-format' target, which can be run when
using the ninja backend by running 'ninja clang-format', this runs the
formatting on all existing files.
Our Meson build doesn't yet support a way to run 'git clang-format',
which runs the formatter between the working directory and commit
provided. Add a new 'style' target to Meson to mimic the target in the
Makefile.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
clang-format: add 'RemoveBracesLLVM' to the main config
In 1b8f306612 (ci/style-check: add `RemoveBracesLLVM` in CI job,
2024-07-23) we added 'RemoveBracesLLVM' to the CI job of running the
clang formatter.
This rule checks and warns against using braces on simple
single-statement bodies of statements. Since we haven't had any issues
regarding this rule, we can now move it into the main clang-format
config and remove it from being CI exclusive.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When clang-format was introduced to the Git project in 6134de6ac1 (clang-format: outline the git project's coding style,
2017-08-14), the 'ColumnLimit' was set to 80. This is inline with our
recommendation in 'Documentation/CodingGuidelines', which states:
We try to keep to at most 80 characters per line.
However while this is recommended limit, this is not the enforced
limit. In some cases in we do overflow this limit to prioritize
readability. Setting the 'ColumnLimit' also means that shorter lines are
concatenated to simply as the result would still be below 80 characters,
which is undesirable.
In the past, we tried to adjust the penalties around line wrapping, once
in 42efde4c29 (clang-format: adjust line break penalties, 2017-09-29)
and another time in 5e9fa0f9fa (clang-format: re-adjust line break
penalties, 2024-10-18). While these settings help tweak the line break
penalties to be more in-line with the requirements of the Git project,
using 'clang-format' still produces a lot of false positives.
So to make 'clang-format' more usable, set the 'ColumnLimit' to 0. This
means that line-wrapping is no-longer a concern of the formatter and
something that the user needs to take care of. The previous commit also
added a more flexible guideline to the '.editorconfig' setting a
'max_line_length' of 120 characters. This should provide some guidance
to users.
In the future, it would be nice to re-instate this limit with adequate
penalties which would follow our guidelines, but currently, it makes
more sense to have a working formatter which we can rely on and which
doesn't create too many false positives.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Tue, 1 Jul 2025 21:17:25 +0000 (14:17 -0700)]
send-pack: clean-up even when taking an early exit
Previous commit has plugged one leak in the normal code path, but
there is an early exit that leaves without releasing any resources
acquired in the function.
This option was introduced in a series of commits from fe3ccc7aab (Merge
branch 'ps/config-subcommands', 2024-05-15) and deprecated
`value-pattern`. But `value-pattern` is still used throughout the doc.
The deprecated modes have been quarantined in the “Deprecated Modes”
section. So let’s only use `--value=<pattern>` in the rest of the doc.
Acked-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
These options were introduced in a series of commits from fe3ccc7aab (Merge branch 'ps/config-subcommands', 2024-05-15).[1]
But they were not documented here.
Document this option and the negated form according to the current
convention.[2]
[1]: `--value` is a replacement for the `value-pattern`
positional argument
[2]: https://lore.kernel.org/git/xmqqcyct1mtq.fsf@gitster.g/
Acked-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This option was introduced in a series of commits from fe3ccc7aab (Merge
branch 'ps/config-subcommands', 2024-05-15). But two styles were used
for the value provided to the option:
These options were introduced in 4e513890008 (builtin/config:
introduce "get" subcommand, 2024-05-06) but not documented here.
Use the description from the source code.
Document this option and the negated form according to the current
convention.[1]
`--show-names` is also the default when `--get-regexp` is given. But
don’t mention it here since all the deprecated modes are quarantined in
the “Deprecated Modes” section.
FreeBSD 13.4 is no longer supported, and 13.5 will be the last
release from that series, so jump instead to 14.3 which should
be supported for another 10 months and will be at that point
the oldest supported release with the interim release of 15.
While at it, move some variables to the environment and make
sure to skip a git grep test that assumes glibc regex.
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Mon, 30 Jun 2025 21:30:31 +0000 (14:30 -0700)]
Merge branch 'jc/merge-compact-summary'
"git merge/pull" has been taught the "--compact-summary" option to
use the compact-summary format, intead of diffstat, when showing
the summary of the incoming changes.
* jc/merge-compact-summary:
merge/pull: extend merge.stat configuration variable to cover --compact-summary
merge/pull: add the "--compact-summary" option
Junio C Hamano [Mon, 30 Jun 2025 21:30:30 +0000 (14:30 -0700)]
Merge branch 'bc/stash-export-import'
An interchange format for stash entries is defined, and subcommand
of "git stash" to import/export has been added.
* bc/stash-export-import:
builtin/stash: provide a way to import stashes from a ref
builtin/stash: provide a way to export stashes to a ref
builtin/stash: factor out revision parsing into a function
object-name: make get_oid quietly return an error
Junio C Hamano [Mon, 30 Jun 2025 21:30:30 +0000 (14:30 -0700)]
Merge branch 'jc/cocci-avoid-regexp-constraint'
Avoid regexp_constraint and instead use comparison_constraint when
listing functions to exclude from application of coccinelle rules,
as spatch can be built with different regexp engine X-<.
daemon: correctly handle soft accept() errors in service_loop
Since df076bdbcc ([PATCH] GIT: Listen on IPv6 as well, if available.,
2005-07-23), the original error checking was included in an inner loop
unchanged, where its effect was different.
Instead of retrying, after a EINTR during accept() in the listening
socket, it will advance to the next one and try with that instead,
leaving the client waiting for another round.
Make sure to retry with the same listener socket that failed originally.
To avoid an unlikely busy loop, fallback to the old behaviour after a
couple of attempts.
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Acked-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jacob Keller [Fri, 27 Jun 2025 22:09:04 +0000 (15:09 -0700)]
send-pack: clean up extra_have oid array
Commit c8009635785e ("fetch-pack, send-pack: clean up shallow oid
array", 2024-09-25) cleaned up the shallow oid array in cmd_send_pack,
but didn't clean up extra_have, which is still leaked at program exit.
I suspect the particular tests in t5539 don't trigger any additions to
the extra_have array, which explains why the tests can pass leak free
despite this gap.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
daemon: remove unnecesary restriction for listener fd
Since df076bdbcc ([PATCH] GIT: Listen on IPv6 as well, if available.,
2005-07-23), any file descriptor assigned to a listening socket was
validated to be within the range to be used in an FDSET later.
6573faff34 (NO_IPV6 support for git daemon, 2005-09-28), moves to
use poll() instead of select(), that doesn't have that restriction,
so remove the original check.
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Acked-by: Phillip Wood <phillip.wood123@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Wed, 25 Jun 2025 21:07:36 +0000 (14:07 -0700)]
Merge branch 'ps/maintenance-ref-lock'
"git maintenance" lacked the care "git gc" had to avoid holding
onto the repository lock for too long during packing refs, which
has been remedied.
* ps/maintenance-ref-lock:
builtin/maintenance: fix locking race when handling "gc" task
builtin/gc: avoid global state in `gc_before_repack()`
usage: allow dying without writing an error message
builtin/maintenance: fix locking race with refs and reflogs tasks
builtin/maintenance: split into foreground and background tasks
builtin/maintenance: fix typedef for function pointers
builtin/maintenance: extract function to run tasks
builtin/maintenance: stop modifying global array of tasks
builtin/maintenance: mark "--task=" and "--schedule=" as incompatible
builtin/maintenance: centralize configuration of explicit tasks
builtin/gc: drop redundant local variable
builtin/gc: use designated field initializers for maintenance tasks
Junio C Hamano [Wed, 25 Jun 2025 21:07:35 +0000 (14:07 -0700)]
Merge branch 'jc/you-still-use-whatchanged'
"git whatchanged" that is longer to type than "git log --raw"
which is its modern rough equivalent has outlived its usefulness
more than 10 years ago. Plan to deprecate and remove it.
* jc/you-still-use-whatchanged:
whatschanged: list it in BreakingChanges document
whatchanged: remove when built with WITH_BREAKING_CHANGES
whatchanged: require --i-still-use-this
tests: prepare for a world without whatchanged
doc: prepare for a world without whatchanged
you-still-use-that??: help deprecating commands for removal
Maxim Cournoyer [Wed, 25 Jun 2025 14:25:11 +0000 (23:25 +0900)]
contrib: better support symbolic port names in git-credential-netrc
To improve support for symbolic port names in netrc files, this
changes does the following:
- Treat symbolic port names as ports, not protocols in git-credential-netrc
- Validate the SMTP server port provided to send-email
- Convert the above symbolic port names to their numerical values.
Before this change, it was not possible to have a SMTP server port set
to "smtps" in a netrc file (e.g. Emacs' ~/.authinfo.gpg), as it would
be registered as a protocol and break the match for a "smtp" protocol
host, as queried for by git-send-email.
Signed-off-by: Maxim Cournoyer <maxim@guixotic.coop> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Maxim Cournoyer [Wed, 25 Jun 2025 14:25:09 +0000 (23:25 +0900)]
contrib: use a more portable shebang for git-credential-netrc
While the installed scripts have their Perl shebang set to PERL_PATH,
it is nevertheless useful to be able to run the uninstalled script for
manual tests while developing. This change makes the shebang more
portable by having the perl command looked from PATH instead of from a
fixed location.
Signed-off-by: Maxim Cournoyer <maxim@guixotic.coop> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In 9d2962a7c4 (receive-pack: use batched reference updates, 2025-05-19)
we updated the 'git-receive-pack(1)' command to use batched reference
updates. One edge case which was missed during this implementation was
when a user pushes multiple branches such as:
Before using batched updates, the references would be applied
sequentially and hence no conflicts would arise. With batched updates,
while the first update applies, the second fails due to D/F conflict. A
similar issue was present in 'git-fetch(1)' and was fixed by separating
out reference pruning into a separate transaction in the commit 'fetch:
use batched reference updates'. Apply a similar mechanism for
'git-receive-pack(1)' and separate out reference deletions into its own
batch.
This means 'git-receive-pack(1)' will now use up to two transactions,
whereas before using batched updates it would use _at least_ two
transactions. So using batched updates is still the better option.
Add a test to validate this behavior.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Karthik Nayak [Fri, 20 Jun 2025 07:15:44 +0000 (09:15 +0200)]
refs/files: skip updates with errors in batched updates
The commit 23fc8e4f61 (refs: implement batch reference update support,
2025-04-08) introduced support for batched reference updates. This
allows users to batch updates together, while allowing some of the
updates to fail.
Under the hood, batched updates use the reference transaction mechanism.
Each update which fails is marked as such. Any failed updates must be
skipped over in the rest of the code, as they wouldn't apply any more.
In two of the loops within 'files_transaction_finish()' of the files
backend, the failed updates aren't skipped over. This can cause a
SEGFAULT otherwise. Add the missing skips and a test to validate the
same.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Tue, 24 Jun 2025 16:48:51 +0000 (09:48 -0700)]
Merge branch 'sa/multi-mailmap-fix'
When asking to apply mailmap to both author and committer field
while showing a commit object, the field that appears later was not
correctly parsed and replaced, which has been corrected.
* sa/multi-mailmap-fix:
cat-file: fix mailmap application for different author and committer
Junio C Hamano [Tue, 24 Jun 2025 16:48:47 +0000 (09:48 -0700)]
Merge branch 'ag/send-email-edit-threading-fix'
"git send-email" incremented its internal message counter when a
message was edited, which made logic that treats the first message
specially misbehave, which has been corrected.
* ag/send-email-edit-threading-fix:
send-email: show the new message id assigned by outlook in the logs
send-email: fix bug resulting in broken threads if a message is edited
Jeff King [Mon, 23 Jun 2025 10:56:25 +0000 (06:56 -0400)]
test-lib: teach test_seq the -f option
The "seq" tool has a "-f" option to produce printf-style formatted
lines. Let's teach our test_seq helper the same trick. This lets us get
rid of some shell loops in test snippets (which are particularly verbose
in our test suite because we have to "|| return 1" to keep the &&-chain
going).
This converts a few call-sites I found by grepping around the test
suite. A few notes on these:
- In "seq", the format specifier is a "%g" float. Since test_seq only
supports integers, I've kept the more natural "%d" (which is what
these call sites were using already).
- Like "seq", test_seq automatically adds a newline to the specified
format. This is what all callers are doing already except for t0021,
but there we do not care about the exact format. We are just trying
to printf a large number of bytes to a file. It's not worth
complicating other callers or adding an option to avoid the newline
in that caller.
- Most conversions are just replacing a shell loop (which does get rid
of an extra fork, since $() requires a subshell). In t0612 we can
replace an awk invocation, which I think makes the end result more
readable, as there's less quoting.
- In t7422 we can replace one loop, but sadly we have to leave the
loop directly above it. This is because that earlier loop wants to
include the seq value twice in the output, which test_seq does not
support (nor does regular seq). If you run:
test_seq -f "foo-%d %d" 10
the second "%d" will always be the empty string. You might naively
think that test_seq could add some extra arguments, like:
# 3 ought to be enough for anyone...
printf "$fmt\n" "$i "$i" $i"
but that just triggers printf to format multiple lines, one per
extra set of arguments.
So we'd have to actually parse the format string, figure out how
many "%" placeholders are there, and then feed it that many
instances of the sequence number. The complexity isn't worth it.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jacob Keller [Mon, 23 Jun 2025 23:11:35 +0000 (16:11 -0700)]
submodule: look up remotes by URL first
The get_default_remote_submodule() function performs a lookup to find
the appropriate remote to use within a submodule. The function first
checks to see if it can find the remote for the current branch. If this
fails, it then checks to see if there is exactly one remote. It will use
this, before finally falling back to "origin" as the default.
If a user happens to rename their default remote from origin, either
manually or by setting something like clone.defaultRemoteName, this
fallback will not work.
In such cases, the submodule logic will try to use a non-existent
remote. This usually manifests as a failure to trigger the submodule
update.
The parent project already knows and stores the submodule URL in either
.gitmodules or its .git/config.
Add a new repo_remote_from_url() helper which will iterate over all the
remotes in a repository and return the first remote which has a matching
URL.
Refactor get_default_remote_submodule to find the submodule and get its
URL. If a valid URL exists, first try to obtain a remote using the new
repo_remote_from_url(). Fall back to the repo_default_remote()
otherwise.
The fallback logic is kept in case for some reason the user has manually
changed the URL within the submodule. Additionally, we still try to use
a remote rather than directly passing the URL in the
fetch_in_submodule() logic. This ensures that an update will properly
update the remote refs within the submodule as expected, rather than
just fetching into FETCH_HEAD.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jacob Keller [Mon, 23 Jun 2025 23:11:33 +0000 (16:11 -0700)]
submodule--helper: improve logic for fallback remote name
The repo_get_default_remote() function in submodule--helper currently
tries to figure out the proper remote name to use for a submodule based
on a few factors.
First, it tries to find the remote for the currently checked out branch.
This works if the submodule is configured to checkout to a branch
instead of a detached HEAD state.
In the detached HEAD state, the code calls back to using "origin", on
the assumption that this is the default remote name. Some users may
change this, such as by setting clone.defaultRemoteName, or by changing
the remote name manually within the submodule repository.
As a first step to improving this situation, refactor to reuse the logic
from remotes_remote_for_branch(). This function uses the remote from the
branch if it has one. If it doesn't then it checks to see if there is
exactly one remote. It uses this remote first before attempting to fall
back to "origin".
To allow using this helper function, introduce a repo_default_remote()
helper to remote.c which takes a repository structure. This helper will
load the remote configuration and get the "HEAD" branch. Then it will
call remotes_remote_for_branch to find the default remote.
Replace calls of repo_get_default_remote() with the calls to this new
function. To maintain consistency with the existing callers, continue
copying the returned string with xstrdup.
This isn't a perfect solution for users who change remote names, but it
should help in cases where the remote name is changed but users haven't
added any additional remotes.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jacob Keller [Mon, 23 Jun 2025 23:11:32 +0000 (16:11 -0700)]
remote: remove the_repository from some functions
The remotes_remote_get_1 (and its caller, remotes_remote_get, have an
implicit dependency on the_repository due to calling
read_branches_file() and read_remotes_file(), both of which use
the_repository. The branch_get() function calls set_merge() which has an
implicit dependency on the_repository as well.
Because of this use of the_repository, the helper functions cannot be
used in code paths which operate on other repositories. A future
refactor of the submodule--helper will want to make use of some of these
functions.
Refactor to break the dependency by passing struct repository *repo
instead of struct remote_state *remote_state in a few places.
The public callers and many other helper functions still depend on
the_repository. A repo-aware function will be exposed in a following
change for git submodule--helper.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jacob Keller [Mon, 23 Jun 2025 23:11:31 +0000 (16:11 -0700)]
dir: move starts_with_dot(_dot)_slash to dir.h
Both submodule--helper.c and submodule-config.c have an implementation
of starts_with_dot_slash and starts_with_dot_dot_slash. The dir.h header
has starts_with_dot(_dot)_slash_native, which sets PATH_MATCH_NATIVE.
Move the helpers to dir.h as static inlines. I thought about renaming
them to postfix with _platform but that felt too long and ugly. On the
other hand it might be slightly confusing with _native.
This simplifies a submodule refactor which wants to use the helpers
earlier in the submodule--helper.c file.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jacob Keller [Mon, 23 Jun 2025 23:11:30 +0000 (16:11 -0700)]
remote: fix tear down of struct remote
The remote_clear() function failed to free the remote->push and
remote->fetch refspec fields.
This should be caught by the leak sanitizer. However, for callers which
use ``the_repository``, the values never go out of scope and the
sanitizer doesn't complain.
A future change is going to add a caller of read_config() for a
submodule repository structure, which would result in the leak sanitizer
complaining.
Fix remote_clear(), updating it to properly call refspec_clear() for
both the push and fetch members.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jacob Keller [Mon, 23 Jun 2025 23:11:29 +0000 (16:11 -0700)]
remote: remove branch->merge_name and fix branch_release()
The branch structure has both branch->merge_name and branch->merge for
tracking the merge information. The former is allocated by add_merge()
and stores the names read from the configuration file. The latter is
allocated by set_merge() which is called by branch_get() when an
external caller requests a branch.
This leads to the confusing situation where branch->merge_nr tracks both
the size of branch->merge (once its allocated) and branch->merge_name.
The branch_release() function incorrectly assumes that branch->merge is
always set when branch->merge_nr is non-zero, and can potentially crash
if read_config() is called without branch_get() being called on every
branch.
In addition, branch_release() fails to free some of the memory
associated with the structure including:
* Failure to free the refspec_item containers in branch->merge[i]
* Failure to free the strings in branch->merge_name[i]
* Failure to free the branch->merge_name parent array.
The set_merge() function sets branch->merge_nr to 0 when there is no
valid remote_name, to avoid external callers seeing a non-zero merge_nr
but a NULL merge array. This results in failure to release most of the
merge data as well.
These issues could be fixed directly, and indeed I initially proposed
such a change at [1] in the past. While this works, there was some
confusion during review because of the inconsistencies.
Instead, its time to clean up the situation properly. Remove
branch->merge_name entirely. Instead, allocate branch->merge earlier
within add_merge() instead of within set_merge(). Instead of having
set_merge() copy from merge_name[i] to merge[i]->src, just have
add_merge() directly initialize merge[i]->src.
Modify the add_merge() to call xstrdup() itself, instead of having
the caller of add_merge() do so. This makes it more obvious which code
owns the memory.
Update all callers which use branch->merge_name[i] to use
branch->merge[i]->src instead.
Add a merge_clear() function which properly releases all of the
merge-related memory, and which sets branch->merge_nr to zero. Use this
both in branch_release() and in set_merge(), fixing the leak when
set_merge() finds no valid remote_name.
Add a set_merge variable to the branch structure, which indicates
whether set_merge() has been called. This replaces the previous use of a
NULL check against the branch->merge array.
With these changes, the merge array is always allocated when merge_nr is
non-zero.
This use of refspec_item to store the names should be safe. External
callers should be using branch_get() to obtain a pointer to the branch,
which will call set_merge(), and the callers internal to remote.c
already handle the partially initialized refpsec_item structure safely.
This end result is cleaner, and avoids duplicating the merge names
twice.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
Link: [1] https://lore.kernel.org/git/20250617-jk-submodule-helper-use-url-v2-1-04cbb003177d@gmail.com/ Signed-off-by: Junio C Hamano <gitster@pobox.com>
So we are passing "$i" as an argument to be filled in, but there is no
"%" placeholder in the format string, which is a bit confusing to read.
We could switch both instances of "$i" to "%d" (and pass $i twice). But
that makes the line even longer. Let's just keep interpolating the value
in the string, and drop the confusing extra "$i" argument.
And since we are not using any printf specifiers at all, it becomes
clear that we can swap it out for echo. We do use a "\n" in the middle
of the string, but breaking this into two separate echo statements
actually makes it easier to read.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Mon, 23 Jun 2025 16:14:11 +0000 (09:14 -0700)]
coccicheck: fail "make" when it fails
With "make coccicheck", we generate contrib/coccinelle/*.cocci.patch
files that contain changes suggested by semantic patches, but "make"
succeeds. Admittedly, not many developers may run "make coccicheck"
in the first place, but it makes it harder to notice when they do
run it after they introduced an iffy piece of code.
Check that the resulting cocci.patch files are all empty.
Junio C Hamano [Wed, 18 Jun 2025 17:55:02 +0000 (10:55 -0700)]
cocci: matching (multiple) identifiers
"make coccicheck" seems to work OK at GitHub CI using
$ spatch --version
spatch version 1.1.1 compiled with OCaml version 4.13.1
OCaml scripting support: yes
Python scripting support: yes
Syntax of regular expressions: PCRE
but not with
$ spatch --version
spatch version 1.3 compiled with OCaml version 5.3.0
OCaml scripting support: yes
Python scripting support: yes
Syntax of regular expressions: Str
Judging from https://ocaml.org/manual/5.3/api/Str.html, I suspect
that this probably is caused by the distinction between BRE vs PCRE.
As there is no reasonably clean way to write the multiple choice
matches portably between these two pattern languages, let's stop
using regexp_constraint and use compare_constraint instead when
listing the function names to exclude.
There are other uses of "!~" but they all want to match a single
simple token, that should work fine either with BRE or PCRE.
Jörg Thalheim [Fri, 20 Jun 2025 15:56:14 +0000 (17:56 +0200)]
imap-send: improve error messages with configuration hints
Replace basic error messages with more helpful ones that guide users
on how to resolve configuration issues. When imap.host or imap.folder
are missing, provide the exact git config commands needed to fix the
problem, along with examples of typical values.
Use the advise() API to display hints in a multi-line format with
proper "hint:" prefixes for each line.
Signed-off-by: Jörg Thalheim <joerg@thalheim.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jörg Thalheim [Fri, 20 Jun 2025 15:56:13 +0000 (17:56 +0200)]
imap-send: fix confusing 'store' terminology in error message
The error message 'no imap store specified' is misleading because
it refers to 'store' when the actual missing configuration is
'imap.folder'. Update the message to use the correct terminology
that matches the configuration variable name.
This reduces confusion for users who might otherwise look for
non-existent 'imap.store' configuration when they see this error.
Signed-off-by: Jörg Thalheim <joerg@thalheim.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>
projects / thirdparty / git.git / log
