| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
A handful of code paths have been corrected to check return values
from functions like curl_easy_duphandle(), deflateInit(), lseek(),
dup(), and strbuf_getline_lf(), resolving several Coverity warnings
about unchecked returns.
* js/coverity-unchecked-returns-fix:
bisect: handle dup() failure when redirecting stdout
bisect: check get_terms return at all call sites
bisect: check strbuf_getline_lf return when reading terms
transport-helper: warn when export-marks file cannot be finalized
transport-helper: check dup() return in get_exporter
compat/pread: check initial lseek for errors
last-modified: handle repo_parse_commit() failures
reftable tests: check reftable_table_init_ref_iterator() return
reftable/block: check deflateInit() return value
reftable: handle block-writer initialization errors
config: propagate launch_editor() failure in show_editor()
http: die on curl_easy_duphandle failure in get_active_slot
|
|
get_active_slot() duplicates the default curl handle via
curl_easy_duphandle() to create a per-slot session handle. The
return value is stored directly in slot->curl without checking
for NULL. curl_easy_duphandle() can return NULL when memory
allocation fails internally, and the libcurl documentation
explicitly states this possibility.
When this happens, slot->curl is NULL and the very next operation
(curl_easy_setopt on line 1632 for CURLOPT_COOKIEFILE) passes
NULL as the curl handle, which is undefined behavior in libcurl
and typically crashes.
Every HTTP operation in git goes through get_active_slot(), so
this affects all remote-https, remote-http, and HTTP-based
operations (clone, fetch, push over HTTP, bundle-uri downloads).
Add a NULL check and die() with a clear message. There is no
reasonable recovery from a failed handle duplication: the process
is out of memory and cannot perform any HTTP operation.
Pointed out by Coverity.
Assisted-by: Claude Opus 4.6
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Concurrent downloads of packfiles via packfile URIs and dumb HTTP are
safer by avoiding concurrent appends to the staging file. Opening in
read-write mode with separate file offsets prevents corruption and
preserves resumability. 'fetch-pack' now tolerates pre-existing
'.keep' files.
* tn/packfile-uri-concurrency:
fetch-pack: accept "pack" output for packfile URIs
http: permit unlinking partial packs on Windows
http: avoid concurrent appends to partial packs
http: accept HTTP 416 for complete partial packs
http: avoid closing index-pack input twice
http-fetch: correct --index-pack-arg documentation
|
|
On Windows, an open file must permit FILE_SHARE_DELETE before another
process can unlink it. MinGW's non-append O_RDWR open enables that
sharing mode only for an existing file; adding O_CREAT falls back to
_wopen(), which cannot set it.
First try opening the partial pack without O_CREAT. If it does not
exist, create it exclusively, close that descriptor, and retry through
the existing-file path. A racing creator retries after EEXIST.
This ensures that every retained descriptor permits another downloader
to unlink the staging path. Add an unlink-while-indexing test that does
not require FIFOs and can therefore run on MinGW.
Signed-off-by: Ted Nyman <tnyman@openai.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Pack requests stage downloads in a predictable partial-pack file so an
interrupted transfer can be resumed. Both packfile URI and ordinary dumb
HTTP requests use this staging path. Opening it in append mode forces
each write to the current end of the file, so concurrent responses can
append duplicate data and corrupt the pack.
Open the partial pack read-write without O_APPEND and seek once to its
current end. Each downloader then retains the offset matching the Range
it requested. Because the staging key must uniquely identify immutable
pack contents, overlapping responses write the same bytes at the same
offsets instead of extending the file with duplicate data.
Duplicate the staging descriptor for index-pack instead of reopening the
path after closing the stream. Another downloader may unlink the staging
path before indexing begins, but index-pack can still read the retained
descriptor.
Exercise resumed transfers and overlapping 200 and 206 responses, and
clarify the staging-key documentation.
Signed-off-by: Ted Nyman <tnyman@openai.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
finish_http_pack_request() passes its staging-file descriptor to
index-pack through child_process.in. start_command() takes ownership
of a supplied descriptor and closes it, even when starting the child
fails.
Do not close the descriptor again after run_command() returns.
Signed-off-by: Ted Nyman <tnyman@openai.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Now that it is OK to call git_hash_discard() even after finalizing the
hash, we no longer need the ctx_valid bool added by a2d8ea5a76 (http:
discard hash in dumb-http http_object_request, 2026-07-02).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
We'd like to add more logic to git_hash_init(), but many callers skip it
and call algop->init_fn() directly. Let's make sure we're consistently
using the wrapper by adding a coccinelle rule.
Besides the coccinelle file itself, this is a purely mechanical
conversion based on the patch it generates. There should be no bare
init_fn() calls left (except for the one in the wrapper).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
* jk/hash-algo-leak-fixes:
hash: add platform-specific discard functions
hash: fix memory leak copying sha256 gcrypt handles
http: discard hash in dumb-http http_object_request
check_stream_oid(): discard hash on read error
patch-id: discard hash when done
csum-file: provide a function to release checkpoints
csum-file: always finalize or discard hash
hash: add discard primitive
csum-file: drop discard_hashfile()
|
|
Usually an object request results in finish_http_object_request()
calling git_hash_final_oid(), after we've received all of the data. But
if we hit an error, we'll bail early and free the http_object_request,
dropping the git_hash_ctx entirely. This can cause a leak for hash
implementations that allocate memory in their context, like OpenSSL >=
3.0.
The obvious fix is for abort_http_object_request() to call
git_hash_discard(), under the assumption that every request is either
finished or aborted. But that's not quite true:
1. Not everybody calls the abort function. Sometimes they jump
straight to release_http_object_request(). So we'd have to put it
there.
2. After the finish function finalizes the hash, we can still
encounter errors! In that case we end up aborting or releasing,
and they must not discard that hash (since that would be a
double-free).
So we'll keep a flag marking the validity of the hash_ctx field of the
request. The lifetime is simple: it is valid immediately after creation,
up until we call finalize. And then our release function can just
conditionally discard the hash based on that flag.
This fixes test failures in t5550 and t5619 when run with:
make SANITIZE=leak \
OPENSSL_SHA256=1 \
GIT_TEST_DEFAULT_HASH=sha256 \
test
The flag handling could be removed if the hash-discard function were
idempotent. This could be done easily-ish by having the underlying
hash functions (like the ones in sha256/openssl.h) set the context
pointer to NULL after free-ing. But it's something that every platform
implementation would have to remember to do, and the benefit for the
callers is not that huge (it would let us shave a few lines here and
probably in a few other spots).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
We lost ability to use https:// proxies during this cycle; this is
a hotfix for the regression.
* js/http-https-proxy-fix:
http: accept https:// proxies again
|
|
Since 663d7abe07ea (http: reject unsupported proxy URL schemes,
2026-05-05), set_curl_proxy_type() returns 0 only for the "http"
and SOCKS variants via dedicated early returns, and -1 for
everything else. The "https" branch configures the CURL handle for
HTTPS proxying but then falls through to the trailing `return -1`
intended for unknown schemes, so the caller in get_curl_handle()
treats a perfectly valid https:// proxy URL as unsupported and
refuses to use it.
Noticed while looking into a Coverity report against the same
function; the unchecked curl_easy_setopt() return values it flags
are orthogonal to this fix.
Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
A memory leak in `fetch_and_setup_pack_index()` when verification of
the downloaded pack index fails has been plugged. Also an obsolete
`unlink()` call on parse failure has been cleaned up.
* lp/http-fetch-pack-index-leak-fix:
http: fix memory leak in fetch_and_setup_pack_index()
http: cleanup function fetch_and_setup_pack_index()
|
|
Inside the function `fetch_and_setup_pack_index()`, when the pack
obtained using `parse_pack_index()` fails to be verified by
`verify_pack_index()`, the function returns without closing and freeing
said pack.
Fix this by calling `close_pack_index()` to munmap the index file for
the leaking pack (which might have been mmapped by `fetch_pack_index()`
or `verify_pack_index()`), and then free it, when the verification
fails.
Signed-off-by: LorenzoPegorari <lorenzo.pegorari2002@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Cleanup the function `fetch_and_setup_pack_index()` by removing the
useless call to the function `unlink()`.
This is not necessary anymore since 63aca3f7f1 (dumb-http: store
downloaded pack idx as tempfile, 2024-10-25), when `fetch_pack_index()`
started registering its return value (in this case `tmp_idx`) as a
tempfile to be deleted at process exit.
Signed-off-by: LorenzoPegorari <lorenzo.pegorari2002@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The "object-file" subsystem still hosts the majority of logic used to
write loose objects. Eventually, we'll want to move this logic into
"odb/source-loose.c", but this isn't yet easily possible because a lot
of the writing logic is still being shared with `force_object_loose()`.
We will eventually detangle this logic so that we can indeed move all of
it into the "loose" source. Meanwhile though, refactor the code so that
it operates on a `struct odb_source_loose` directly to already make the
dependency explicit.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Misspelt proxy URL (e.g., httt://...) did not trigger any warning
or failure, which has been corrected.
* aw/validate-proxy-url-scheme:
http: reject unsupported proxy URL schemes
|
|
An explicit proxy URL with an unrecognized scheme such as
htpp://127.0.0.1 is currently accepted.
Git parses the URL, extracts the host part, and then passes only that
host to libcurl. Because no proxy type is selected for the unknown
scheme, Git leaves libcurl at its default HTTP proxy type, so the typo
is silently treated as an HTTP proxy.
Reject proxy URLs with explicit unsupported schemes instead of silently
accepting them. Keep the existing host:port-without-scheme behavior
unchanged.
Implement the SOCKS proxy handling with a shared table-driven mapping.
Add a regression test to cover the unsupported-scheme case.
Signed-off-by: Aliwoto <aminnimaj@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
When a server advertises Negotiate (SPNEGO) authentication, the
"auto" mode of http.emptyAuth should detect this as an "exotic"
method and proactively send empty credentials, allowing libcurl to
use the system Kerberos ticket without prompting the user.
However, two features interact to prevent this from working:
The Negotiate-stripping logic, introduced in 4dbe66464b
(remote-curl: fall back to Basic auth if Negotiate fails,
2015-01-08), removes CURLAUTH_GSSNEGOTIATE from the allowed
methods on the first 401 response. The empty-auth auto-detection,
introduced in 40a18fc77c (http: add an "auto" mode for
http.emptyauth, 2017-02-25), then checks the remaining methods
for anything "exotic" -- but Negotiate has already been removed,
so auto mode never activates for servers whose only non-Basic/Digest
method is Negotiate (e.g., Apache with mod_auth_kerb offering
Basic + Negotiate).
Fix this by delaying the Negotiate stripping in auto mode: on the
first 401, keep Negotiate in the allowed methods so that auto mode
can detect it and retry with empty credentials. If that attempt
fails (no valid Kerberos ticket), strip Negotiate on the second 401
and fall through to credential_fill() as usual.
To support this, also teach http_reauth_prepare() to skip
credential_fill() when empty auth is about to be attempted, since
filling real credentials would bypass the empty-auth mechanism.
The true and false modes are unchanged: true sends empty credentials
on the very first request (before any 401), and false never sends
them.
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
All three HTTP retry paths (http_request_recoverable, post_rpc,
probe_rpc) call credential_fill() directly when handling
HTTP_REAUTH. Extract this into a helper function so that a
subsequent commit can add pre-fill logic (such as attempting
empty-auth before prompting) in one place.
No functional change.
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Further work to adjust the codebase for C23 that changes functions
like strchr() that discarded constness when they return a pointer into
a const string to preserve constness.
* jk/c23-const-preserving-fixes-more:
git-compat-util: fix CONST_OUTPARAM typo and indentation
refs/files-backend: drop const to fix strchr() warning
http: drop const to fix strstr() warning
range-diff: drop const to fix strstr() warnings
pkt-line: make packet_reader.line non-const
skip_prefix(): check const match between in and out params
pseudo-merge: fix disk reads from find_pseudo_merge()
find_last_dir_sep(): convert inline function to macro
run-command: explicitly cast away constness when assigning to void
pager: explicitly cast away strchr() constness
transport-helper: drop const to fix strchr() warnings
http: add const to fix strchr() warnings
convert: add const to fix strchr() warnings
|
|
In redact_sensitive_header(), a C23 implementation of libc will complain
that strstr() assigns the result from "const char *cookie" to "char
*semicolon".
Ultimately the memory is writable. We're fed a strbuf, generate a const
pointer "sensitive_header" within it using skip_iprefix(), and then
assign the result to "cookie". So we can solve this by dropping the
const from "cookie" and "sensitive_header".
However, this runs afoul of skip_iprefix(), which wants a "const char
**" for its out-parameter. We can solve that by teaching skip_iprefix()
the same "make sure out is at least as const as in" magic that we
recently taught to skip_prefix().
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The HTTP transport learned to react to "429 Too Many Requests".
* vp/http-rate-limit-retries:
http: add support for HTTP 429 rate limit retries
strbuf_attach: fix call sites to pass correct alloc
strbuf: pass correct alloc to strbuf_attach() in strbuf_reencode()
|
|
Add retry logic for HTTP 429 (Too Many Requests) responses to handle
server-side rate limiting gracefully. When Git's HTTP client receives
a 429 response, it can now automatically retry the request after an
appropriate delay, respecting the server's rate limits.
The implementation supports the RFC-compliant Retry-After header in
both delay-seconds (integer) and HTTP-date (RFC 2822) formats. If a
past date is provided, Git retries immediately without waiting.
Retry behavior is controlled by three new configuration options
(http.maxRetries, http.retryAfter, and http.maxRetryTime) which are
documented in git-config(1).
The retry logic implements a fail-fast approach: if any delay
(whether from server header or configuration) exceeds maxRetryTime,
Git fails immediately with a clear error message rather than capping
the delay. This provides better visibility into rate limiting issues.
The implementation includes extensive test coverage for basic retry
behavior, Retry-After header formats (integer and HTTP-date),
configuration combinations, maxRetryTime limits, invalid header
handling, environment variable overrides, and edge cases.
Signed-off-by: Vaidas Pilkauskas <vaidas.pilkauskas@shopify.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The "files" backend is implemented as a pointer in the `struct
odb_source`. This contradicts our typical pattern for pluggable backends
like we use it for example in the ref store or for object database
streams, where we typically embed the generic base structure in the
specialized implementation. This pattern has a couple of small benefits:
- We avoid an extra allocation.
- We hide implementation details in the generic structure.
- We can easily downcast from a generic backend to the specialized
structure and vice versa because the offsets are known at compile
time.
- It becomes trivial to identify locations where we depend on backend
specific logic because the cast needs to be explicit.
Refactor our "files" object database source to do the same and embed the
`struct odb_source` in the `struct odb_source_files`.
There are still a bunch of sites in our code base where we do have to
access internals of the "files" backend. The intent is that those will
go away over time, but this will certainly take a while. Meanwhile,
provide a `odb_source_files_downcast()` function that can convert a
generic source into a "files" source.
As we only have a single source the downcast succeeds unconditionally
for now. Eventually though the intent is to make the cast `BUG()` in
case the caller requests to downcast a non-"files" backend to a "files"
backend.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Introduce a new "files" object database source. This source encapsulates
access to both loose object files and the packfile store, similar to how
the "files" backend for refs encapsulates access to loose refs and the
packed-refs file.
Note that for now the "files" source is still a direct member of a
`struct odb_source`. This architecture will be reversed in the next
commit so that the files source contains a `struct odb_source`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The packfile store is a member of `struct object_database`, which means
that we have a single store per database. This doesn't really make much
sense though: each source connected to the database has its own set of
packfiles, so there is a conceptual mismatch here. This hasn't really
caused much of a problem in the past, but with the advent of pluggable
object databases this is becoming more of a problem because some of the
sources may not even use packfiles in the first place.
Move the packfile store down by one level from the object database into
the object database source. This ensures that each source now has its
own packfile store, and we can eventually start to abstract it away
entirely so that the caller doesn't even know what kind of store it
uses.
Note that we only need to adjust a relatively small number of callers,
way less than one might expect. This is because most callers are using
`repo_for_each_pack()`, which handles enumeration of all packfiles that
exist in the repository. So for now, none of these callers need to be
adapted. The remaining callers that iterate through the packfiles
directly and that need adjustment are those that are a bit more tangled
with packfiles. These will be adjusted over time.
Note that this patch only moves the packfile store, and there is still a
bunch of functions that seemingly operate on a packfile store but that
end up iterating over all sources. These will be adjusted in subsequent
commits.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The dumb HTTP protocol directly fetches packfiles from the remote server
and temporarily stores them in a list of packfiles. Those packfiles are
not yet added to the repository's packfile store until we finalize the
whole fetch.
Refactor the code to instead use a `struct packfile_list` to store those
packs. This prepares us for a subsequent change where the `->next`
pointer of `struct packed_git` will go away.
Note that this refactoring creates some temporary duplication of code,
as we now have both `packfile_list_find_oid()` and `find_oid_pack()`.
The latter function will be removed in a subsequent commit though.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
We have a bunch of different sites that want to iterate through all
packs of a given `struct packfile_store`. This pattern is somewhat
verbose and repetitive, which makes it somewhat cumbersome.
Introduce a new macro `repo_for_each_pack()` that removes some of the
boilerplate.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
* ps/packfile-store:
packfile: refactor `get_packed_git_mru()` to work on packfile store
packfile: refactor `get_all_packs()` to work on packfile store
packfile: refactor `get_packed_git()` to work on packfile store
packfile: move `get_multi_pack_index()` into "midx.c"
packfile: introduce function to load and add packfiles
packfile: refactor `install_packed_git()` to work on packfile store
packfile: split up responsibilities of `reprepare_packed_git()`
packfile: refactor `prepare_packed_git()` to work on packfile store
packfile: reorder functions to avoid function declaration
odb: move kept cache into `struct packfile_store`
odb: move MRU list of packfiles into `struct packfile_store`
odb: move packfile map into `struct packfile_store`
odb: move initialization bit into `struct packfile_store`
odb: move list of packfiles into `struct packfile_store`
packfile: introduce a new `struct packfile_store`
|
|
The `get_all_packs()` function prepares the packfile store and then
returns its packfiles. Refactor it to accept a packfile store instead of
a repository to clarify its scope.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The `install_packed_git()` functions adds a packfile to a specific
object store. Refactor it to accept a packfile store instead of a
repository to clarify its scope.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Adjust to the way newer versions of cURL selectivel enables tracing
options, so that our tests can continue to work.
* jk/curl-global-trace-components:
curl: add support for curl_global_trace() components
|
|
In addition to the regular trace information produced by
CURLOPT_VERBOSE, recent curl versions can enable or disable tracing of
specific subsystems using a call to curl_global_trace().
This level of detail may or may not be useful for us in Git as mere
users of libcurl, but there's one case where we need it for a test. In
t5564, we set up a socks proxy, access it with GIT_TRACE_CURL set, and
expect to find socks-related messages in the output. This test is broken
in the release candidates for libcurl 8.16, as those socks messages are
no longer produced in the trace.
The problem bisects to curl's commit ab5e0bfddc (pytest: add SOCKS tests
and scoring, 2025-07-21). There the socks messages were moved from
generic infof() messages to the component-specific CURL_TRC_CF() system.
And so we do not see them by default, but only if "socks" is enabled as
a logging component.
Teach Git's http code to accept a component list from the
environment and pass it into curl_global_trace(). We can then use
that in the test to enable the correct component.
It should be safe to do so unconditionally. In older versions of curl
which don't support this call, setting the environment variable is a
noop. Likewise, any versions of curl which don't recognize the "socks"
component should silently ignore it. The manpage for curl_global_trace()
says this:
The config string is a list of comma-separated component names. Names
are case-insensitive and unknown names are ignored. The special name
"all" applies to all components. Names may be prefixed with '+' or '-'
to enable or disable detailed logging for a component.
The list of component names is not part of curl's public API. Names may
be added or disappear in future versions of libcurl. Since unknown
names are silently ignored, outdated log configurations does not cause
errors when upgrading libcurl. Given that, some names can be expected
to be fairly stable and are listed below for easy reference.
So this should let us make the test work on all versions without
worrying about confusing older (or newer) versions. For the same reason,
I've opted not to document this interface. This is deep internal voodoo
for which we can make no promises to users. In fact, I was tempted to
simply hard-code "socks" to let our test pass and not expose anything.
But I suspect a little run-time flexibility may come in handy in the
future when debugging or dealing with similar logging issues.
I also considered just putting "all" into such a hard-coded default. But
if you try it, you will see that many of the components are quite
verbose and likely not interesting. They would clutter up our trace
output if we enabled them by default.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Reduce implicit assumption and dependence on the_repository in the
object-file subsystem.
* ps/object-file-wo-the-repository:
object-file: get rid of `the_repository` in index-related functions
object-file: get rid of `the_repository` in `force_object_loose()`
object-file: get rid of `the_repository` in `read_loose_object()`
object-file: get rid of `the_repository` in loose object iterators
object-file: remove declaration for `for_each_file_in_obj_subdir()`
object-file: inline `for_each_loose_file_in_objdir_buf()`
object-file: get rid of `the_repository` when writing objects
odb: introduce `odb_write_object()`
loose: write loose objects map via their source
object-file: get rid of `the_repository` in `finalize_object_file()`
object-file: get rid of `the_repository` in `loose_object_info()`
object-file: get rid of `the_repository` when freshening objects
object-file: inline `check_and_freshen()` functions
object-file: get rid of `the_repository` in `has_loose_object()`
object-file: stop using `the_hash_algo`
object-file: fix -Wsign-compare warnings
|
|
In "config.c" we host both the business logic to read and write config
files as well as the logic to parse specific Git-related variables. On
the one hand this is mixing concerns, but even more importantly it means
that we cannot easily remove the dependency on `the_repository` in our
config parsing logic.
Move the logic into "environment.c". This file is a grab bag of all
kinds of global state already, so it is quite a good fit. Furthermore,
it also hosts most of the global variables that we're parsing the config
values into, making this an even better fit.
Note that there is one hidden change: in `parse_fsync_components()` we
use an `int` to iterate through `ARRAY_SIZE(fsync_component_names)`. But
as -Wsign-compare warnings are enabled in this file this causes a
compiler warning. The issue is fixed by using a `size_t` instead.
This change allows us to drop the `USE_THE_REPOSITORY_VARIABLE`
declaration.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
In 036876a1067 (config: hide functions using `the_repository` by
default, 2024-08-13) we have moved around a bunch of functions in the
config subsystem that depend on `the_repository`. Those function have
been converted into mere wrappers around their equivalent function that
takes in a repository as parameter, and the intent was that we'll
eventually remove those wrappers to make the dependency on the global
repository variable explicit at the callsite.
Follow through with that intent and remove `git_config()`. All callsites
are adjusted so that they use `repo_config(the_repository, ...)`
instead. While some callsites might already have a repository available,
this mechanical conversion is the exact same as the current situation
and thus cannot cause any regression. Those sites should eventually be
cleaned up in a later patch series.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
We implicitly depend on `the_repository` when moving an object file into
place in `finalize_object_file()`. Get rid of this global dependency by
passing in a repository.
Note that one might be pressed to inject an object database instead of a
repository. But the function doesn't really care about the ODB at all.
All it does is to move a file into place while checking whether there is
any collision. As such, the functionality it provides is independent of
the object database and only needs the repository as parameter so that
it can adjust permissions of the file we are about to finalize.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Code clean-up around object access API.
* ps/object-store:
odb: rename `read_object_with_reference()`
odb: rename `pretend_object_file()`
odb: rename `has_object()`
odb: rename `repo_read_object_file()`
odb: rename `oid_object_info()`
odb: trivial refactorings to get rid of `the_repository`
odb: get rid of `the_repository` when handling submodule sources
odb: get rid of `the_repository` when handling the primary source
odb: get rid of `the_repository` in `for_each()` functions
odb: get rid of `the_repository` when handling alternates
odb: get rid of `the_repository` in `odb_mkstemp()`
odb: get rid of `the_repository` in `assert_oid_type()`
odb: get rid of `the_repository` in `find_odb()`
odb: introduce parent pointers
object-store: rename files to "odb.{c,h}"
object-store: rename `object_directory` to `odb_source`
object-store: rename `raw_object_store` to `object_database`
|
|
* ps/object-store:
odb: rename `read_object_with_reference()`
odb: rename `pretend_object_file()`
odb: rename `has_object()`
odb: rename `repo_read_object_file()`
odb: rename `oid_object_info()`
odb: trivial refactorings to get rid of `the_repository`
odb: get rid of `the_repository` when handling submodule sources
odb: get rid of `the_repository` when handling the primary source
odb: get rid of `the_repository` in `for_each()` functions
odb: get rid of `the_repository` when handling alternates
odb: get rid of `the_repository` in `odb_mkstemp()`
odb: get rid of `the_repository` in `assert_oid_type()`
odb: get rid of `the_repository` in `find_odb()`
odb: introduce parent pointers
object-store: rename files to "odb.{c,h}"
object-store: rename `object_directory` to `odb_source`
object-store: rename `raw_object_store` to `object_database`
|
|
In the preceding commits we have renamed the structures contained in
"object-store.h" to `struct object_database` and `struct odb_backend`.
As such, the code files "object-store.{c,h}" are confusingly named now.
Rename them to "odb.{c,h}" accordingly.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The `object_directory` structure is used as an access point for a single
object directory like ".git/objects". While the structure isn't yet
fully self-contained, the intent is for it to eventually contain all
information required to access objects in one specific location.
While the name "object directory" is a good fit for now, this will
change over time as we continue with the agenda to make pluggable object
databases a thing. Eventually, objects may not be accessed via any kind
of directory at all anymore, but they could instead be backed by any
kind of durable storage mechanism. While it seems quite far-fetched for
now, it is thinkable that eventually this might even be some form of a
database, for example.
As such, the current name of this structure will become worse over time
as we evolve into the direction of pluggable ODBs. Immediate next steps
will start to carve out proper self-contained object directories, which
requires us to pass in these object directories as parameters. Based on
our modern naming schema this means that those functions should then be
named after their subsystem, which means that we would start to bake the
current name into the codebase more and more.
Let's preempt this by renaming the structure. There have been a couple
alternatives that were discussed:
- `odb_backend` was discarded because it led to the association that
one object database has a single backend, but the model is that one
alternate has one backend. Furthermore, "backend" is more about the
actual backing implementation and less about the high-level concept.
- `odb_alternate` was discarded because it is a bit of a stretch to
also call the main object directory an "alternate".
Instead, pick `odb_source` as the new name. It makes it sufficiently
clear that there can be multiple sources and does not cause confusion
when mixed with the already-existing "alternate" terminology.
In the future, this change allows us to easily introduce for example a
`odb_files_source` and other format-specific implementations.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
As of Homebrew's update to cURL v8.14.0, there are new compile errors to
be observed in the `osx-gcc` job of Git's CI builds:
In file included from http.h:8,
from imap-send.c:36:
In function 'setup_curl',
inlined from 'curl_append_msgs_to_imap' at imap-send.c:1460:9,
inlined from 'cmd_main' at imap-send.c:1581:9:
/usr/local/Cellar/curl/8.14.0/include/curl/typecheck-gcc.h:50:15: error: call to '_curl_easy_setopt_err_long' declared with attribute warning: curl_easy_setopt expects a long argument [-Werror=attribute-warning]
50 | _curl_easy_setopt_err_long(); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/local/Cellar/curl/8.14.0/include/curl/curl.h:54:7: note: in definition of macro 'CURL_IGNORE_DEPRECATION'
54 | statements \
| ^~~~~~~~~~
imap-send.c:1423:9: note: in expansion of macro 'curl_easy_setopt'
1423 | curl_easy_setopt(curl, CURLOPT_PORT, srvc->port);
| ^~~~~~~~~~~~~~~~
[... many more instances of nearly identical warnings...]
See for example this CI workflow run:
https://github.com/git/git/actions/runs/15454602308/job/43504278284#step:4:307
The most likely explanation is the entry "typecheck-gcc.h: fix the
typechecks" in cURL's release notes (https://curl.se/ch/8.14.0.html).
Nearly identical compile errors afflicted recently-updated Debian
setups, which have been addressed by `jk/curl-easy-setopt-typefix`.
However, on macOS Git is built with different build options, which
uncovered more instances of `int` values that need to be cast to
constants, which were not covered by 6f11c42e8edc (curl: fix integer
constant typechecks with curl_easy_setopt(), 2025-06-04). Let's
explicitly convert even those remaining `int` constants in
`curl_easy_setopt()` calls to `long` parameters.
In addition to looking at the compile errors of the `osx-gcc` job, I
verified that there are no other instances of the same issue that need
to be handled in this manner (and that might not be caught by our CI
builds because of yet other build options that might skip those code
parts), I ran the following command and inspected all 23 results
manually to ensure that the fix is now actually complete:
git grep -n curl_easy_setopt |
grep -ve ',.*, *[A-Za-z_"&]' \
-e ',.*, *[-0-9]*L)' \
-e ',.*,.* (long)'
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
As with the previous two commits, we should be passing long integers,
not regular ones, to curl_easy_setopt(), and compiling against curl 8.14
loudly complains if we don't.
This patch catches the remaining cases, which are ones where we pass
curl's own symbolic constants. We'll cast them to long manually in each
call.
It seems kind of weird to me that curl doesn't define these constants as
longs, since the point of them is to pass to curl_easy_setopt(). But in
the curl documentation and examples, they clearly show casting them as
part of the setopt calls. It may be that there is some reason not to
push the type into the macro, like backwards compatibility. I didn't
dig, as it doesn't really matter: we have to follow what existing curl
versions ask for anyway.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The curl documentation specifies that curl_easy_setopt() takes either:
...a long, a function pointer, an object pointer or a curl_off_t,
depending on what the specific option expects.
But when we pass an integer constant like "0", it will by default be a
regular non-long int. This has always been wrong, but seemed to work in
practice (I didn't dig into curl's implementation to see whether this
might actually be triggering undefined behavior, but it seems likely and
regardless we should do what the docs say).
This is especially important since curl has a type-checking macro that
causes building against curl 8.14 to produce many warnings. The specific
commit is due to their 79b4e56b3 (typecheck-gcc.h: fix the typechecks,
2025-04-22). Curiously, it does only seem to trigger when compiled with
-O2 for me.
We can fix it by just marking the constants with a long "L".
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The function `loose_object_path()` is a trivial wrapper around
`odb_loose_path()`, with the only exception that it always uses the
primary object database of the given repository. This doesn't really add
a ton of value though, so let's drop the function and inline it at every
callsite.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
* ps/object-file-cleanup:
object-store: merge "object-store-ll.h" and "object-store.h"
object-store: remove global array of cached objects
object: split out functions relating to object store subsystem
object-file: drop `index_blob_stream()`
object-file: split up concerns of `HASH_*` flags
object-file: split out functions relating to object store subsystem
object-file: move `xmmap()` into "wrapper.c"
object-file: move `git_open_cloexec()` to "compat/open.c"
object-file: move `safe_create_leading_directories()` into "path.c"
object-file: move `mkdir_in_gitdir()` into "path.c"
|
|
The "object-store-ll.h" header has been introduced to keep transitive
header dependendcies and compile times at bay. Now that we have created
a new "object-store.c" file though we can easily move the last remaining
additional bit of "object-store.h", the `odb_path_map`, out of the
header.
Do so. As the "object-store.h" header is now equivalent to its low-level
alternative we drop the latter and inline it into the former.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
curl supports a few options to control when and how often it should
instruct the OS to send TCP keepalives, like KEEPIDLE, KEEPINTVL, and
KEEPCNT. Until this point, there hasn't been a way for users to change
what values are used for these options, forcing them to rely on curl's
defaults.
But we do unconditionally enable TCP keepalives without giving users an
ability to tweak any fine-grained parameters. Ordinarily this isn't a
problem, particularly for users that have fast-enough connections,
and/or are talking to a server that has generous or nonexistent
thresholds for killing a connection it hasn't heard from in a while.
But it can present a problem when one or both of those assumptions fail.
For instance, I can reliably get an in-progress clone to be killed from
the remote end when cloning from some forges while using trickle to
limit my clone's bandwidth.
For those users and others who wish to more finely tune the OS's
keepalive behavior, expose configuration and environment variables which
allow setting curl's KEEPIDLE, KEEPINTVL, and KEEPCNT options.
Note that while KEEPIDLE and KEEPINTVL were added in curl 7.25.0,
KEEPCNT was added much more recently in curl 8.9.0. Per f7c094060c
(git-curl-compat: remove check for curl 7.25.0, 2024-10-23), both
KEEPIDLE and KEEPINTVL are set unconditionally. But since we may be
compiled with a curl that isn't as new as 8.9.0, only set KEEPCNT when
we have CURLOPT_TCP_KEEPCNT to begin with.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Acked-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
At the end of `get_curl_handle()` we call `set_curl_keepalive()` to
enable TCP keepalive probes on our CURL handle. `set_curl_keepalive()`
dates back to 47ce115370 (http: use curl's tcp keepalive if available,
2013-10-14), which conditionally compiled different variants of
`set_curl_keepalive()` depending on what version of curl we were
compiled with[^1].
As of f7c094060c (git-curl-compat: remove check for curl 7.25.0,
2024-10-23), we no longer conditionally compile `set_curl_keepalive()`
since we no longer support pre-7.25.0 versions of curl. But the version
of that function that we kept is really just a thin wrapper around
setting the TCP_KEEPALIVE option, so there's no reason to keep it in its
own function.
Inline the definition of `set_curl_keepalive()` to within
`get_curl_handle()` so that the setup of our CURL handle is
self-contained.
[1]: The details are spelled out in 47ce115370, but the gist is curl
7.25.0 and newer use CURLOPT_TCP_KEEPALIVE, older versions use
CURLOPT_SOCKOPTFUNCTION with a custom callback, and older versions
that predate even that option do nothing.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Acked-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
