summaryrefslogtreecommitdiff
path: root/builtin/stash.c
AgeCommit message (Collapse)AuthorFilesLines
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>
2019-10-11Merge branch 'js/stash-apply-in-secondary-worktree'Libravatar Junio C Hamano1-0/+4
"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 subdirectoryLibravatar Johannes Schindelin1-0/+4
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 cacheLibravatar Thomas Gummerer1-4/+7
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 NLibravatar Elijah Newren1-0/+2
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'Libravatar Junio C Hamano1-23/+9
"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-indexLibravatar Thomas Gummerer1-23/+9
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 indexLibravatar Thomas Gummerer1-4/+5
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'Libravatar Junio C Hamano1-1/+1
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'Libravatar Junio C Hamano1-0/+4
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'Libravatar Junio C Hamano1-31/+40
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'Libravatar Junio C Hamano1-4/+4
Code clean-up. * tb/stash-in-c-unused-param-fix: stash: drop unused parameter
2019-04-22Merge branch 'ps/stash-in-c'Libravatar Junio C Hamano1-0/+1635
"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 necessaryLibravatar Thomas Gummerer1-0/+4
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 pointerLibravatar Thomas Gummerer1-30/+38
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 parameterLibravatar Thomas Gummerer1-4/+4
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 againLibravatar Johannes Schindelin1-2/+3
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 offLibravatar Johannes Schindelin1-1/+4
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 againLibravatar Johannes Schindelin1-0/+35
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`Libravatar Paul-Sebastian Ungureanu1-0/+1595
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>