Commit Graph

77252 Commits (v2.50.0-rc1)

Author SHA1 Message Date
Patrick Steinhardt 9451446c61 ci: perform build and smoke tests for Meson docs
Our "documentation" CI job performs a couple of tests against our
documentation. Part of these tests is to check whether documentation
builds at all and whether it spits out the expected set of files. We
don't yet have such a test for Meson, which means that we wouldn't
notice at all if building the documentation were to break. As a result,
breakages as fixed by 87eccc3a81 (meson: fix building technical and
howto docs, 2025-03-02) are easy to go unnoticed.

Address this test gap by starting to build both manpages and HTML sites
as part of the CI job.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-12 10:21:53 -07:00
Patrick Steinhardt bbd831ce54 meson: don't install git-pack-redundant(1) docs with breaking changes
When breaking changes are enabled we continue to install documentation
of the git-pack-redundant(1) command even though it is completely
disabled and thus inaccessible. Improve this by only installing the
documentation in case breaking changes aren't enabled.

Based-on-patch-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-12 09:20:21 -07:00
Patrick Steinhardt 24a3f30c0d meson: don't compile git-pack-redundant(1) with breaking changes
We continue to compile the git-pack-redundant(1) builtin with Meson when
breaking changes are enabled even though we ultimately don't expose this
command at all. This is mostly harmless, but given that the intent of
the build option is to be as close as possible to the state where the
breaking change has been fully implemented this isn't optimal either.

Improve the situation by not compiling the builtin when breaking changes
are enabled.

Based-on-patch-by: Phillip Wood <phillip.wood123@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-12 09:20:21 -07:00
Patrick Steinhardt 7208d84305 meson: define WITH_BREAKING_CHANGES when enabling breaking changes
While Meson already supports the `-Dbreaking_changes=true` option, it
only wires up the build option that propagates into the tests. The build
option is only used for our tests to enable the `WITH_BREAKING_CHANGES`
prerequisite though, and does not influence the code that is actually
being built.

The omission went unnoticed because we only have tests right now that
get disabled when breaking changes are enabled, but not the other way
round. In other words, we don't have any tests that verify that breaking
changes behave as expected.

Fix the build issue by setting the `WITH_BREAKING_CHANGES` preprocessor
macro when breaking changes are enabled. Note that the `libgit_c_args`
array is defined after the current spot where we handle the option, so
to not have multiple sites where we handle it we instead move it after
the array has been defined.

Based-on-patch-by: Phillip Wood <phillip.wood123@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-12 09:20:20 -07:00
Junio C Hamano de3dec1187 name-rev: remove "--stdin" support
As part of Git 3.0, remove the hidden synonym for "--annotate-stdin"
for real.  As this does not change the fact that it used to be
called "--stdin" in older version of Git, keep that passage in the
documentation for "--annotate-stdin".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-12 08:48:54 -07:00
Junio C Hamano b0db798928 t6120: further modernize
There is absolutely no reason why a pattern given to grep to find
'warning: --stdin is deprecated' must be quoted within a pair of
single quotes, or the pattern to look for the literal string as ERE.

Quote the test body with a pair of single quotes like everybody
else, and quote the needle string in a pair of double quotes.  Also
use test_grep instead of "grep -E".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-12 08:48:54 -07:00
Junio C Hamano a733ec8a9e t6120: avoid hiding "git" exit status
A handful of tests invoke "git" on the upstream side of a pipe,
hiding its exit status.  Correct them.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-12 08:48:54 -07:00
Junio C Hamano 926d18e58d t: introduce WITH_BREAKING_CHANGES prerequisite
Earlier c5bc9a7f (Makefile: wire up build option for deprecated
features, 2025-01-22) made an unfortunate decision to introduce the
WITHOUT_BREAKING_CHANGES prerequisite to perform tests that ensure
the historical behaviour that may be different from what we will
have in the future.  It would inevitably invite double-negation when
we need to add tests to ensure the behaviour we want to have in the
future.

