summaryrefslogtreecommitdiff
path: root/builtin/stash.c
AgeCommit message (Collapse)Author
2020-04-29Merge branch 'en/fill-directory-exponential'Junio C Hamano
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'Junio C Hamano
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 hunksJohannes Schindelin
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 matchesElijah Newren
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-27Merge branch 'tg/retire-scripted-stash'Junio C Hamano
"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'Junio C Hamano
"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 settingThomas Gummerer
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 levelThomas Gummerer
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 optionAlexandr Miloslavskiy
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 parsingAlexandr Miloslavskiy
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'Junio C Hamano
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-22built-in stash: use the built-in `git add -p` if so configuredJohannes Schindelin
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'Junio C Hamano
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 itThomas Gummerer
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'Junio C Hamano
"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 correctlyJohannes Schindelin
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'Junio C Hamano
"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'Junio C Hamano
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 submodulesJakob Jarmar
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>
2019-10-11Merge branch 'js/stash-apply-in-secondary-worktree'Junio C Hamano
"git stash apply" in a subdirectory of a secondary worktree failed to access the worktree correctly, which has been corrected. * js/stash-apply-in-secondary-worktree: stash apply: report status correctly even in a worktree's subdirectory
2019-10-06stash apply: report status correctly even in a worktree's subdirectoryJohannes Schindelin
When Git wants to spawn a child Git process inside a worktree's subdirectory while `GIT_DIR` is set, we need to take care of specifying the work tree's top-level directory explicitly because it cannot be discovered: the current directory is _not_ the top-level directory of the work tree, and neither is it inside the parent directory of `GIT_DIR`. This fixes the problem where `git stash apply` would report pretty much everything deleted or untracked when run inside a worktree's subdirectory. To make sure that we do not introduce the "reverse problem", i.e. when `GIT_WORK_TREE` is defined but `GIT_DIR` is not, we simply make sure that both are set. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-20stash: make sure to write refreshed cacheThomas Gummerer
When converting stash into C, calls to 'git update-index --refresh' were replaced with the 'refresh_cache()' function. That is fine as long as the index is only needed in-core, and not re-read from disk. However in many cases we do actually need the refreshed index to be written to disk, for example 'merge_recursive_generic()' discards the in-core index before re-reading it from disk, and in the case of 'apply --quiet', the 'refresh_cache()' we currently have is pointless without writing the index to disk. Always write the index after refreshing it to ensure there are no regressions in this compared to the scripted stash. In the future we can consider avoiding the write where possible after making sure none of the subsequent calls actually need the refreshed cache, and it is not expected to be refreshed after stash exits or it is written somewhere else already. Reported-by: Jeff King <peff@peff.net> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-08-19Ensure index matches head before invoking merge machinery, round NElijah Newren
This is the bug that just won't die; there always seems to be another form of it somewhere. See the commit message of 55f39cf7551b ("merge: fix misleading pre-merge check documentation", 2018-06-30) for a more detailed explanation), but in short: <quick summary> builtin/merge.c contains this important requirement for merge strategies: ...the index must be in sync with the head commit. The strategies are responsible to ensure this. This condition is important to enforce because there are two likely failure cases when the index isn't in sync with the head commit: * we silently throw away changes the user had staged before the merge * we accidentally (and silently) include changes in the merge that were not part of either of the branches/trees being merged Discarding users' work and mis-merging are both bad outcomes, especially when done silently, so naturally this rule was stated sternly -- but, unfortunately totally ignored in practice unless and until actual bugs were found. But, fear not: the bugs from this were fixed in commit ee6566e8d70d ("[PATCH] Rewrite read-tree", 2005-09-05) through a rewrite of read-tree (again, commit 55f39cf7551b has a more detailed explanation of how this affected merge). And it was fixed again in commit 160252f81626 ("git-merge-ours: make sure our index matches HEAD", 2005-11-03) ...and it was fixed again in commit 3ec62ad9ffba ("merge-octopus: abort if index does not match HEAD", 2016-04-09) ...and again in commit 65170c07d466 ("merge-recursive: avoid incorporating uncommitted changes in a merge", 2017-12-21) ...and again in commit eddd1a411d93 ("merge-recursive: enforce rule that index matches head before merging", 2018-06-30) ...with multiple testcases added to the testsuite that could be enumerated in even more commits. Then, finally, in a patch in the same series as the last fix above, the documentation about this requirement was fixed in commit 55f39cf7551b ("merge: fix misleading pre-merge check documentation", 2018-06-30), and we all lived happily ever after... </quick summary> Unfortunately, "ever after" apparently denotes a limited time and it expired today. The merge-recursive rule to enforce that index matches head was at the beginning of merge_trees() and would only trigger when opt->call_depth was 0. Since merge_recursive() doesn't call merge_trees() until after returning from recursing, this meant that the check wasn't triggered by merge_recursive() until it had first finished all the intermediate merges to create virtual merge bases. That is a potentially HUGE amount of computation (and writing of intermediate merge results into the .git/objects directory) before it errors out and says, in effect, "Sorry, I can't do any merging because you have some local changes that would be overwritten." Trying to enforce that all of merge_trees(), merge_recursive(), and merge_recursive_generic() checked the index == head condition earlier resulted in a bunch of broken tests. It turns out that merge_recursive() has code to drop and reload the cache while recursing to create intermediate virtual merge bases, but unfortunately that code runs even when no recursion is necessary. This unconditional dropping and reloading of the cache masked a few bugs: * builtin/merge-recursive.c: didn't even bother loading the index. * builtin/stash.c: feels like a fake 'builtin' because it repeatedly invokes git subprocesses all over the place, mixed with other operations. In particular, invoking "git reset" will reset the index on disk, but the parent process that invoked it won't automatically have its in-memory index updated. * t3030-merge-recursive.h: this test has always been broken in that it didn't make sure to make index match head before running. But, it didn't care about the index or even the merge result, just the verbose output while running. While commit eddd1a411d93 ("merge-recursive: enforce rule that index matches head before merging", 2018-06-30) should have uncovered this broken test, it used a test_must_fail wrapper around the merge-recursive call because it was known that the merge resulted in a rename/rename conflict. Thus, that fix only made this test fail for a different reason, and since the index == head check didn't happen until after coming all the way back out of the recursion, the testcase had enough information to pass the one check that it did perform. So, load the index in builtin/merge-recursive.c, reload the in-memory index in builtin/stash.c, and modify the t3030 testcase to correctly setup the index and make sure that the test fails in the expected way (meaning it reports a rename/rename conflict). This makes sure that all callers actually make the index match head. The next commit will then enforce the condition that index matches head earlier so this problem doesn't return in the future. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-25Merge branch 'tg/stash-keep-index-with-removed-paths'Junio C Hamano
"git stash --keep-index" did not work correctly on paths that have been removed, which has been fixed. * tg/stash-keep-index-with-removed-paths: stash: fix handling removed files with --keep-index
2019-07-16stash: fix handling removed files with --keep-indexThomas Gummerer
git stash push --keep-index is supposed to keep all changes that have been added to the index, both in the index and on disk. Currently this doesn't behave correctly when a file is removed from the index. Instead of keeping it deleted on disk, --keep-index currently restores the file. Fix that behaviour by using 'git checkout' in no-overlay mode which can faithfully restore the index and working tree. This also simplifies the code. Note that this will overwrite untracked files if the untracked file has the same name as a file that has been deleted in the index. Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-06-19stash: fix show referencing stash indexThomas Gummerer
In the conversion of 'stash show' to C in dc7bd382b1 ("stash: convert show to builtin", 2019-02-25), 'git stash show <n>', where n is the index of a stash got broken, if n is not a file or a valid revision by itself. 'stash show' accepts any flag 'git diff' accepts for changing the output format. Internally we use 'setup_revisions()' to parse these command line flags. Currently we pass the whole argv through to 'setup_revisions()', which includes the stash index. As the stash index is not a valid revision or a file in the working tree in most cases however, this 'setup_revisions()' call (and thus the whole command) ends up failing if we use this form of 'git stash show'. Instead of passing the whole argv to 'setup_revisions()', only pass the flags (and the command name) through, while excluding the stash reference. The stash reference is parsed (and validated) in 'get_stash_info()' already. This separate parsing also means that we currently do produce the correct output if the command succeeds. Reported-by: Mike Hommey <mh@glandium.org> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-04-25Merge branch 'jk/unused-params-even-more'Junio C Hamano
Code cleanup. * jk/unused-params-even-more: parse_opt_ref_sorting: always use with NONEG flag pretty: drop unused strbuf from parse_padding_placeholder() pretty: drop unused "type" parameter in needs_rfc2047_encoding() parse-options: drop unused ctx parameter from show_gitcomp() fetch_pack(): drop unused parameters report_path_error(): drop unused prefix parameter unpack-trees: drop unused error_type parameters unpack-trees: drop name_entry from traverse_by_cache_tree() test-date: drop unused "now" parameter from parse_dates() update-index: drop unused prefix_length parameter from do_reupdate() log: drop unused "len" from show_tagger() log: drop unused rev_info from early output revision: drop some unused "revs" parameters
2019-04-22Merge branch 'tg/stash-in-c-show-default-to-p-fix'Junio C Hamano
A regression fix. * tg/stash-in-c-show-default-to-p-fix: stash: setup default diff output format if necessary
2019-04-22Merge branch 'js/stash-in-c-pathspec-fix'Junio C Hamano
Further fixes to "git stash" reimplemented in C. * js/stash-in-c-pathspec-fix: stash: pass pathspec as pointer built-in stash: handle :(glob) pathspecs again legacy stash: fix "rudimentary backport of -q"
2019-04-22Merge branch 'tb/stash-in-c-unused-param-fix'Junio C Hamano
Code clean-up. * tb/stash-in-c-unused-param-fix: stash: drop unused parameter
2019-04-22Merge branch 'ps/stash-in-c'Junio C Hamano
"git stash" rewritten in C. * ps/stash-in-c: (28 commits) tests: add a special setup where stash.useBuiltin is off stash: optionally use the scripted version again stash: add back the original, scripted `git stash` stash: convert `stash--helper.c` into `stash.c` stash: replace all `write-tree` child processes with API calls stash: optimize `get_untracked_files()` and `check_changes()` stash: convert save to builtin stash: make push -q quiet stash: convert push to builtin stash: convert create to builtin stash: convert store to builtin stash: convert show to builtin stash: convert list to builtin stash: convert pop to builtin stash: convert branch to builtin stash: convert drop and clear to builtin stash: convert apply to builtin stash: mention options in `show` synopsis stash: add tests for `git stash show` config stash: rename test cases to be more descriptive ...
2019-03-21stash: setup default diff output format if necessaryThomas Gummerer
In the scripted 'git stash show' when no arguments are passed, we just pass '--stat' to 'git diff'. When any argument is passed to 'stash show', we no longer pass '--stat' to 'git diff', and pass whatever flags are passed directly through to 'git diff'. By default 'git diff' shows the patch output. So when a user uses 'git stash show --patience', they would be shown the diff as expected, using the patience algorithm. '--patience' in this case only changes the diff algorithm, but does not cause 'git diff' to show the diff by itself. The diff is shown because that's the default behaviour of 'git diff'. In the C version of 'git stash show', we try to emulate that behaviour using the internal diff API. However we forgot to set up the default output format, in case it wasn't set by any of the flags that were passed through. So 'git stash show --patience' in the builtin version of stash would be completely silent, while it would show the diff in the scripted version. The same thing would happen for other flags that only affect the way a patch is displayed, rather than switching to a different output format than the default one. Fix this by setting up the default output format for 'git diff'. Reported-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-12stash: pass pathspec as pointerThomas Gummerer
Passing the pathspec by value is potentially confusing, as the copy is only a shallow copy, so save the overhead of the copy, and pass the pathspec struct as a pointer. In addition use copy_pathspec to copy the pathspec into rev.prune_data, so the copy is a proper deep copy, and owned by the revision API, as that's what the API expects. Reported-by: Jeff King <peff@peff.net> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-11stash: drop unused parameterThomas Gummerer
Drop the unused prefix parameter in do_drop_stash. We also have an unused "prefix" parameter in the 'create_stash' function, however we leave that in place for symmetry with the other top-level functions. Reported-by: Jeff King <peff@peff.net> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-08built-in stash: handle :(glob) pathspecs againJohannes Schindelin
When passing a list of pathspecs to, say, `git add`, we need to be careful to use the original form, not the parsed form of the pathspecs. This makes a difference e.g. when calling git stash -- ':(glob)**/*.txt' where the original form includes the `:(glob)` prefix while the parsed form does not. However, in the built-in `git stash`, we passed the parsed (i.e. incorrect) form, and `git add` would fail with the error message: fatal: pathspec '**/*.txt' did not match any files at the stage where `git stash` drops the changes from the worktree, even if `refs/stash` has been actually updated successfully. This fixes https://github.com/git-for-windows/git/issues/2037 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-07tests: add a special setup where stash.useBuiltin is offJohannes Schindelin
Add a GIT_TEST_STASH_USE_BUILTIN=false test mode which is equivalent to running with stash.useBuiltin=false. This is needed to spot that we're not introducing any regressions in the legacy stash version while we're carrying both it and the new built-in version. This imitates the equivalent treatment for the built-in rebase in 62c23938fae5 (tests: add a special setup where rebase.useBuiltin is off, 2018-11-14). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-07stash: optionally use the scripted version againJohannes Schindelin
We recently converted the `git stash` command from Unix shell scripts to builtins. Let's end users a way out when they discover a bug in the builtin command: `stash.useBuiltin`. As the file name `git-stash` is already in use, let's rename the scripted backend to `git-legacy-stash`. To make the test suite pass with `stash.useBuiltin=false`, this commit also backports rudimentary support for `-q` (but only *just* enough to appease the test suite), and adds a super-ugly hack to force exit code 129 for `git stash -h`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-07stash: convert `stash--helper.c` into `stash.c`Paul-Sebastian Ungureanu
The old shell script `git-stash.sh` was removed and replaced entirely by `builtin/stash.c`. In order to do that, `create` and `push` were adapted to work without `stash.sh`. For example, before this commit, `git stash create` called `git stash--helper create --message "$*"`. If it called `git stash--helper create "$@"`, then some of these changes wouldn't have been necessary. This commit also removes the word `helper` since now stash is called directly and not by a shell script. Signed-off-by: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>