git/http-push.c, branch v2.50.0

curl: pass `long` values where expected

2025-06-06T15:12:24Z

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 Signed-off-by: Junio C Hamano

curl: fix integer constant typechecks with curl_easy_setopt()

2025-06-04T21:17:53Z

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 Signed-off-by: Junio C Hamano

treewide: convert users of `repo_has_object_file()` to `has_object()`

2025-04-29T17:08:13Z

As the comment of `repo_has_object_file()` and its `_with_flags()` variant tells us, these functions are considered to be deprecated in favor of `has_object()`. There are a couple of slight benefits in favor of the replacement: - The new function has a short-and-sweet name. - More explicit defaults: `has_object()` doesn't fetch missing objects via promisor remotes, and neither does it reload packfiles if an object wasn't found by default. This ensures that it becomes immediately obvious when a simple object existence check may result in expensive actions. Most importantly though, it is confusing that we have two sets of functions that ultimately do the same thing, but with different defaults. Start sunsetting `repo_has_object_file()` and its `_with_flags()` sibling by replacing all callsites with `has_object()`: - `repo_has_object_file(...)` is equivalent to `has_object(..., HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR)`. - `repo_has_object_file_with_flags(..., OBJECT_INFO_QUICK | OBJECT_INFO_SKIP_FETCH_OBJECT)` is equivalent to `has_object(..., 0)`. - `repo_has_object_file_with_flags(..., OBJECT_INFO_SKIP_FETCH_OBJECT)` is equivalent to `has_object(..., HAS_OBJECT_RECHECK_PACKED)`. - `repo_has_object_file_with_flags(..., OBJECT_INFO_QUICK)` is equivalent to `has_object(..., HAS_OBJECT_FETCH_PROMISOR)`. The replacements should be functionally equivalent. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

object-store: merge "object-store-ll.h" and "object-store.h"

2025-04-15T15:24:37Z

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 Signed-off-by: Junio C Hamano

object-file: split out functions relating to object store subsystem

2025-04-15T15:24:36Z

While we have the "object-store.h" header, most of the functionality for object stores is actually hosted in "object-file.c". This makes it hard to find relevant functions and causes us to mix up concerns. Split out functions relating to the object store subsystem into a new "object-store.c" file. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

global: adapt callers to use generic hash context helpers

2025-01-31T18:06:11Z

Adapt callers to use generic hash context helpers instead of using the hash algorithm to update them. This makes the callsites easier to reason about and removes the possibility that the wrong hash algorithm is used to update the hash context's state. And as a nice side effect this also gets rid of a bunch of users of `the_hash_algo`. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

hash: stop typedeffing the hash context

2025-01-31T18:06:10Z

We generally avoid using `typedef` in the Git codebase. One exception though is the `git_hash_ctx`, likely because it used to be a union rather than a struct until the preceding commit refactored it. But now that it is a normal `struct` there isn't really a need for a typedef anymore. Drop the typedef and adapt all callers accordingly. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

global: trivial conversions to fix `-Wsign-compare` warnings

2024-12-06T11:20:04Z

We have a bunch of loops which iterate up to an unsigned boundary using a signed index, which generates warnigs because we compare a signed and unsigned value in the loop condition. Address these sites for trivial cases and enable `-Wsign-compare` warnings for these code units. This patch only adapts those code units where we can drop the `DISABLE_SIGN_COMPARE_WARNINGS` macro in the same step. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

global: mark code units that generate warnings with `-Wsign-compare`

2024-12-06T11:20:02Z

Mark code units that generate warnings with `-Wsign-compare`. This allows for a structured approach to get rid of all such warnings over time in a way that can be easily measured. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

packfile: convert find_sha1_pack() to use object_id

2024-10-25T21:35:46Z

The find_sha1_pack() function has a few problems: - it's badly named, since it works with any object hash - it takes the hash as a bare pointer rather than an object_id struct We can fix both of these easily, as all callers actually have a real object_id anyway. I also found the existence of this function somewhat confusing, as it is about looking in an arbitrary set of linked packed_git structs. It's good for things like dumb-http which are looking in downloaded remote packs, and not our local packs. But despite the name, it is not a good way to find the pack which contains a local object (it skips the use of the midx, the pack mru list, and so on). So let's also add an explanatory comment above the declaration that may point people in the right direction. I suspect the calls in fast-import.c, which use the packed_git list from the repository struct, could actually just be using find_pack_entry(). But since we'd need to keep it anyway for dumb-http, I didn't dig further there. If we eventually drop dumb-http support, then it might be worth examining them to see if we can get rid of the function entirely. Signed-off-by: Jeff King Signed-off-by: Taylor Blau