Introduce WITH_BREAKING_CHANGES prerequisite and replace the
existing uses of WITHOUT_BREAKING_CHANGES prerequisite.  To catch
any future topics that add more uses of WITHOUT_BREAKING_CHANGES,
mark it as a removed prerequisite.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-12 08:48:21 -07:00
Jiang Xin 4d53aae14b Merge branch 'tl/zh_CN_2.49.0_rnd' of github.com:dyrone/git
* 'tl/zh_CN_2.49.0_rnd' of github.com:dyrone/git:
  l10n: zh_CN: updated translation for 2.49
2025-03-12 19:36:40 +08:00
Teng Long ed99a5d9b8 l10n: zh_CN: updated translation for 2.49
Helped-by: 依云 <lilydjwg@gmail.com>
Helped-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
Signed-off-by: Teng Long <dyroneteng@gmail.com>
2025-03-12 14:52:52 +08:00
Jiang Xin 2bd71e1c16 Merge branch '2.49-uk-update' of github.com:arkid15r
* '2.49-uk-update' of github.com:arkid15r/git-ukrainian-l10n:
  l10n: uk: add 2.49 translation
2025-03-12 11:10:40 +08:00
Arkadii Yakovets 5b75ad9ee8
l10n: uk: add 2.49 translation
Co-authored-by: Kate Golovanova <kate@kgthreads.com>
Co-authored-by: Mikhail T. <Mikhail.Teterin@BNY.com>
Co-authored-by: Tamara Lazerka <lazerkatamara@gmail.com>
Signed-off-by: Arkadii Yakovets <ark@cho.red>
Signed-off-by: Kate Golovanova <kate@kgthreads.com>
Signed-off-by: Mikhail T. <Mikhail.Teterin@BNY.com>
Signed-off-by: Tamara Lazerka <lazerkatamara@gmail.com>
2025-03-11 19:48:31 -07:00
Abhijeetsingh Meena 5337daddc7 dir.h: remove duplicate forward declaration of struct repository
The `struct repository;` forward declaration appears twice in `dir.h`:
once at line 10 and again at line 46. This duplication is unnecessary
and likely unintentional.

Removing the second declaration has no impact on compilation, as verified
by a clean build.

Signed-off-by: Abhijeetsingh Meena <abhijeet040403@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-11 15:13:21 -07:00
Junio C Hamano 1c24d55a2f t: extend test_lazy_prereq
Allow test_lazy_prereq script to signal a programming error by
exiting with status 125 (like how bisect scripts do).  This is used
to signal a deprecated-and-then-removed prerequisite that should
never be used in tests anymore.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-11 15:05:23 -07:00
Junio C Hamano ab362fc6f4 t: document test_lazy_prereq
The t/README file talked about test_set_prereq but lacked
explanation on test_lazy_prereq, which is a more modern way to
define prerequisites.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-11 15:05:23 -07:00
Emir SARI f17f45f387
l10n: tr: Update Turkish translations for 2.49.0
Signed-off-by: Emir SARI <emir_sari@icloud.com>
2025-03-11 15:05:57 +03:00
Jiang Xin 00cbbbe90a Merge branch 'vi-2.49' of github.com:Nekosha/git-po
* 'vi-2.49' of github.com:Nekosha/git-po:
  l10n: Updated translation for vi-2.49
2025-03-11 07:35:07 +08:00
Jiang Xin b50b68dfd4 Merge branch 'master' of github.com:alshopov/git-po
* 'master' of github.com:alshopov/git-po:
  l10n: bg.po: Updated Bulgarian translation (5836t)
2025-03-11 07:33:18 +08:00
Jiang Xin aa77e3afef Merge branch 'fr_v2.49' of github.com:jnavila/git
* 'fr_v2.49' of github.com:jnavila/git:
  l10n: fr: 2.49 round 2
2025-03-11 07:23:32 +08:00
Jiang Xin 2d8902bb24 Merge branch 'master' of github.com:nafmo/git-l10n-sv
* 'master' of github.com:nafmo/git-l10n-sv:
  l10n: sv.po: Fix Swedish typos
  l10n: sv.po: Update Swedish translation
