summaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* tests: simplify construction of large blocks of textEric Sunshine2021-12-137-202/+205
| | | | | | | | | | | Take advantage of here-docs to create large blocks of text rather than using a series of `echo` statements. Not only are here-docs a natural fit for such a task, but there is less opportunity for a broken &&-chain. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* t9107: use shell parameter expansion to avoid breaking &&-chainEric Sunshine2021-12-131-4/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This test intentionally breaks the &&-chain when using `expr` to parse "[<path>]:<ref>" since the pattern matching operation will return 1 (failure) when <path> is empty even though an empty <path> is legitimate in this test and should not cause the test to fail. However, it is possible to parse the input without breaking the &&-chain by using shell parameter expansion (i.e. `${i%%...}`). Other ways to avoid the problem would be `{ expr $i : ... ||:; }` or test_might_fail(), however, parameter expansion seems simplest. IMPLEMENTATION NOTE The rewritten `if` expression: if test "$ref" = "${ref#refs/remotes/}"`; then continue; fi is perhaps a bit subtle. At first glance, it looks like it will `continue` the loop if $ref starts with "refs/remotes/", but in fact it's the opposite: the loop will `continue` if $ref does not start with "refs/remotes/". In the original, `expr` would only match if the ref started with "refs/remotes/", and $ref would end up empty if it didn't, so `test -z` would `continue` the loop if the ref did not start with "refs/remotes/". With parameter expansion, ${ref#refs/remotes/} attempts to strip "refs/remotes/" from $ref. If it fails, meaning that $ref does not start with "refs/remotes/", then the expansion will just be $ref unchanged, and it will `continue` the loop. On the other hand, if stripping succeeds, meaning that $ref begins with "refs/remotes/", then the expansion will be the value of $ref with "refs/remotes/" removed, hence `continue` will not be taken. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* t6300: make `%(raw:size) --shell` test more robustEric Sunshine2021-12-131-4/+1
| | | | | | | | | | | | | | | | | This test populates its `expect` file solely by appending content but fails to ensure that the file starts out empty. The test succeeds only because no earlier test populated a file of the exact same name, however this is an accident waiting to happen. Make the test more robust by ensuring that it contains exactly the intended content. While at it, simplify the implementation via a straightforward `sed` application and by avoiding dropping out of the single-quote context within the test body (thus eliminating a hard-to-digest combination of apostrophes and backslashes). Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* t5516: drop unnecessary subshell and command invocationEric Sunshine2021-12-131-4/+1
| | | | | | | | | | | | To create its "expect" file, this test pipes into `sort` the output of `git for-each-ref` and a copy of that same output but with a minor textual transformation applied. To do so, it employs a subshell and commands `cat` and `sed` even though the same result can be accomplished by `sed` alone (without a subshell). Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* t4202: clarify intent by creating expected content less cleverlyEric Sunshine2021-12-131-21/+21
| | | | | | | | | | | | | | | | | | | | | | | | | | Several tests assign the output of `$(...)` command substitution to an "expect" variable, taking advantage of the fact that `$(...)` folds out the final line terminator while leaving internal line terminators intact. They do this because the "actual" string with which "expect" will be compared is shaped the same way. However, this intent (having internal line terminators, but no final line terminator) is not necessarily obvious at first glance and may confuse casual readers. The intent can be made more obvious by using `printf` instead, with which line termination is stated clearly: printf "sixth\nthird" In fact, many other tests in this script already use `printf` for precisely this purpose, thus it is an established pattern. Therefore, convert these tests to employ `printf`, as well. While at it, modernize the tests to use test_cmp() to compare the expected and actual output rather than using the semi-deprecated `verbose test "$x" = "$y"`. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* t1020: avoid aborting entire test script when one test failsEric Sunshine2021-12-131-3/+3
| | | | | | | | | | | | | Although `exit 1` is the proper way to signal a test failure from within a subshell, its use outside any subshell should be avoided since it aborts the entire script rather than aborting only the failed test. Instead, a simple `return 1` is the proper idiom for signaling failure outside a subshell since it aborts only the test in question, not the entire script. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* t1010: fix unnoticed failure on WindowsEric Sunshine2021-12-131-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | On Microsoft Windows, a directory name should never end with a period. Quoting from Microsoft documentation[1]: Do not end a file or directory name with a space or a period. Although the underlying file system may support such names, the Windows shell and user interface does not. Naming a directory with a trailing period is indeed perilous: % git init foo % cd foo % mkdir a. % git status warning: could not open directory 'a./': No such file or directory The t1010 "setup" test: for d in a a. a0 do mkdir "$d" && echo "$d/one" >"$d/one" && git add "$d" done && runs afoul of this Windows limitation, as can be observed when running the test verbosely: error: open("a./one"): No such file or directory error: unable to index file 'a./one' fatal: adding files failed The reason this problem has gone unnoticed for so long is twofold. First, the failed `git add` is swallowed silently because the loop is not terminated explicitly by `|| return 1` to signal the failure. Second, none of the tests in this script care about the literal directory names ("a", "a.", "a0") or the specific number of tree entries. They care instead about the order of entries in the tree, and that the tree synthesized in the index and created by `git write-tree` matches the tree created by the output of `git ls-tree` fed into `git mktree`, thus the absence of "a./one" has no impact on the tests. Skipping these tests on Windows by, for instance, checking the FUNNYNAMES predicate would avoid the problem, however, the funny-looking name is not what is being tested here. Rather, the tests are about checking that `git mktree` produces stable results for various input conditions, such as when the input order is not consistent or when an object is missing. Therefore, resolve the problem simply by using a directory name which is legal on Windows and sorts the same as "a.". While at it, add the missing `|| return 1` to the loop body in order to catch this sort of problem in the future. [1]: https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* t/lib-pager: use sane_unset() to avoid breaking &&-chainEric Sunshine2021-12-091-1/+1
| | | | | | | | | | | This test intentionally breaks the &&-chain following `unset` since it doesn't know if `unset` will succeed or fail and doesn't want a local `unset` failure to abort the test overall. We can do better by using sane_unset() which can be linked into the &&-chain as usual. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* The first batch to start the current cycleJunio C Hamano2021-11-301-4/+35
| | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'mc/clean-smudge-with-llp64'Junio C Hamano2021-11-3011-19/+89
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | The clean/smudge conversion code path has been prepared to better work on platforms where ulong is narrower than size_t. * mc/clean-smudge-with-llp64: clean/smudge: allow clean filters to process extremely large files odb: guard against data loss checking out a huge file git-compat-util: introduce more size_t helpers odb: teach read_blob_entry to use size_t t1051: introduce a smudge filter test for extremely large files test-lib: add prerequisite for 64-bit platforms test-tool genzeros: generate large amounts of data more efficiently test-genzeros: allow more than 2G zeros in Windows
| * clean/smudge: allow clean filters to process extremely large filesMatt Cooper2021-11-032-1/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The filter system allows for alterations to file contents when they're moved between the database and the worktree. We already made sure that it is possible for smudge filters to produce contents that are larger than `unsigned long` can represent (which matters on systems where `unsigned long` is narrower than `size_t`, most notably 64-bit Windows). Now we make sure that clean filters can _consume_ contents that are larger than that. Note that this commit only allows clean filters' _input_ to be larger than can be represented by `unsigned long`. This change makes only a very minute dent into the much larger project to teach Git to use `size_t` instead of `unsigned long` wherever appropriate. Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Matt Cooper <vtbassmatt@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * odb: guard against data loss checking out a huge fileMatt Cooper2021-11-033-9/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This introduces an additional guard for platforms where `unsigned long` and `size_t` are not of the same size. If the size of an object in the database would overflow `unsigned long`, instead we now exit with an error. A complete fix will have to update _many_ other functions throughout the codebase to use `size_t` instead of `unsigned long`. It will have to be implemented at some stage. This commit puts in a stop-gap for the time being. Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Matt Cooper <vtbassmatt@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * git-compat-util: introduce more size_t helpersJohannes Schindelin2021-11-031-0/+25
| | | | | | | | | | | | | | We will use them in the next commit. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * odb: teach read_blob_entry to use size_tMatt Cooper2021-11-034-6/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | There is mixed use of size_t and unsigned long to deal with sizes in the codebase. Recall that Windows defines unsigned long as 32 bits even on 64-bit platforms, meaning that converting size_t to unsigned long narrows the range. This mostly doesn't cause a problem since Git rarely deals with files larger than 2^32 bytes. But adjunct systems such as Git LFS, which use smudge/clean filters to keep huge files out of the repository, may have huge file contents passed through some of the functions in entry.c and convert.c. On Windows, this results in a truncated file being written to the workdir. I traced this to one specific use of unsigned long in write_entry (and a similar instance in write_pc_item_to_fd for parallel checkout). That appeared to be for the call to read_blob_entry, which expects a pointer to unsigned long. By altering the signature of read_blob_entry to expect a size_t, write_entry can be switched to use size_t internally (which all of its callers and most of its callees already used). To avoid touching dozens of additional files, read_blob_entry uses a local unsigned long to call a chain of functions which aren't prepared to accept size_t. Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Matt Cooper <vtbassmatt@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * t1051: introduce a smudge filter test for extremely large filesMatt Cooper2021-11-031-0/+15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The filter system allows for alterations to file contents when they're added to the database or working tree. ("Smudge" when moving to the working tree; "clean" when moving to the database.) This is used natively to handle CRLF to LF conversions. It's also employed by Git-LFS to replace large files from the working tree with small tracking files in the repo and vice versa. Git reads the entire smudged file into memory to convert it into a "clean" form to be used in-core. While this is inefficient, there's a more insidious problem on some platforms due to inconsistency between using unsigned long and size_t for the same type of data (size of a file in bytes). On most 64-bit platforms, unsigned long is 64 bits, and size_t is typedef'd to unsigned long. On Windows, however, unsigned long is only 32 bits (and therefore on 64-bit Windows, size_t is typedef'd to unsigned long long in order to be 64 bits). Practically speaking, this means 64-bit Windows users of Git-LFS can't handle files larger than 2^32 bytes. Other 64-bit platforms don't suffer this limitation. This commit introduces a test exposing the issue; future commits make it pass. The test simulates the way Git-LFS works by having a tiny file checked into the repository and expanding it to a huge file on checkout. Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Matt Cooper <vtbassmatt@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * test-lib: add prerequisite for 64-bit platformsCarlo Marcelo Arenas Belón2021-11-031-0/+4
| | | | | | | | | | | | | | | | | | | | | | Allow tests that assume a 64-bit `size_t` to be skipped in 32-bit platforms and regardless of the size of `long`. This imitates the `LONG_IS_64BIT` prerequisite. Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * test-tool genzeros: generate large amounts of data more efficientlyJohannes Schindelin2021-11-031-2/+15
| | | | | | | | | | | | | | | | | | | | | | | | | | In this developer's tests, producing one gigabyte worth of NULs in a busy loop that writes out individual bytes, unbuffered, took ~27sec. Writing chunked 256kB buffers instead only took ~0.6sec This matters because we are about to introduce a pair of test cases that want to be able to produce 5GB of NULs, and we cannot use `/dev/zero` because of the HP NonStop platform's lack of support for that device. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * test-genzeros: allow more than 2G zeros in WindowsCarlo Marcelo Arenas Belón2021-11-031-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | d5cfd142ec (tests: teach the test-tool to generate NUL bytes and use it, 2019-02-14), add a way to generate zeroes in a portable way without using /dev/zero (needed by HP NonStop), but uses a long variable that is limited to 2^31 in Windows. Use instead a (POSIX/C99) intmax_t that is at least 64bit wide in 64-bit Windows to use in a future test. Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'ab/sh-retire-helper-functions'Junio C Hamano2021-11-307-41/+14
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Make a few helper functions unused and then lose them. * ab/sh-retire-helper-functions: git-sh-setup: remove "sane_grep", it's not needed anymore git-sh-setup: remove unused sane_egrep() function git-instaweb: unconditionally assume that gitweb is mod_perl capable Makefile: remove $(NO_CURL) from $(SCRIPT_DEFINES) Makefile: remove $(GIT_VERSION) from $(SCRIPT_DEFINES) Makefile: move git-SCRIPT-DEFINES adjacent to $(SCRIPT_DEFINES)
| * | git-sh-setup: remove "sane_grep", it's not needed anymoreÆvar Arnfjörð Bjarmason2021-10-227-22/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Remove the sane_grep() shell function in git-sh-setup. The two reasons for why it existed don't apply anymore: 1. It was added due to GNU grep supporting GREP_OPTIONS. See e1622bfcbad (Protect scripted Porcelains from GREP_OPTIONS insanity, 2009-11-23). Newer versions of GNU grep ignore that, but even on older versions its existence won't matter, none of these sane_grep() uses care about grep's output, they're merely using it to check if a string exists in a file or stream. We also don't care about the "LC_ALL=C" that "sane_grep" was using, these greps for fixed or ASCII strings will behave the same under any locale. 2. The SANE_TEXT_GREP added in 71b401032b9 (sane_grep: pass "-a" if grep accepts it, 2016-03-08) isn't needed either, none of these grep uses deal with binary data. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | git-sh-setup: remove unused sane_egrep() functionÆvar Arnfjörð Bjarmason2021-10-221-4/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The is_zero_oid() function in git-submodule.sh has not been used since e83e3333b57 (submodule: port submodule subcommand 'summary' from shell to C, 2020-08-13), so we can remove it, and the sane_egrep() function, dead is_zero_oid() was the only function which still referenced it. Unlike some other functions in git-sh-setup.sh, this function has not been documented in git-sh-setup(1), so per [1] it should be OK to remove it. I'm still unclear about the future of some of the other functions[2], but any questions in that area should not apply here. 1. https://lore.kernel.org/git/xmqqr1dtgnn8.fsf@gitster.g/ 1. https://lore.kernel.org/git/87tuiwjfvi.fsf@evledraar.gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | git-instaweb: unconditionally assume that gitweb is mod_perl capableÆvar Arnfjörð Bjarmason2021-10-221-4/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Remove a check for whether mod_perl is a supported mode of gitweb.cgi added in a51d37c1df6 (Add git-instaweb, instantly browse the working repo with gitweb, 2006-07-01). The reason for the check was to support users who had a newer version of git and an older version of gitweb, it was then subsequently adjusted for changes in the script in f0e588dffc1 (git-instaweb: fix mod_perl detection for apache2, 2009-08-08). It's a fair bet that nobody's running a git from 2021 and gitweb from pre-2007 anymore, so we can unconditionally assume that this will be supported by gitweb.cgi. This allows a subsequent commit to remove the sane_grep() wrapper, this change is split up from that since this is the only case where the "grep" invocation could be removed entirely. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | Makefile: remove $(NO_CURL) from $(SCRIPT_DEFINES)Ævar Arnfjörð Bjarmason2021-10-221-2/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Stop including $(NO_CURL) in $(SCRIPT_DEFINES). The "@NO_CURL@" replacement added in 6c5c62f3401 (Print an error if cloning a http repo and NO_CURL is set, 2006-02-15) has not been referenced by anything in-tree since 49eb8d39c78 (Remove contrib/examples/*, 2018-03-25). That commit removed the reference from contrib/examples/*, but this @@NO_CURL@@ hasn't been used since git-pull.sh was the primary entry point for "git pull". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | Makefile: remove $(GIT_VERSION) from $(SCRIPT_DEFINES)Ævar Arnfjörð Bjarmason2021-10-221-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Remove the $(GIT_VERSION) from $(SCRIPT_DEFINES). Now every time HEAD changes in a development copy we don't need to re-build the scripts and script libraries. This has not been needed since 2b9391bc675 (Makefile: do not replace @@GIT_VERSION@@ in shell scripts, 2012-06-20). On my setup this changes the re-making of 44 targets in a development copy where moved HEAD to 27. The $(GIT_VERSION) was seemingly left here by mistake or omission. We didn't need it since 2b9391bc675, but in the later e4dd89ab984 (Makefile: update scripts when build-time parameters change, 2012-06-20) it was added to SCRIPT_DEFINES. The two were part of the same series of patches, and given the summary in [1] and [2] it looks like this was probably a case of some earlier version of a later patch being combined with an updated earlier patch. 1. https://lore.kernel.org/git/20120619232231.GA6328@sigill.intra.peff.net/ 2. https://lore.kernel.org/git/20120619232453.GB6496@sigill.intra.peff.net/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | Makefile: move git-SCRIPT-DEFINES adjacent to $(SCRIPT_DEFINES)Ævar Arnfjörð Bjarmason2021-10-221-8/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When "GIT-SCRIPT-DEFINES" was added in e4dd89ab984 (Makefile: update scripts when build-time parameters change, 2012-06-20) the rules for generating the scripts themselves were moved further away from the "cmd_munge_script" added in 46bac904581 (Do not install shell libraries executable, 2010-01-31). Let's move these around so that the variables and defines needed by given targets immediately precede them. This is not needed for any subsequent changes to work, but makes the code consistent with how GIT-PERL-DEFINES is structured. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'tb/plug-pack-bitmap-leaks'Junio C Hamano2021-11-308-45/+84
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Leakfix. * tb/plug-pack-bitmap-leaks: pack-bitmap.c: more aggressively free in free_bitmap_index() pack-bitmap.c: don't leak type-level bitmaps midx.c: write MIDX filenames to strbuf builtin/multi-pack-index.c: don't leak concatenated options builtin/repack.c: avoid leaking child arguments builtin/pack-objects.c: don't leak memory via arguments t/helper/test-read-midx.c: free MIDX within read_midx_file() midx.c: don't leak MIDX from verify_midx_file midx.c: clean up chunkfile after reading the MIDX
| * | | pack-bitmap.c: more aggressively free in free_bitmap_index()Taylor Blau2021-10-291-0/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The function free_bitmap_index() is somewhat lax in what it frees. There are two notable examples: - While it does call kh_destroy_oid_map on the "bitmaps" map, which maps commit OIDs to their corresponding bitmaps, the bitmaps themselves are not freed. Note here that we recycle already-freed ewah_bitmaps into a pool, but these are handled correctly by ewah_pool_free(). - We never bother to free the extended index's "positions" map, which we always allocate in load_bitmap(). Fix both of these. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | pack-bitmap.c: don't leak type-level bitmapsTaylor Blau2021-10-291-0/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | test_bitmap_walk() is used to implement `git rev-list --test-bitmap`, which compares the result of the on-disk bitmaps with ones generated on-the-fly during a revision walk. In fa95666a40 (pack-bitmap.c: harden 'test_bitmap_walk()' to check type bitmaps, 2021-08-24), we hardened those tests to also check the four special type-level bitmaps, but never freed those bitmaps. We should have, since each required an allocation when we EWAH-decompressed them. Free those, plugging that leak, and also free the base (the scratch-pad bitmap), too. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | midx.c: write MIDX filenames to strbufTaylor Blau2021-10-294-37/+49
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | To ask for the name of a MIDX and its corresponding .rev file, callers invoke get_midx_filename() and get_midx_rev_filename(), respectively. These both invoke xstrfmt(), allocating a chunk of memory which must be freed later on. This makes callers in pack-bitmap.c somewhat awkward. Specifically, midx_bitmap_filename(), which is implemented like: return xstrfmt("%s-%s.bitmap", get_midx_filename(midx->object_dir), hash_to_hex(get_midx_checksum(midx))); this leaks the second argument to xstrfmt(), which itself was allocated with xstrfmt(). This caller could assign both the result of get_midx_filename() and the outer xstrfmt() to a temporary variable, remembering to free() the former before returning. But that involves a wasteful copy. Instead, get_midx_filename() and get_midx_rev_filename() take a strbuf as an output parameter. This way midx_bitmap_filename() can manipulate and pass around a temporary buffer which it detaches back to its caller. That allows us to implement the function without copying or open-coding get_midx_filename() in a way that doesn't leak. Update the other callers of get_midx_filename() and get_midx_rev_filename() accordingly. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | builtin/multi-pack-index.c: don't leak concatenated optionsTaylor Blau2021-10-291-0/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The `multi-pack-index` builtin dynamically allocates an array of command-line option for each of its separate modes by calling add_common_options() to concatante the common options with sub-command specific ones. Because this operation allocates a new array, we have to be careful to remember to free it. We already do this in the repack and write sub-commands, but verify and expire don't. Rectify this by calling FREE_AND_NULL as the other modes do. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | builtin/repack.c: avoid leaking child argumentsTaylor Blau2021-10-291-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `git repack` invokes a handful of child processes: one to write the actual pack, and optionally ones to repack promisor objects and update the MIDX. Most of these are freed automatically by calling `start_command()` (which invokes it on error) and `finish_command()` which calls it automatically. But repack_promisor_objects() can initialize a child_process, populate its array of arguments, and then return from the function before even calling start_command(). Make sure that the prepared list of arguments is freed by calling child_process_clear() ourselves to avoid leaking memory along this path. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | builtin/pack-objects.c: don't leak memory via argumentsTaylor Blau2021-10-281-4/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When constructing arguments to pass to setup_revision(), pack-objects only frees the memory used by that array after calling get_object_list(). Ensure that we call strvec_clear() whether or not we use the arguments array by cleaning up whenever we exit the function (and rewriting one early return to jump to a label which frees the memory and then returns). We could avoid setting this array up altogether unless we are in the if-else block that calls get_object_list(), but setting up the argument array is intermingled with lots of other side-effects, e.g.: if (exclude_promisor_objects) { use_internal_rev_list = 1; fetch_if_missing = 0; strvec_push(&rp, "--exclude-promisor-objects"); } So it would be awkward to check exclude_promisor_objects twice: first to set use_internal_rev_list and fetch_if_missing, and then again above get_object_list() to push the relevant argument onto the array. Instead, leave the array's construction alone and make sure to free it unconditionally. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | t/helper/test-read-midx.c: free MIDX within read_midx_file()Taylor Blau2021-10-281-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When calling `read_midx_file()` to show information about a MIDX or list the objects contained within it we fail to call `close_midx()`, leaking the memory allocated to store that MIDX. Fix this by calling `close_midx()` before exiting the function. We can drop the "early" return when `show_objects` is non-zero, since the next instruction is also a return. (We could just as easily put a `cleanup` label here as with previous patches. But the only other time we terminate the function early is when we fail to load a MIDX in the first place. `close_midx()` does handle a NULL argument, but the extra complexity is likely not warranted). Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | midx.c: don't leak MIDX from verify_midx_fileTaylor Blau2021-10-281-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The function midx.c:verify_midx_file() allocates a MIDX struct by calling load_multi_pack_index(). But when cleaning up, it calls free() without freeing any resources associated with the MIDX. Call the more appropriate close_midx() which does free those resources, which causes t5319.3 to pass when Git is compiled with SANITIZE=leak. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | midx.c: clean up chunkfile after reading the MIDXTaylor Blau2021-10-211-1/+2
| |/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In order to read the contents of a MIDX, we initialize a chunkfile structure which can read the table of contents and assign pointers into different sections of the file for us. We do call free(), since the chunkfile struct is heap allocated, but not the more appropriate free_chunkfile(), which also frees memory that the structure itself owns. Call that instead to avoid leaking memory in this function. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'tp/send-email-completion'Junio C Hamano2021-11-304-22/+54
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The command line complation for "git send-email" options have been tweaked to make it easier to keep it in sync with the command itself. * tp/send-email-completion: send-email docs: add format-patch options send-email: programmatically generate bash completions
| * | | send-email docs: add format-patch optionsThiago Perrotta2021-10-282-3/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git-send-email(1) does not mention that "git format-patch" options are accepted. Augment SYNOPSIS and DESCRIPTION to mention it. Update git-send-email.perl USAGE to be consistent with git-send-email(1). Signed-off-by: Thiago Perrotta <tbperrotta@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | send-email: programmatically generate bash completionsThiago Perrotta2021-10-283-19/+48
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git send-email --git-completion-helper" only prints "format-patch" flags. Make it print "send-email" flags as well, extracting them programmatically from its three existing "GetOptions". Introduce a "uniq" subroutine, otherwise --cc-cover, --to-cover and other flags would show up twice. In addition, deduplicate flags common to both "send-email" and "format-patch", like --from. Remove extraneous flags: --h and --git-completion-helper. Add trailing "=" to options that expect an argument, inline with the format-patch implementation. Add a completion test for "send-email --validate", a send-email flag. Signed-off-by: Thiago Perrotta <tbperrotta@gmail.com> Based-on-patch-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'jc/unsetenv-returns-an-int'Junio C Hamano2021-11-302-2/+4
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The compatibility implementation for unsetenv(3) were written to mimic ancient, non-POSIX, variant seen in an old glibc; it has been changed to return an integer to match the more modern era. * jc/unsetenv-returns-an-int: unsetenv(3) returns int, not void
| * | | | unsetenv(3) returns int, not voidJunio C Hamano2021-10-302-2/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This compatilibity implementation has been returning a wrong type, ever since 731043fd (Add compat/unsetenv.c ., 2006-01-25) added to the system, yet nobody noticed it in the past 16 years, presumably because no code checks failures in their unsetenv() calls. Sigh. For now, make it always succeed. Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | Merge branch 'jc/fix-ref-sorting-parse'Junio C Hamano2021-11-308-52/+95
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Things like "git -c branch.sort=bogus branch new HEAD", i.e. the operation modes of the "git branch" command that do not need the sort key information, no longer errors out by seeing a bogus sort key. * jc/fix-ref-sorting-parse: for-each-ref: delay parsing of --sort=<atom> options
| * | | | | for-each-ref: delay parsing of --sort=<atom> optionsJunio C Hamano2021-10-208-52/+95
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The for-each-ref family of commands invoke parsers immediately when it sees each --sort=<atom> option, and die before even seeing the other options on the command line when the <atom> is unrecognised. Instead, accumulate them in a string list, and have them parsed into a ref_sorting structure after the command line parsing is done. As a consequence, "git branch --sort=bogus -h" used to fail to give the brief help, which arguably may have been a feature, now does so, which is more consistent with how other options work. The patch is smaller than the actual extent of the "damage" to the codebase, thanks to the fact that the original code consistently used OPT_REF_SORT() macro to handle command line options. We only needed to replace the variable used for the list, and implementation of the callback function used in the macro. The old rule was for the users of the API to: - Declare ref_sorting and ref_sorting_tail variables; - OPT_REF_SORT() macro will instantiate ref_sorting instance (which may barf and die) and append it to the tail; - Append to the tail each ref_sorting read from the configuration by parsing in the config callback (which may barf and die); - See if ref_sorting is null and use ref_sorting_default() instead. Now the rule is not all that different but is simpler: - Declare ref_sorting_options string list. - OPT_REF_SORT() macro will append it to the string list; - Append to the string list the sort key read from the configuration; - call ref_sorting_options() to turn the string list to ref_sorting structure (which also deals with the default value). As side effects, this change also cleans up a few issues: - 95be717c (parse_opt_ref_sorting: always use with NONEG flag, 2019-03-20) muses that "git for-each-ref --no-sort" should simply clear the sort keys accumulated so far; it now does. - The implementation detail of "struct ref_sorting" and the helper function parse_ref_sorting() can now be private to the ref-filter API implementation. - If you set branch.sort to a bogus value, the any "git branch" invocation, not only the listing mode, would abort with the original code; now it doesn't Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | Merge branch 'ab/ref-filter-leakfix' into jc/fix-ref-sorting-parseJunio C Hamano2021-10-205-16/+34
| |\ \ \ \ \ | | |_|_|/ / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | * ab/ref-filter-leakfix: branch: use ref_sorting_release() ref-filter API user: add and use a ref_sorting_release() tag: use a "goto cleanup" pattern, leak less memory
* | | | | | Merge branch 'so/stash-staged'Junio C Hamano2021-11-303-12/+113
|\ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git stash" learned the "--staged" option to stash away what has been added to the index (and nothing else). * so/stash-staged: stash: get rid of unused argument in stash_staged() stash: implement '--staged' option for 'push' and 'save'
| * | | | | | stash: get rid of unused argument in stash_staged()Sergey Organov2021-10-281-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Unused 'ps' argument was a left-over from original copy-paste of stash_patch(). Removed. Signed-off-by: Sergey Organov <sorganov@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | stash: implement '--staged' option for 'push' and 'save'Sergey Organov2021-10-183-12/+113
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Stash only the changes that are staged. This mode allows to easily stash-out for later reuse some changes unrelated to the current work in progress. Unlike 'stash push --patch', --staged supports use of any tool to select the changes to stash-out, including, but not limited to 'git add --interactive'. Signed-off-by: Sergey Organov <sorganov@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | Merge branch 'jc/tutorial-format-patch-base'Junio C Hamano2021-11-301-13/+28
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Teach and encourage first-time contributors to this project to state the base commit when they submit their topic. * jc/tutorial-format-patch-base: MyFirstContribution: teach to use "format-patch --base=auto"
| * | | | | | | MyFirstContribution: teach to use "format-patch --base=auto"Junio C Hamano2021-10-231-13/+28
| | |_|_|/ / / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Let's encourage first-time contributors to tell us what commit they based their work on with the format-patch invocation. As the example already forks from origin/master and branch.autosetupmerge by default records the upstream when the psuh branch was created, we can use --base=auto for this. Also, mention that the range of commits can simply be given with `@{u}` if they are on the `psuh` branch already. As we are getting one more option on the command line, and spending one paragraph each to explain them, let's reformat that part of the description as a bulleted list. Helped-by: Bagas Sanjaya <bagasdotme@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | Merge branch 'ab/refs-errno-cleanup'Junio C Hamano2021-11-3010-143/+294
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The "remainder" of hn/refs-errno-cleanup topic. * ab/refs-errno-cleanup: (21 commits) refs API: post-migration API renaming [2/2] refs API: post-migration API renaming [1/2] refs API: don't expose "errno" in run_transaction_hook() refs API: make expand_ref() & repo_dwim_log() not set errno refs API: make resolve_ref_unsafe() not set errno refs API: make refs_ref_exists() not set errno refs API: make refs_resolve_refdup() not set errno refs tests: ignore ignore errno in test-ref-store helper refs API: ignore errno in worktree.c's find_shared_symref() refs API: ignore errno in worktree.c's add_head_info() refs API: make files_copy_or_rename_ref() et al not set errno refs API: make loose_fill_ref_dir() not set errno refs API: make resolve_gitlink_ref() not set errno refs API: remove refs_read_ref_full() wrapper refs/files: remove "name exist?" check in lock_ref_oid_basic() reflog tests: add --updateref tests refs API: make refs_rename_ref_available() static refs API: make parse_loose_ref_contents() not set errno refs API: make refs_read_raw_ref() not set errno refs API: add a version of refs_resolve_ref_unsafe() with "errno" ...
| * | | | | | | refs API: post-migration API renaming [2/2]Ævar Arnfjörð Bjarmason2021-10-167-29/+22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Rename the transitory refs_werrres_ref_unsafe() function to refs_resolve_ref_unsafe(), now that all callers of the old function have learned to pass in a "failure_errno" parameter. The coccinelle semantic patch added in the preceding commit works, but I couldn't figure out how to get spatch(1) to re-flow these argument lists (and sometimes make lines way too long), so this rename was done with: perl -pi -e 's/refs_werrres_ref_unsafe/refs_resolve_ref_unsafe/g' \ $(git grep -l refs_werrres_ref_unsafe -- '*.c') But after that "make contrib/coccinelle/refs.cocci.patch" comes up empty, so the result would have been the same. Let's remove that transitory semantic patch file, we won't need to retain it for any other in-flight changes, refs_werrres_ref_unsafe() only existed within this patch series. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>