aboutsummaryrefslogtreecommitdiffstats
path: root/http-backend.c
diff options
context:
space:
mode:
authorJunio C Hamano <gitster@pobox.com>2022-10-02 08:43:56 -0700
committerJunio C Hamano <gitster@pobox.com>2022-10-02 08:43:56 -0700
commit3dcec76d9df911ed8321007b1d197c1a206dc164 (patch)
tree17f1b818715fb25bbdf8c9f83261c9ce9317fd31 /http-backend.c
parentMerge tag 'l10n-2.38.0-rnd3' of https://github.com/git-l10n/git-po (diff)
downloadgit-3dcec76d9df911ed8321007b1d197c1a206dc164.tar.gz
git-3dcec76d9df911ed8321007b1d197c1a206dc164.zip
Git 2.38v2.38.0
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'http-backend.c')
0 files changed, 0 insertions, 0 deletions
generated by cgit v1.2.3 (git 2.39.1) at 2025-12-13 12:35:51 +0000
net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 2024-07-10chainlint.pl: check line numbers in expected outputJeff King74-894/+913 While working on chainlint.pl recently, we introduced some bugs that showed incorrect line numbers in the output. But it was hard to notice, since we sanitize the output by removing all of the line numbers! It would be nice to retain these so we can catch any regressions. The main reason we sanitize is for maintainability: we concatenate all of the test snippets into a single file, so it's hard for each ".expect" file to know at which offset its test input will be found. We can handle that by storing the per-test line numbers in the ".expect" files, and then dynamically offsetting them as we build the concatenated test and expect files together. The changes to the ".expect" files look like tedious boilerplate, but it actually makes adding new tests easier. You can now just run: perl chainlint.pl chainlint/foo.test | tail -n +2 >chainlint/foo.expect to save the output of the script minus the comment headers (after checking that it is correct, of course). Whereas before you had to strip the line numbers. The conversions here were done mechanically using something like the script above, and then spot-checked manually. It would be possible to do all of this in shell via the Makefile, but it gets a bit complicated (and requires a lot of extra processes). Instead, I've written a short perl script that generates the concatenated files (we already depend on perl, since chainlint.pl uses it). Incidentally, this improves a few other things: - we incorrectly used $(CHAINLINTTMP_SQ) inside a double-quoted string. So if your test directory required quoting, like: make "TEST_OUTPUT_DIRECTORY=/tmp/h'orrible" we'd fail the chainlint tests. - the shell in the Makefile didn't handle &&-chaining correctly in its loops (though in practice the "sed" and "cat" invocations are not likely to fail). - likewise, the sed invocation to strip numbers was hiding the exit code of chainlint.pl itself. In practice this isn't a big deal; since there are linter violations in the test files, we expect it to exit non-zero. But we could later use exit codes to distinguish serious errors from expected ones. - we now use a constant number of processes, instead of scaling with the number of test scripts. So it should be a little faster (on my machine, "make check-chainlint" goes from 133ms to 73ms). There are some alternatives to this approach, but I think this is still a good intermediate step: 1. We could invoke chainlint.pl individually on each test file, and compare it to the expected output (and possibly using "make" to avoid repeating already-done checks). This is a much bigger change (and we'd have to figure out what to do with the "# LINT" lines in the inputs). But in this case we'd still want the "expect" files to be annotated with line numbers. So most of what's in this patch would be needed anyway. 2. Likewise, we could run a single chainlint.pl and feed it all of the scripts (with "--jobs=1" to get deterministic output). But we'd still need to annotate the scripts as we did here, and we'd still need to either assemble the "expect" file, or break apart the script output to compare to each individual ".expect" file. So we may pursue those in the long run, but this patch gives us more robust tests without too much extra work or moving in a useless direction. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 2024-07-10chainlint.pl: force CRLF conversion when opening input filesJeff King1-1/+1 The lexer in chainlint.pl can't handle CRLF line endings; it complains about an internal error in scan_token() if we see one. For example, in our Windows CI environment: $ perl chainlint.pl chainlint/for-loop.test | cat -v Thread 2 terminated abnormally: internal error scanning character '^M' This doesn't break "make check-chainlint" (yet), because we assemble a concatenated input by passing the contents of each file through "sed". And the "sed" we use will strip out the CRLFs. But the next patch is going to rework this a bit, which does break check-chainlint on Windows. Plus it's probably nicer to folks on Windows who might work on chainlint itself and write new tests. In theory we could fix the parser to handle this, but it's not really worth the trouble. We should be able to ask the input layer to translate the line endings for us. In fact, I'd expect this to happen by default, as perl's documentation claims Win32 uses the ":unix:crlf" PERLIO layer by default ("unix" here just refers to using read/write syscalls, and then "crlf" layers the translation on top). However, this doesn't seem to be the case in our Windows CI environment. I didn't dig into the exact reason, but it is perhaps because we are using an msys build of perl rather than a "true" Win32 build. At any rate, it is easy-ish to just ask explicitly for the conversion. In the above example, setting PERLIO=crlf in the environment is enough to make it work. Curiously, though, this doesn't work when invoking chainlint via "make". Again, I didn't dig into it, but it may have to do with msys programs calling Windows programs or vice versa. We can make it work consistently by just explicitly asking for CRLF translation when we open the files. This will even work on non-Windows platforms, though we wouldn't really expect to find CRLF files there. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 2024-07-10chainlint.pl: do not spawn more threads than we have scriptsJeff King1-0/+1 The chainlint.pl script spawns worker threads to check many scripts in parallel. This is good if you feed it a lot of scripts. But if you give it few (or one), then the overhead of spawning the threads dominates. We can easily notice that we have fewer scripts than threads and scale back as appropriate. This patch reduces the time to run: time for i in chainlint/*.test; do perl chainlint.pl $i done >/dev/null on my system from ~4.1s to ~1.1s, where I have 8+8 cores. As with the previous patch, this isn't the usual way we run chainlint (we feed many scripts at once, which is why it supports threading in the first place). So this won't make a big difference in the real world, but it may help us out in the future, and it makes experimenting with and debugging the chainlint tests a bit more pleasant. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 2024-07-10chainlint.pl: only start threads if jobs > 1Jeff King1-1/+2 If the system supports threads, chainlint.pl will always spawn worker threads to do the real work. But when --jobs=1, this is pointless, since we could just do the work in the main thread. And spawning even a single thread has a high overhead. For example, on my Linux system, running: for i in chainlint/*.test; do perl chainlint.pl --jobs=1 $i done >/dev/null takes ~1.7s without this patch, and ~1.1s after. We don't usually spawn a bunch of individual chainlint.pl processes (instead we feed several scripts at once, and the parallelism outweighs the setup cost). But it's something we've considered doing, and since we already have fallback code for systems without thread support, it's pretty easy to make this work. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 2024-07-10chainlint.pl: add test_expect_success call to test snippetsJeff King73-3/+145 The chainlint tests are a series of individual files, each holding a test body. The "make check-chainlint" target assembles them into a single file, adding a "test_expect_success" function call around each. Let's instead include that function call in the files themselves. This is a little more boilerplate, but has several advantages: 1. You can now run chainlint manually on snippets with just "perl chainlint.perl chainlint/foo.test". This can make developing and debugging a little easier. 2. Many of the tests implicitly relied on the syntax of the lines added by the Makefile (in particular the use of single-quotes). This assumption is much easier to see when the single-quotes are alongside the test body. 3. We had no way to test how the chainlint program handled various test_expect_success lines themselves. Now we'll be able to check variations. The change to the .test files was done mechanically, using the same test names they would have been assigned by the Makefile (this is important to match the expected output). The Makefile has the minimal change to drop the extra lines; there are more cleanups possible but a future patch in this series will rewrite this substantially anyway. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 2024-06-28More post 2.45.2 updates from the 'master' frontJunio C Hamano2-1/+81 Signed-off-by: Junio C Hamano <gitster@pobox.com> 2024-06-27The seventeenth batchJunio C Hamano1-0/+18 Signed-off-by: Junio C Hamano <gitster@pobox.com> 2024-06-24The sixteenth batchJunio C Hamano1-0/+24 Signed-off-by: Junio C Hamano <gitster@pobox.com> 2024-06-20The fifteenth batchJunio C Hamano1-0/+35 Signed-off-by: Junio C Hamano <gitster@pobox.com> 2024-06-20commit: remove find_header_mem()René Scharfe2-19/+2 cfc5cf428b (receive-pack.c: consolidate find header logic, 2022-01-06) introduced find_header_mem() and turned find_commit_header() into a thin wrapper. Since then, the latter has become the last remaining caller of the former. Remove it to restore find_commit_header() to the state before cfc5cf428b, get rid of a strlen(3) call and resolve a NEEDSWORK note in the process. Signed-off-by: René Scharfe <l.s.r@web.de> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 2024-06-20t5500: fix mistaken $SERVER reference in helper functionJeff King1-1/+1 The end of t5500 contains two tests which use a single helper function, fetch_filter_blob_limit_zero(). It takes a parameter to point to the path of the server repository, which we store locally as $SERVER. The first caller uses the relative path "server", while the second points into the httpd document root. Commit 07ef3c6604 (fetch test: use more robust test for filtered objects, 2019-12-23) refactored some lines, but accidentally switched "$SERVER" to "server" in one spot. That means the second caller is looking at the server directory from the previous test rather than its own. This happens to work out because the "server" directory from the first test is still hanging around, and the contents of the two are identical. But it was clearly not the intended behavior, and is fragile to cleaning up the leftovers from the first test. Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 2024-06-20mingw: drop bogus (and unneeded) declaration of `_pgmptr`Johannes Schindelin1-1/+0 In 08809c09aa13 (mingw: add a helper function to attach GDB to the current process, 2020-02-13), I added a declaration that was not needed. Back then, that did not matter, but now that the declaration of that symbol was changed in mingw-w64's headers, it causes the following compile error: CC compat/mingw.o compat/mingw.c: In function 'open_in_gdb': compat/mingw.c:35:9: error: function declaration isn't a prototype [-Werror=strict-prototypes] 35 | extern char *_pgmptr; | ^~~~~~ In file included from C:/git-sdk-64/usr/src/git/build-installers/mingw64/lib/gcc/x86_64-w64-mingw32/14.1.0/include/mm_malloc.h:27, from C:/git-sdk-64/usr/src/git/build-installers/mingw64/lib/gcc/x86_64-w64-mingw32/14.1.0/include/xmmintrin.h:34, from C:/git-sdk-64/usr/src/git/build-installers/mingw64/lib/gcc/x86_64-w64-mingw32/14.1.0/include/immintrin.h:31, from C:/git-sdk-64/usr/src/git/build-installers/mingw64/lib/gcc/x86_64-w64-mingw32/14.1.0/include/x86intrin.h:32, from C:/git-sdk-64/usr/src/git/build-installers/mingw64/include/winnt.h:1658, from C:/git-sdk-64/usr/src/git/build-installers/mingw64/include/minwindef.h:163, from C:/git-sdk-64/usr/src/git/build-installers/mingw64/include/windef.h:9, from C:/git-sdk-64/usr/src/git/build-installers/mingw64/include/windows.h:69, from C:/git-sdk-64/usr/src/git/build-installers/mingw64/include/winsock2.h:23, from compat/../git-compat-util.h:215, from compat/mingw.c:1: compat/mingw.c:35:22: error: '__p__pgmptr' redeclared without dllimport attribute: previous dllimport ignored [-Werror=attributes] 35 | extern char *_pgmptr; | ^~~~~~~ Let's just drop the declaration and get rid of this compile error. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> 2024-06-20fetch-pack: fix segfault when fscking without --lock-packJeff King2-1/+13 The fetch-pack internals have multiple options related to creating ".keep" lock-files for the received pack: - if args.lock_pack is set, then we tell index-pack to create a .keep file. In the fetch-pack plumbing command, this is triggered by passing "-k" twice. - if the caller passes in a pack_lockfiles string list, then we use it to record the path of the keep-file created by index-pack. We get that name by reading the stdout of index-pack. In the fetch-pack command, this is triggered by passing the (undocumented) --lock-pack option; without it, we pass in a NULL string list. So it's possible to ask index-pack to create the lock-file (using "-k -k") but not ask to record it (by avoiding "--lock-pack"). This worked fine until 5476e1efde (fetch-pack: print and use dangling .gitmodules, 2021-02-22), but now it causes a segfault. Before that commit, if pack_lockfiles was NULL, we wouldn't bother reading the output from index-pack at all. But since that commit, index-pack may produce extra output if we asked it to fsck. So even if nobody cares about the lockfile path, we still need to read it to skip to the output we do care about. We correctly check that we didn't get a NULL lockfile path (which can happen if we did not ask it to create a .keep file at all), but we missed the case where the lockfile path is not NULL (due to "-k -k") but the pack_lockfiles string_list is NULL (because nobody passed "--lock-pack"), and segfault trying to add to the NULL string-list. We can fix this by skipping the append to the string list when either the value or the list is NULL. In that case we must also free the lockfile path to avoid leaking it when it's non-NULL. Nobody noticed the bug for so long because the transport code used by "git fetch" always passes in a pack_lockfiles pointer, and remote-curl (the main user of the fetch-pack plumbing command) always passes --lock-pack. Reported-by: Kirill Smelkov <kirr@nexedi.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 2024-06-18merge: avoid write merge state when unable to write indexKyle Zhao2-1/+11 Writing the merge state after the index write fails is meaningless and could potentially cause Git to lose changes. Signed-off-by: Kyle Zhao <kylezhao@tencent.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 2024-06-17The fourteenth batchJunio C Hamano1-0/+37 Signed-off-by: Junio C Hamano <gitster@pobox.com> 2024-06-17attr: fix msan issue in read_attr_from_indexKyle Lippincott1-1/+2 Memory sanitizer (msan) is detecting a use of an uninitialized variable (`size`) in `read_attr_from_index`: ==2268==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x5651f3416504 in read_attr_from_index git/attr.c:868:11 #1 0x5651f3415530 in read_attr git/attr.c #2 0x5651f3413d74 in bootstrap_attr_stack git/attr.c:968:6 #3 0x5651f3413d74 in prepare_attr_stack git/attr.c:1004:2 #4 0x5651f3413d74 in collect_some_attrs git/attr.c:1199:2 #5 0x5651f3413144 in git_check_attr git/attr.c:1345:2 #6 0x5651f34728da in convert_attrs git/convert.c:1320:2 #7 0x5651f3473425 in would_convert_to_git_filter_fd git/convert.c:1373:2 #8 0x5651f357a35e in index_fd git/object-file.c:2630:34 #9 0x5651f357aa15 in index_path git/object-file.c:2657:7 #10 0x5651f35db9d9 in add_to_index git/read-cache.c:766:7 #11 0x5651f35dc170 in add_file_to_index git/read-cache.c:799:9 #12 0x5651f321f9b2 in add_files git/builtin/add.c:346:7 #13 0x5651f321f9b2 in cmd_add git/builtin/add.c:565:18 #14 0x5651f321d327 in run_builtin git/git.c:474:11 #15 0x5651f321bc9e in handle_builtin git/git.c:729:3 #16 0x5651f321a792 in run_argv git/git.c:793:4 #17 0x5651f321a792 in cmd_main git/git.c:928:19 #18 0x5651f33dde1f in main git/common-main.c:62:11 The issue exists because `size` is an output parameter from `read_blob_data_from_index`, but it's only modified if `read_blob_data_from_index` returns non-NULL. The read of `size` when calling `read_attr_from_buf` unconditionally may read from an uninitialized value. `read_attr_from_buf` checks that `buf` is non-NULL before reading from `size`, but by then it's already too late: the uninitialized read will have happened already. Furthermore, there's no guarantee that the compiler won't reorder things so that it checks `size` before checking `!buf`. Make the call to `read_attr_from_buf` conditional on `buf` being non-NULL, ensuring that `size` is not read if it's never set. Signed-off-by: Kyle Lippincott <spectral@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 2024-06-14pack-bitmap.c: ensure pseudo-merge offset reads are boundedTaylor Blau1-0/+5 After reading the pseudo-merge extension's metadata table, we allocate an array to store information about each pseudo-merge, including its byte offset within the .bitmap file itself. This is done like so: pseudo_merge_ofs = index_end - 24 - (index->pseudo_merges.nr * sizeof(uint64_t)); for (i = 0; i < index->pseudo_merges.nr; i++) { index->pseudo_merges.v[i].at = get_be64(pseudo_merge_ofs); pseudo_merge_ofs += sizeof(uint64_t); } But if the pseudo-merge table is corrupt, we'll keep calling get_be64() past the end of the pseudo-merge extension, potentially reading off the end of the mmap'd region. Prevent this by ensuring that we have at least `table_size - 24` many bytes available to read (adding 24 to the left-hand side of our inequality to account for the length of the metadata component). This is sufficient to prevent us from reading off the end of the pseudo-merge extension, and ensures that all of the get_be64() calls below are in bounds. Helped-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 2024-06-14Documentation/technical/bitmap-format.txt: add missing position tableTaylor Blau1-0/+9 While investigating a benign Coverity warning on the new pseudo-merge implementation, I was struggling to understand the (paraphrased) below: ofs = index_end - 24 - (index->pseudo_merges.nr * sizeof(uint64_t)); for (i = 0; i < index->pseudo_merges.nr; i++) { index->pseudo_merges.v[i].at = get_be64(ofs); ofs += sizeof(uint64_t); } , in pack-bitmap.c::load_bitmap_header(). Looking at the documentation, the diagram describing the on-disk format (prior to this patch) suggested that the optional extended lookup table immediately preceded the trailing metadata portion. If that were the case, that would make the above code from load_bitmap_header() incorrect, as we'd be blindly reading into the extended offset table. But later on in the documentation there is a description of the pseudo-merge position table as immediately preceding the trailing metadata portion of the extension. And indeed, we do write the position table in pack-bitmap-write.c: /* write positions for all pseudo merges */ for (i = 0; i < writer->pseudo_merges_nr; i++) hashwrite_be64(f, pseudo_merge_ofs[i]); hashwrite_be32(f, writer->pseudo_merges_nr); hashwrite_be32(f, kh_size(writer->pseudo_merge_commits)); hashwrite_be64(f, table_start - start); hashwrite_be64(f, hashfile_total(f) - start + sizeof(uint64_t)); So this is purely a case of the diagram being out of sync with the textual description and actual implementation of the format specification. Add the missing component back to the format diagram to avoid further confusion in this area. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 2024-06-14BreakingChanges: document that we do not plan to deprecate git-checkoutPatrick Steinhardt1-0/+12 The git-checkout(1) command is seen by many as hard to understand because it connects two somewhat unrelated features: switching between branches and restoring worktree files from arbitrary revisions. In 2019, we thus implemented two new commands git-switch(1) and git-restore(1) to split out these separate concerns into standalone functions. This "replacement" of git-checkout(1) has repeatedly triggered concerns for our userbase that git-checkout(1) will eventually go away. This is not the case though: the use of that command is still widespread, and it is not expected that this will change anytime soon. Document that all three commands will remain for the foreseeable future. This decision may be revisited in case we ever figure out that most everyone has given up on any of the commands. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com> 2024-06-14BreakingChanges: document removal of graftingPatrick Steinhardt1-0/+13 The grafting mechanism for objects has been deprecated in e650d0643b (docs: mark info/grafts as outdated, 2014-03-05), which is more than a decade ago. The mechanism can lead to hard-to-debug issues and has a superior replacement with replace refs. Follow through with the deprecation and mark grafts for removal in Git 3.0. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com> 2024-06-14BreakingChanges: document upcoming change from "sha1" to "sha256"Patrick Steinhardt1-0/+30 Starting with 8e42eb0e9a (doc: sha256 is no longer experimental, 2023-07-31), the "sha256" object format is no longer considered to be experimental. Furthermore, the SHA-1 hash function is actively recommended against by for example NIST and FIPS 140-2, and attacks against it are becoming more practical both due to new weaknesses (SHAppening, SHAttered, Shambles) and due to the ever-increasing computing power. It is only a matter of time before it can be considered to be broken completely. Let's plan for this event by being active instead of waiting for it to happend and announce that the default object format is going to change from "sha1" to "sha256" with Git 3.0. All major Git implementations (libgit2, JGit, go-git) support the "sha256" object format and are thus prepared for this change. The most important missing piece in the puzzle is support in forges. But while GitLab recently gained experimental support for the "sha256" object format though, to the best of my knowledge GitHub doesn't support it yet. Ideally, announcing this upcoming change will encourage forges to start building that support. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>