2025-03-11 07:22:07 +08:00
seoyeon-kwon ec507acbfd l10n: ko: fix minor typo in Korean translation
Signed-off-by: seoyeon-kwon <seoyeon.kwon@navercorp.com>
2025-03-11 07:20:03 +08:00
Ruggero Turra ee01097f28 l10n: it: fix spelling of "sorgente" (Italian for "source")
Signed-off-by: Ruggero Turra <ruggero.turra@cern.ch>
2025-03-11 07:16:20 +08:00
Luke Shumaker d9cb0e6ff8 fast-export, fast-import: add support for signed-commits
fast-export has a --signed-tags= option that controls how to handle tag
signatures.  However, there is no equivalent for commit signatures; it
just silently strips the signature out of the commit (analogously to
--signed-tags=strip).

While signatures are generally problematic for fast-export/fast-import
(because hashes are likely to change), if they're going to support tag
signatures, there's no reason to not also support commit signatures.

So, implement a --signed-commits= option that mirrors the --signed-tags=
option.

On the fast-export side, try to be as much like signed-tags as possible,
in both implementation and in user-interface.  This will change the
default behavior to '--signed-commits=abort' from what is now
'--signed-commits=strip'.  In order to provide an escape hatch for users
of third-party tools that call fast-export and do not yet know of the
--signed-commits= option, add an environment variable
'FAST_EXPORT_SIGNED_COMMITS_NOABORT=1' that changes the default to
'--signed-commits=warn-strip'.

Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:24:56 -07:00
Luke Shumaker dda9bff3c5 fast-export: do not modify memory from get_commit_buffer
fast-export's helper function find_encoding() takes a `const char *`, but
modifies that memory despite the `const`.  Ultimately, this memory came
from get_commit_buffer(), and you're not supposed to modify the memory
that you get from get_commit_buffer().

So, get rid of find_encoding() in favor of commit.h:find_commit_header(),
which gives back a string length, rather than mutating the memory to
insert a '\0' terminator.

Because find_commit_header() detects the "\n\n" string that separates the
headers and the commit message, move the call to be above the
`message = strstr(..., "\n\n")` call.  This helps readability, and allows
for the value of `encoding` to be used for a better value of "..." so that
the same memory doesn't need to be checked twice.  Introduce a
`commit_buffer_cursor` variable to avoid writing an awkward
`encoding ? encoding + encoding_len : committer_end` expression.

Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:24:56 -07:00
Luke Shumaker 87f2a9195e git-fast-export.adoc: clarify why 'verbatim' may not be a good idea
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:24:55 -07:00
Luke Shumaker 3b24d86c56 fast-export: rename --signed-tags='warn' to 'warn-verbatim'
The --signed-tags= option takes one of five arguments specifying how to
handle signed tags during export.  Among these arguments, 'strip' is to
'warn-strip' as 'verbatim' is to 'warn' (the unmentioned argument is
'abort', which stops the fast-export process entirely).  That is,
signatures are either stripped or copied verbatim while exporting, with
or without a warning.

Match the pattern and rename 'warn' to 'warn-verbatim' to make it clear
that it instructs fast-export to copy signatures verbatim.

To maintain backwards compatibility, 'warn' is still recognized as
deprecated synonym of 'warn-verbatim'.

Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:24:55 -07:00
Christian Couder 73ca6d2001 fast-export: fix missing whitespace after switch
"Documentation/CodingGuidelines" says that there should be whitespaces
around operators like 'if', 'switch', 'for', etc.

Let's fix this in "builtin/fast-export.c".

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:24:55 -07:00
Luke Shumaker d007dc2a3e git-fast-import.adoc: add missing LF in the BNF
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:24:55 -07:00
Arnav Bhate 2bfd3b3685 decorate: fix sign comparison warnings
There are multiple instances where ints have been initialized with
values of unsigned ints, and where negative values don't mean anything.
When such ints are compared with unsigned ints, it causes sign comparison
warnings.

Also, some of these are used just as stand-ins for their initial
values, never being modified, thus obscuring the specific conditions
under which certain operations happen.

Replace int with unsigned int for 2 variables, and replace the
intermediate variables with their initial values for 2 other variables.

