summaryrefslogtreecommitdiffstats
AgeCommit message (Collapse)AuthorLines
2024-07-13run-command: declare the `git_shell_path()` function globallyJohannes Schindelin-1/+6
The intention is to use it in `git var GIT_SHELL_PATH`, therefore we need this function to stop being file-local only. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-13run-command(win32): resolve the path to the Unix shell earlyJohannes Schindelin-4/+6
In 776297548e (Do not use SHELL_PATH from build system in prepare_shell_cmd on Windows, 2012-04-17), the hard-coded path to the Unix shell was replaced by passing `sh` instead when executing Unix shell scripts in Git. This was done because the hard-coded path to the Unix shell is incorrect on Windows because it not only is a Unix-style absolute path instead of a Windows one, but Git uses the runtime prefix feature on Windows, i.e. the correct path cannot be hard-coded. Naturally, the `sh` argument will be resolved to the full path of said executable eventually. To help fixing the bug where `git var GIT_SHELL_PATH` currently does not reflect that logic, but shows that incorrect hard-coded Unix-style absolute path, let's resolve the full path to the `sh` executable early in the `git_shell_path()` function so that we can use it in `git var`, too, and be sure that the output is equivalent to what `run_command()` does when it is asked to execute a command-line using a Unix shell. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-13mingw(is_msys2_sh): handle forward slashes in the `sh.exe` path, tooJohannes Schindelin-1/+1
Whether the full path to the MSYS2 Bash is specified using backslashes or forward slashes, in either case the command-line arguments need to be quoted in the MSYS2-specific manner instead of using regular Win32 command-line quoting rules. In preparation for `prepare_shell_cmd()` to use the full path to `sh.exe` (with forward slashes for consistency), let's teach the `is_msys2_sh()` function about this; Otherwise 5580.4 'clone with backslashed path' would fail once `prepare_shell_cmd()` uses the full path instead of merely `sh`. This patch relies on the just-introduced fix where `fspathcmp()` handles backslashes and forward slashes as equivalent on Windows. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-13win32: override `fspathcmp()` with a directory separator-aware versionJohannes Schindelin-4/+53
On Windows, the backslash is the directory separator, even if the forward slash can be used, too, at least since Windows NT. This means that the paths `a/b` and `a\b` are equivalent, and `fspathcmp()` needs to be made aware of that fact. Note that we have to override both `fspathcmp()` and `fspathncmp()`, and the former cannot be a mere pre-processor constant that transforms calls to `fspathcmp(a, b)` into `fspathncmp(a, b, (size_t)-1)` because the function `report_collided_checkout()` in `unpack-trees.c` wants to assign `list.cmp = fspathcmp`. Also note that `fspatheq()` does _not_ need to be overridden because it calls `fspathcmp()` internally. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-13strvec: declare the `strvec_push_nodup()` function globallyJohannes Schindelin-1/+4
This function differs from `strvec_push()` in that it takes ownership of the allocated string that is passed as second argument. This is useful when appending elements to the string array that have been freshly allocated and serve no further other purpose after that. Without declaring this function globally, call sites would allocate the memory, only to have `strvec_push()` duplicate the string, and then the first copy would need to be released. Having this function globally avoids that kind of unnecessary work. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-13run-command: refactor getting the Unix shell path into its own functionJohannes Schindelin-5/+10
This encapsulates the platform-specific logic better. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-12cmake: fix build of `t-oidtree`Johannes Schindelin-1/+2
When the `oidtree` test helper was turned into a unit test, a new `lib-oid` source file was added as dependency. This was only done in the Makefile so far, but also needs to be done in the CMake definition. This is a companion of ed548408723d (t/: migrate helper/test-oidtree.c to unit-tests/t-oidtree.c, 2024-06-08). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-12t-reftable-merged: add test for REFTABLE_FORMAT_ERRORChandra Pratap-0/+3
When calling reftable_new_merged_table(), if the hash ID of the passed reftable_table parameter doesn't match the passed hash_id parameter, a REFTABLE_FORMAT_ERROR is thrown. This case is currently left unexercised, so add a test for the same. Mentored-by: Patrick Steinhardt <ps@pks.im> Mentored-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-12t-reftable-merged: use reftable_ref_record_equal to compare ref recordsChandra Pratap-1/+1
In the test t_merged_single_record() defined in t-reftable-merged.c, the 'input' and 'expected' ref records are checked for equality by comparing their update indices. It is very much possible for two different ref records to have the same update indices. Use reftable_ref_record_equal() instead for a stronger check. Mentored-by: Patrick Steinhardt <ps@pks.im> Mentored-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-12t-reftable-merged: add tests for reftable_merged_table_max_update_indexChandra Pratap-0/+2
reftable_merged_table_max_update_index() as defined by reftable/ merged.{c, h} returns the maximum update index in a merged table. Since this function is currently unexercised, add tests for it. Mentored-by: Patrick Steinhardt <ps@pks.im> Mentored-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-12t-reftable-merged: improve the const-correctness of helper functionsChandra Pratap-10/+9
In t-reftable-merged.c, a number of helper functions used by the tests can be re-defined with parameters made 'const' which makes it easier to understand if they're read-only or not. Re-define these functions along these lines. Mentored-by: Patrick Steinhardt <ps@pks.im> Mentored-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-12t-reftable-merged: improve the test t_merged_single_record()Chandra Pratap-5/+10
In t-reftable-merged.c, the test t_merged_single_record() ensures that a ref ('a') which occurs in only one of the records ('r2') can be retrieved. Improve this test by adding another record 'r3' to ensure that ref 'a' only occurs in 'r2' and that merged tables don't simply read the last record. Mentored-by: Patrick Steinhardt <ps@pks.im> Mentored-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-12t: harmonize t-reftable-merged.c with coding guidelinesChandra Pratap-40/+28
Harmonize the newly ported test unit-tests/t-reftable-merged.c with the following guidelines: - Single line control flow statements like 'for' and 'if' must omit curly braces. - Structs must be 0-initialized with '= { 0 }' instead of '= { NULL }'. - Array indices should preferably be of type 'size_t', not 'int'. - It is fine to use C99 initial declaration in 'for' loop. While at it, use 'ARRAY_SIZE(x)' to store the number of elements in an array instead of hardcoding them. Mentored-by: Patrick Steinhardt <ps@pks.im> Mentored-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-12t: move reftable/merged_test.c to the unit testing frameworkChandra Pratap-57/+60
reftable/merged_test.c exercises the functions defined in reftable/merged.{c, h}. Migrate reftable/merged_test.c to the unit testing framework. Migration involves refactoring the tests to use the unit testing framework instead of reftable's test framework and renaming the tests according to unit-tests' naming conventions. Also, move strbuf_add_void() and noop_flush() from reftable/test_framework.c to the ported test. This is because both these functions are used in the merged tests and reftable/test_framework.{c, h} is not #included in the ported test. Mentored-by: Patrick Steinhardt <ps@pks.im> Mentored-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-12builtin/push: call set_refspecs after validating remoteKarthik Nayak-14/+24
When an end-user runs "git push" with an empty string for the remote repository name, e.g. $ git push '' main "git push" fails with a BUG(). Even though this is a nonsense request that we want to fail, we shouldn't hit a BUG(). Instead we want to give a sensible error message, e.g., 'bad repository'". This is because since 9badf97c42 (remote: allow resetting url list, 2024-06-14), we reset the remote URL if the provided URL is empty. When a user of 'remotes_remote_get' tries to fetch a remote with an empty repo name, the function initializes the remote via 'make_remote'. But the remote is still not a valid remote, since the URL is empty, so it tries to add the URL alias using 'add_url_alias'. This in-turn will call 'add_url', but since the URL is empty we call 'strvec_clear' on the `remote->url`. Back in 'remotes_remote_get', we again check if the remote is valid, which fails, so we return 'NULL' for the 'struct remote *' value. The 'builtin/push.c' code, calls 'set_refspecs' before validating the remote. This worked with empty repo names earlier since we would get a remote, albeit with an empty URL. With the new changes, we get a 'NULL' remote value, this causes the check for remote to fail and raises the BUG in 'set_refspecs'. Do a simple fix by doing remote validation first. Also add a test to validate the bug fix. With this, we can also now directly pass remote to 'set_refspecs' instead of it trying to lazily obtain it. Helped-by: Jeff King <peff@peff.net> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-12Git 2.46-rc0v2.46.0-rc0Junio C Hamano-1/+7
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-12Merge branch 'rs/simplify-submodule-helper-super-prefix-invocation'Junio C Hamano-11/+6
Code clean-up. * rs/simplify-submodule-helper-super-prefix-invocation: submodule--helper: use strvec_pushf() for --super-prefix
2024-07-12Merge branch 'as/pathspec-h-typofix'Junio C Hamano-1/+1
Typofix. * as/pathspec-h-typofix: pathspec: fix typo "glossary-context.txt" -> "glossary-content.txt"
2024-07-11doc: update http.cookieFile with in-memory cookie processingPiotr Szlazak-1/+5
Documentation only mentions how to read cookies from the given file and how to save them to the file using http.saveCookies. But underlying libcURL allows the HTTP cookies used only in memory; cookies from the server will be accepted and sent back in successive requests within same connection, by using an empty string as the filename. Document this. Signed-off-by: Piotr Szlazak <piotr.szlazak@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-11test-lib: GIT_TEST_SANITIZE_LEAK_LOG enabled by defaultRubén Justo-47/+17
As we currently describe in t/README, it can happen that: Some tests run "git" (or "test-tool" etc.) without properly checking the exit code, or git will invoke itself and fail to ferry the abort() exit code to the original caller. Therefore, GIT_TEST_SANITIZE_LEAK_LOG=true is needed to be set to capture all memory leaks triggered by our tests. It seems unnecessary to force users to remember this option, as forgetting it could lead to missed memory leaks. We could solve the problem by making it "true" by default, but that might suggest we think "false" makes sense, which isn't the case. Therefore, the best approach is to remove the option entirely while maintaining the capability to detect memory leaks in blind spots of our tests. Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-10t/.gitattributes: ignore whitespace in chainlint expect filesJeff King-1/+1
The ".expect" files in t/chainlint/ are snippets of expected output from the chainlint script, and do not necessarily conform to our usual code style. Especially with the recent change to retain line numbers, blank lines in the input script end up with trailing whitespace as we print "3 " for line 3, for example. The point of these files is to match the output verbatim, so let's not complain about the trailing spaces. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-10t: convert some here-doc test bodiesJeff King-117/+117
The t1404 script checks a lot of output from Git which contains single quotes. Because the test snippets are themselves wrapped in the same single-quotes, we have to resort to using $SQ to match them. This is error-prone and makes the tests harder to read. Instead, let's use the new here-doc feature added in the previous commit, which lets us write anything in the test body we want (except the here-doc end marker on a line by itself, of course). Note that we do use "\" in our marker to avoid interpolation (which is the whole point). But we don't use "<<-", as we want to preserve whitespace in the snippet (and running with "-v" before and after shows that we produce the exact same output, except with the ugly $SQ references fixed). I just converted every test here, even though only some of them use $SQ. But it would be equally correct to mix-and-match styles if we don't mind the inconsistency. I've also converted a few tests in t0600 which were moved from t1404 (I had written this patch before they were moved, but it seemed worth porting over the changes rather than losing them). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-10test-lib: allow test snippets as here-docsJeff King-5/+35
Most test snippets are wrapped in single quotes, like: test_expect_success 'some description' ' do_something ' This sometimes makes the snippets awkward to write, because you can't easily use single quotes within them. We sometimes work around this with $SQ, or by loosening regexes to use "." instead of a literal quote, or by using double quotes when we'd prefer to use single-quotes (and just adding extra backslash-escapes to avoid interpolation). This commit adds another option: feeding the snippet via the function's stdin. This doesn't conflict with anything the snippet would want to do, because we always redirect its stdin from /dev/null anyway (which we'll continue to do). A few notes on the implementation: - it would be nice to push this down into test_run_, but we can't, as test_expect_success and test_expect_failure want to see the actual script content to report it for verbose-mode. A helper function limits the amount of duplication in those callers here. - The helper function is a little awkward to call, as you feed it the name of the variable you want to set. The more natural thing in shell would be command substitution like: body=$(body_or_stdin "$2") but that loses trailing whitespace. There are tricks around this, like: body=$(body_or_stdin "$2"; printf .) body=${body%.} but we'd prefer to keep such tricks in the helper, not in each caller. - I implemented the helper using a sequence of "read" calls. Together with "-r" and unsetting the IFS, this preserves incoming whitespace. An alternative is to use "cat" (which then requires the gross "." trick above). But this saves us a process, which is probably a good thing. The "read" builtin does use more read() syscalls than necessary (one per byte), but that is almost certainly a win over a separate process. Both are probably slower than passing a single-quoted string, but the difference is lost in the noise for a script that I converted as an experiment. - I handle test_expect_success and test_expect_failure here. If we like this style, we could easily extend it to other spots (e.g., lazy_prereq bodies) on top of this patch. - even though we are using "local", we have to be careful about our variable names. Within test_expect_success, any variable we declare with local will be seen as local by the test snippets themselves (so it wouldn't persist between tests like normal variables would). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-10chainlint.pl: add tests for test body in heredocJeff King-0/+50
The chainlint.pl script recently learned about the upcoming: test_expect_success 'some test' - <<\EOT TEST_BODY EOT syntax, where TEST_BODY should be checked in the usual way. Let's make sure this works by adding a few tests. The "here-doc-body" file tests the basic syntax, including an embedded here-doc which we should still be able to recognize. Likewise the "here-doc-body-indent" checks the same thing, but using the "<<-" operator. We wouldn't expect this to be used normally, but we would not want to accidentally miss a body that uses it. The "pathological" variant checks the opposite: we don't get confused by an indented tag within the here-doc body. The "here-doc-double" tests the handling of two here-doc tags on the same line. This is not something we'd expect anybody to do in practice, but the code was written defensively to handle this, so let's make sure it works. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-10chainlint.pl: recognize test bodies defined via heredocEric Sunshine-5/+22
In order to check tests for semantic problems, chainlint.pl scans test scripts, looking for tests defined as: test_expect_success [prereq] title ' body ' where `body` is a single string which is then treated as a standalone chunk of code and "linted" to detect semantic issues. (The same happens for `test_expect_failure` definitions.) The introduction of test definitions in which the test body is instead presented via a heredoc rather than as a single string creates a blind spot in the linting process since such invocations are not recognized by chainlint.pl. Prepare for this new style by also recognizing tests defined as: test_expect_success [prereq] title - <<\EOT body EOT A minor complication is that chainlint.pl has never considered heredoc bodies significant since it doesn't scan them for semantic problems, thus it has always simply thrown them away. However, with the new `test_expect_success` calling sequence, heredoc bodies become meaningful, thus need to be captured. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-10chainlint.pl: check line numbers in expected outputJeff King-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 King-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 King-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 King-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 King-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-07-09http.c: cookie file tighteningJunio C Hamano-0/+9
The http.cookiefile configuration variable is used to call curl_easy_setopt() to set CURLOPT_COOKIEFILE and if http.savecookies is set, the same value is used for CURLOPT_COOKIEJAR. The former is used only to read cookies at startup, the latter is used to write cookies at the end. The manual pages https://curl.se/libcurl/c/CURLOPT_COOKIEFILE.html and https://curl.se/libcurl/c/CURLOPT_COOKIEJAR.html talk about two interesting special values. * "" (an empty string) given to CURLOPT_COOKIEFILE means not to read cookies from any file upon startup. * It is not specified what "" (an empty string) given to CURLOPT_COOKIEJAR does; presumably open a file whose name is an empty string and write cookies to it? In any case, that is not what we want to see happen, ever. * "-" (a dash) given to CURLOPT_COOKIEFILE makes cURL read cookies from the standard input, and given to CURLOPT_COOKIEJAR makes cURL write cookies to the standard output. Neither of which we want ever to happen. So, let's make sure we avoid these nonsense cases. Specifically, when http.cookies is set to "-", ignore it with a warning, and when it is set to "" and http.savecookies is set, ignore http.savecookies with a warning. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-09http: allow authenticating proactivelybrian m. carlson-6/+192
When making a request over HTTP(S), Git only sends authentication if it receives a 401 response. Thus, if a repository is open to the public for reading, Git will typically never ask for authentication for fetches and clones. However, there may be times when a user would like to authenticate nevertheless. For example, a forge may give higher rate limits to users who authenticate because they are easier to contact in case of excessive use. Or it may be useful for a known heavy user, such as an internal service, to proactively authenticate so its use can be monitored and, if necessary, throttled. Let's make this possible with a new option, "http.proactiveAuth". This option specifies a type of authentication which can be used to authenticate against the host in question. This is necessary because we lack the WWW-Authenticate header to provide us details; similarly, we cannot accept certain types of authentication because we require information from the server, such as a nonce or challenge, to successfully authenticate. If we're in auto mode and we got a username and password, set the authentication scheme to Basic. libcurl will not send authentication proactively unless there's a single choice of allowed authentication, and we know in this case we didn't get an authtype entry telling us what scheme to use, or we would have taken a different codepath and written the header ourselves. In any event, of the other schemes that libcurl supports, Digest and NTLM require a nonce or challenge, which means that they cannot work with proactive auth, and GSSAPI does not use a username and password at all, so Basic is the only logical choice among the built-in options. Note that the existing http_proactive_auth variable signifies proactive auth if there are already credentials, which is different from the functionality we're adding, which always seeks credentials even if none are provided. Nonetheless, t5540 tests the existing behavior for WebDAV-based pushes to an open repository without credentials, so we preserve it. While at first this may seem an insecure and bizarre decision, it may be that authentication is done with TLS certificates, in which case it might actually provide a quite high level of security. Expand the variable to use an enum to handle the additional cases and a helper function to distinguish our new cases from the old ones. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-09doc: mention that proxies must be completely transparentbrian m. carlson-0/+5
We already document in the FAQ that proxies must be completely transparent and not modify the request or response in any way, but add similar documentation to the http.proxy entry. We know that while the FAQ is very useful, users sometimes are less likely to read in favor of the documentation specific to an option or command, so adding it in both places will help users be adequately informed. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-09gitfaq: add entry about syncing working treesbrian m. carlson-0/+52
Users very commonly want to sync their working tree with uncommitted changes across machines, often to carry across in-progress work or stashes. Despite this not being a recommended approach, users want to do it and are not dissuaded by suggestions not to, so let's recommend a sensible technique. The technique that many users are using is their preferred cloud syncing service, which is a bad idea. Users have reported problems where they end up with duplicate files that won't go away (with names like "file.c 2"), broken references, oddly named references that have date stamps appended to them, missing objects, and general corruption and data loss. That's because almost all of these tools sync file by file, which is a great technique if your project is a single word processing document or spreadsheet, but is utterly abysmal for Git repositories because they don't necessarily snapshot the entire repository correctly. They also tend to sync the files immediately instead of when the repository is quiescent, so writing multiple files, as occurs during a commit or a gc, can confuse the tools and lead to corruption. We know that the old standby, rsync, is up to the task, provided that the repository is quiescent, so let's suggest that and dissuade people from using cloud syncing tools. Let's tell people about common things they should be aware of before doing this and that this is still potentially risky. Additionally, let's tell people that Git's security model does not permit sharing working trees across users in case they planned to do that. While we'd still prefer users didn't try to do this, hopefully this will lead them in a safer direction. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-09gitfaq: give advice on using eol attribute in gitattributesbrian m. carlson-4/+17
In the FAQ, we tell people how to use the text attribute, but we fail to explain what to do with the eol attribute. As we ourselves have noticed, most shell implementations do not care for carriage returns, and as such, people will practically always want them to use LF endings. Similar things can be said for batch files on Windows, except with CRLF endings. Since these are common things to have in a repository, let's help users make a good decision by recommending that they use the gitattributes file to correctly check out the endings. In addition, let's correct the cross-reference to this question, which originally referred to "the following entry", even though a new entry has been inserted in between. The cross-reference notation should prevent this from occurring and provide a link in formats, such as HTML, which support that. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-09gitfaq: add documentation on proxiesbrian m. carlson-0/+36
Many corporate environments and local systems have proxies in use. Note the situations in which proxies can be used and how to configure them. At the same time, note what standards a proxy must follow to work with Git. Explicitly call out certain classes that are known to routinely have problems reported various places online, including in the Git for Windows issue tracker and on Stack Overflow, and recommend against the use of such software, noting that they are associated with myriad security problems (including, for example, breaking sandboxing and image integrity[0], and, for TLS middleboxes, the use of insecure protocols and ciphers and lack of certificate verification[1]). Don't mention the specific nature of these security problems in the FAQ entry because they are extremely numerous and varied and we wish to keep the FAQ entry relatively brief. [0] https://issues.chromium.org/issues/40285192 [1] https://faculty.cc.gatech.edu/~mbailey/publications/ndss17_interception.pdf Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-08ci: unify bash calling conventionJunio C Hamano-1/+1
Under ci/ hierarchy, we run scripts under either "sh" (any Bourne compatible POSIX shell would work) or specifically "bash" (as they require features from bash, e.g., ${parameter/pattern/string} expansion). As we have the CI environment under our control, we can expect that /bin/sh will always be fine to run the scripts that only require a Bourne shell, but we may not know where "bash" is installed depending on the distro used. So let's make sure we start these scripts with either one of these: #!/bin/sh #!/usr/bin/env bash Yes, the latter has to assume that everybody installs "env" at that path and not as /bin/env or /usr/local/bin/env, but this currently is the best we could do. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-08The ninteenth batchJunio C Hamano-0/+28
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-08Merge branch 'ds/sparse-lstat-caching'Junio C Hamano-52/+164
The code to deal with modified paths that are out-of-cone in a sparsely checked out working tree has been optimized. * ds/sparse-lstat-caching: sparse-index: improve lstat caching of sparse paths sparse-index: count lstat() calls sparse-index: use strbuf in path_found() sparse-index: refactor path_found() sparse-checkout: refactor skip worktree retry logic
2024-07-08Merge branch 'xx/bundie-uri-fixes'Junio C Hamano-14/+243
When bundleURI interface fetches multiple bundles, Git failed to take full advantage of all bundles and ended up slurping duplicated objects. * xx/bundie-uri-fixes: unbundle: extend object verification for fetches fetch-pack: expose fsckObjects configuration logic bundle-uri: verify oid before writing refs
2024-07-08Merge branch 'ps/leakfixes-more'Junio C Hamano-272/+591
More memory leaks have been plugged. * ps/leakfixes-more: (29 commits) builtin/blame: fix leaking ignore revs files builtin/blame: fix leaking prefixed paths blame: fix leaking data for blame scoreboards line-range: plug leaking find functions merge: fix leaking merge bases builtin/merge: fix leaking `struct cmdnames` in `get_strategy()` sequencer: fix memory leaks in `make_script_with_merges()` builtin/clone: plug leaking HEAD ref in `wanted_peer_refs()` apply: fix leaking string in `match_fragment()` sequencer: fix leaking string buffer in `commit_staged_changes()` commit: fix leaking parents when calling `commit_tree_extended()` config: fix leaking "core.notesref" variable rerere: fix various trivial leaks builtin/stash: fix leak in `show_stash()` revision: free diff options builtin/log: fix leaking commit list in git-cherry(1) merge-recursive: fix memory leak when finalizing merge builtin/merge-recursive: fix leaking object ID bases builtin/difftool: plug memory leaks in `run_dir_diff()` object-name: free leaking object contexts ...
2024-07-08Merge branch 'tb/path-filter-fix'Junio C Hamano-58/+736
The Bloom filter used for path limited history traversal was broken on systems whose "char" is unsigned; update the implementation and bump the format version to 2. * tb/path-filter-fix: bloom: introduce `deinit_bloom_filters()` commit-graph: reuse existing Bloom filters where possible object.h: fix mis-aligned flag bits table commit-graph: new Bloom filter version that fixes murmur3 commit-graph: unconditionally load Bloom filters bloom: prepare to discard incompatible Bloom filters bloom: annotate filters with hash version repo-settings: introduce commitgraph.changedPathsVersion t4216: test changed path filters with high bit paths t/helper/test-read-graph: implement `bloom-filters` mode bloom.h: make `load_bloom_filter_from_graph()` public t/helper/test-read-graph.c: extract `dump_graph_info()` gitformat-commit-graph: describe version 2 of BDAT commit-graph: ensure Bloom filters are read with consistent settings revision.c: consult Bloom filters for root commits t/t4216-log-bloom.sh: harden `test_bloom_filters_not_used()`
2024-07-08Merge branch 'db/date-underflow-fix'Junio C Hamano-7/+56
date parser updates to be more careful about underflowing epoch based timestamp. * db/date-underflow-fix: date: detect underflow/overflow when parsing dates with timezone offset t0006: simplify prerequisites
2024-07-08Merge branch 'rj/pager-die-upon-exec-failure'Junio C Hamano-13/+6
When GIT_PAGER failed to spawn, depending on the code path taken, we failed immediately (correct) or just spew the payload to the standard output (incorrect). The code now always fail immediately when GIT_PAGER fails. * rj/pager-die-upon-exec-failure: pager: die when paging to non-existing command
2024-07-08Merge branch 'ss/doc-eol-attr-fix'Junio C Hamano-1/+1
Doc update. * ss/doc-eol-attr-fix: doc: fix case error of eol attribute in example
2024-07-08Merge branch 'jc/archive-prefix-with-add-virtual-file'Junio C Hamano-4/+6
"git archive --add-virtual-file=<path>:<contents>" never paid attention to the --prefix=<prefix> option but the documentation said it would. The documentation has been corrected. * jc/archive-prefix-with-add-virtual-file: archive: document that --add-virtual-file takes full path
2024-07-08advice: warn when sparse index expandsDerrick Stolee-1/+49
Typically, forcing a sparse index to expand to a full index means that Git could not determine the status of a file outside of the sparse-checkout and needed to expand sparse trees into the full list of sparse blobs. This operation can be very slow when the sparse-checkout is much smaller than the full tree at HEAD. When users are in this state, there is usually a modified or untracked file outside of the sparse-checkout mentioned by the output of 'git status'. There are a number of reasons why this is insufficient: 1. Users may not have a full understanding of which files are inside or outside of their sparse-checkout. This is more common in monorepos that manage the sparse-checkout using custom tools that map build dependencies into sparse-checkout definitions. 2. In some cases, an empty directory could exist outside the sparse-checkout and these empty directories are not reported by 'git status' and friends. 3. If the user has '.gitignore' or 'exclude' files, then 'git status' will squelch the warnings and not demonstrate any problems. In order to help users who are in this state, add a new advice message to indicate that a sparse index is expanded to a full index. This message should be written at most once per process, so add a static global 'give_advice_on_expansion' to sparse-index.c. Further, there is a case in 'git sparse-checkout set' that uses the sparse index as an in-memory data structure (even when writing a full index) so we need to disable the message in that kind of case. The t1092-sparse-checkout-compatibility.sh test script compares the behavior of several Git commands across full and sparse repositories, including sparse repositories with and without a sparse index. We need to disable the advice in the sparse-index repo to avoid differences in stderr. By leaving the advice on in the sparse-checkout repo (without the sparse index), we can test the behavior of disabling the advice in convert_to_sparse(). (Indeed, these tests are how that necessity was discovered.) Add a test that reenables the advice and demonstrates that the message is output. The advice message is defined outside of expand_index() to avoid super- wide lines. It is also defined as a macro to avoid compile issues with -Werror=format-security. Signed-off-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-08doc: fix the max number of branches shown by "show-branch"Rikita Ishikawa-1/+1
The number to be displayed is calculated by the following defined in object.h: #define REV_SHIFT 2 #define MAX_REVS (FLAG_BITS - REV_SHIFT) FLAG_BITS is currently 28, so 26 is the correct number. Signed-off-by: Rikita Ishikawa <lagrange.resolvent@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-07gitweb: rss/atom change published/updated date to committer dateJesús Ariel Cabello Mateos-2/+2
The author date is used for published/updated date in the rss/atom feed stream. Change it to the committer date that reflects the "published/updated" definition better and makes rss/atom feeds more linear. Gitlab/Github rss/atom feeds use the committer date. Additionally, to be consistent, also use the committer date to determine the date of the last commit to send in the feed instead of the author date. Signed-off-by: Jesús Ariel Cabello Mateos <080ariel@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-07Merge https://github.com/j6t/git-guiJunio C Hamano-1601/+1674
* https://github.com/j6t/git-gui: git-gui: fix inability to quit after closing another instance git-gui: sv.po: Update Swedish translation (576t0f0u) git-gui: note the new maintainer Makefile(s): do not enforce "all indents must be done with tab" Makefile(s): avoid recipe prefix in conditional statements doc: switch links to https doc: update links to current pages git-gui: po: fix typo in French "aperçu"