summaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2023-02-24ref-filter: drop unused atom parameter from get_worktree_path()Jeff King1-2/+2
The get_worktree_path() function is used to populate the %(worktreepath) value, but it has never used its "atom" parameter since it was added in 2582083fa1 (ref-filter: add worktreepath atom, 2019-04-28). Normally we'd use the atom struct to cache any work we do, but in this case there's a global hashmap that does that caching already. So we can just drop the unused parameter. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23diff.h: remove unnecessary include of object.hElijah Newren1-1/+0
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23Remove unnecessary includes of builtin.hElijah Newren3-3/+1
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23treewide: replace cache.h with more direct headers, where possibleElijah Newren12-11/+30
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23replace-object.h: move read_replace_refs declaration from cache.h to hereElijah Newren15-9/+21
Adjust several files to be more explicit about their dependency on replace-objects to accommodate this change. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23object-store.h: move struct object_info from cache.hElijah Newren4-65/+67
Move struct object_info, and a few related #define's from cache.h to object-store.h. A surprising effect of this change is that replace-object.h, which includes object-store.h, now needs to directly include cache.h since that is where read_replace_refs is declared and that variable is used in one of its inline functions. The next commit will move that declaration and fix that unfortunate new direct inclusion of cache.h. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23dir.h: refactor to no longer need to include cache.hElijah Newren5-35/+43
Moving a few functions around allows us to make dir.h no longer need to include cache.h. This commit is best viewed with: git log -1 -p --color-moved Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23object.h: stop depending on cache.h; make cache.h depend on object.hElijah Newren17-32/+36
Things should be able to depend on object.h without pulling in all of cache.h. Move an enum to allow this. Note that a couple files previously depended on things brought in through cache.h indirectly (revision.h -> commit.h -> object.h -> cache.h). As such, this change requires making existing dependencies more explicit in half a dozen files. The inclusion of strbuf.h in some headers if of particular note: these headers directly embedded a strbuf in some new structs, meaning they should have been including strbuf.h all along but were indirectly getting the necessary definitions. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23ident.h: move ident-related declarations out of cache.hElijah Newren11-59/+79
These functions were all defined in a separate ident.c already, so create ident.h and move the declarations into that file. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23pretty.h: move has_non_ascii() declaration from commit.hElijah Newren3-3/+4
The function is defined in pretty.c, so this moves the declaration to a more logical place. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23cache.h: remove dependence on hex.h; make other files include it explicitlyElijah Newren173-1/+172
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23hex.h: move some hex-related declarations from cache.hElijah Newren7-82/+95
hex.c contains code for hex-related functions, but for some reason these functions were declared in the catch-all cache.h. Move the function declarations into a hex.h header instead. This also allows us to remove includes of cache.h from a few C files. For now, we make cache.h include hex.h, so that it is easier to review the direct changes being made by this patch. In the next patch, we will remove that, and add the necessary direct '#include "hex.h"' in the hundreds of C files that need it. Note that reviewing the header changes in this commit might be simplified via git log --no-walk -p --color-moved $COMMIT -- '*.h'` In particular, it highlights the simple movement of code in .h files rather nicely. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23hash.h: move some oid-related declarations from cache.hElijah Newren7-37/+42
These defines and enum are all oid-related and as such seem to make more sense being included in hash.h. Further, moving them there allows us to remove some includes of cache.h in other files. The change to line-log.h might look unrelated, but line-log.h includes diffcore.h, which previously included cache.h, which included the kitchen sink. Since this patch makes diffcore.h no longer include cache.h, the compiler complains about the 'struct string_list *' function parameter. Add a forward declaration for struct string_list to address this. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23alloc.h: move ALLOC_GROW() functions from cache.hElijah Newren91-134/+219
This allows us to replace includes of cache.h with includes of the much smaller alloc.h in many places. It does mean that we also need to add includes of alloc.h in a number of C files. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23treewide: remove unnecessary cache.h includes in source filesElijah Newren29-30/+25
We had several C files include cache.h unnecessarily. Replace those with an include of "git-compat-util.h" instead. Much like the previous commit, these have all been verified via both ensuring that gcc -E $SOURCE_FILE | grep '"cache.h"' found no hits and that make DEVELOPER=1 ${OBJECT_FILE_FOR_SOURCE_FILE} successfully compiles without warnings. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23treewide: remove unnecessary cache.h includesElijah Newren10-11/+7
We had several header files include cache.h unnecessarily. Remove those. These have all been verified via both ensuring that gcc -E $HEADER | grep '"cache.h"' found no hits and that cat >temp.c <<EOF && #include "git-compat-util.h" #include "$HEADER" int main() {} EOF gcc -c temp.c successfully compiles without warnings. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23treewide: remove unnecessary git-compat-util.h includes in headersElijah Newren11-15/+0
For sanity, we should probably do one of the following: (a) make C and header files both depend upon everything they need (b) consistently exclude git-compat-util.h from headers and require it be the first include in C files Currently, we have some of the headers following (a) and others following (b), which makes things messy. In the past I was pushed towards (b), as per [1] and [2]. Further, during this series I discovered that this mixture empirically will mean that we end up with C files that do not directly include git-compat-util.h, and do include headers that don't include git-compat-util.h, with the result that we likely have headers included before an indirect inclusion of git-compat-util.h. Since git-compat-util.h has tricky platform-specific stuff that is meant to be included before everything else, this state of affairs is risky and may lead to things breaking in subtle ways (and only on some platforms) as per [1] and [2]. Since including git-compat-util.h in existing header files makes it harder for us to catch C files that are missing that include, let's switch to (b) to make the enforcement of this rule easier. Remove the inclusion of git-compat-util.h from header files other than the ones that have been approved as alternate first includes. [1] https://lore.kernel.org/git/20180811173406.GA9119@sigill.intra.peff.net/ [2] https://lore.kernel.org/git/20180811174301.GA9287@sigill.intra.peff.net/ Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23treewide: ensure one of the appropriate headers is sourced firstElijah Newren14-3/+19
We had several C files ignoring the rule to include one of the appropriate headers first; fix that. While at it, the rule in Documentation/CodingGuidelines about which header to include has also fallen out of sync, so update the wording to mention other allowed headers. Unfortunately, C files in reftable/ don't actually follow the previous or updated rule. If you follow the #include chain in its C files, reftable/system.h _tends_ to be first (i.e. record.c first includes record.h, which first includes basics.h, which first includees system.h), but not always (e.g. publicbasics.c includes another header first that does not include system.h). However, I'm going to punt on making actual changes to the C files in reftable/ since I do not want to risk bringing it out-of-sync with any version being used externally. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23rebase -i: fix parsing of "fixup -C<commit>"Phillip Wood4-6/+42
If the user omits the space between "-C" and the commit in a fixup command then it is parsed as an ordinary fixup and the commit message is not updated as it should be. Fix this by making the space between "-C" and "<commit>" optional as it is for the "merge" command. Note that set_replace_editor() is changed to set $GIT_SEQUENCE_EDITOR instead of $EDITOR in order to be able to replace the todo list and reword commits with $FAKE_COMMIT_MESSAGE. This is safe as all the existing users are using set_replace_editor() to replace the todo list. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23rebase -i: match whole word in is_command()Phillip Wood3-12/+14
When matching an unabbreviated command is_command() only does a prefix match which means it parses "pickled" as TODO_PICK. parse_insn_line() does error out because is_command() only advances as far as the end of "pick" so it looks like the command name is not followed by a space but the error message is "missing arguments for pick" rather than telling the user that the "pickled" is not a valid command. Fix this by ensuring the match is follow by whitespace or the end of the string as we already do for abbreviated commands. The (*bol = p) at the end of the condition is a bit cute for my taste but I decided to leave it be for now. Rather than add new tests the existing tests for bad commands are adapted to use a bad command name that triggers the prefix matching bug. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23t5559: make SSL/TLS the defaultJeff King1-0/+1
The point of t5559 is run the regular t5551 tests with HTTP/2. But it does so with the "h2c" protocol, which uses cleartext upgrades from HTTP/1.1 to HTTP/2 (rather than learning about HTTP/2 support during the TLS negotiation). This has a few problems: - it's not very indicative of the real world. In practice, most servers that support HTTP/2 will also support TLS. - support for upgrading does not seem as robust. In particular, we've run into bugs in some versions of Apache's mod_http2 that trigger only with the upgrade mode. See: https://lore.kernel.org/git/Y8ztIqYgVCPILJlO@coredump.intra.peff.net/ So the upside is that this change makes our HTTP/2 tests more robust and more realistic. The downside is that if we can't set up SSL for any reason, we'll skip the tests (even though you _might_ have been able to run the HTTP/2 tests the old way). We could probably have a conditional fallback, but it would be complicated for little gain, and it's not even clear it would help (i.e., would any test environment even have HTTP/2 but not SSL support?). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23t5559: fix test failures with LIB_HTTPD_SSLJeff King1-5/+18
One test needs to be tweaked in order for t5559 to pass with SSL/TLS set up. When we make our initial clone, we check that the curl trace of requests is what we expected. But we need to fix two things: - along with ignoring "data" lines from the trace, we need to ignore "SSL data" lines - when TLS is used, the server is able to tell the client (via ALPN) that it supports HTTP/2 before the first HTTP request is made. So rather than request an upgrade using an HTTP header, it can just speak HTTP/2 immediately With this patch, running: LIB_HTTPD_SSL=1 ./t5559-http-fetch-smart-http2.sh works, whereas it did not before. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23t/lib-httpd: enable HTTP/2 "h2" protocol, not just h2cJeff King1-1/+1
Commit 73c49a4474 (t: run t5551 tests with both HTTP and HTTP/2, 2022-11-11) added Apache config to enable HTTP/2. However, it only enabled the "h2c" protocol, which allows cleartext HTTP/2 (generally based on an upgrade header during an HTTP/1.1 request). This is what t5559 is generally testing, since by default we don't set up SSL/TLS. However, it should be possible to run t5559 with LIB_HTTPD_SSL set. In that case, Apache will advertise support for HTTP/2 via ALPN during the TLS handshake. But we need to tell it support "h2" (the non-cleartext version) to do so. Without that, then curl does not even try to do the HTTP/1.1 upgrade (presumably because after seeing that we did TLS but didn't get the ALPN indicator, it assumes it would be fruitless). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23t/lib-httpd: respect $HTTPD_PROTO in expect_askpass()Jeff King1-3/+3
When the HTTP tests are run with LIB_HTTPD_SSL in the environment, then we access the test server as https://. This causes expect_askpass to complain, because it tries to blindly match "http://" in the prompt shown to the user. We can adjust this to use $HTTPD_PROTO, which is set during the setup phase. Note that this is enough for t5551 and t5559 to pass when run with https, but there are similar problems in other scripts that will need to be fixed before the whole suite can run with LIB_HTTPD_SSL. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23t5551: drop curl trace lines without headersJeff King1-1/+1
We pick apart a curl trace, looking for "=> Send header:" and so on, and matching against an expected set of requests and responses. We remove "== Info" lines entirely. However, our parser is fooled when running the test with LIB_HTTPD_SSL on Ubuntu 20.04 (as found in our linux-gcc CI job), as curl hands us an "Info" buffer with a newline, and we get: == Info: successfully set certificate verify locations: == Info: CAfile: /etc/ssl/certs/ca-certificates.crt CApath: /etc/ssl/certs => Send SSL data[...] which results in the "CApath" line ending up in the cleaned-up output, causing the test to fail. Arguably the tracing code should detect this and put it on two separate "== Info" lines. But this is actually a curl bug, fixed by their 80d73bcca (tls: provide the CApath verbose log on its own line, 2020-08-18). It's simpler to just work around it here. Since we are using GIT_TRACE_CURL, every line should just start with one of "<=", "==", or "=>", and we can throw away anything else. In fact, we can just replace the pattern for deleting "*" lines. Those were from the old GIT_CURL_VERBOSE output, but we switched over in 14e24114d9 (t5551-http-fetch-smart.sh: use the GIT_TRACE_CURL environment var, 2016-09-05). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23t5551: handle v2 protocol in cookie testJeff King1-8/+11
After making a request, we check that it stored the expected cookies. This depends on the protocol version, because the cookies we store depend on the exact requests we made (and for ls-remote, v2 will always hit /git-upload-pack to get the refs, whereas v0 is happy with the initial ref advertisement). As a result, hardly anybody runs this test, as you'd have to manually set GIT_TEST_PROTOCOL_VERSION=0 to do so. Let's teach it to handle both protocol versions. One way to do this would be to make the expectation conditional on the protocol used. But there's a simpler solution. The reason that v0 doesn't hit /git-upload-pack is that ls-remote doesn't fetch any objects. If we instead do a fetch (making sure there's an actual object to grab), then both v0 and v2 will hit the same endpoints and set the same cookies. Note that we do have to clean up our new tag here; otherwise it confuses the later "clone 2,000 tags" test. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23t5551: simplify expected cookie fileJeff King1-3/+2
After making an HTTP request that should store cookies, we check that the expected values are in the cookie file. We don't want to look at the whole file, because it has noisy comments at the top that we shouldn't depend on. But we strip out the interesting bits using "tail -3", which is brittle. It requires us to put an extra blank line in our expected output, and it would fail to notice any reordering or extra content in the cookie file. Instead, let's just grep for non-blank lines that are not comments, which more directly describes what we're interested in. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23t5551: handle v2 protocol in upload-pack service testJeff King1-13/+3
We perform a clone and a fetch, and then check that we saw the expected requests in Apache's access log. In the v2 protocol, there will be one extra request to /git-upload-pack for each operation (since the initial /info/refs probe is just used to upgrade the protocol). As a result, this test is a noop unless the use of the v0 protocol is forced. Which means that hardly anybody runs it, since you have to do so manually. Let's update it to handle v2 and run it always. We could do this by just conditionally adding in the extra POST lines. But if we look at the origin of the test in 7da4e2280c (test smart http fetch and push, 2009-10-30), the point is really just to make sure that the smart git-upload-pack service was used at all. So rather than counting up the individual requests, let's just make sure we saw each of the expected types. This is a bit looser, but makes maintenance easier. Since we're now matching with grep, we can also loosen the HTTP/1.1 match, which allows this test to pass when run with HTTP/2 via t5559. That lets: GIT_TEST_PROTOCOL_VERSION=0 ./t5559-http-fetch-smart-http2.sh run to completion, which previously failed (and of course it works if you use v2, as well). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23t5551: handle v2 protocol when checking curl traceJeff King1-11/+25
After cloning an http repository, we check the curl trace to make sure the expected requests were made. But since the expected trace was never updated to handle v2, it is only run when you ask the test suite to run in v0 mode (which hardly anybody does). Let's update it to handle both protocols. This isn't too hard since v2 just sends an extra header and an extra request. So we can just annotate those extra lines and strip them out for v0 (and drop the annotations for v2). I didn't bother handling v1 here, as it's not really of practical interest (it would drop the extra v2 request, but still have the "git-protocol" lines). There's a similar tweak needed at the end. Since we check the "accept-encoding" value loosely, we grep for it rather than finding it in the verbatim trace. This grep insists that there are exactly 2 matches, but of course in v2 with the extra request there are 3. We could tweak the number, but it's simpler still to just check that we saw at least one match. The verbatim check already confirmed how many instances of the header we have; we're really just checking here that "gzip" is in the value (it's possible, of course, that the headers could have different values, but that seems like an unlikely bug). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23t5551: stop forcing clone to run with v0 protocolJeff King1-1/+1
In the "clone http repository" test, we check the curl trace to make sure the expected requests were made. This whole script was marked to handle only the v0 protocol in d790ee1707 (tests: fix protocol version for overspecifications, 2019-02-25). That makes sense, since v2 requires an extra request, so tests as specific as this would fail unless modified. Later, in preparation for v2 becoming the default, this was tweaked by 8a1b0978ab (test: request GIT_TEST_PROTOCOL_VERSION=0 when appropriate, 2019-12-23). There we run the trace check only if the user has explicitly asked to test protocol version 0. But it also forced the clone itself to run with the v0 protocol. This makes the check for "can we expect a v0 trace" silly; it will always be v0. But much worse, it means that the clone we are testing is not like the one that normal users would run. They would use the defaults, which are now v2. And since this is supposed to be a basic check of clone-over-http, we should do the same. Let's fix this by dropping the extra v0 override. The test still passes because the trace checking only kicks in if we asked to use v0 explicitly (this is the same as before; even though we were running a v0 clone, unless you specifically set GIT_TEST_PROTOCOL_VERSION=0, the trace check was always skipped). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23t5551: handle HTTP/2 when checking curl traceJeff King1-3/+11
We check that the curl trace of a clone has the lines we expect, but this won't work when we run the test under t5559, because a few details are different under HTTP/2 (but nobody noticed because it only happens when you manually set GIT_TEST_PROTOCOL_VERSION to "0"). We can handle both HTTP protocols with a few tweaks: - we'll drop the HTTP "101 Switching Protocols" response, as well as various protocol upgrade headers. These details aren't interesting to us. We just want to make sure the correct protocol was used (and we do in the main request/response lines). - successful HTTP/2 responses just say "200" and not "200 OK"; we can normalize these - replace HTTP/1.1 with a variable in the request/response lines. We can use the existing $HTTP_PROTO for this, as it's already set to "HTTP/2" when appropriate. We do need to tweak the fallback value to "HTTP/1.1" to match what curl will write (prior to this patch, the fallback value didn't matter at all; we only checked if it was the literal string "HTTP/2"). Note that several lines still expect HTTP/1.1 unconditionally. The first request does so because the client requests an upgrade during the request. The POST request and response do so because you can't do an upgrade if there is a request body. (This will all be different if we trigger HTTP/2 via ALPN, but the tests aren't yet capable of that). This is enough to let: GIT_TEST_PROTOCOL_VERSION=0 ./t5559-http-fetch-smart-http2.sh pass the "clone http repository" test (but there are some other failures later on). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23t5551: lower-case headers in expected curl traceJeff King1-25/+30
There's a test in t5551 which checks the curl trace (after simplifying it a bit). It doesn't work with HTTP/2, because in that case curl outputs all of the headers in lower-case. Even though this test is run with HTTP/2 by t5559, nobody has noticed because checking the trace only happens if GIT_TEST_PROTOCOL_VERSION is manually set to "0". Let's fix this by lower-casing all of the header names in the trace, and then checking for those in our expected code (this is easier than making HTTP/2 traces look like HTTP/1.1, since HTTP/1.1 uses title-casing). Sadly, we can't quite do this in our existing sed script. This works if you have GNU sed: s/^\\([><]\\) \\([A-Za-z0-9-]*:\\)/\1 \L\2\E/ but \L is a GNU-ism, and I don't think there's a portable solution. We could just "tr A-Z a-z" on the way in, of course, but that makes the non-header parts harder to read (e.g., lowercase "post" requests). But to paraphrase Baron Munchausen, I have learned from experience that a modicum of Perl can be most efficacious. Note that this doesn't quite get the test passing with t5559; there are more fixes needed on top. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23t5551: drop redundant grep for Accept-LanguageJeff King1-4/+1
Commit b0c4adcdd7 (remote-curl: send Accept-Language header to server, 2022-07-11) added tests to make sure the header is sent via HTTP. However, it checks in two places: 1. In the expected trace output, we check verbatim for the header and its value. 2. Afterwards, we grep for the header again in the trace file. This (2) is probably cargo-culted from the earlier grep for Accept-Encoding. It is needed for the encoding because we smudge the value of that header when doing the verbatim check; see 1a53e692af (remote-curl: accept all encodings supported by curl, 2018-05-22). But we don't do so for the language header, so any problem that the "grep" would catch in (2) would already have been caught by (1). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23t5541: simplify and move "no empty path components" testJeff King2-18/+9
Commit 9ee6bcd398 (t5541-http-push: add test for URLs with trailing slash, 2010-04-08) added a test that clones a URL with a trailing slash, and confirms that we don't send a doubled slash (like "$url//info/refs") to the server. But this test makes no sense in t5541, which is about pushing. It should have been added in t5551. Let's move it there. But putting it at the end is tricky, since it checks the entire contents of the Apache access log. We could get around this by clearing the log before our test. But there's an even simpler solution: just make sure no doubled slashes appear in the log (fortunately, "http://" does not appear in the log itself). As a bonus, this also lets us drop the check for the v0 protocol (which is otherwise necessary since v2 makes multiple requests, and check_access_log insists on exactly matching the number of requests, even though we don't care about that here). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23t5541: stop marking "used receive-pack service" test as v0 onlyJeff King1-6/+1
We have a test which checks to see if a request to git-receive-pack was made. Originally, it was checking the entire set of requests made in the script so far, including clones, and thus it would break when run with the v2 protocol (since that implies an extra request for fetches). Since the previous commit, though, we are only checking the requests made by a single push. And since there is no v2 push protocol, the test now passes no matter what's in GIT_TEST_PROTOCOL_VERSION. We can just run it all the time. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23t5541: run "used receive-pack service" test earlierJeff King1-26/+18
There's a test in t5541 that confirms that "git push" makes two requests (a GET to /info/refs, and a POST to /git-receive-pack). However, it's a noop unless GIT_TEST_PROTOCOL_VERSION is set to "0", due to 8a1b0978ab (test: request GIT_TEST_PROTOCOL_VERSION=0 when appropriate, 2019-12-23). This means that almost nobody runs it. And indeed, it has been broken since b0c4adcdd7 (remote-curl: send Accept-Language header to server, 2022-07-11). But the fault is not in that commit, but in how brittle the test is. It runs after several operations have been performed, which means that it expects to see the complete set of requests made so far in the script. Commit b0c4adcdd7 added a new test, which means that the "used receive-pack service" test must be updated, too. Let's fix this by making the test less brittle. We'll move it higher in the script, right after the first push has completed. And we'll clear the access log right before doing the push, so we'll see only the requests from that command. This is technically testing less, in that we won't check that all of those other requests also correctly used smart http. But there's no particular reason think that if the first one did, the others wouldn't. After this patch, running: GIT_TEST_PROTOCOL_VERSION=0 ./t5541-http-push-smart.sh passes, whereas it did not before. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-22credential: new attribute password_expiry_utcM Hickford6-2/+125
Some passwords have an expiry date known at generation. This may be years away for a personal access token or hours for an OAuth access token. When multiple credential helpers are configured, `credential fill` tries each helper in turn until it has a username and password, returning early. If Git authentication succeeds, `credential approve` stores the successful credential in all helpers. If authentication fails, `credential reject` erases matching credentials in all helpers. Helpers implement corresponding operations: get, store, erase. The credential protocol has no expiry attribute, so helpers cannot store expiry information. Even if a helper returned an improvised expiry attribute, git credential discards unrecognised attributes between operations and between helpers. This is a particular issue when a storage helper and a credential-generating helper are configured together: [credential] helper = storage # eg. cache or osxkeychain helper = generate # eg. oauth `credential approve` stores the generated credential in both helpers without expiry information. Later `credential fill` may return an expired credential from storage. There is no workaround, no matter how clever the second helper. The user sees authentication fail (a retry will succeed). Introduce a password expiry attribute. In `credential fill`, ignore expired passwords and continue to query subsequent helpers. In the example above, `credential fill` ignores the expired password and a fresh credential is generated. If authentication succeeds, `credential approve` replaces the expired password in storage. If authentication fails, the expired credential is erased by `credential reject`. It is unnecessary but harmless for storage helpers to self prune expired credentials. Add support for the new attribute to credential-cache. Eventually, I hope to see support in other popular storage helpers. Example usage in a credential-generating helper https://github.com/hickford/git-credential-oauth/pull/16 Signed-off-by: M Hickford <mirth.hickford@gmail.com> Reviewed-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-22The seventeenth batchJunio C Hamano1-0/+12
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-22Merge branch 'ab/hook-api-with-stdin'Junio C Hamano8-36/+54
Extend the run-hooks API to allow feeding data from the standard input when running the hook script(s). * ab/hook-api-with-stdin: hook: support a --to-stdin=<path> option sequencer: use the new hook API for the simpler "post-rewrite" call hook API: support passing stdin to hooks, convert am's 'post-rewrite' run-command: allow stdin for run_processes_parallel run-command.c: remove dead assignment in while-loop
2023-02-22Merge branch 'ab/various-leak-fixes'Junio C Hamano77-62/+182
Leak fixes. * ab/various-leak-fixes: push: free_refs() the "local_refs" in set_refspecs() push: refactor refspec_append_mapped() for subsequent leak-fix receive-pack: release the linked "struct command *" list grep API: plug memory leaks by freeing "header_list" grep.c: refactor free_grep_patterns() builtin/merge.c: free "&buf" on "Your local changes..." error builtin/merge.c: use fixed strings, not "strbuf", fix leak show-branch: free() allocated "head" before return commit-graph: fix a parse_options_concat() leak http-backend.c: fix cmd_main() memory leak, refactor reg{exec,free}() http-backend.c: fix "dir" and "cmd_arg" leaks in cmd_main() worktree: fix a trivial leak in prune_worktrees() repack: fix leaks on error with "goto cleanup" name-rev: don't xstrdup() an already dup'd string various: add missing clear_pathspec(), fix leaks clone: use free() instead of UNLEAK() commit-graph: use free_commit_graph() instead of UNLEAK() bundle.c: don't leak the "args" in the "struct child_process" tests: mark tests as passing with SANITIZE=leak
2023-02-22Merge branch 'jk/doc-ls-remote-matching'Junio C Hamano2-7/+14
Doc update. * jk/doc-ls-remote-matching: doc/ls-remote: clarify pattern format doc/ls-remote: cosmetic cleanups for examples
2023-02-22Merge branch 'rs/cache-tree-strbuf-growth-fix'Junio C Hamano1-2/+1
Remove unnecessary explicit sizing of strbuf. * rs/cache-tree-strbuf-growth-fix: cache-tree: fix strbuf growth in prime_cache_tree_rec()
2023-02-22Merge branch 'ab/the-index-compatibility'Junio C Hamano12-87/+62
Remove more remaining uses of macros that relies on the_index singleton instance without explicitly spelling it out. * ab/the-index-compatibility: cocci & cache.h: remove "USE_THE_INDEX_COMPATIBILITY_MACROS" cache-tree API: remove redundant update_main_cache_tree() cocci & cache-tree.h: migrate "write_cache_as_tree" to "*_index_*" cocci & cache.h: apply pending "index_cache_pos" rule cocci & cache.h: fully apply "active_nr" part of index-compatibility builtin/rm.c: use narrower "USE_THE_INDEX_VARIABLE"
2023-02-22Merge branch 'en/name-rev-make-taggerdate-much-less-important'Junio C Hamano2-11/+9
"git name-rev" heuristics update. * en/name-rev-make-taggerdate-much-less-important: name-rev: fix names by dropping taggerdate workaround
2023-02-21range-diff: let '--abbrev' option takes effectTeng Long2-4/+39
As mentioned in 'git-range-diff.txt': "`git range-diff` also accepts the regular diff options (see linkgit:git-diff[1])...", but '--abbrev' is not in the "regular" scope. In Git, the "abbrev" of an object may not be a fixed value in different repositories, depending on the needs of the them(Linus mentioned in e6c587c7 in 2016: "the Linux kernel project needs 11 to 12 hexdigits" at that time ), that's why a user may want to display abbrev according to a specified length. Although a similar effect can be achieved through configuration (like: git -c core.abbrev=<abbrev>), but based on ease of use (many users may not know that the -c option can be specified) and the description in existing document, supporting users to directly use '--abbrev', could be a good way. Signed-off-by: Teng Long <dyroneteng@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-21fetch: choose a sensible default with --jobs=0 againMatthias Aßhauer2-0/+8
prior to 51243f9 (run-command API: don't fall back on online_cpus(), 2022-10-12) `git fetch --multiple --jobs=0` would choose some default amount of jobs, similar to `git -c fetch.parallel=0 fetch --multiple`. While our documentation only ever promised that `fetch.parallel` would fall back to a "sensible default", it makes sense to do the same for `--jobs`. So fall back to online_cpus() and not BUG() out. This fixes https://github.com/git-for-windows/git/issues/4302 Reported-by: Drew Noakes <drnoakes@microsoft.com> Signed-off-by: Matthias Aßhauer <mha1993@live.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-21trace.c, git.c: remove unnecessary parameter to trace_repo_setup()idriss fekir3-6/+5
trace_repo_setup() of trace.c is called with the argument 'prefix' from only one location, run_builtin of git.c, which sets 'prefix' to the return value of setup_git_directory() or setup_git_directory_gently() (a wrapper of the former). Now that "prefix" is in startup_info there is no need for the parameter of trace_repo_setup() because setup_git_directory() sets "startup_info->prefix" to the same value it returns. It would be less confusing to use "prefix" from startup_info instead of passing it as an argument. Signed-off-by: Idriss Fekir <mcsm224@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-21t2015-checkout-unborn.sh: changes the style for cdAshutosh Pandey1-5/+6
the `cd` followed the old style which wasn't consistent with the rest of the test suite, so this commit makes it consistent with the current style of the test suite for `cd` in subshell. Signed-off-by: Ashutosh Pandey <ashutosh.pandeyhlr007@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-21diff: teach diff to read algorithm from diff driverJohn Cai6-11/+97
It can be useful to specify diff algorithms per file type. For example, one may want to use the minimal diff algorithm for .json files, another for .c files, etc. The diff machinery already checks attributes for a diff driver. Teach the diff driver parser a new type "algorithm" to look for in the config, which will be used if a driver has been specified through the attributes. Enforce precedence of the diff algorithm by favoring the command line option, then looking at the driver attributes & config combination, then finally the diff.algorithm config. To enforce precedence order, use a new `ignore_driver_algorithm` member during options parsing to indicate the diff algorithm was set via command line args. Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-21diff: consolidate diff algorithm option parsingJohn Cai1-14/+43
A subsequent commit will need the ability to tell if the diff algorithm was set through the command line through setting a new member of diff_options. While this logic can be added to the diff_opt_diff_algorithm() callback, the `--minimal` and `--histogram` options are handled via OPT_BIT without a callback. Remedy this by consolidating the options parsing logic for --minimal and --histogram into one callback. This way we can modify `diff_options` in that function. As an additional refactor, the logic that sets the diff algorithm in diff_opt_diff_algorithm() can be refactored into a helper that will allow multiple callsites to set the diff algorithm. Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>