Signed-off-by: Arnav Bhate <bhatearnav@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:21:13 -07:00
Patrick Steinhardt 7d70b29c4f hash: stop depending on `the_repository` in `null_oid()`
The `null_oid()` function returns the object ID that only consists of
zeroes. Naturally, this ID also depends on the hash algorithm used, as
the number of zeroes is different between SHA1 and SHA256. Consequently,
the function returns the hash-algorithm-specific null object ID.

This is currently done by depending on `the_hash_algo`, which implicitly
makes us depend on `the_repository`. Refactor the function to instead
pass in the hash algorithm for which we want to retrieve the null object
ID. Adapt callsites accordingly by passing in `the_repository`, thus
bubbling up the dependency on that global variable by one layer.

There are a couple of trivial exceptions for subsystems that already got
rid of `the_repository`. These subsystems instead use the repository
that is available via the calling context:

  - "builtin/grep.c"
  - "grep.c"
  - "refs/debug.c"

There are also two non-trivial exceptions:

  - "diff-no-index.c": Here we know that we may not have a repository
    initialized at all, so we cannot rely on `the_repository`. Instead,
    we adapt `diff_no_index()` to get a `struct git_hash_algo` as
    parameter. The only caller is located in "builtin/diff.c", where we
    know to call `repo_set_hash_algo()` in case we're running outside of
    a Git repository. Consequently, it is fine to continue passing
    `the_repository->hash_algo` even in this case.

  - "builtin/ls-files.c": There is an in-flight patch series that drops
    `USE_THE_REPOSITORY_VARIABLE` in this file, which causes a semantic
    conflict because we use `null_oid()` in `show_submodule()`. The
    value is passed to `repo_submodule_init()`, which may use the object
    ID to resolve a tree-ish in the superproject from which we want to
    read the submodule config. As such, the object ID should refer to an
    object in the superproject, and consequently we need to use its hash
    algorithm.

    This means that we could in theory just not bother about this edge
    case at all and just use `the_repository` in "diff-no-index.c". But
    doing so would feel misdesigned.

Remove the `USE_THE_REPOSITORY_VARIABLE` preprocessor define in
"hash.c".

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:16:20 -07:00
Patrick Steinhardt 8ca9fa60a6 hash: fix "-Wsign-compare" warnings
There are a couple of trivial "-Wsign-compare" warnings in "hash.c". Fix
them.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:16:20 -07:00
Patrick Steinhardt 172d0f686b object-file: split out logic regarding hash algorithms
While we have a "hash.h" header, the actual implementation of the
subsystem is hosted by "object-file.c". This makes it harder than
necessary to find the actual implementation of the hash subsystem and
intermingles the different concerns with one another.

Split out the implementation of hash algorithms into a new, separate
"hash.c" file.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:16:20 -07:00
Patrick Steinhardt 19be71db9c delta-islands: stop depending on `the_repository`
There are multiple sites in "delta-islands.c" where we use the
global `the_repository` variable, either explicitly or implicitly by
using `the_hash_algo`.

Refactor the code to stop using `the_repository`. In most cases this is
trivial because we already had a repository available in the calling
context, with the only exception being `propagate_island_marks()`. Adapt
it so that the repository gets passed in via a parameter.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:16:20 -07:00
Patrick Steinhardt f6e174b2d8 object-file-convert: stop depending on `the_repository`
There are multiple sites in "object-file-convert.c" where we use the
global `the_repository` variable, either explicitly or implicitly by
using `the_hash_algo`. All of these callsites are transitively called
from `convert_object_file()`, which indeed has no repo as input.

Refactor the function so that it receives a repository as a parameter
and pass it through to all internal functions to get rid of the
dependency. Remove the `USE_THE_REPOSITORY_VARIABLE` define.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:16:19 -07:00
Patrick Steinhardt 1a6768d1dd pack-bitmap-write: stop depending on `the_repository`
There are multiple sites in "pack-bitmap-write.c" where we use the
global `the_repository` variable, either explicitly or implicitly by
using `the_hash_algo`.

Refactor the code so that the `struct bitmap_writer` stores the
repository it is getting initialized with. Like this, we can adapt
callsites that use `the_repository` to instead use the repository
provided by the writer.

