summaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2022-01-14repo_read_index: clear SKIP_WORKTREE bit from files present in worktreeLibravatar Elijah Newren9-80/+72
The fix is short (~30 lines), but the description is not. Sorry. There is a set of problems caused by files in what I'll refer to as the "present-despite-SKIP_WORKTREE" state. This commit aims to not just fix these problems, but remove the entire class as a possibility -- for those using sparse checkouts. But first, we need to understand the problems this class presents. A quick outline: * Problems * User facing issues * Problem space complexity * Maintenance and code correctness challenges * SKIP_WORKTREE expectations in Git * Suggested solution * Pros/Cons of suggested solution * Notes on testcase modifications === User facing issues === There are various ways for users to get files to be present in the working copy despite having the SKIP_WORKTREE bit set for that file in the index. This may come from: * various git commands not really supporting the SKIP_WORKTREE bit[1,2] * users grabbing files from elsewhere and writing them to the worktree (perhaps even cached in their editor) * users attempting to "abort" a sparse-checkout operation with a not-so-early Ctrl+C (updating $GIT_DIR/info/sparse-checkout and the working tree is not atomic)[3]. Once users have present-despite-SKIP_WORKTREE files, any modifications users make to these files will be ignored, possibly to users' confusion. Further: * these files will degrade performance for the sparse-index case due to requiring the index to be expanded (see commit 55dfcf9591 ("sparse-checkout: clear tracked sparse dirs", 2021-09-08) for why we try to delete entire directories outside the sparse cone). * these files will not be updated by by standard commands (switch/checkout/pull/merge/rebase will leave them alone unless conflicts happen -- and even then, the conflicted file may be written somewhere else to avoid overwriting the SKIP_WORKTREE file that is present and in the way) * there is nothing in Git that users can use to discover such files (status, diff, grep, etc. all ignore it) * there is no reasonable mechanism to "recover" from such a condition (neither `git sparse-checkout reapply` nor `git reset --hard` will correct it). So, not only are users modifications ignored, but the files get progressively more stale over time. At some point in the future, they may change their sparseness specification or disable sparse-checkouts. At that time, all present-despite-SKIP_WORKTREE files will show up as having lots of modifications because they represent a version from a different branch or commit. These might include user-made local changes from days before, but the only way to tell is to have users look through them all closely. If these users come to others for help, there will be no logs that explain the issue; it's just a mysterious list of changes. Users might adamantly claim (correctly, as it turns out) that they didn't modify these files, while others presume they did. [1] https://lore.kernel.org/git/xmqqbmb1a7ga.fsf@gitster-ct.c.googlers.com/ [2] https://lore.kernel.org/git/CABPp-BH9tju7WVm=QZDOvaMDdZbpNXrVWQdN-jmfN8wC6YVhmw@mail.gmail.com/ [3] https://lore.kernel.org/git/CABPp-BFnFpzwGC11TLoLs8YK5yiisA5D5-fFjXnJsbESVDwZsA@mail.gmail.com/ === Problem space complexity === SKIP_WORKTREE has been part of Git for over a decade. Duy did lots of work on it initially, and several others have since come along and put lots of work into it. Stolee spent most of 2021 on the sparse-index, with lots of bugfixes along the way including to non-sparse-index cases as we are still trying to get sparse checkouts to behave reasonably. Basically every codepath throughout the treat needs to be aware of an additional type of file: tracked-but-not-present. The extra type results in lots of extra testcases and lots of extra code everywhere. But, the sad thing is that we actually have more than one extra type. We have tracked, tracked-but-not-present (SKIP_WORKTREE), and tracked-but-promised-to-not-be-present-but-is-present-anyway (present-despite-SKIP_WORKTREE). Two types is a monumental amount of effort to support, and adding a third feels a bit like insanity[4]. [4] Some examples of which can be seen at https://lore.kernel.org/git/CABPp-BGJ_Nvi5TmgriD9Bh6eNXE2EDq2f8e8QKXAeYG3BxZafA@mail.gmail.com/ === Maintenance and code correctness challenges === Matheus' patches to grep stalled for nearly a year, in part because of complications of how to handle sparse-checkouts appropriately in all cases[5][6] (with trying to sanely figure out how to sanely handle present-despite-SKIP_WORKTREE files being one of the complications). His rm/add follow-ups also took months because of those kinds of issues[7]. The corner cases with things like submodules and SKIP_WORKTREE with the addition of present-despite-SKIP_WORKTREE start becoming really complex[8]. We've had to add ugly logic to merge-ort to attempt to handle present-despite-SKIP_WORKTREE files[9], and basically just been forced to give up in merge-recursive knowing full well that we'll sometimes silently discard user modifications. Despite stash essentially being a merge, it needed extra code (beyond what was in merge-ort and merge-recursive) to manually tweak SKIP_WORKTREE bits in order to avoid a few different bugs that'd result in an early abort with a partial stash application[10]. [5] See https://lore.kernel.org/git/5f3f7ac77039d41d1692ceae4b0c5df3bb45b74a.1612901326.git.matheus.bernardino@usp.br/#t and the dates on the thread; also Matheus and I had several conversations off-list trying to resolve the issues over that time [6] ...it finally kind of got unstuck after https://lore.kernel.org/git/CABPp-BGJ_Nvi5TmgriD9Bh6eNXE2EDq2f8e8QKXAeYG3BxZafA@mail.gmail.com/ [7] See for example https://lore.kernel.org/git/CABPp-BHwNoVnooqDFPAsZxBT9aR5Dwk5D9sDRCvYSb8akxAJgA@mail.gmail.com/#t and quotes like "The core functionality of sparse-checkout has always been only partially implemented", a statement I still believe is true today. [8] https://lore.kernel.org/git/pull.809.git.git.1592356884310.gitgitgadget@gmail.com/ [9] See commit 66b209b86a ("merge-ort: implement CE_SKIP_WORKTREE handling with conflicted entries", 2021-03-20) [10] See commit ba359fd507 ("stash: fix stash application in sparse-checkouts", 2020-12-01) === SKIP_WORKTREE expectations in Git === A couple quotes: * From [11] (before the "sparse-checkout" command existed): If it needs too many special cases, hacks, and conditionals, then it is not worth the complexity---if it is easier to write a correct code by allowing Git to populate working tree files, it is perfectly fine to do so. In a sense, the sparse checkout "feature" itself is a hack by itself, and that is why I think this part should be "best effort" as well. * From the git-sparse-checkout manual (still present today): THIS COMMAND IS EXPERIMENTAL. ITS BEHAVIOR, AND THE BEHAVIOR OF OTHER COMMANDS IN THE PRESENCE OF SPARSE-CHECKOUTS, WILL LIKELY CHANGE IN THE FUTURE. [11] https://lore.kernel.org/git/xmqqbmb1a7ga.fsf@gitster-ct.c.googlers.com/ === Suggested solution === SKIP_WORKTREE was written to allow sparse-checkouts, in particular, as the name of the option implies, to allow the file to NOT be in the worktree but consider it to be unchanged rather than deleted. The suggests a simple solution: present-despite-SKIP_WORKTREE files should not exist, for those using sparse-checkouts. Enforce this at index loading time by checking if core.sparseCheckout is true; if so, check files in the index with the SKIP_WORKTREE bit set to verify that they are absent from the working tree. If they are present, unset the bit (in memory, though any commands that write to the index will record the update). Users can, of course, can get the SKIP_WORKTREE bit back such as by running `git sparse-checkout reapply` (if they have ensured the file is unmodified and doesn't match the specified sparsity patterns). === Pros/Cons of suggested solution === Pros: * Solves the user visible problems reported above, which I've been complaining about for nearly a year but couldn't find a solution to. * Helps prevent slow performance degradation with a sparse-index. * Much easier behavior in sparse-checkouts for users to reason about * Very simple, ~30 lines of code. * Significantly simplifies some ugly testcases, and obviates the need to test an entire class of potential issues. * Reduces code complexity, reasoning, and maintenance. Avoids disagreements about weird corner cases[12]. * It has been reported that some users might be (ab)using SKIP_WORKTREE as a let-me-modify-but-keep-the-file-in-the-worktree mechanism[13, and a few other similar references]. These users know of multiple caveats and shortcomings in doing so; perhaps not surprising given the "SKIP_WORKTREE expecations" section above. However, these users use `git update-index --skip-worktree`, and not `git sparse-checkout` or core.sparseCheckout=true. As such, these users would be unaffected by this change and can continue abusing the system as before. [12] https://lore.kernel.org/git/CABPp-BH9tju7WVm=QZDOvaMDdZbpNXrVWQdN-jmfN8wC6YVhmw@mail.gmail.com/ [13] https://stackoverflow.com/questions/13630849/git-difference-between-assume-unchanged-and-skip-worktree Cons: * When core.sparseCheckout is enabled, this adds a performance cost to reading the index. I'll defer discussion of this cost to a subsequent patch, since I have some optimizations to add. === Notes on testcase modifications === The good: * t1011: Compare to two cases above it ('read-tree will not throw away dirty changes, non-sparse'); since the file is present, it should match the non-sparse case now * t1092: sparse-index & sparse-checkout now match full-worktree behavior in more cases! Yaay for consistency! * t6428, t7012: look at how much simpler the tests become! Merge and stash can just fail early telling the user there's a file in the way, instead of not noticing until it's about to write a file and then have to implement sudden crash avoidance. Hurray for sanity! * t7817: sparse behavior better matches full tree behavior. Hurray for sanity! The confusing: * t3705: These changes were ONLY needed on Windows, but they don't hurt other platforms. Let's discuss each individually: * core.sparseCheckout should be false by default. Nothing in this testcase toggles that until many, many tests later. However, early tests (#5 in particular) were testing `update-index --skip-worktree` behavior in a non-sparse-checkout, but the Windows tests in CI were behaving as if core.sparseCheckout=true had been specified somewhere. I do not have access to a Windows machine. But I just manually did what should have been a no-op and turned the config off. And it fixed the test. * I have no idea why the leftover .gitattributes file from this test was causing failures for test #18 on Windows, but only with these changes of mine. Test #18 was checking for empty stderr, and specifically wanted to know that some error completely unrelated to file endings did not appear. The leftover .gitattributes file thus caused some spurious stderr unrelated to the thing being checked. Since other tests did not intend to test normalization, just proactively remove the .gitattributes file. I'm certain this is cleaner and better, I'm just unsure why/how this didn't trigger problems before. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-14unpack-trees: fix accidental loss of user changesLibravatar Elijah Newren2-2/+4
For sparse-checkouts, we don't want unpack-trees to error out on files that are missing from the worktree, so there has traditionally been logic to make it skip the verify_uptodate() check for these. Unfortunately, it was skipping the verify_uptodate() check for files that were expected to *become* SKIP_WORKTREE. For files that were not already SKIP_WORKTREE, that can cause us to later delete the file in apply_sparse_checkout(). Only skip the check for files that were already SKIP_WORKTREE as well to avoid lightly discarding important changes users may have made to files. Note 1: unpack-trees.c is already a bit complex, and the logic around CE_SKIP_WORKTREE and CE_NEW_SKIP_WORKTREE in that file are no exception. I also tried just replacing CE_NEW_SKIP_WORKTREE with CE_SKIP_WORKTREE in the verify_uptodate() check instead of checking for both flags, and found that it also fixed this bug and passed all the tests. I also attempted to devise a few testcases that might trip either variant of my fix and was unable to find any problems. It may be that just checking CE_SKIP_WORKTREE is a better fix, but I'm not sure. I thought it was a bit safer to strictly reduce the number of cases where we skip the up-to-date check rather than just toggling which kind of cases skip it, and thus went with the current variant of the fix. Note 2: I also wondered if verify_absent() might have a similar bug, but despite my attempts to try to devise a testcase that would trigger such a thing, I couldn't find any problematic testcases. Thus, this patch makes no attempt to apply similar changes to verify_absent() and verify_absent_if_directory(). Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-14t1011: add testcase demonstrating accidental loss of user modificationsLibravatar Elijah Newren1-0/+21
If a user has a file with local modifications that is not marked as SKIP_WORKTREE, but the sparsity patterns are such that it should be marked that way, and the user then invokes a command like * git checkout -q HEAD^ or * git read-tree -mu HEAD^ Then the file will be deleted along with all the users' modifications. Add a testcase demonstrating this problem. Note: This bug only triggers if something other than 'HEAD' is given; if the commands above had specified 'HEAD', then the users' file would be left alone. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-13Merge branch 'vd/sparse-clean-etc' into en/present-despite-skippedLibravatar Junio C Hamano8-17/+360
* vd/sparse-clean-etc: update-index: reduce scope of index expansion in do_reupdate update-index: integrate with sparse index update-index: add tests for sparse-checkout compatibility checkout-index: integrate with sparse index checkout-index: add --ignore-skip-worktree-bits option checkout-index: expand sparse checkout compatibility tests clean: integrate with sparse index reset: reorder wildcard pathspec conditions reset: fix validation in sparse index test
2022-01-13update-index: reduce scope of index expansion in do_reupdateLibravatar Victoria Dye2-4/+15
Replace unconditional index expansion in 'do_reupdate()' with one scoped to only where a full index is needed. A full index is only required in 'do_reupdate()' when a sparse directory in the index differs from HEAD; in that case, the index is expanded and the operation restarted. Because the index should only be expanded if a sparse directory is modified, add a test ensuring the index is not expanded when differences only exist within the sparse cone. Signed-off-by: Victoria Dye <vdye@github.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-13update-index: integrate with sparse indexLibravatar Victoria Dye3-3/+25
Enable use of the sparse index with `update-index`. Most variations of `update-index` work without explicitly expanding the index or making any other updates in or outside of `update-index.c`. The one usage requiring additional changes is `--cacheinfo`; if a file inside a sparse directory was specified, the index would not be expanded until after the cache tree is invalidated, leading to a mismatch between the index and cache tree. This scenario is handled by rearranging `add_index_entry_with_check`, allowing `index_name_stage_pos` to expand the index *before* attempting to invalidate the relevant cache tree path, avoiding cache tree/index corruption. Signed-off-by: Victoria Dye <vdye@github.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-13update-index: add tests for sparse-checkout compatibilityLibravatar Victoria Dye2-0/+168
Introduce tests for a variety of `git update-index` use cases, including performance scenarios. Tests are intended to exercise `update-index` with options that change the commands interaction with the index (e.g., `--again`) and with files/directories inside and outside a sparse checkout cone. Of note is that these tests clearly establish the behavior of `git update-index --add` with untracked, outside-of-cone files. Unlike `git add`, which fails with an error when provided with such files, `update-index` succeeds in adding them to the index. Additionally, the `skip-worktree` flag is *not* automatically added to the new entry. Although this is pre-existing behavior, there are a couple of reasons to avoid changing it in favor of consistency with e.g. `git add`: * `update-index` is low-level command for modifying the index; while it can perform operations similar to those of `add`, it traditionally has fewer "guardrails" preventing a user from doing something they may not want to do (in this case, adding an outside-of-cone, non-`skip-worktree` file to the index) * `update-index` typically only exits with an error code if it is incapable of performing an operation (e.g., if an internal function call fails); adding a new file outside the sparse checkout definition is still a valid operation, albeit an inadvisable one * `update-index` does not implicitly set flags (e.g., `skip-worktree`) when creating new index entries with `--add`; if flags need to be updated, options like `--[no-]skip-worktree` allow a user to intentionally set them All this to say that, while there are valid reasons to consider changing the treatment of outside-of-cone files in `update-index`, there are also sufficient reasons for leaving it as-is. Co-authored-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Victoria Dye <vdye@github.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-13checkout-index: integrate with sparse indexLibravatar Victoria Dye2-3/+36
Add repository settings to allow usage of the sparse index. When using the `--all` option, sparse directories are ignored by default due to the `skip-worktree` flag, so there is no need to expand the index. If `--ignore-skip-worktree-bits` is specified, the index is expanded in order to check out all files. When checking out individual files, existing behavior in a full index is to exit with an error if a directory is specified (as the directory name will not match an index entry). However, it is possible in a sparse index to match a directory name to a sparse directory index entry, but checking out that sparse directory still results in an error on checkout. To reduce some potential confusion for users, `checkout_file(...)` explicitly exits with an informative error if provided with a sparse directory name. The test corresponding to this scenario verifies the error message, which now differs between sparse index and non-sparse index checkouts. Signed-off-by: Victoria Dye <vdye@github.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-13checkout-index: add --ignore-skip-worktree-bits optionLibravatar Victoria Dye3-12/+38
Update `checkout-index` to no longer refresh files that have the `skip-worktree` bit set, exiting with an error if `skip-worktree` filenames are directly provided to `checkout-index`. The newly-added `--ignore-skip-worktree-bits` option provides a mechanism to replicate the old behavior, checking out *all* files specified (even those with `skip-worktree` enabled). The ability to toggle whether files should be checked-out based on `skip-worktree` already exists in `git checkout` and `git restore` (both of which have an `--ignore-skip-worktree-bits` option). The change to, by default, ignore `skip-worktree` files is especially helpful for sparse-checkout; it prevents inadvertent creation of files outside the sparse definition on disk and eliminates the need to expand a sparse index when using the `--all` option. Internal usage of `checkout-index` in `git stash` and `git filter-branch` do not make explicit use of files with `skip-worktree` enabled, so `--ignore-skip-worktree-bits` is not added to them. Helped-by: Elijah Newren <newren@gmail.com> Signed-off-by: Victoria Dye <vdye@github.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-13checkout-index: expand sparse checkout compatibility testsLibravatar Victoria Dye2-0/+55
Add tests to cover `checkout-index`, with a focus on cases interesting in a sparse checkout (e.g., files specified outside sparse checkout definition). New tests are intended to serve as a baseline for existing and/or expected behavior and performance when integrating `checkout-index` with the sparse index. Note that the test 'checkout-index --all' is marked as 'test_expect_failure', indicating that `update-index --all` will be modified in a subsequent patch to behave as the test expects. Signed-off-by: Victoria Dye <vdye@github.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-13clean: integrate with sparse indexLibravatar Victoria Dye2-0/+24
Remove full index requirement for `git clean` and test to ensure the index is not expanded in `git clean`. Add to existing test for `git clean` to verify cleanup of untracked files in sparse directories is consistent between sparse index and non-sparse index checkouts. Signed-off-by: Victoria Dye <vdye@github.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-13reset: reorder wildcard pathspec conditionsLibravatar Victoria Dye1-3/+9
Rearrange conditions in method determining whether index expansion is necessary when a pathspec is specified for `git reset`, placing less expensive condition first. Additionally, add details & examples to related code comments to help with readability. Helped-by: Elijah Newren <newren@gmail.com> Signed-off-by: Victoria Dye <vdye@github.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-13reset: fix validation in sparse index testLibravatar Victoria Dye1-4/+2
Update t1092 test 'reset with pathspecs outside sparse definition' to verify index contents. The use of `rev-parse` verifies the contents of HEAD, not the index, providing no real validation of the reset results. Conversely, `ls-files` reports the contents of the index (OIDs, flags, filenames), which are then compared across checkouts to ensure compatible index states. Fixes 741a2c9ffa (reset: expand test coverage for sparse checkouts, 2021-09-27). Signed-off-by: Victoria Dye <vdye@github.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-12Last minute fixes before -rc1Libravatar Junio C Hamano1-1/+10
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-12Merge branch 'ps/lockfile-cleanup-fix'Libravatar Junio C Hamano4-11/+33
Some lockfile code called free() in signal-death code path, which has been corrected. * ps/lockfile-cleanup-fix: fetch: fix deadlock when cleaning up lockfiles in async signals
2022-01-12Merge branch 'ma/header-dup-cleanup'Libravatar Junio C Hamano1-2/+0
Code clean-up. * ma/header-dup-cleanup: cache.h: drop duplicate `ensure_full_index()` declaration
2022-01-12Merge branch 'fs/gpg-unknown-key-test-fix'Libravatar Junio C Hamano1-20/+2
Test simplification. * fs/gpg-unknown-key-test-fix: t/gpg: simplify test for unknown key
2022-01-12Merge branch 'ak/protect-any-current-branch'Libravatar Junio C Hamano1-1/+1
* ak/protect-any-current-branch: branch: missing space fix at line 313
2022-01-12Merge branch 'jt/pack-header-lshift-overflow'Libravatar Junio C Hamano1-1/+1
* jt/pack-header-lshift-overflow: packfile: fix off-by-one error in decoding logic
2022-01-12Merge branch 'rb/nonstop-lacks-uncompress2'Libravatar Junio C Hamano1-0/+1
* rb/nonstop-lacks-uncompress2: build: NonStop ships with an older zlib
2022-01-12Merge branch 'ma/windows-dynload-fix'Libravatar Junio C Hamano5-10/+15
Fix calling dynamically loaded functions on Windows. * ma/windows-dynload-fix: lazyload: use correct calling conventions
2022-01-12Merge branch 'fs/ssh-signing-key-lifetime'Libravatar Junio C Hamano2-1/+9
"git merge $signed_tag" started to drop the tag message from the default merge message it uses by accident, which has been corrected. * fs/ssh-signing-key-lifetime: fmt-merge-msg: prevent use-after-free with signed tags
2022-01-12build: NonStop ships with an older zlibLibravatar Randall S. Becker1-0/+1
Notably, it lacks uncompress2(); use the fallback we ship in our tree instead. Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-12packfile: fix off-by-one error in decoding logicLibravatar Junio C Hamano1-1/+1
shift count being exactly at 7-bit smaller than the long is OK; on 32-bit architecture, shift count starts at 4 and goes through 11, 18 and 25, at which point the guard triggers one iteration too early. Reported-by: Marc Strapetz <marc.strapetz@syntevo.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-12t/gpg: simplify test for unknown keyLibravatar Fabian Stelzer1-20/+2
To test for a key that is completely unknown to the keyring we need one to sign the commit with. This was done by generating a new key and not add it into the keyring. To avoid the key generation overhead and problems where GPG did hang in CI during it, switch GNUPGHOME to the empty $GNUPGHOME_NOT_USED instead, therefore making all used keys unknown for this single `verify-commit` call. Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Fabian Stelzer <fs@gigacodes.de> Reviewed-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-12branch: missing space fix at line 313Libravatar Bagas Sanjaya1-1/+1
The message introduced by commit 593a2a5d06 (branch: protect branches checked out in all worktrees, 2021-12-01) is missing a space in the first line, add it. Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-10fmt-merge-msg: prevent use-after-free with signed tagsLibravatar Taylor Blau2-1/+9
When merging a signed tag, fmt_merge_msg_sigs() is responsible for populating the body of the merge message with the names of the signed tags, their signatures, and the validity of those signatures. In 02769437e1 (ssh signing: use sigc struct to pass payload, 2021-12-09), check_signature() was taught to pass the object payload via the sigc struct instead of passing the payload buffer separately. In effect, 02769437e1 causes buf, and sigc.payload to point at the same region in memory. This causes a problem for fmt_tag_signature(), which wants to read from this location, since it is freed beforehand by signature_check_clear() (which frees it via sigc's `payload` member). That makes the subsequent use in fmt_tag_signature() a use-after-free. As a result, merge messages did not contain the body of any signed tags. Luckily, they tend not to contain garbage, either, since the result of strstr()-ing the object buffer in fmt_tag_signature() is guarded: const char *tag_body = strstr(buf, "\n\n"); if (tag_body) { tag_body += 2; strbuf_add(tagbuf, tag_body, buf + len - tag_body); } Unfortunately, the tests in t6200 did not catch this at the time because they do not search for the body of signed tags in fmt-merge-msg's output. Resolve this by waiting to call signature_check_clear() until after its contents can be safely discarded. Harden ourselves against any future regressions in this area by making sure we can find signed tag messages in the output of fmt-merge-msg, too. Reported-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-10Git 2.35-rc0Libravatar Junio C Hamano2-1/+60
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-10Merge branch 'en/stash-df-fix'Libravatar Junio C Hamano2-4/+29
"git stash apply" forgot to attempt restoring untracked files when it failed to restore changes to tracked ones. * en/stash-df-fix: stash: do not return before restoring untracked files
2022-01-10Merge branch 'ms/t-readme-typofix'Libravatar Junio C Hamano1-1/+1
Typofix. * ms/t-readme-typofix: t/README: fix typo
2022-01-10Merge branch 'ja/i18n-similar-messages'Libravatar Junio C Hamano51-178/+189
Similar message templates have been consolidated so that translators need to work on fewer number of messages. * ja/i18n-similar-messages: i18n: turn even more messages into "cannot be used together" ones i18n: ref-filter: factorize "%(foo) atom used without %(bar) atom" i18n: factorize "--foo outside a repository" i18n: refactor "unrecognized %(foo) argument" strings i18n: factorize "no directory given for --foo" i18n: factorize "--foo requires --bar" and the like i18n: tag.c factorize i18n strings i18n: standardize "cannot open" and "cannot read" i18n: turn "options are incompatible" into "cannot be used together" i18n: refactor "%s, %s and %s are mutually exclusive" i18n: refactor "foo and bar are mutually exclusive"
2022-01-10Merge branch 'en/merge-ort-renorm-with-rename-delete-conflict-fix'Libravatar Junio C Hamano2-3/+42
A corner case bug in the ort merge strategy has been corrected. * en/merge-ort-renorm-with-rename-delete-conflict-fix: merge-ort: fix bug with renormalization and rename/delete conflicts
2022-01-10Merge branch 'jc/doc-submitting-patches-choice-of-base'Libravatar Junio C Hamano1-14/+39
Extend the guidance to choose the base commit to build your work on, and hint/nudge contributors to read others' changes. * jc/doc-submitting-patches-choice-of-base: SubmittingPatchs: clarify choice of base and testing
2022-01-10Merge branch 'jl/subtree-check-parents-argument-passing-fix'Libravatar Junio C Hamano1-4/+3
Fix performance-releated bug in "git subtree" (in contrib/). * jl/subtree-check-parents-argument-passing-fix: subtree: fix argument handling in check_parents
2022-01-10Merge branch 'lh/use-gnu-color-in-grep'Libravatar Junio C Hamano1-3/+3
The color palette used by "git grep" has been updated to match that of GNU grep. * lh/use-gnu-color-in-grep: grep: align default colors with GNU grep ones
2022-01-10Merge branch 'js/branch-track-inherit'Libravatar Junio C Hamano16-67/+312
"git -c branch.autosetupmerge=inherit branch new old" makes "new" to have the same upstream as the "old" branch, instead of marking "old" itself as its upstream. * js/branch-track-inherit: config: require lowercase for branch.*.autosetupmerge branch: add flags and config to inherit tracking branch: accept multiple upstream branches for tracking
2022-01-10Merge branch 'ab/usage-die-message'Libravatar Junio C Hamano9-34/+71
Code clean-up to hide vreportf() from public API. * ab/usage-die-message: config API: use get_error_routine(), not vreportf() usage.c + gc: add and use a die_message_errno() gc: return from cmd_gc(), don't call exit() usage.c API users: use die_message() for error() + exit 128 usage.c API users: use die_message() for "fatal :" + exit 128 usage.c: add a die_message() routine
2022-01-10Merge branch 'jz/apply-3-corner-cases'Libravatar Junio C Hamano2-1/+21
"git apply --3way" bypasses the attempt to do a three-way application in more cases to address the regression caused by the recent change to use direct application as a fallback. * jz/apply-3-corner-cases: git-apply: skip threeway in add / rename cases
2022-01-10Merge branch 'hn/reftable-fixes'Libravatar Junio C Hamano8-11/+101
Assorted fixlets in reftable code. * hn/reftable-fixes: reftable: support preset file mode for writing reftable: signal overflow reftable: fix typo in header
2022-01-10Merge branch 'ab/reflog-prep'Libravatar Junio C Hamano3-120/+150
Code refactoring in the reflog part of refs API. * ab/reflog-prep: reflog + refs-backend: move "verbose" out of the backend refs files-backend: assume cb->newlog if !EXPIRE_REFLOGS_DRY_RUN reflog: reduce scope of "struct rev_info" reflog expire: don't use lookup_commit_reference_gently() reflog expire: refactor & use "tip_commit" only for UE_NORMAL reflog expire: use "switch" over enum values reflog: change one->many worktree->refnames to use a string_list reflog expire: narrow scope of "cb" in cmd_reflog_expire() reflog delete: narrow scope of "cmd" passed to count_reflog_ent()
2022-01-10Merge branch 'ab/do-not-limit-stash-help-to-push'Libravatar Junio C Hamano2-0/+20
"git stash" by default triggers its "push" action, but its implementation also made "git stash -h" to show short help only for "git stash push", which has been corrected. * ab/do-not-limit-stash-help-to-push: stash: don't show "git stash push" usage on bad "git stash" usage
2022-01-10Merge branch 'ab/makefile-hook-list-dependency-fix'Libravatar Junio C Hamano1-2/+2
Fix dependency rules to generate hook-list.h header file. * ab/makefile-hook-list-dependency-fix: Makefile: correct the dependency graph of hook-list.h
2022-01-10Merge branch 'ab/makefile-pager-env-is-used-only-by-pager.c'Libravatar Junio C Hamano1-5/+6
* ab/makefile-pager-env-is-used-only-by-pager.c: Makefile: move -DPAGER_ENV from BASIC_CFLAGS to EXTRA_CPPFLAGS
2022-01-10Merge branch 'ab/makefile-msgfmt-wo-stats'Libravatar Junio C Hamano1-1/+1
Make the recipe that runs msgfmt less noisy. * ab/makefile-msgfmt-wo-stats: Makefile: don't invoke msgfmt with --statistics
2022-01-10Merge branch 'hn/refs-debug-update'Libravatar Junio C Hamano6-20/+19
Debugging support for refs API. * hn/refs-debug-update: refs: centralize initialization of the base ref_store. refs: print error message in debug output refs: pass gitdir to packed_ref_store_create
2022-01-10Merge branch 'ds/fetch-pull-with-sparse-index'Libravatar Junio C Hamano8-82/+168
"git fetch" and "git pull" are now declared sparse-index clean. Also "git ls-files" learns the "--sparse" option to help debugging. * ds/fetch-pull-with-sparse-index: test-read-cache: remove --table, --expand options t1091/t3705: remove 'test-tool read-cache --table' t1092: replace 'read-cache --table' with 'ls-files --sparse' ls-files: add --sparse option fetch/pull: use the sparse index
2022-01-10Merge branch 'hn/ref-api-tests-update'Libravatar Junio C Hamano3-11/+24
Test updates. * hn/ref-api-tests-update: t7004: use "test-tool ref-store" for reflog inspection t7004: create separate tags for different tests t5550: require REFFILES t5540: require REFFILES
2022-01-10Merge branch 'jh/p4-remove-unused'Libravatar Junio C Hamano1-76/+0
Remove a few commands from "git p4" that aren't very useful. * jh/p4-remove-unused: git-p4: remove "rollback" verb git-p4: remove "debug" verb
2022-01-10Merge branch 'ja/perf-use-specified-shell'Libravatar Junio C Hamano1-1/+1
Perf tests were run with end-user's shell, but it has been corrected to use the shell specified by $TEST_SHELL_PATH. * ja/perf-use-specified-shell: t/perf: do not run tests in user's $SHELL
2022-01-10Merge branch 'hn/test-ref-store-show-hash-algo'Libravatar Junio C Hamano1-4/+5
Debugging support for refs API. * hn/test-ref-store-show-hash-algo: test-ref-store: print hash algorithm