git/serve.c, branch jch

hash: use uint32_t for object_id algorithm

2026-02-08T01:41:01Z

We currently use an int for this value, but we'll define this structure from Rust in a future commit and we want to ensure that our data types are exactly identical. To make that possible, use a uint32_t for the hash algorithm. Signed-off-by: brian m. carlson Signed-off-by: Junio C Hamano

Use legacy hash for legacy formats

2025-07-01T21:58:24Z

We have a large variety of data formats and protocols where no hash algorithm was defined and the default was assumed to always be SHA-1. Instead of explicitly stating SHA-1, let's use the constant to represent the legacy hash algorithm (which is still SHA-1) so that it's clear for documentary purposes that it's a legacy fallback option and not an intentional choice to use SHA-1. Signed-off-by: brian m. carlson Signed-off-by: Junio C Hamano

Add 'promisor-remote' capability to protocol v2

2025-02-18T19:05:37Z

When a server S knows that some objects from a repository are available from a promisor remote X, S might want to suggest to a client C cloning or fetching the repo from S that C may use X directly instead of S for these objects. Note that this could happen both in the case S itself doesn't have the objects and borrows them from X, and in the case S has the objects but knows that X is better connected to the world (e.g., it is in a $LARGEINTERNETCOMPANY datacenter with petabit/s backbone connections) than S. Implementation of the latter case, which would require S to omit in its response the objects available on X, is left for future improvement though. Then C might or might not, want to get the objects from X. If S and C can agree on C using X directly, S can then omit objects that can be obtained from X when answering C's request. To allow S and C to agree and let each other know about C using X or not, let's introduce a new "promisor-remote" capability in the protocol v2, as well as a few new configuration variables: - "promisor.advertise" on the server side, and: - "promisor.acceptFromServer" on the client side. By default, or if "promisor.advertise" is set to 'false', a server S will not advertise the "promisor-remote" capability. If S doesn't advertise the "promisor-remote" capability, then a client C replying to S shouldn't advertise the "promisor-remote" capability either. If "promisor.advertise" is set to 'true', S will advertise its promisor remotes with a string like: promisor-remote=[;]... where each element contains information about a single promisor remote in the form: name=[,url=] where is the urlencoded name of a promisor remote and is the urlencoded URL of the promisor remote named . For now, the URL is passed in addition to the name. In the future, it might be possible to pass other information like a filter-spec that the client may use when cloning from S, or a token that the client may use when retrieving objects from X. It is C's responsibility to arrange how it can reach X though, so pieces of information that are usually outside Git's concern, like proxy configuration, must not be distributed over this protocol. It might also be possible in the future for "promisor.advertise" to have other values. For example a value like "onlyName" could prevent S from advertising URLs, which could help in case C should use a different URL for X than the URL S is using. (The URL S is using might be an internal one on the server side for example.) By default or if "promisor.acceptFromServer" is set to "None", C will not accept to use the promisor remotes that might have been advertised by S. In this case, C will not advertise any "promisor-remote" capability in its reply to S. If "promisor.acceptFromServer" is set to "All" and S advertised some promisor remotes, then on the contrary, C will accept to use all the promisor remotes that S advertised and C will reply with a string like: promisor-remote=[;]... where the elements are the urlencoded names of all the promisor remotes S advertised. In a following commit, other values for "promisor.acceptFromServer" will be implemented, so that C will be able to decide the promisor remotes it accepts depending on the name and URL it received from S. So even if that name and URL information is not used much right now, it will be needed soon. Helped-by: Taylor Blau Helped-by: Patrick Steinhardt Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano

serve: stop using `the_repository`

2024-12-18T18:44:30Z

Stop using `the_repository` in the "serve" subsystem by passing in a repository when advertising capabilities or serving requests. Adjust callers accordingly by using `the_repository`. While there may be some callers that have a repository available in their context, this trivial conversion allows for easier verification and bubbles up the use of `the_repository` by one level. 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

drop trailing newline from warning/error/die messages

2024-09-05T16:07:12Z