Remove the `USE_THE_REPOSITORY_VARIABLE` define.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:16:19 -07:00
Patrick Steinhardt e5957ca321 pack-revindex: stop depending on `the_repository`
There are multiple sites in "pack-revindex.c" where we use the global
`the_repository` variable, either explicitly or implicitly by using
`the_hash_algo`. In all of those cases we already have a repository
available in the calling context though.

Refactor the code to instead use the caller-provided repository and
remove the `USE_THE_REPOSITORY_VARIABLE` define.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:16:19 -07:00
Patrick Steinhardt 7ebf19ce55 pack-check: stop depending on `the_repository`
There are multiple sites in "pack-check.c" where we use the global
`the_repository` variable, either explicitly or implicitly by using
`the_hash_algo`. In all of those cases we already have a repository
available in the calling context though.

Refactor the code to instead use the caller-provided repository and
remove the `USE_THE_REPOSITORY_VARIABLE` define.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:16:19 -07:00
Patrick Steinhardt 7835ee75cd environment: move access to "core.bigFileThreshold" into repo settings
The "core.bigFileThreshold" setting is stored in a global variable and
populated via `git_default_core_config()`. This may cause issues in
the case where one is handling multiple different repositories in a
single process with different values for that config key, as we may or
may not see the correct value in that case. Furthermore, global state
blocks our path towards libification.

Refactor the code so that we instead store the value in `struct
repo_settings`, where the value is computed as-needed and cached.

Note that this change requires us to adapt one test in t1050 that
verifies that we die when parsing an invalid "core.bigFileThreshold"
value. The exercised Git command doesn't use the value at all, and thus
it won't hit the new code path that parses the value. This is addressed
by using git-hash-object(1) instead, which does read the value.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:16:18 -07:00
Patrick Steinhardt 2582846f2f pack-write: stop depending on `the_repository` and `the_hash_algo`
There are a couple of functions in "pack-write.c" that implicitly depend
on `the_repository` or `the_hash_algo`. Remove this dependency by
injecting the repository via a parameter and adapt callers accordingly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:16:18 -07:00
Patrick Steinhardt 74d414c9f1 object: stop depending on `the_repository`
There are a couple of functions exposed by "object.c" that implicitly
depend on `the_repository`. Remove this dependency by injecting the
repository via a parameter. Adapt callers accordingly by simply using
`the_repository`, except in cases where the subsystem is already free of
the repository. In that case, we instead pass the repository provided by
the caller's context.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:16:18 -07:00
Patrick Steinhardt 228457c9d9 csum-file: stop depending on `the_repository`
There are multiple sites in "csum-file.c" where we use the global
`the_repository` variable, either explicitly or implicitly by using
`the_hash_algo`.

Refactor the code to stop using `the_repository` by adapting functions
to receive required data as parameters. Adapt callsites accordingly by
either using `the_repository->hash_algo`, or by using a context-provided
hash algorithm in case the subsystem already got rid of its dependency
on `the_repository`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:16:18 -07:00
Jeff King c702dd4856 fetch: use ref prefix list to skip ls-refs
In git-fetch we have an optimization to avoid issuing an ls-refs command
to the server if we don't care about the value of any refs (e.g.,
because we are fetching exact object ids), saving a round-trip to the
server. This comes from e70a3030e7 (fetch: do not list refs if fetching
only hashes, 2018-09-27).

It uses an explicit flag "must_list_refs" to decide when we need to do
so. That was needed back then, because the list of ref-prefixes was not
always complete. If it was empty, it did not necessarily mean that we
were not interested in any refs). But that is no longer the case; an
empty list of prefixes means that we truly do not care about any refs.

And so rather than an explicit flag, we can just check whether we are
interested in any ref prefixes. This simplifies the code slightly, as
there is now a single source of truth for the decision.

It also fixes a bug in / optimizes a very unlikely case, which is:

  git fetch $remote ^foo $oid

I.e., a negative refspec combined with an exact oid fetch. This is
somewhat nonsense, in that there are no positive refspecs mentioning
refs to countermand with the negative one. But we should be able to do
this without issuing an ls-refs command (excluding "foo" from the empty
set will obviously still be the empty set).

