kernel/git - git - PowerEL Git System

Commit Graph

Author	SHA1	Message	Date
Victoria Dye	4b8b0f6fa2	stash: make internal resets quiet and refresh index Add the options '-q' and '--refresh' to the 'git reset' executed in 'reset_head()', and '--refresh' to the 'git reset -q' executed in 'do_push_stash(...)'. 'stash' is implemented such that git commands invoked as part of it (e.g., 'clean', 'read-tree', 'reset', etc.) have their informational output silenced. However, the 'reset' in 'reset_head()' is not called with '-q', leading to the potential for a misleading printout from 'git stash apply --index' if the stash included a removed file: Unstaged changes after reset: D <deleted file> Not only is this confusing in its own right (since, after the reset, 'git stash' execution would stage the deletion in the index), it would be printed even when the stash was applied with the '-q' option. As a result, the messaging is removed entirely by calling 'git status' with '-q'. Additionally, because the default behavior of 'git reset -q' is to skip refreshing the index, but later operations in 'git stash' subcommands expect a non-stale index, enable '--refresh' as well. Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Elia Pinto	4fcea603c7	builtin/stash.c: delete duplicate include entry.h is included more than once. Signed-off-by: Elia Pinto <gitter.spiros@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
John Cai	758b4d2be8	stash: call reflog_delete() in reflog.c Now that cmd_reflog_delete has been libified an exported it into a new reflog.c library so we can call it directly from builtin/stash.c. This not only gives us a performance gain since we don't need to create a subprocess, but it also allows us to use the ref transactions api in the future. Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Glen Choo	ceaf037f61	stash: strip "refs/heads/" with skip_prefix When generating a message for a stash, "git stash" only records the part of the branch name to the right of the last "/". e.g. if HEAD is at "foo/bar/baz", "git stash" generates a message prefixed with "WIP on baz:" instead of "WIP on foo/bar/baz:". Fix this by using skip_prefix() to skip "refs/heads/" instead of looking for the last instance of "/". Reported-by: Kraymer <kraymer@gmail.com> Reported-by: Daniel Hahler <git@thequod.de> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Johannes Schindelin	e9b272e4c1	stash: stop warning about the obsolete `stash.useBuiltin` config setting In `8a2cd3f512` (stash: remove the stash.useBuiltin setting, 2020-03-03), we removed support for `stash.useBuiltin`, but left a warning in its place. After almost two years, and several major versions, it is time to remove even that warning. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Elijah Newren	ff5b7913f0	sequencer, stash: fix running from worktree subdir In commits `bc3ae46b42` ("rebase: do not attempt to remove startup_info->original_cwd", 2021-12-09) and `0fce211ccc` ("stash: do not attempt to remove startup_info->original_cwd", 2021-12-09), we wanted to allow the subprocess to know which directory the parent process was running from, so that the subprocess could protect it. However... When run from a non-main worktree, setup_git_directory() will note that the discovered git directory (/PATH/TO/.git/worktree/non-main-worktree) does not match DEFAULT_GIT_DIR_ENVIRONMENT (see setup_discovered_git_dir()), and decide to set GIT_DIR in the environment. This matters because... Whenever git is run with the GIT_DIR environment variable set, and GIT_WORK_TREE not set, it presumes that '.' is the working tree. So... This combination results in the subcommand being very confused about the working tree. Fix it by also setting the GIT_WORK_TREE environment variable along with setting cmd.dir. A possibly more involved fix we could consider for later would be to make setup.c set GIT_WORK_TREE whenever (a) it discovers both the git directory and the working tree and (b) it decides to set GIT_DIR in the environment. I did not attempt that here as such would be too big of a change for a 2.35.1 release. Test-case-by: Glen Choo <chooglen@google.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Jean-Noël Avila	246cac8505	i18n: turn even more messages into "cannot be used together" ones Even if some of these messages are not subject to gettext i18n, this helps bring a single style of message for a given error type. Signed-off-by: Jean-Noël Avila <jn.avila@free.fr> Reviewed-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Jean-Noël Avila	6fa00ee843	i18n: factorize "--foo requires --bar" and the like They are all replaced by "the option '%s' requires '%s'", which is a new string but replaces 17 previous unique strings. Signed-off-by: Jean-Noël Avila <jn.avila@free.fr> Reviewed-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Jean-Noël Avila	12909b6b8a	i18n: turn "options are incompatible" into "cannot be used together" Signed-off-by: Jean-Noël Avila <jn.avila@free.fr> Reviewed-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Elijah Newren	71cade5a0b	stash: do not return before restoring untracked files In commit `bee8691f19` ("stash: restore untracked files AFTER restoring tracked files", 2021-09-10), we correctly identified that we should restore changes to tracked files before attempting to restore untracked files, and accordingly moved the code for restoring untracked files a few lines down in do_apply_stash(). Unfortunately, the intervening lines had some early return statements meaning that we suddenly stopped restoring untracked files in some cases. Even before the previous commit, there was another possible issue with the current code -- a post-stash-apply 'git status' that was intended to be run after restoring the stash was skipped when we hit a conflict (or other error condition), which seems slightly inconsistent. Fix both issues by saving the return status, and letting other functionality run before returning. Reported-by: AJ Henderson Test-case-by: Randall S. Becker <randall.becker@nexbridge.ca> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Ævar Arnfjörð Bjarmason	fa476be8f0	parse-options API: add a usage_msg_optf() Add a usage_msg_optf() as a shorthand for the sort of usage_msg_opt(xstrfmt(...)) used in builtin/stash.c. I'll make more use of this function in builtin/cat-file.c shortly. The disconnect between the "..." and "fmt" is a bit unusual, but it works just fine and this keeps it consistent with usage_msg_opt(), i.e. a caller of it can be moved to usage_msg_optf() and not have to have its arguments re-arranged. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Ævar Arnfjörð Bjarmason	ca7990cea5	stash: don't show "git stash push" usage on bad "git stash" usage Change the usage message emitted by "git stash --invalid-option" to emit usage information for "git stash" in general, and not just for the "push" command. I.e. before: $ git stash --invalid-option error: unknown option `invalid-option' usage: git stash [push [-p\|--patch] [-k\|--[no-]keep-index] [-q\|--quiet] [-u\|--include-untracked] [-a\|--all] [-m\|--message <message>] [--] [<pathspec>...]] [...] After: $ git stash --invalid-option error: unknown option `invalid-option' usage: git stash list [<options>] or: git stash show [<options>] [<stash>] or: git stash drop [-q\|--quiet] [<stash>] or: git stash ( pop \| apply ) [--index] [-q\|--quiet] [<stash>] or: git stash branch <branchname> [<stash>] or: git stash clear or: git stash [push [-p\|--patch] [-S\|--staged] [-k\|--[no-]keep-index] [-q\|--quiet] [-u\|--include-untracked] [-a\|--all] [-m\|--message <message>] [--pathspec-from-file=<file> [--pathspec-file-nul]] [--] [<pathspec>...]] or: git stash save [-p\|--patch] [-S\|--staged] [-k\|--[no-]keep-index] [-q\|--quiet] [-u\|--include-untracked] [-a\|--all] [<message>] [...] That we emitted the usage for just "push" in the case of the subcommand not being explicitly specified was an unintentional side-effect of how it was implemented. When it was converted to C in `d553f538b8` (stash: convert push to builtin, 2019-02-25) the pattern of having per-subcommand usage information was rightly continued. The "git-stash.sh" shellscript did not have that, and always printed the equivalent of "git_stash_usage". But in doing so the case of push being implicit and explicit was conflated. A variable was added to track this in `8c3713cede` (stash: eliminate crude option parsing, 2020-02-17), but it did not update the usage output accordingly. This still leaves e.g. "git stash push -h" emitting the "git_stash_usage" output, instead of "git_stash_push_usage". That should be fixed, but is a much deeper misbehavior in parse_options() not being aware of subcommands at all. I.e. in how PARSE_OPT_KEEP_UNKNOWN and PARSE_OPT_NO_INTERNAL_HELP combine in commands such as "git stash". Perhaps PARSE_OPT_KEEP_UNKNOWN should imply PARSE_OPT_NO_INTERNAL_HELP, or better yet parse_options() should be extended to fully handle these subcommand cases that we handle manually in "git stash", "git commit-graph", "git multi-pack-index" etc. All of those musings would be a much bigger change than this isolated fix though, so let's leave that for some other time. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Elijah Newren	0fce211ccc	stash: do not attempt to remove startup_info->original_cwd Since stash spawns a `clean` subprocess, make sure we run that from the startup_info->original_cwd directory, so that the `clean` processs knows to protect that directory. Also, since the `clean` command might no longer run from the toplevel, pass the ':/' magic pathspec to ensure we still clean from the toplevel. Acked-by: Derrick Stolee <stolee@gmail.com> Acked-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Sergey Organov	a8a6e0682d	stash: get rid of unused argument in stash_staged() Unused 'ps' argument was a left-over from original copy-paste of stash_patch(). Removed. Signed-off-by: Sergey Organov <sorganov@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Sergey Organov	41a28eb6c1	stash: implement '--staged' option for 'push' and 'save' Stash only the changes that are staged. This mode allows to easily stash-out for later reuse some changes unrelated to the current work in progress. Unlike 'stash push --patch', --staged supports use of any tool to select the changes to stash-out, including, but not limited to 'git add --interactive'. Signed-off-by: Sergey Organov <sorganov@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Elijah Newren	94b7f1563a	Comment important codepaths regarding nuking untracked files/dirs In the last few commits we focused on code in unpack-trees.c that mistakenly removed untracked files or directories. There may be more of those, but in this commit we change our focus: callers of toplevel commands that are expected to remove untracked files or directories. As noted previously, we have toplevel commands that are expected to delete untracked files such as 'read-tree --reset', 'reset --hard', and 'checkout --force'. However, that does not mean that other highlevel commands that happen to call these other commands thought about or conveyed to users the possibility that untracked files could be removed. Audit the code for such callsites, and add comments near existing callsites to mention whether these are safe or not. My auditing is somewhat incomplete, though; it skipped several cases: * git-rebase--preserve-merges.sh: is in the process of being deprecated/removed, so I won't leave a note that there are likely more bugs in that script. * contrib/git-new-workdir: why is the -f flag being used in a new empty directory?? It shouldn't hurt, but it seems useless. * git-p4.py: Don't see why -f is needed for a new dir (maybe it's not and is just superfluous), but I'm not at all familiar with the p4 stuff * git-archimport.perl: Don't care; arch is long since dead * git-cvs*.perl: Don't care; cvs is long since dead Also, the reset --hard in builtin/worktree.c looks safe, due to only running in an empty directory. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Elijah Newren	480d3d6bf9	Change unpack_trees' 'reset' flag into an enum Traditionally, unpack_trees_options->reset was used to signal that it was okay to delete any untracked files in the way. This was used by `git read-tree --reset`, but then started appearing in other places as well. However, many of the other uses should not be deleting untracked files in the way. Change this value to an enum so that a value of 1 (i.e. "true") can be split into two: UNPACK_RESET_PROTECT_UNTRACKED, UNPACK_RESET_OVERWRITE_UNTRACKED In order to catch accidental misuses (i.e. where folks call it the way they traditionally used to), define the special enum value of UNPACK_RESET_INVALID = 1 which will trigger a BUG(). Modify existing callers so that read-tree --reset reset --hard checkout --force continue using the UNPACK_RESET_OVERWRITE_UNTRACKED logic, while other callers, including am checkout without --force stash (though currently dead code; reset always had a value of 0) numerous callers from rebase/sequencer to reset_head() will use the new UNPACK_RESET_PROTECT_UNTRACKED value. Also, note that it has been reported that 'git checkout <treeish> <pathspec>' currently also allows overwriting untracked files[1]. That case should also be fixed, but it does not use unpack_trees() and thus is outside the scope of the current changes. [1] https://lore.kernel.org/git/15dad590-087e-5a48-9238-5d2826950506@gmail.com/ Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Elijah Newren	1b5f37334a	Remove ignored files by default when they are in the way Change several commands to remove ignored files by default when they are in the way. Since some commands (checkout, merge) take a --no-overwrite-ignore option to allow the user to configure this, and it may make sense to add that option to more commands (and in the case of merge, actually plumb that configuration option through to more of the backends than just the fast-forwarding special case), add little comments about where such flags would be used. Incidentally, this fixes a test failure in t7112. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Elijah Newren	04988c8d18	unpack-trees: introduce preserve_ignored to unpack_trees_options Currently, every caller of unpack_trees() that wants to ensure ignored files are overwritten by default needs to: * allocate unpack_trees_options.dir * flip the DIR_SHOW_IGNORED flag in unpack_trees_options.dir->flags * call setup_standard_excludes AND then after the call to unpack_trees() needs to * call dir_clear() * deallocate unpack_trees_options.dir That's a fair amount of boilerplate, and every caller uses identical code. Make this easier by instead introducing a new boolean value where the default value (0) does what we want so that new callers of unpack_trees() automatically get the appropriate behavior. And move all the handling of unpack_trees_options.dir into unpack_trees() itself. While preserve_ignored = 0 is the behavior we feel is the appropriate default, we defer fixing commands to use the appropriate default until a later commit. So, this commit introduces several locations where we manually set preserve_ignored=1. This makes it clear where code paths were previously preserving ignored files when they should not have been; a future commit will flip these to instead use a value of 0 to get the behavior we want. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Ævar Arnfjörð Bjarmason	5d70198efe	parse-options API users: align usage output in C-strings In preparation for having continued usage lines properly aligned in "git <cmd> -h" output, let's have the "[" on the second such lines align with the "[" on the first line. In some cases this makes the output worse, because e.g. the "git ls-remote -h" output had been aligned to account for the extra whitespace that the usage_with_options_internal() function in parse-options.c would add. In other cases such as builtin/stash.c (not changed here), we were aligned in the C strings, but since that didn't account for the extra padding in usage_with_options_internal() it would come out looking misaligned, e.g. code like this: N_("git stash [push [-p\|--patch] [-k\|--[no-]keep-index] [-q\|--quiet]\n" " [-u\|--include-untracked] [-a\|--all] [-m\|--message <message>]\n" Would emit: or: git stash [push [-p\|--patch] [-k\|--[no-]keep-index] [-q\|--quiet] [-u\|--include-untracked] [-a\|--all] [-m\|--message <message>] Let's change all the usage arrays which use such continued usage output via "\n"-embedding to be like builtin/stash.c. This makes the output worse temporarily, but in a subsequent change I'll improve the usage_with_options_internal() to take this into account, at which point all of the strings being changed here will emit prettier output. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Elijah Newren	bee8691f19	stash: restore untracked files AFTER restoring tracked files If a user deletes a file and places a directory of untracked files there, then stashes all these changes, the untracked directory of files cannot be restored until after the corresponding file in the way is removed. So, restore changes to tracked files before restoring untracked files. There is no counterpart problem to worry about with the user deleting an untracked file and then add a tracked one in its place. Git does not track untracked files, and so will not know the untracked file was deleted, and thus won't be able to stash the removal of that file. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Elijah Newren	3d40e3723b	stash: avoid feeding directories to update-index When a file is removed from the cache, but there is a file of the same name present in the working directory, we would normally treat that file in the working directory as untracked. However, in the case of stash, doing that would prevent a simple 'git stash push', because the untracked file would be in the way of restoring the deleted file. git stash, however, blindly assumes that whatever is in the working directory for a deleted file is wanted and passes that path along to update-index. That causes problems when the working directory contains a directory with the same name as the deleted file. Add some code for this special case that will avoid passing directory names to update-index. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Ævar Arnfjörð Bjarmason	ce93a4c612	dir.[ch]: replace dir_init() with DIR_INIT Remove the dir_init() function and replace it with a DIR_INIT macro. In many cases in the codebase we need to initialize things with a function for good reasons, e.g. needing to call another function on initialization. The "dir_init()" function was not one such case, and could trivially be replaced with a more idiomatic macro initialization pattern. The only place where we made use of its use of memset() was in dir_clear() itself, which resets the contents of an an existing struct pointer. Let's use the new "memcpy() a 'blank' struct on the stack" idiom to do that reset. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Denton Liu	af5cd44b6f	stash show: use stash.showIncludeUntracked even when diff options given 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>	4 years ago
Sergey Organov	1e20a407fe	stash list: stop passing "-m" to "git log" Passing "-m" in "git log --first-parent -m" is not needed as --first-parent implies --diff-merges=first-parent anyway. OTOH, it will stop being harmless once we let "-m" imply "-p". While we are at it, fix corresponding test description in t3903-stash to match what it actually tests. Signed-off-by: Sergey Organov <sorganov@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Alex Henrie	4901884a23	stash: don't translate literal commands Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Denton Liu	1ff595d218	stash show: fix segfault with --{include,only}-untracked 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>	4 years ago
Derrick Stolee	a02912019a	stash: ensure full index 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>	4 years ago
Matheus Tavares	d052cc0382	entry: extract a header file for entry.c functions 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>	4 years ago
Denton Liu	0af760e261	stash show: learn stash.showIncludeUntracked 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>	4 years ago
Denton Liu	d3c7bf73bd	stash show: teach --include-untracked and --only-untracked 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>	4 years ago
Johannes Sixt	b865734760	replace "parameters" by "arguments" in error messages 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>	4 years ago
Denton Liu	3e885f0277	stash: declare ref_stash as an array 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>	4 years ago
Elijah Newren	ba359fd507	stash: fix stash application in sparse-checkouts 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>	4 years ago
Elijah Newren	b34ab4a43b	stash: remove unnecessary process forking 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>	4 years ago
Kyle Meyer	eaf5341538	stash: add missing space to an error message 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>	4 years ago
René Scharfe	4f44c5659b	stash: simplify reflog emptiness check 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>	4 years ago
Jonathan Tan	f24c30e0b6	wt-status: tolerate dangling marks 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>	5 years ago
Elijah Newren	eceba53214	dir: fix problematic API to avoid memory leaks 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>	5 years ago
Elijah Newren	dad4f23ce5	dir: make clear_directory() free all relevant memory 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>	5 years ago
Jeff King	d70a9eb611	strvec: rename struct fields 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>	5 years ago
Jeff King	f6d8942b1f	strvec: fix indentation in renamed calls 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>	5 years ago
Jeff King	22f9b7f3f5	strvec: convert builtin/ callers away from argv_array name 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>	5 years ago
Jeff King	dbbcd44fb4	strvec: rename files from argv-array to strvec 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>	5 years ago
Johannes Schindelin	7723436149	stash -p: (partially) fix bug concerning split hunks 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>	5 years ago
Elijah Newren	95c11ecc73	Fix error-prone fill_directory() API; make it only return matches 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 `404ebceda0` (dir: also check directories for matching pathspecs, 2019-09-17) and `89a1f4aaf7` (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>	5 years ago
Thomas Gummerer	8a2cd3f512	stash: remove the stash.useBuiltin setting 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>	5 years ago
Thomas Gummerer	b0c7362d19	stash: get git_stash_config at the top level 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>	5 years ago
Alexandr Miloslavskiy	8a98758a8d	stash push: support the --pathspec-from-file option 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>	5 years ago
Alexandr Miloslavskiy	8c3713cede	stash: eliminate crude option parsing 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>	5 years ago

1 2 3

107 Commits (1dff6dc016a3b20e92910004ade1c13339b6fe7f)