Our error reporting routines append a trailing newline, and the strings we pass to them should not include them (otherwise we get an extra blank line after the message). These cases were all found by looking at the results of: git grep -P '[^_](error|error_errno|warning|die|die_errno)\(.*\\n"[,)]' '*.c' Note that we _do_ sometimes include a newline in the middle of such messages, to create multiline output (hence our grep matching "," or ")" after we see the newline, so we know we're at the end of the string). It's possible that one or more of these cases could intentionally be including a blank line at the end, but having looked at them all manually, I think these are all just mistakes. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

hash-ll: merge with "hash.h"

2024-06-14T17:26:33Z

The "hash-ll.h" header was introduced via d1cbe1e6d8 (hash-ll.h: split out of hash.h to remove dependency on repository.h, 2023-04-22) to make explicit the split between hash-related functions that rely on the global `the_repository`, and those that don't. This split is no longer necessary now that we we have removed the reliance on `the_repository`. Merge "hash-ll.h" back into "hash.h". This causes some code units to not include "repository.h" anymore, which requires us to add some forward declarations. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

global: introduce `USE_THE_REPOSITORY_VARIABLE` macro

2024-06-14T17:26:33Z

Use of the `the_repository` variable is deprecated nowadays, and we slowly but steadily convert the codebase to not use it anymore. Instead, callers should be passing down the repository to work on via parameters. It is hard though to prove that a given code unit does not use this variable anymore. The most trivial case, merely demonstrating that there is no direct use of `the_repository`, is already a bit of a pain during code reviews as the reviewer needs to manually verify claims made by the patch author. The bigger problem though is that we have many interfaces that implicitly rely on `the_repository`. Introduce a new `USE_THE_REPOSITORY_VARIABLE` macro that allows code units to opt into usage of `the_repository`. The intent of this macro is to demonstrate that a certain code unit does not use this variable anymore, and to keep it from new dependencies on it in future changes, be it explicit or implicit For now, the macro only guards `the_repository` itself as well as `the_hash_algo`. There are many more known interfaces where we have an implicit dependency on `the_repository`, but those are not guarded at the current point in time. Over time though, we should start to add guards as required (or even better, just remove them). Define the macro as required in our code units. As expected, most of our code still relies on the global variable. Nearly all of our builtins rely on the variable as there is no way yet to pass `the_repository` to their entry point. For now, declare the macro in "biultin.h" to keep the required changes at least a little bit more contained. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

upload-pack: disallow object-info capability by default

2024-02-28T22:42:01Z

We added an "object-info" capability to the v2 upload-pack protocol in a2ba162cda (object-info: support for retrieving object info, 2021-04-20). In the almost 3 years since, we have not added any client-side support, and it does not appear to exist in other implementations either (JGit understands the verb on the server side, but not on the client side). Since this largely unused code is accessible over the network by default, it increases the attack surface of upload-pack. I don't know of any particularly severe problem, but one issue is that because of the request/response nature of the v2 protocol, it will happily read an unbounded number of packets, adding each one to a string list (without regard to whether they are objects we know about, duplicates, etc). This may be something we want to improve in the long run, but in the short term it makes sense to disable the feature entirely. We'll add a config option as an escape hatch for anybody who wants to develop the feature further. A more gentle option would be to add the config option to let people disable it manually, but leave it enabled by default. But given that there's no client side support, that seems like the wrong balance with security. Disabling by default will slow adoption a bit once client-side support does become available (there were some patches[1] in 2022, but nothing got merged and there's been nothing since). But clients have to deal with older servers that do not understand the option anyway (and the capability system handles that), so it will just be a matter of servers flipping their config at that point (and hopefully once any unbounded allocations have been addressed). [jk: this is a patch that GitHub has been running for several years, but rebased forward and with a new commit message for upstream] [1] https://lore.kernel.org/git/20220208231911.725273-1-calvinwan@google.com/ Signed-off-by: Taylor Blau Signed-off-by: Jeff King Signed-off-by: Junio C Hamano