However, the current code does not do so. The negative refspec is not
counted as a noop in un-setting the must_list_refs flag (hardly the
fault of e70a3030e7, as negative refspecs did not appear until much
later). But by using the prefix list as a source of truth, this
naturally just works; the negative refspec does not add a prefix to ask
about, and hence does not trigger the ls-refs call.

This is esoteric enough that I didn't bother adding a test. The real
value here is in the code simplification.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:13:46 -07:00
Jeff King 20010b8c20 fetch: avoid ls-refs only to ask for HEAD symref update
When we fetch from a configured remote, we may try to update the local
refs/remotes/<origin>/HEAD, and so we ask the server to advertise its
HEAD to us.

But if we aren't otherwise asking about any refs at all, then we know
this HEAD update can never happen! To consider a new value for HEAD,
the set_head() function uses guess_remote_head(). And even if it sees an
explicit symref value for HEAD, it will only report that as a match if
we also saw that remote ref advertised, and it mapped to a local
tracking ref via get_fetch_map().

In other words, a fetch like this:

  git fetch origin $exact_oid:refs/heads/foo

can never update HEAD, because we will never have fetched (nor even see
the advertisement for) the ref that HEAD points to.

Currently the command above will still call ls-refs to ask about the
HEAD, even though it is pointless. This patch teaches it to skip the
ls-refs call entirely in this case, which avoids a round-trip to the
server.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:13:46 -07:00
Jeff King 095bc13f35 fetch: stop protecting additions to ref-prefix list
When using the ref-prefix feature of protocol v2, a client which sends
no prefixes at all will get the full advertisement. And so the code in
git-fetch was historically loose about setting up that list based on our
refspecs. There were cases where we needed to know about some refs, so
we just didn't add anything to the ref-prefix list.

And hence further code, like that for tag-following and updating
origin/HEAD, had to be careful about adding to an empty list. E.g., see
the bug fixed by bd52d9a058 (fetch: fix following tags when fetching
specific OID, 2025-03-07).

But the previous commit removed the last such case, and now we know an
empty ref-prefix list (at least inside git-fetch's do_fetch() function)
means that we really don't need to see any refs. So we can drop those
extra conditionals.

This simplifies the code a little. But it also means that some cases can
now use ref prefixes when they would not otherwise. As the test shows,
fetching an exact oid into a local ref can now avoid enumerating all of
the refs. The refspec itself doesn't need to know about any remote refs,
and the tag auto-following can just ask about refs/tags/.

The same is true for asking about HEAD to update the local origin/HEAD.
I didn't add a test for that yet, though, as we can optimize it even
further.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:13:45 -07:00
Jeff King 625ed92134 fetch: ask server to advertise HEAD for config-less fetch
If we're not given any refspecs (either on the command line or via
config) and we have no branch merge config, then we fetch the remote
HEAD into our local FETCH_HEAD. In that case we do not send any
ref-prefix option to the server at all, and we see the full
advertisement.

But this is sub-optimal. We only care about HEAD, so we can just ask
for that, and ignore all of the other refs.

The new test demonstrates a case where we see fewer refs (in this case
only one less, but in theory we could be ignoring millions of them).

This also removes the only case where we care about seeing some refs
from the other side, but don't add anything to the ref_prefixes list.
Cleaning this up means one less maintenance burden. Before this patch,
any code which wanted to add to the list had to make sure the list was
not empty, since an empty list meant "ask for everything". Now it really
means "we are not interested in any refs".

This should let us optimize a few more cases in subsequent patches.

Note that we'll add "HEAD" to the list of prefixes, and later code for
updating "refs/remotes/<remote>/HEAD" may likewise do so. In theory this
could cause duplicates in the list, but in practice these can't both
trigger. We hit our new case only if there are no refspecs, and the
"<remote>/HEAD" feature is enabled only when we are fetching from a
remote with configured refspecs. We could be defensive with a flag, but
it didn't seem worth it to me (the absolute worse case is a useless
redundant ref-prefix line sent to the server).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:13:45 -07:00
Jeff King 36b12c3248 refspec_ref_prefixes(): clean up refspec_item logic
The point of refspec_ref_prefixes() is to look over the set of refspecs
and set up an appropriate list of "ref-prefix" strings to send to the
server.

