summaryrefslogtreecommitdiff
path: root/builtin/stash.c
AgeCommit message (Collapse)AuthorFilesLines
2021-06-10Merge branch 'ah/stash-usage-i18n-fix'Libravatar Junio C Hamano1-2/+2
i18n update. * ah/stash-usage-i18n-fix: stash: don't translate literal commands
2021-05-22Merge branch 'dl/stash-show-untracked-fixup'Libravatar Junio C Hamano1-4/+1
Another brown paper bag inconsistency fix for a new feature introduced during this cycle. * dl/stash-show-untracked-fixup: stash show: use stash.showIncludeUntracked even when diff options given
2021-05-22stash show: use stash.showIncludeUntracked even when diff options givenLibravatar Denton Liu1-4/+1
If options pertaining to how the diff is displayed is provided to `git stash show`, the command will ignore the stash.showIncludeUntracked configuration variable, defaulting to not showing any untracked files. This is unintuitive behaviour since the format of the diff output and whether or not to display untracked files are orthogonal. Use stash.showIncludeUntracked even when diff options are given. Of course, this is still overridable via the command-line options. Update the documentation to explicitly say which configuration variables will be overridden when a diff options are given. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-17stash: don't translate literal commandsLibravatar Alex Henrie1-2/+2
Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-16Merge branch 'dl/stash-show-untracked-fixup'Libravatar Junio C Hamano1-2/+6
The code to handle options recently added to "git stash show" around untracked part of the stash segfaulted when these options were used on a stash entry that does not record untracked part. * dl/stash-show-untracked-fixup: stash show: fix segfault with --{include,only}-untracked t3905: correct test title
2021-05-13stash show: fix segfault with --{include,only}-untrackedLibravatar Denton Liu1-2/+6
When `git stash show --include-untracked` or `git stash show --only-untracked` is run on a stash that doesn't include an untracked entry, a segfault occurs. This happens because we do not check whether the untracked entry is actually present and just attempt to blindly dereference it. Ensure that the untracked entry is present before actually attempting to dereference it. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-04-30Merge branch 'ds/sparse-index-protections'Libravatar Junio C Hamano1-0/+2
Builds on top of the sparse-index infrastructure to mark operations that are not ready to mark with the sparse index, causing them to fall back on fully-populated index that they always have worked with. * ds/sparse-index-protections: (47 commits) name-hash: use expand_to_path() sparse-index: expand_to_path() name-hash: don't add directories to name_hash revision: ensure full index resolve-undo: ensure full index read-cache: ensure full index pathspec: ensure full index merge-recursive: ensure full index entry: ensure full index dir: ensure full index update-index: ensure full index stash: ensure full index rm: ensure full index merge-index: ensure full index ls-files: ensure full index grep: ensure full index fsck: ensure full index difftool: ensure full index commit: ensure full index checkout: ensure full index ...
2021-04-14stash: ensure full indexLibravatar Derrick Stolee1-0/+2
Before iterating over all cache entries, ensure that a sparse index is expanded to a full index to avoid unexpected behavior. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-04-02Merge branch 'mt/parallel-checkout-part-1'Libravatar Junio C Hamano1-0/+2
Preparatory API changes for parallel checkout. * mt/parallel-checkout-part-1: entry: add checkout_entry_ca() taking preloaded conv_attrs entry: move conv_attrs lookup up to checkout_entry() entry: extract update_ce_after_write() from write_entry() entry: make fstat_output() and read_blob_entry() public entry: extract a header file for entry.c functions convert: add classification for conv_attrs struct convert: add get_stream_filter_ca() variant convert: add [async_]convert_to_working_tree_ca() variants convert: make convert_attrs() and convert structs public
2021-03-23entry: extract a header file for entry.c functionsLibravatar Matheus Tavares1-0/+1
The declarations of entry.c's public functions and structures currently reside in cache.h. Although not many, they contribute to the size of cache.h and, when changed, cause the unnecessary recompilation of modules that don't really use these functions. So let's move them to a new entry.h header. While at it let's also move a comment related to checkout_entry() from entry.c to entry.h as it's more useful to describe the function there. Original-patch-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-22Merge branch 'dl/stash-show-untracked'Libravatar Junio C Hamano1-1/+61
"git stash show" learned to optionally show untracked part of the stash. * dl/stash-show-untracked: stash show: learn stash.showIncludeUntracked stash show: teach --include-untracked and --only-untracked
2021-03-05stash show: learn stash.showIncludeUntrackedLibravatar Denton Liu1-0/+8
The previous commit teaches `git stash show --include-untracked`. It may be desirable for a user to be able to always enable the --include-untracked behavior. Teach the stash.showIncludeUntracked config option which allows users to do this in a similar manner to stash.showPatch. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-05stash show: teach --include-untracked and --only-untrackedLibravatar Denton Liu1-1/+53
Stash entries can be made with untracked files via `git stash push --include-untracked`. However, because the untracked files are stored in the third parent of the stash entry and not the stash entry itself, running `git stash show` does not include the untracked files as part of the diff. With --include-untracked, untracked paths, which are recorded in the third-parent if it exists, are shown in addition to the paths that have modifications between the stash base and the working tree in the stash. It is possible to manually craft a malformed stash entry where duplicate untracked files in the stash entry will mask tracked files. We detect and error out in that case via a custom unpack_trees() callback: stash_worktree_untracked_merge(). Also, teach stash the --only-untracked option which only shows the untracked files of a stash entry. This is similar to `git show stash^3` but it is nice to provide a convenient abstraction for it so that users do not have to think about the underlying implementation. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-02-25Merge branch 'js/params-vs-args'Libravatar Junio C Hamano1-1/+1
Messages update. * js/params-vs-args: replace "parameters" by "arguments" in error messages
2021-02-23replace "parameters" by "arguments" in error messagesLibravatar Johannes Sixt1-1/+1
When an error message informs the user about an incorrect command invocation, it should refer to "arguments", not "parameters". Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-02-11stash: declare ref_stash as an arrayLibravatar Denton Liu1-1/+1
Save sizeof(const char *) bytes by declaring ref_stash as an array instead of having a redundant pointer to an array. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-02-05Merge branch 'en/stash-apply-sparse-checkout' into maintLibravatar Junio C Hamano1-49/+116
"git stash" did not work well in a sparsely checked out working tree. * en/stash-apply-sparse-checkout: stash: fix stash application in sparse-checkouts stash: remove unnecessary process forking t7012: add a testcase demonstrating stash apply bugs in sparse checkouts
2021-01-15Merge branch 'en/stash-apply-sparse-checkout'Libravatar Junio C Hamano1-49/+116
"git stash" did not work well in a sparsely checked out working tree. * en/stash-apply-sparse-checkout: stash: fix stash application in sparse-checkouts stash: remove unnecessary process forking t7012: add a testcase demonstrating stash apply bugs in sparse checkouts
2020-12-01stash: fix stash application in sparse-checkoutsLibravatar Elijah Newren1-2/+48
sparse-checkouts are built on the patterns in the $GIT_DIR/info/sparse-checkout file, where commands have modified behavior for paths that do not match those patterns. The differences in behavior, as far as the bugs concerned here, fall into three different categories (with git subcommands that fall into each category listed): * commands that only look at files matching the patterns: * status * diff * clean * update-index * commands that remove files from the working tree that do not match the patterns, and restore files that do match them: * read-tree * switch * checkout * reset (--hard) * commands that omit writing files to the working tree that do not match the patterns, unless those files are not clean: * merge * rebase * cherry-pick * revert There are some caveats above, e.g. a plain `git diff` ignores files outside the sparsity patterns but will show diffs for paths outside the sparsity patterns when revision arguments are passed. (Technically, diff is treating the sparse paths as matching HEAD.) So, there is some internal inconsistency among these commands. There are also additional commands that should behave differently in the face of sparse-checkouts, as the sparse-checkout documentation alludes to, but the above is sufficient for me to explain how `git stash` is affected. What is relevant here is that logically 'stash' should behave like a merge; it three-way merges the changes the user had in progress at stash creation time, the HEAD at the time the stash was created, and the current HEAD, in order to get the stashed changes applied to the current branch. However, this simplistic view doesn't quite work in practice, because stash tweaks it a bit due to two factors: (1) flags like --keep-index and --include-untracked (why we used two different verbs, 'keep' and 'include', is a rant for another day) modify what should be staged at the end and include more things that should be quasi-merged, (2) stash generally wants changes to NOT be staged. It only provides exceptions when (a) some of the changes had conflicts and thus we want to use stages to denote the clean merges and higher order stages to mark the conflicts, or (b) if there is a brand new file we don't want it to become untracked. stash has traditionally gotten this special behavior by first doing a merge, and then when it's clean, applying a pipeline of commands to modify the result. This series of commands for unstaging-non-newly-added-files came from the following commands: git diff-index --cached --name-only --diff-filter=A $CTREE >"$a" git read-tree --reset $CTREE git update-index --add --stdin <"$a" rm -f "$a" Looking back at the different types of special sparsity handling listed at the beginning of this message, you may note that we have at least one of each type covered here: merge, diff-index, and read-tree. The weird mix-and-match led to 3 different bugs: (1) If a path merged cleanly and it didn't match the sparsity patterns, the merge backend would know to avoid writing it to the working tree and keep the SKIP_WORKTREE bit, simply only updating it in the index. Unfortunately, the subsequent commands would essentially undo the changes in the index and thus simply toss the changes altogether since there was nothing left in the working tree. This means the stash is only partially applied. (2) If a path existed in the worktree before `git stash apply` despite having the SKIP_WORKTREE bit set, then the `git read-tree --reset` would print an error message of the form error: Entry 'modified' not uptodate. Cannot merge. and cause stash to abort early. (3) If there was a brand new file added by the stash, then the diff-index command would save that pathname to the temporary file, the read-tree --reset would remove it from the index, and the update-index command would barf due to no such file being present in the working copy; it would print a message of the form: error: NEWFILE: does not exist and --remove not passed fatal: Unable to process path NEWFILE and then cause stash to abort early. Basically, the whole idea of unstage-unless-brand-new requires special care when you are dealing with a sparse-checkout. Fix these problems by applying the following simple rule: When we unstage files, if they have the SKIP_WORKTREE bit set, clear that bit and write the file out to the working directory. (*) If there's already a file present in the way, rename it first. This fixes all three problems in t7012.13 and allows us to mark it as passing. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-12-01stash: remove unnecessary process forkingLibravatar Elijah Newren1-49/+70
When stash was converted from shell to a builtin, it merely transliterated the forking of various git commands from shell to a C program that would fork the same commands. Some of those were converted over to actual library calls, but much of the pipeline-of-commands design still remains. Fix some of this by replacing the portion corresponding to git diff-index --cached --name-only --diff-filter=A $CTREE >"$a" git read-tree --reset $CTREE git update-index --add --stdin <"$a" rm -f "$a" into a library function that does the same thing. (The read-tree --reset was already partially converted over to a library call, but as an independent piece.) Note here that this came after a merge operation was performed. The merge machinery always stages anything that cleanly merges, and the above code only runs if there are no conflicts. Its purpose is to make it so that when there are no conflicts, all the changes from the stash are unstaged. However, that causes brand new files from the stash to become untracked, so the code above first saves those files off and then re-adds them afterwards. We replace the whole series of commands with a simple function that will unstage files that are not newly added. This doesn't fix any bugs in the usage of these commands, it simply matches the existing behavior but makes it into a single atomic operation that we can then operate on as a whole. A subsequent commit will take advantage of this to fix issues with these commands in sparse-checkouts. This conversion incidentally fixes t3906.1, because the separate update-index process would die with the following error messages: error: uninitialized_sub: is a directory - add files inside instead fatal: Unable to process path uninitialized_sub The unstaging of the directory as a submodule meant it was no longer tracked, and thus as an uninitialized directory it could not be added back using `git update-index --add`, thus resulting in this error and early abort. Most of the submodule tests in 3906 continue to fail after this change, this change was just enough to push the first of those tests to success. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-30Merge branch 'km/stash-error-message-fix'Libravatar Junio C Hamano1-1/+1
Error message fix. * km/stash-error-message-fix: stash: add missing space to an error message
2020-11-24stash: add missing space to an error messageLibravatar Kyle Meyer1-1/+1
Restore a space that was lost in 8a0fc8d19d (stash: convert apply to builtin, 2019-02-25). Signed-off-by: Kyle Meyer <kyle@kyleam.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-01stash: simplify reflog emptiness checkLibravatar René Scharfe1-14/+13
Calling rev-parse to check if the drop subcommand removed the last stash and treating its failure as confirmation is fragile, as the command can fail for other reasons, e.g. because the system is out of memory. Directly check if the reflog is empty instead, which is more robust. Reported-by: Marek Mrva <mrva@eof-studios.com> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-09Merge branch 'jt/interpret-branch-name-fallback'Libravatar Junio C Hamano1-1/+1
"git status" has trouble showing where it came from by interpreting reflog entries that recordcertain events, e.g. "checkout @{u}", and gives a hard/fatal error. Even though it inherently is impossible to give a correct answer because the reflog entries lose some information (e.g. "@{u}" does not record what branch the user was on hence which branch 'the upstream' needs to be computed, and even if the record were available, the relationship between branches may have changed), at least hide the error to allow "status" show its output. * jt/interpret-branch-name-fallback: wt-status: tolerate dangling marks refs: move dwim_ref() to header file sha1-name: replace unsigned int with option struct
2020-09-02wt-status: tolerate dangling marksLibravatar Jonathan Tan1-1/+1
When a user checks out the upstream branch of HEAD, the upstream branch not being a local branch, and then runs "git status", like this: git clone $URL client cd client git checkout @{u} git status no status is printed, but instead an error message: fatal: HEAD does not point to a branch (This error message when running "git branch" persists even after checking out other things - it only stops after checking out a branch.) This is because "git status" reads the reflog when determining the "HEAD detached" message, and thus attempts to DWIM "@{u}", but that doesn't work because HEAD no longer points to a branch. Therefore, when calculating the status of a worktree, tolerate dangling marks. This is done by adding an additional parameter to dwim_ref() and repo_dwim_ref(). Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-08-18dir: fix problematic API to avoid memory leaksLibravatar Elijah Newren1-2/+2
The dir structure seemed to have a number of leaks and problems around it. First I noticed that parent_hashmap and recursive_hashmap were being leaked (though Peff noticed and submitted fixes before me). Then I noticed in the previous commit that clear_directory() was only taking responsibility for a subset of fields within dir_struct, despite the fact that entries[] and ignored[] we allocated internally to dir.c. That, of course, resulted in many callers either leaking or haphazardly trying to free these arrays and their contents. Digging further, I found that despite the pretty clear documentation near the top of dir.h that folks were supposed to call clear_directory() when the user no longer needed the dir_struct, there were four callers that didn't bother doing that at all. However, two of them clearly thought about leaks since they had an UNLEAK(dir) directive, which to me suggests that the method to free the data was too unclear. I suspect the non-obviousness of the API and its holes led folks to avoid it, which then snowballed into further problems with the entries[], ignored[], parent_hashmap, and recursive_hashmap problems. Rename clear_directory() to dir_clear() to be more in line with other data structures in git, and introduce a dir_init() to handle the suggested memsetting of dir_struct to all zeroes. I hope that a name like "dir_clear()" is more clear, and that the presence of dir_init() will provide a hint to those looking at the code that they need to look for either a dir_clear() or a dir_free() and lead them to find dir_clear(). Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-08-18dir: make clear_directory() free all relevant memoryLibravatar Elijah Newren1-3/+0
The calling convention for the dir API is supposed to end with a call to clear_directory() to free up no longer needed memory. However, clear_directory() didn't free dir->entries or dir->ignored. I believe this was an oversight, but a number of callers noticed memory leaks and started free'ing these. Unfortunately, they did so somewhat haphazardly (sometimes freeing the entries in the arrays, and sometimes only free'ing the arrays themselves). This suggests the callers weren't trying to make sure any possible memory used might be free'd, but just the memory they noticed their usecase definitely had allocated. Fix this mess by moving all the duplicated free'ing logic into clear_directory(). End by resetting dir to a pristine state so it could be reused if desired. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-07-30strvec: rename struct fieldsLibravatar Jeff King1-4/+4
The "argc" and "argv" names made sense when the struct was argv_array, but now they're just confusing. Let's rename them to "nr" (which we use for counts elsewhere) and "v" (which is rather terse, but reads well when combined with typical variable names like "args.v"). Note that we have to update all of the callers immediately. Playing tricks with the preprocessor is hard here, because we wouldn't want to rewrite unrelated tokens. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-07-28strvec: fix indentation in renamed callsLibravatar Jeff King1-19/+19
Code which split an argv_array call across multiple lines, like: argv_array_pushl(&args, "one argument", "another argument", "and more", NULL); was recently mechanically renamed to use strvec, which results in mis-matched indentation like: strvec_pushl(&args, "one argument", "another argument", "and more", NULL); Let's fix these up to align the arguments with the opening paren. I did this manually by sifting through the results of: git jump grep 'strvec_.*,$' and liberally applying my editor's auto-format. Most of the changes are of the form shown above, though I also normalized a few that had originally used a single-tab indentation (rather than our usual style of aligning with the open paren). I also rewrapped a couple of obvious cases (e.g., where previously too-long lines became short enough to fit on one), but I wasn't aggressive about it. In cases broken to three or more lines, the grouping of arguments is sometimes meaningful, and it wasn't worth my time or reviewer time to ponder each case individually. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-07-28strvec: convert builtin/ callers away from argv_array nameLibravatar Jeff King1-57/+57
We eventually want to drop the argv_array name and just use strvec consistently. There's no particular reason we have to do it all at once, or care about interactions between converted and unconverted bits. Because of our preprocessor compat layer, the names are interchangeable to the compiler (so even a definition and declaration using different names is OK). This patch converts all of the files in builtin/ to keep the diff to a manageable size. The conversion was done purely mechanically with: git ls-files '*.c' '*.h' | xargs perl -i -pe ' s/ARGV_ARRAY/STRVEC/g; s/argv_array/strvec/g; ' and then selectively staging files with "git add builtin/". We'll deal with any indentation/style fallouts separately. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-07-28strvec: rename files from argv-array to strvecLibravatar Jeff King1-1/+1
This requires updating #include lines across the code-base, but that's all fairly mechanical, and was done with: git ls-files '*.c' '*.h' | xargs perl -i -pe 's/argv-array.h/strvec.h/' Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-29Merge branch 'en/fill-directory-exponential'Libravatar Junio C Hamano1-12/+5
The directory traversal code had redundant recursive calls which made its performance characteristics exponential with respect to the depth of the tree, which was corrected. * en/fill-directory-exponential: completion: fix 'git add' on paths under an untracked directory Fix error-prone fill_directory() API; make it only return matches dir: replace double pathspec matching with single in treat_directory() dir: include DIR_KEEP_UNTRACKED_CONTENTS handling in treat_directory() dir: replace exponential algorithm with a linear one dir: refactor treat_directory to clarify control flow dir: fix confusion based on variable tense dir: fix broken comment dir: consolidate treat_path() and treat_one_path() dir: fix simple typo in comment t3000: add more testcases testing a variety of ls-files issues t7063: more thorough status checking
2020-04-28Merge branch 'js/stash-p-fix'Libravatar Junio C Hamano1-1/+1
Allowing the user to split a patch hunk while "git stash -p" does not work well; a band-aid has been added to make this (partially) work better. * js/stash-p-fix: stash -p: (partially) fix bug concerning split hunks t3904: fix incorrect demonstration of a bug
2020-04-08stash -p: (partially) fix bug concerning split hunksLibravatar Johannes Schindelin1-1/+1
When trying to stash part of the worktree changes by splitting a hunk and then only partially accepting the split bits and pieces, the user is presented with a rather cryptic error: error: patch failed: <file>:<line> error: test: patch does not apply Cannot remove worktree changes and the command would fail to stash the desired parts of the worktree changes (even if the `stash` ref was actually updated correctly). We even have a test case demonstrating that failure, carrying it for four years already. The explanation: when splitting a hunk, the changed lines are no longer separated by more than 3 lines (which is the amount of context lines Git's diffs use by default), but less than that. So when staging only part of the diff hunk for stashing, the resulting diff that we want to apply to the worktree in reverse will contain those changes to be dropped surrounded by three context lines, but since the diff is relative to HEAD rather than to the worktree, these context lines will not match. Example time. Let's assume that the file README contains these lines: We the people and the worktree added some lines so that it contains these lines instead: We are the kind people and the user tries to stash the line containing "are", then the command will internally stage this line to a temporary index file and try to revert the diff between HEAD and that index file. The diff hunk that `git stash` tries to revert will look somewhat like this: @@ -1776,3 +1776,4 We +are the people It is obvious, now, that the trailing context lines overlap with the part of the original diff hunk that the user did *not* want to stash. Keeping in mind that context lines in diffs serve the primary purpose of finding the exact location when the diff does not apply precisely (but when the exact line number in the file to be patched differs from the line number indicated in the diff), we work around this by reducing the amount of context lines: the diff was just generated. Note: this is not a *full* fix for the issue. Just as demonstrated in t3701's 'add -p works with pathological context lines' test case, there are ambiguities in the diff format. It is very rare in practice, of course, to encounter such repeated lines. The full solution for such cases would be to replace the approach of generating a diff from the stash and then applying it in reverse by emulating `git revert` (i.e. doing a 3-way merge). However, in `git stash -p` it would not apply to `HEAD` but instead to the worktree, which makes this non-trivial to implement as long as we also maintain a scripted version of `add -i`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-01Fix error-prone fill_directory() API; make it only return matchesLibravatar Elijah Newren1-12/+5
Traditionally, the expected calling convention for the dir.c API was: fill_directory(&dir, ..., pathspec) foreach entry in dir->entries: if (dir_path_match(entry, pathspec)) process_or_display(entry) This may have made sense once upon a time, because the fill_directory() call could use cheap checks to avoid doing full pathspec matching, and an external caller may have wanted to do other post-processing of the results anyway. However: * this structure makes it easy for users of the API to get it wrong * this structure actually makes it harder to understand fill_directory() and the functions it uses internally. It has tripped me up several times while trying to fix bugs and restructure things. * relying on post-filtering was already found to produce wrong results; pathspec matching had to be added internally for multiple cases in order to get the right results (see commits 404ebceda01c (dir: also check directories for matching pathspecs, 2019-09-17) and 89a1f4aaf765 (dir: if our pathspec might match files under a dir, recurse into it, 2019-09-17)) * it's bad for performance: fill_directory() already has to do lots of checks and knows the subset of cases where it still needs to do more checks. Forcing external callers to do full pathspec matching means they must re-check _every_ path. So, add the pathspec matching within the fill_directory() internals, and remove it from external callers. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-03-26Merge branch 'tg/retire-scripted-stash'Libravatar Junio C Hamano1-39/+11
"git stash" has kept an escape hatch to use the scripted version for a few releases, which got stale. It has been removed. * tg/retire-scripted-stash: stash: remove the stash.useBuiltin setting stash: get git_stash_config at the top level
2020-03-09Merge branch 'am/pathspec-f-f-more'Libravatar Junio C Hamano1-38/+41
"git rm" and "git stash" learns the new "--pathspec-from-file" option. * am/pathspec-f-f-more: stash push: support the --pathspec-from-file option stash: eliminate crude option parsing doc: stash: synchronize <pathspec> description doc: stash: document more options doc: stash: split options from description (2) doc: stash: split options from description (1) rm: support the --pathspec-from-file option doc: rm: synchronize <pathspec> description
2020-03-05stash: remove the stash.useBuiltin settingLibravatar Thomas Gummerer1-37/+10
Remove the stash.useBuiltin setting which was added as an escape hatch to disable the builtin version of stash first released with Git 2.22. Carrying the legacy version is a maintenance burden, and has in fact become out of date failing a test since the 2.23 release, without anyone noticing until now. So users would be getting a hint to fall back to a potentially buggy version of the tool. We used to shell out to git config to get the useBuiltin configuration to avoid changing any global state before spawning legacy-stash. However that is no longer necessary, so just use the 'git_config' function to get the setting instead. Similar to what we've done in d03ebd411c ("rebase: remove the rebase.useBuiltin setting", 2019-03-18), where we remove the corresponding setting for rebase, we leave the documentation in place, so people can refer back to it when searching for it online, and so we can refer to it in the commit message. Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-03-03stash: get git_stash_config at the top levelLibravatar Thomas Gummerer1-3/+2
In the next commit we're adding another config variable to be read from 'git_stash_config', that is valid for the top level command instead of just a subset. Move the 'git_config' invocation for 'git_stash_config' to the top-level to prepare for that. Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-19stash push: support the --pathspec-from-file optionLibravatar Alexandr Miloslavskiy1-0/+20
Decisions taken for simplicity: 1) For now, `--pathspec-from-file` is declared incompatible with `--patch`, even when <file> is not `-`. Such use case is not really expected. 2) It is not allowed to pass pathspec in both args and file. Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-19stash: eliminate crude option parsingLibravatar Alexandr Miloslavskiy1-38/+21
Eliminate crude option parsing and rely on real parsing instead, because 1) Crude parsing is crude, for example it's not capable of handling things like `git stash -m Message` 2) Adding options in two places is inconvenient and prone to bugs As a side result, the case of `git stash -m Message` gets fixed. Also give a good error message instead of just throwing usage at user. ---- Some review of what's been happening to this code: Before [1], `git-stash.sh` only verified that all args begin with `-` : # The default command is "push" if nothing but options are given seen_non_option= for opt do case "$opt" in --) break ;; -*) ;; *) seen_non_option=t; break ;; esac done Later, [1] introduced the duplicate code I'm now removing, also making the previous test more strict by white-listing options. ---- [1] Commit 40af1468 ("stash: convert `stash--helper.c` into `stash.c`" 2019-02-26) Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-05Merge branch 'js/patch-mode-in-others-in-c'Libravatar Junio C Hamano1-11/+14
The effort to move "git-add--interactive" to C continues. * js/patch-mode-in-others-in-c: commit --interactive: make it work with the built-in `add -i` built-in add -p: implement the "worktree" patch modes built-in add -p: implement the "checkout" patch modes built-in stash: use the built-in `git add -p` if so configured legacy stash -p: respect the add.interactive.usebuiltin setting built-in add -p: implement the "stash" and "reset" patch modes built-in add -p: prepare for patch modes other than "stage"
2019-12-21built-in stash: use the built-in `git add -p` if so configuredLibravatar Johannes Schindelin1-11/+14
The scripted version of `git stash` called directly into the Perl script `git-add--interactive.perl`, and this was faithfully converted to C. However, we have a much better way to do this now: call the internal API directly, which will now incidentally also respect the `add.interactive.useBuiltin` setting. Let's just do this. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-01Merge branch 'tg/stash-refresh-index'Libravatar Junio C Hamano1-4/+3
Recent update to "git stash pop" made the command empty the index when run with the "--quiet" option, which has been corrected. * tg/stash-refresh-index: stash: make sure we have a valid index before writing it
2019-11-14stash: make sure we have a valid index before writing itLibravatar Thomas Gummerer1-4/+3
In 'do_apply_stash()' we refresh the index in the end. Since 34933d0eff ("stash: make sure to write refreshed cache", 2019-09-11), we also write that refreshed index when --quiet is given to 'git stash apply'. However if '--index' is not given to 'git stash apply', we also discard the index in the else clause just before. We need to do so because we use an external 'git update-index --add --stdin', which leads to an out of date in-core index. Later we call 'refresh_and_write_cache', which now leads to writing the discarded index, which means we essentially write an empty index file. This is obviously not correct, or the behaviour the user wanted. We should not modify the users index without being asked to do so. Make sure to re-read the index after discarding the current in-core index, to avoid dealing with outdated information. Instead we could also drop the 'discard_cache()' + 'read_cache()', however that would make it easy to fall into the same trap as 34933d0eff did, so it's better to avoid that. We can also drop the 'refresh_and_write_cache' completely in the quiet case. Previously in legacy stash we relied on 'git status' to refresh the index after calling 'git read-tree' when '--index' was passed to 'git apply'. However the 'reset_tree()' call that replaced 'git read-tree' always passes options that are equivalent to '-m', making the refresh of the index unnecessary. Reported-by: Grzegorz Rajchman <rayman17@gmail.com> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-10Merge branch 'js/update-index-ignore-removal-for-skip-worktree'Libravatar Junio C Hamano1-2/+3
"git stash save" in a working tree that is sparsely checked out mistakenly removed paths that are outside the area of interest. * js/update-index-ignore-removal-for-skip-worktree: stash: handle staged changes in skip-worktree files correctly update-index: optionally leave skip-worktree entries alone
2019-11-02stash: handle staged changes in skip-worktree files correctlyLibravatar Johannes Schindelin1-2/+3
When calling `git stash` while changes were staged for files that are marked with the `skip-worktree` bit (e.g. files that are excluded in a sparse checkout), the files are recorded as _deleted_ instead. The reason is that `git stash` tries to construct the tree reflecting the worktree essentially by copying the index to a temporary one and then updating the files from the worktree. Crucially, it calls `git diff-index` to update also those files that are in the HEAD but have been unstaged in the index. However, when the temporary index is updated via `git update-index --add --remove`, skip-worktree entries mark the files as deleted by mistake. Let's use the newly-introduced `--ignore-skip-worktree-entries` option of `git update-index` to prevent exactly this from happening. Note that the regression test case deliberately avoids replicating the scenario described above and instead tries to recreate just the symptom. Reported by Dan Thompson. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-18Merge branch 'jj/stash-reset-only-toplevel'Libravatar Junio C Hamano1-1/+1
"git stash save" lost local changes to submodules, which has been corrected. * jj/stash-reset-only-toplevel: stash: avoid recursive hard reset on submodules
2019-10-15Merge branch 'en/merge-recursive-cleanup'Libravatar Junio C Hamano1-0/+2
The merge-recursive machiery is one of the most complex parts of the system that accumulated cruft over time. This large series cleans up the implementation quite a bit. * en/merge-recursive-cleanup: (26 commits) merge-recursive: fix the fix to the diff3 common ancestor label merge-recursive: fix the diff3 common ancestor label for virtual commits merge-recursive: alphabetize include list merge-recursive: add sanity checks for relevant merge_options merge-recursive: rename MERGE_RECURSIVE_* to MERGE_VARIANT_* merge-recursive: split internal fields into a separate struct merge-recursive: avoid losing output and leaking memory holding that output merge-recursive: comment and reorder the merge_options fields merge-recursive: consolidate unnecessary fields in merge_options merge-recursive: move some definitions around to clean up the header merge-recursive: rename merge_options argument to opt in header merge-recursive: rename 'mrtree' to 'result_tree', for clarity merge-recursive: use common name for ancestors/common/base_list merge-recursive: fix some overly long lines cache-tree: share code between functions writing an index as a tree merge-recursive: don't force external callers to do our logging merge-recursive: remove useless parameter in merge_trees() merge-recursive: exit early if index != head Ensure index matches head before invoking merge machinery, round N merge-recursive: remove another implicit dependency on the_repository ...
2019-10-15stash: avoid recursive hard reset on submodulesLibravatar Jakob Jarmar1-1/+1
git stash push does not recursively stash submodules, but if submodule.recurse is set, it may recursively reset --hard them. Having only the destructive action recurse is likely to be surprising behaviour, and unlikely to be desirable, so the easiest fix should be to ensure that the call to git reset --hard never recurses into submodules. This matches the behavior of check_changes_tracked_files, which ignores submodules. Signed-off-by: Jakob Jarmar <jakob@jarmar.se> Signed-off-by: Junio C Hamano <gitster@pobox.com>