The logic for handling individual refspec_items has some confusing bits.
The final part of our if/else cascade checks this:

  else if (item->src && !item->exact_sha1)
	prefix = item->src;

But we know that "item->exact_sha1" can never be true, because earlier
we did:

  if (item->exact_sha1 || item->negative)
	continue;

This is due to 6c301adb0a (fetch: do not pass ref-prefixes for fetch by
exact SHA1, 2018-05-31), which added the continue. So it is tempting to
remove the extra exact_sha1 at the end of the cascade, leaving the one
at the top of the loop.

But I don't think that's quite right. The full cascade is:

  if (rs->fetch == REFSPEC_FETCH)
	prefix = item->src;
  else if (item->dst)
	prefix = item->dst;
  else if (item->src && !item->exact_sha1)
	prefix = item->src;

which all comes from 6373cb598e (refspec: consolidate ref-prefix
generation logic, 2018-05-16). That first "if" is supposed to handle
fetches, where we care about the source name, since that is coming from
the server. And the rest should be for pushes, where we care about the
destination, since that's the name the server will use. And we get that
either explicitly from "dst" (for something like "foo:bar") or
implicitly from the source (a refspec like "foo" is treated as
"foo:foo").

But how should exact_sha1 interact with those? For a fetch, exact_sha1
always means we do not care about sending a name to the server (there is
no server refname at all). But pushing an exact sha1 should still care
about the destination on the server! It is only if we have to fall back
to the implicit source that we need to care if it is a real ref (though
arguably such a push does not even make sense; where would the server
store it?).

So I think that 6c301adb0a "broke" the push case by always skipping
exact_sha1 items, even though a push should only care about the
destination.

Of course this is all completely academic. We have still not implemented
a v2 push protocol, so even though we do call this function for pushes,
we'd never actually send these ref-prefix lines.

However, given the effort I spent to figure out what was going on here,
and the overlapping exact_sha1 checks, I'd like to rewrite this to
preemptively fix the bug, and hopefully make it less confusing.

This splits the "if" at the top-level into fetch vs push, and then each
handles exact_sha1 appropriately itself. The check for negative refspecs
remains outside of either (there is no protocol support for them, so we
never send them to the server, but rather use them only to reduce the
advertisement we receive).

The resulting behavior should be identical for fetches, but hopefully
sets us up better for a potential future v2 push.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:13:45 -07:00
Jeff King 821d8f2157 t5516: beef up exact-oid ref prefixes test
Commit 6c301adb0a (fetch: do not pass ref-prefixes for fetch by exact
SHA1, 2018-05-31) added a test that fetching an exact oid with the v2
protocol works. Originally it failed without the code change from that
commit, because fetch failed with "no matching remote head".

That changed in 0177565148 (transport: do not list refs if possible,
2018-09-27), which made fetch more forgiving of this case.

But that now meant the test passes even without its fix! So let's also
have it check the packet listing to make sure we did not ask for the
bogus prefix (ultimately this is less important than whether the command
fails, since it's just an optimization, but we should make sure not to
regress it).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:13:45 -07:00
Jeff King 6ea26f34c9 t5516: drop NEEDSWORK about v2 reachability behavior
When this test was added in 6c301adb0a (fetch: do not pass ref-prefixes
for fetch by exact SHA1, 2018-05-31), there was still some uncertainty
about the v2 protocol's looser behavior with serving objects that are
not directly pointed at by a ref.

At this point that behavior is well established, and I do not think we
would ever change v2 to match the v0 behavior (and if we did,
remembering to update this test is the least of our concerns).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:13:45 -07:00
Jeff King 2de68c046e t5516: prefer "oid" to "sha1" in some test titles
These old tests refer to object ids as "sha1". These days we prefer
the more algorithm-agnostic "oid".

There are a few more tests that mention sha1 in the title and also use
it in variables throughout the test. I've left them for now, as changing
them is more involved (and they're linked to the allowTipSHA1InWant
config, which as a v0-only thing actually is always sha1).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:13:44 -07:00
Jeff King 09781e379b t5702: fix typo in test name
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-10 13:13:44 -07:00