From 6a7aca6f016e397838926250764318b767650bd5 Mon Sep 17 00:00:00 2001 From: Alexandr Miloslavskiy Date: Thu, 16 Jan 2020 16:09:18 +0000 Subject: [PATCH 01/27] doc: rm: synchronize description This patch continues the effort that is already applied to `git commit`, `git reset`, `git checkout` etc. 1) Changed outdated descriptions to mention pathspec instead. 2) Added reference to 'linkgit:gitglossary[7]'. 3) Removed content that merely repeated gitglossary. 4) Merged the remainder of "discussion" into ``. Signed-off-by: Alexandr Miloslavskiy Signed-off-by: Junio C Hamano --- Documentation/git-rm.txt | 50 +++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 29 deletions(-) diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt index b5c46223c4..e02a08e5ef 100644 --- a/Documentation/git-rm.txt +++ b/Documentation/git-rm.txt @@ -8,16 +8,16 @@ git-rm - Remove files from the working tree and from the index SYNOPSIS -------- [verse] -'git rm' [-f | --force] [-n] [-r] [--cached] [--ignore-unmatch] [--quiet] [--] ... +'git rm' [-f | --force] [-n] [-r] [--cached] [--ignore-unmatch] [--quiet] [--] ... DESCRIPTION ----------- -Remove files from the index, or from the working tree and the index. -`git rm` will not remove a file from just your working directory. -(There is no option to remove a file only from the working tree -and yet keep it in the index; use `/bin/rm` if you want to do that.) -The files being removed have to be identical to the tip of the branch, -and no updates to their contents can be staged in the index, +Remove files matching pathspec from the index, or from the working tree +and the index. `git rm` will not remove a file from just your working +directory. (There is no option to remove a file only from the working +tree and yet keep it in the index; use `/bin/rm` if you want to do +that.) The files being removed have to be identical to the tip of the +branch, and no updates to their contents can be staged in the index, though that default behavior can be overridden with the `-f` option. When `--cached` is given, the staged content has to match either the tip of the branch or the file on disk, @@ -26,15 +26,20 @@ allowing the file to be removed from just the index. OPTIONS ------- -...:: - Files to remove. Fileglobs (e.g. `*.c`) can be given to - remove all matching files. If you want Git to expand - file glob characters, you may need to shell-escape them. - A leading directory name - (e.g. `dir` to remove `dir/file1` and `dir/file2`) can be - given to remove all files in the directory, and recursively - all sub-directories, - but this requires the `-r` option to be explicitly given. +...:: + Files to remove. A leading directory name (e.g. `dir` to remove + `dir/file1` and `dir/file2`) can be given to remove all files in + the directory, and recursively all sub-directories, but this + requires the `-r` option to be explicitly given. ++ +The command removes only the paths that are known to Git. ++ +File globbing matches across directory boundaries. Thus, given two +directories `d` and `d2`, there is a difference between using +`git rm 'd*'` and `git rm 'd/*'`, as the former will also remove all +of directory `d2`. ++ +For more details, see the 'pathspec' entry in linkgit:gitglossary[7]. -f:: --force:: @@ -69,19 +74,6 @@ OPTIONS for each file removed. This option suppresses that output. -DISCUSSION ----------- - -The list given to the command can be exact pathnames, -file glob patterns, or leading directory names. The command -removes only the paths that are known to Git. Giving the name of -a file that you have not told Git about does not remove that file. - -File globbing matches across directory boundaries. Thus, given -two directories `d` and `d2`, there is a difference between -using `git rm 'd*'` and `git rm 'd/*'`, as the former will -also remove all of directory `d2`. - REMOVING FILES THAT HAVE DISAPPEARED FROM THE FILESYSTEM -------------------------------------------------------- There is no option for `git rm` to remove from the index only From 5f393dc3aa62ba3e13a9890af4e769ad84812fbc Mon Sep 17 00:00:00 2001 From: Alexandr Miloslavskiy Date: Mon, 17 Feb 2020 17:25:16 +0000 Subject: [PATCH 02/27] rm: support the --pathspec-from-file option Decisions taken for simplicity: 1) It is not allowed to pass pathspec in both args and file. Adjustments were needed for `if (!argc)` block: This code actually means "pathspec is not present". Previously, pathspec could only come from commandline arguments, so testing for `argc` was a valid way of testing for the presence of pathspec. But this is no longer true with `--pathspec-from-file`. During the entire `--pathspec-from-file` story, I tried to keep its behavior very close to giving pathspec on commandline, so that switching from one to another doesn't involve any surprises. However, throwing usage at user in the case of empty `--pathspec-from-file` would puzzle because there's nothing wrong with "usage" (that is, argc/argv array). On the other hand, throwing usage in the old case also feels bad to me. While it's less of a puzzle, I (as user) never liked the experience of comparing my commandline to "usage", trying to spot a difference. Since it's already known what the error is, it feels a lot better to give that specific error to user. Judging from [1] it doesn't seem that showing usage in this case was important (the patch was to avoid segfault), and it doesn't fit into how other commands react to empty pathspec (see for example `git add` with a custom message). Therefore, I decided to show new error text in both cases. In order to continue testing for error early, I moved `parse_pathspec()` higher. Now it happens before `read_cache()` / `hold_locked_index()` / `setup_work_tree()`, which shouldn't cause any issues. [1] Commit 7612a1ef ("git-rm: honor -n flag" 2006-06-09) Signed-off-by: Alexandr Miloslavskiy Signed-off-by: Junio C Hamano --- Documentation/git-rm.txt | 17 +++++++- builtin/rm.c | 28 ++++++++++--- t/t3601-rm-pathspec-file.sh | 79 +++++++++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 7 deletions(-) create mode 100755 t/t3601-rm-pathspec-file.sh diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt index e02a08e5ef..ab750367fd 100644 --- a/Documentation/git-rm.txt +++ b/Documentation/git-rm.txt @@ -8,7 +8,9 @@ git-rm - Remove files from the working tree and from the index SYNOPSIS -------- [verse] -'git rm' [-f | --force] [-n] [-r] [--cached] [--ignore-unmatch] [--quiet] [--] ... +'git rm' [-f | --force] [-n] [-r] [--cached] [--ignore-unmatch] + [--quiet] [--pathspec-from-file= [--pathspec-file-nul]] + [--] [...] DESCRIPTION ----------- @@ -73,6 +75,19 @@ For more details, see the 'pathspec' entry in linkgit:gitglossary[7]. `git rm` normally outputs one line (in the form of an `rm` command) for each file removed. This option suppresses that output. +--pathspec-from-file=:: + Pathspec is passed in `` instead of commandline args. If + `` is exactly `-` then standard input is used. Pathspec + elements are separated by LF or CR/LF. Pathspec elements can be + quoted as explained for the configuration variable `core.quotePath` + (see linkgit:git-config[1]). See also `--pathspec-file-nul` and + global `--literal-pathspecs`. + +--pathspec-file-nul:: + Only meaningful with `--pathspec-from-file`. Pathspec elements are + separated with NUL character and all other characters are taken + literally (including newlines and quotes). + REMOVING FILES THAT HAVE DISAPPEARED FROM THE FILESYSTEM -------------------------------------------------------- diff --git a/builtin/rm.c b/builtin/rm.c index 19ce95a901..4858631e0f 100644 --- a/builtin/rm.c +++ b/builtin/rm.c @@ -235,7 +235,8 @@ static int check_local_mod(struct object_id *head, int index_only) } static int show_only = 0, force = 0, index_only = 0, recursive = 0, quiet = 0; -static int ignore_unmatch = 0; +static int ignore_unmatch = 0, pathspec_file_nul; +static char *pathspec_from_file; static struct option builtin_rm_options[] = { OPT__DRY_RUN(&show_only, N_("dry run")), @@ -245,6 +246,8 @@ static struct option builtin_rm_options[] = { OPT_BOOL('r', NULL, &recursive, N_("allow recursive removal")), OPT_BOOL( 0 , "ignore-unmatch", &ignore_unmatch, N_("exit with a zero status even if nothing matched")), + OPT_PATHSPEC_FROM_FILE(&pathspec_from_file), + OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul), OPT_END(), }; @@ -259,8 +262,24 @@ int cmd_rm(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, builtin_rm_options, builtin_rm_usage, 0); - if (!argc) - usage_with_options(builtin_rm_usage, builtin_rm_options); + + parse_pathspec(&pathspec, 0, + PATHSPEC_PREFER_CWD, + prefix, argv); + + if (pathspec_from_file) { + if (pathspec.nr) + die(_("--pathspec-from-file is incompatible with pathspec arguments")); + + parse_pathspec_file(&pathspec, 0, + PATHSPEC_PREFER_CWD, + prefix, pathspec_from_file, pathspec_file_nul); + } else if (pathspec_file_nul) { + die(_("--pathspec-file-nul requires --pathspec-from-file")); + } + + if (!pathspec.nr) + die(_("No pathspec was given. Which files should I remove?")); if (!index_only) setup_work_tree(); @@ -270,9 +289,6 @@ int cmd_rm(int argc, const char **argv, const char *prefix) if (read_cache() < 0) die(_("index file corrupt")); - parse_pathspec(&pathspec, 0, - PATHSPEC_PREFER_CWD, - prefix, argv); refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, &pathspec, NULL, NULL); seen = xcalloc(pathspec.nr, 1); diff --git a/t/t3601-rm-pathspec-file.sh b/t/t3601-rm-pathspec-file.sh new file mode 100755 index 0000000000..7de21f8bcf --- /dev/null +++ b/t/t3601-rm-pathspec-file.sh @@ -0,0 +1,79 @@ +#!/bin/sh + +test_description='rm --pathspec-from-file' + +. ./test-lib.sh + +test_tick + +test_expect_success setup ' + echo A >fileA.t && + echo B >fileB.t && + echo C >fileC.t && + echo D >fileD.t && + git add fileA.t fileB.t fileC.t fileD.t && + git commit -m "files" && + + git tag checkpoint +' + +restore_checkpoint () { + git reset --hard checkpoint +} + +verify_expect () { + git status --porcelain --untracked-files=no -- fileA.t fileB.t fileC.t fileD.t >actual && + test_cmp expect actual +} + +test_expect_success 'simplest' ' + restore_checkpoint && + + cat >expect <<-\EOF && + D fileA.t + EOF + + echo fileA.t | git rm --pathspec-from-file=- && + verify_expect +' + +test_expect_success '--pathspec-file-nul' ' + restore_checkpoint && + + cat >expect <<-\EOF && + D fileA.t + D fileB.t + EOF + + printf "fileA.t\0fileB.t\0" | git rm --pathspec-from-file=- --pathspec-file-nul && + verify_expect +' + +test_expect_success 'only touches what was listed' ' + restore_checkpoint && + + cat >expect <<-\EOF && + D fileB.t + D fileC.t + EOF + + printf "fileB.t\nfileC.t\n" | git rm --pathspec-from-file=- && + verify_expect +' + +test_expect_success 'error conditions' ' + restore_checkpoint && + echo fileA.t >list && + + test_must_fail git rm --pathspec-from-file=list -- fileA.t 2>err && + test_i18ngrep -e "--pathspec-from-file is incompatible with pathspec arguments" err && + + test_must_fail git rm --pathspec-file-nul 2>err && + test_i18ngrep -e "--pathspec-file-nul requires --pathspec-from-file" err && + + >empty_list && + test_must_fail git rm --pathspec-from-file=empty_list 2>err && + test_i18ngrep -e "No pathspec was given. Which files should I remove?" err +' + +test_done From 2b7460d16719b751f025094ba9c8d10c81a6c4b1 Mon Sep 17 00:00:00 2001 From: Alexandr Miloslavskiy Date: Mon, 17 Feb 2020 17:25:17 +0000 Subject: [PATCH 03/27] doc: stash: split options from description (1) This patch moves blocks of text as-is to make it easier to review the next patch. Signed-off-by: Alexandr Miloslavskiy Signed-off-by: Junio C Hamano --- Documentation/git-stash.txt | 68 +++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 33 deletions(-) diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt index 53e1a1205d..31ab780f5a 100644 --- a/Documentation/git-stash.txt +++ b/Documentation/git-stash.txt @@ -58,31 +58,6 @@ non-option arguments are not allowed to prevent a misspelled subcommand from making an unwanted stash entry. The two exceptions to this are `stash -p` which acts as alias for `stash push -p` and pathspecs, which are allowed after a double hyphen `--` for disambiguation. -+ -When pathspec is given to 'git stash push', the new stash entry records the -modified states only for the files that match the pathspec. The index -entries and working tree files are then rolled back to the state in -HEAD only for these files, too, leaving files that do not match the -pathspec intact. -+ -If the `--keep-index` option is used, all changes already added to the -index are left intact. -+ -If the `--include-untracked` option is used, all untracked files are also -stashed and then cleaned up with `git clean`, leaving the working directory -in a very clean state. If the `--all` option is used instead then the -ignored files are stashed and cleaned in addition to the untracked files. -+ -With `--patch`, you can interactively select hunks from the diff -between HEAD and the working tree to be stashed. The stash entry is -constructed such that its index state is the same as the index state -of your repository, and its worktree contains only the changes you -selected interactively. The selected changes are then rolled back -from your worktree. See the ``Interactive Mode'' section of -linkgit:git-add[1] to learn how to operate the `--patch` mode. -+ -The `--patch` option implies `--keep-index`. You can use -`--no-keep-index` to override this. save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] []:: @@ -128,14 +103,6 @@ pop [--index] [-q|--quiet] []:: Applying the state can fail with conflicts; in this case, it is not removed from the stash list. You need to resolve the conflicts by hand and call `git stash drop` manually afterwards. -+ -If the `--index` option is used, then tries to reinstate not only the working -tree's changes, but also the index's ones. However, this can fail, when you -have conflicts (which are stored in the index, where you therefore can no -longer apply the changes as they were originally). -+ -When no `` is given, `stash@{0}` is assumed, otherwise `` must -be a reference of the form `stash@{}`. apply [--index] [-q|--quiet] []:: @@ -185,6 +152,41 @@ store:: reflog. This is intended to be useful for scripts. It is probably not the command you want to use; see "push" above. +If the `--all` option is used instead then the +ignored files are stashed and cleaned in addition to the untracked files. + +If the `--include-untracked` option is used, all untracked files are also +stashed and then cleaned up with `git clean`, leaving the working directory +in a very clean state. + +If the `--index` option is used, then tries to reinstate not only the working +tree's changes, but also the index's ones. However, this can fail, when you +have conflicts (which are stored in the index, where you therefore can no +longer apply the changes as they were originally). + +If the `--keep-index` option is used, all changes already added to the +index are left intact. + +With `--patch`, you can interactively select hunks from the diff +between HEAD and the working tree to be stashed. The stash entry is +constructed such that its index state is the same as the index state +of your repository, and its worktree contains only the changes you +selected interactively. The selected changes are then rolled back +from your worktree. See the ``Interactive Mode'' section of +linkgit:git-add[1] to learn how to operate the `--patch` mode. ++ +The `--patch` option implies `--keep-index`. You can use +`--no-keep-index` to override this. + +When pathspec is given to 'git stash push', the new stash entry records the +modified states only for the files that match the pathspec. The index +entries and working tree files are then rolled back to the state in +HEAD only for these files, too, leaving files that do not match the +pathspec intact. + +When no `` is given, `stash@{0}` is assumed, otherwise `` must +be a reference of the form `stash@{}`. + DISCUSSION ---------- From 0093abc286fe3f195f702de28c6d9af009dda8e0 Mon Sep 17 00:00:00 2001 From: Alexandr Miloslavskiy Date: Mon, 17 Feb 2020 17:25:18 +0000 Subject: [PATCH 04/27] doc: stash: split options from description (2) Together with the previous patch, this brings docs for `git stash` to the common layout used for most other commands (see for example docs for `git add`, `git commit`, `git checkout`, `git reset`) where all options are documented in a separate list. After some thinking and having a look at docs for `git svn` and `git `submodule`, I have arrived at following conclusions: * Options should be described in a list rather then text to facilitate lookup for user. * Single list is better then multiple lists because it avoids copy&pasting descriptions between subcommands (or, without copy&pasting, user will have to look up missing options in other subcommands). * As a consequence, commands section should only give brief info and list possible options. Since options have good enough names, user will only need to look up the "interesting" options. * Every option should list which subcommands support it. I have decided to use alphabetical sorting in the list of options to facilitate lookup for user. There is some text editing done to make old descriptions better fit into the list-style format. Signed-off-by: Alexandr Miloslavskiy Signed-off-by: Junio C Hamano --- Documentation/git-stash.txt | 88 +++++++++++++++++++++++-------------- 1 file changed, 55 insertions(+), 33 deletions(-) diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt index 31ab780f5a..69281fcf43 100644 --- a/Documentation/git-stash.txt +++ b/Documentation/git-stash.txt @@ -43,8 +43,8 @@ created stash, `stash@{1}` is the one before it, `stash@{2.hours.ago}` is also possible). Stashes may also be referenced by specifying just the stash index (e.g. the integer `n` is equivalent to `stash@{n}`). -OPTIONS -------- +COMMANDS +-------- push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message ] [--] [...]:: @@ -86,7 +86,7 @@ show [] []:: Show the changes recorded in the stash entry as a diff between the stashed contents and the commit back when the stash entry was first - created. When no `` is given, it shows the latest one. + created. By default, the command shows the diffstat, but it will accept any format known to 'git diff' (e.g., `git stash show -p stash@{1}` to view the second most recent entry in patch form). @@ -116,8 +116,7 @@ branch []:: the commit at which the `` was originally created, applies the changes recorded in `` to the new working tree and index. If that succeeds, and `` is a reference of the form - `stash@{}`, it then drops the ``. When no `` - is given, applies the latest one. + `stash@{}`, it then drops the ``. + This is useful if the branch on which you ran `git stash push` has changed enough that `git stash apply` fails due to conflicts. Since @@ -133,9 +132,6 @@ clear:: drop [-q|--quiet] []:: Remove a single stash entry from the list of stash entries. - When no `` is given, it removes the latest one. - i.e. `stash@{0}`, otherwise `` must be a valid stash - log reference of the form `stash@{}`. create:: @@ -152,40 +148,66 @@ store:: reflog. This is intended to be useful for scripts. It is probably not the command you want to use; see "push" above. -If the `--all` option is used instead then the -ignored files are stashed and cleaned in addition to the untracked files. +OPTIONS +------- +-a:: +--all:: + This option is only valid for `push` and `save` commands. ++ +All ignored and untracked files are also stashed and then cleaned +up with `git clean`. -If the `--include-untracked` option is used, all untracked files are also -stashed and then cleaned up with `git clean`, leaving the working directory -in a very clean state. +-u:: +--include-untracked:: + This option is only valid for `push` and `save` commands. ++ +All untracked files are also stashed and then cleaned up with +`git clean`. -If the `--index` option is used, then tries to reinstate not only the working -tree's changes, but also the index's ones. However, this can fail, when you -have conflicts (which are stored in the index, where you therefore can no -longer apply the changes as they were originally). +--index:: + This option is only valid for `pop` and `apply` commands. ++ +Tries to reinstate not only the working tree's changes, but also +the index's ones. However, this can fail, when you have conflicts +(which are stored in the index, where you therefore can no longer +apply the changes as they were originally). -If the `--keep-index` option is used, all changes already added to the -index are left intact. +-k:: +--keep-index:: +--no-keep-index:: + This option is only valid for `push` and `save` commands. ++ +All changes already added to the index are left intact. -With `--patch`, you can interactively select hunks from the diff -between HEAD and the working tree to be stashed. The stash entry is -constructed such that its index state is the same as the index state -of your repository, and its worktree contains only the changes you -selected interactively. The selected changes are then rolled back -from your worktree. See the ``Interactive Mode'' section of -linkgit:git-add[1] to learn how to operate the `--patch` mode. +-p:: +--patch:: + This option is only valid for `push` and `save` commands. ++ +Interactively select hunks from the diff between HEAD and the +working tree to be stashed. The stash entry is constructed such +that its index state is the same as the index state of your +repository, and its worktree contains only the changes you selected +interactively. The selected changes are then rolled back from your +worktree. See the ``Interactive Mode'' section of linkgit:git-add[1] +to learn how to operate the `--patch` mode. + The `--patch` option implies `--keep-index`. You can use `--no-keep-index` to override this. -When pathspec is given to 'git stash push', the new stash entry records the -modified states only for the files that match the pathspec. The index -entries and working tree files are then rolled back to the state in -HEAD only for these files, too, leaving files that do not match the -pathspec intact. +...:: + This option is only valid for `push` command. ++ +The new stash entry records the modified states only for the files +that match the pathspec. The index entries and working tree files +are then rolled back to the state in HEAD only for these files, +too, leaving files that do not match the pathspec intact. -When no `` is given, `stash@{0}` is assumed, otherwise `` must -be a reference of the form `stash@{}`. +:: + This option is only valid for `apply`, `branch`, `drop`, `pop`, + `show` commands. ++ +A reference of the form `stash@{}`. When no `` is +given, the latest stash is assumed (that is, `stash@{0}`). DISCUSSION ---------- From b22909144ccb717e6f3c96562451fdfeca592c9c Mon Sep 17 00:00:00 2001 From: Alexandr Miloslavskiy Date: Mon, 17 Feb 2020 17:25:19 +0000 Subject: [PATCH 05/27] doc: stash: document more options Signed-off-by: Alexandr Miloslavskiy Signed-off-by: Junio C Hamano --- Documentation/git-stash.txt | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt index 69281fcf43..4303b5abc2 100644 --- a/Documentation/git-stash.txt +++ b/Documentation/git-stash.txt @@ -194,6 +194,18 @@ to learn how to operate the `--patch` mode. The `--patch` option implies `--keep-index`. You can use `--no-keep-index` to override this. +-q:: +--quiet:: + This option is only valid for `apply`, `drop`, `pop`, `push`, + `save`, `store` commands. ++ +Quiet, suppress feedback messages. + +\--:: + This option is only valid for `push` command. ++ +Separates pathspec from options for disambiguation purposes. + ...:: This option is only valid for `push` command. + From 3f3d8068f577ccbf11a9343fbfec3b7c679a6772 Mon Sep 17 00:00:00 2001 From: Alexandr Miloslavskiy Date: Mon, 17 Feb 2020 17:25:20 +0000 Subject: [PATCH 06/27] doc: stash: synchronize description This patch continues the effort that is already applied to `git commit`, `git reset`, `git checkout` etc. 1) Added reference to 'linkgit:gitglossary[7]'. 2) Fixed mentions of incorrectly plural "pathspecs". Signed-off-by: Alexandr Miloslavskiy Signed-off-by: Junio C Hamano --- Documentation/git-stash.txt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt index 4303b5abc2..f10d8a2a5c 100644 --- a/Documentation/git-stash.txt +++ b/Documentation/git-stash.txt @@ -56,13 +56,13 @@ push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q For quickly making a snapshot, you can omit "push". In this mode, non-option arguments are not allowed to prevent a misspelled subcommand from making an unwanted stash entry. The two exceptions to this -are `stash -p` which acts as alias for `stash push -p` and pathspecs, +are `stash -p` which acts as alias for `stash push -p` and pathspec elements, which are allowed after a double hyphen `--` for disambiguation. save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] []:: This option is deprecated in favour of 'git stash push'. It - differs from "stash push" in that it cannot take pathspecs. + differs from "stash push" in that it cannot take pathspec. Instead, all non-option arguments are concatenated to form the stash message. @@ -213,6 +213,8 @@ The new stash entry records the modified states only for the files that match the pathspec. The index entries and working tree files are then rolled back to the state in HEAD only for these files, too, leaving files that do not match the pathspec intact. ++ +For more details, see the 'pathspec' entry in linkgit:gitglossary[7]. :: This option is only valid for `apply`, `branch`, `drop`, `pop`, From 8c3713cede71e4741dac546f787734627329f55b Mon Sep 17 00:00:00 2001 From: Alexandr Miloslavskiy Date: Mon, 17 Feb 2020 17:25:21 +0000 Subject: [PATCH 07/27] 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 Signed-off-by: Junio C Hamano --- builtin/stash.c | 59 +++++++++++++++++------------------------------- t/t3903-stash.sh | 5 ++++ 2 files changed, 26 insertions(+), 38 deletions(-) diff --git a/builtin/stash.c b/builtin/stash.c index 4ad3adf4ba..7bc4c5d696 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -1448,8 +1448,10 @@ done: return ret; } -static int push_stash(int argc, const char **argv, const char *prefix) +static int push_stash(int argc, const char **argv, const char *prefix, + int push_assumed) { + int force_assume = 0; int keep_index = -1; int patch_mode = 0; int include_untracked = 0; @@ -1471,10 +1473,22 @@ static int push_stash(int argc, const char **argv, const char *prefix) OPT_END() }; - if (argc) + if (argc) { + force_assume = !strcmp(argv[0], "-p"); argc = parse_options(argc, argv, prefix, options, git_stash_push_usage, - 0); + PARSE_OPT_KEEP_DASHDASH); + } + + if (argc) { + if (!strcmp(argv[0], "--")) { + argc--; + argv++; + } else if (push_assumed && !force_assume) { + die("subcommand wasn't specified; 'push' can't be assumed due to unexpected token '%s'", + argv[0]); + } + } parse_pathspec(&ps, 0, PATHSPEC_PREFER_FULL | PATHSPEC_PREFIX_ORIGIN, prefix, argv); @@ -1547,7 +1561,6 @@ static int use_builtin_stash(void) int cmd_stash(int argc, const char **argv, const char *prefix) { - int i = -1; pid_t pid = getpid(); const char *index_file; struct argv_array args = ARGV_ARRAY_INIT; @@ -1580,7 +1593,7 @@ int cmd_stash(int argc, const char **argv, const char *prefix) (uintmax_t)pid); if (!argc) - return !!push_stash(0, NULL, prefix); + return !!push_stash(0, NULL, prefix, 0); else if (!strcmp(argv[0], "apply")) return !!apply_stash(argc, argv, prefix); else if (!strcmp(argv[0], "clear")) @@ -1600,45 +1613,15 @@ int cmd_stash(int argc, const char **argv, const char *prefix) else if (!strcmp(argv[0], "create")) return !!create_stash(argc, argv, prefix); else if (!strcmp(argv[0], "push")) - return !!push_stash(argc, argv, prefix); + return !!push_stash(argc, argv, prefix, 0); else if (!strcmp(argv[0], "save")) return !!save_stash(argc, argv, prefix); else if (*argv[0] != '-') usage_msg_opt(xstrfmt(_("unknown subcommand: %s"), argv[0]), git_stash_usage, options); - if (strcmp(argv[0], "-p")) { - while (++i < argc && strcmp(argv[i], "--")) { - /* - * `akpqu` is a string which contains all short options, - * except `-m` which is verified separately. - */ - if ((strlen(argv[i]) == 2) && *argv[i] == '-' && - strchr("akpqu", argv[i][1])) - continue; - - if (!strcmp(argv[i], "--all") || - !strcmp(argv[i], "--keep-index") || - !strcmp(argv[i], "--no-keep-index") || - !strcmp(argv[i], "--patch") || - !strcmp(argv[i], "--quiet") || - !strcmp(argv[i], "--include-untracked")) - continue; - - /* - * `-m` and `--message=` are verified separately because - * they need to be immediately followed by a string - * (i.e.`-m"foobar"` or `--message="foobar"`). - */ - if (starts_with(argv[i], "-m") || - starts_with(argv[i], "--message=")) - continue; - - usage_with_options(git_stash_usage, options); - } - } - + /* Assume 'stash push' */ argv_array_push(&args, "push"); argv_array_pushv(&args, argv); - return !!push_stash(args.argc, args.argv, prefix); + return !!push_stash(args.argc, args.argv, prefix, 1); } diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh index ea56e85e70..3ad23e2502 100755 --- a/t/t3903-stash.sh +++ b/t/t3903-stash.sh @@ -285,6 +285,11 @@ test_expect_success 'stash --no-keep-index' ' test bar,bar2 = $(cat file),$(cat file2) ' +test_expect_success 'dont assume push with non-option args' ' + test_must_fail git stash -q drop 2>err && + test_i18ngrep -e "subcommand wasn'\''t specified; '\''push'\'' can'\''t be assumed due to unexpected token '\''drop'\''" err +' + test_expect_success 'stash --invalid-option' ' echo bar5 >file && echo bar6 >file2 && From 8a98758a8d9dbd807fc2f1576e9141c1b720c5c1 Mon Sep 17 00:00:00 2001 From: Alexandr Miloslavskiy Date: Mon, 17 Feb 2020 17:25:22 +0000 Subject: [PATCH 08/27] 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 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 Signed-off-by: Junio C Hamano --- Documentation/git-stash.txt | 20 ++++++- builtin/stash.c | 20 +++++++ t/t3909-stash-pathspec-file.sh | 100 +++++++++++++++++++++++++++++++++ 3 files changed, 139 insertions(+), 1 deletion(-) create mode 100755 t/t3909-stash-pathspec-file.sh diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt index f10d8a2a5c..31f1beb65b 100644 --- a/Documentation/git-stash.txt +++ b/Documentation/git-stash.txt @@ -15,6 +15,7 @@ SYNOPSIS 'git stash' branch [] 'git stash' [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet] [-u|--include-untracked] [-a|--all] [-m|--message ] + [--pathspec-from-file= [--pathspec-file-nul]] [--] [...]] 'git stash' clear 'git stash' create [] @@ -46,7 +47,7 @@ stash index (e.g. the integer `n` is equivalent to `stash@{n}`). COMMANDS -------- -push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message ] [--] [...]:: +push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message ] [--pathspec-from-file= [--pathspec-file-nul]] [--] [...]:: Save your local modifications to a new 'stash entry' and roll them back to HEAD (in the working tree and in the index). @@ -194,6 +195,23 @@ to learn how to operate the `--patch` mode. The `--patch` option implies `--keep-index`. You can use `--no-keep-index` to override this. +--pathspec-from-file=:: + This option is only valid for `push` command. ++ +Pathspec is passed in `` instead of commandline args. If +`` is exactly `-` then standard input is used. Pathspec +elements are separated by LF or CR/LF. Pathspec elements can be +quoted as explained for the configuration variable `core.quotePath` +(see linkgit:git-config[1]). See also `--pathspec-file-nul` and +global `--literal-pathspecs`. + +--pathspec-file-nul:: + This option is only valid for `push` command. ++ +Only meaningful with `--pathspec-from-file`. Pathspec elements are +separated with NUL character and all other characters are taken +literally (including newlines and quotes). + -q:: --quiet:: This option is only valid for `apply`, `drop`, `pop`, `push`, diff --git a/builtin/stash.c b/builtin/stash.c index 7bc4c5d696..74d92595a2 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -27,6 +27,7 @@ static const char * const git_stash_usage[] = { N_("git stash clear"), N_("git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n" " [-u|--include-untracked] [-a|--all] [-m|--message ]\n" + " [--pathspec-from-file= [--pathspec-file-nul]]\n" " [--] [...]]"), N_("git stash save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n" " [-u|--include-untracked] [-a|--all] []"), @@ -1456,7 +1457,9 @@ static int push_stash(int argc, const char **argv, const char *prefix, int patch_mode = 0; int include_untracked = 0; int quiet = 0; + int pathspec_file_nul = 0; const char *stash_msg = NULL; + const char *pathspec_from_file = NULL; struct pathspec ps; struct option options[] = { OPT_BOOL('k', "keep-index", &keep_index, @@ -1470,6 +1473,8 @@ static int push_stash(int argc, const char **argv, const char *prefix, N_("include ignore files"), 2), OPT_STRING('m', "message", &stash_msg, N_("message"), N_("stash message")), + OPT_PATHSPEC_FROM_FILE(&pathspec_from_file), + OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul), OPT_END() }; @@ -1492,6 +1497,21 @@ static int push_stash(int argc, const char **argv, const char *prefix, parse_pathspec(&ps, 0, PATHSPEC_PREFER_FULL | PATHSPEC_PREFIX_ORIGIN, prefix, argv); + + if (pathspec_from_file) { + if (patch_mode) + die(_("--pathspec-from-file is incompatible with --patch")); + + if (ps.nr) + die(_("--pathspec-from-file is incompatible with pathspec arguments")); + + parse_pathspec_file(&ps, 0, + PATHSPEC_PREFER_FULL | PATHSPEC_PREFIX_ORIGIN, + prefix, pathspec_from_file, pathspec_file_nul); + } else if (pathspec_file_nul) { + die(_("--pathspec-file-nul requires --pathspec-from-file")); + } + return do_push_stash(&ps, stash_msg, quiet, keep_index, patch_mode, include_untracked); } diff --git a/t/t3909-stash-pathspec-file.sh b/t/t3909-stash-pathspec-file.sh new file mode 100755 index 0000000000..55e050cfd4 --- /dev/null +++ b/t/t3909-stash-pathspec-file.sh @@ -0,0 +1,100 @@ +#!/bin/sh + +test_description='stash --pathspec-from-file' + +. ./test-lib.sh + +test_tick + +test_expect_success setup ' + >fileA.t && + >fileB.t && + >fileC.t && + >fileD.t && + git add fileA.t fileB.t fileC.t fileD.t && + git commit -m "Files" && + + git tag checkpoint +' + +restore_checkpoint () { + git reset --hard checkpoint +} + +verify_expect () { + git stash show --name-status >actual && + test_cmp expect actual +} + +test_expect_success 'simplest' ' + restore_checkpoint && + + # More files are written to make sure that git didnt ignore + # --pathspec-from-file, stashing everything + echo A >fileA.t && + echo B >fileB.t && + echo C >fileC.t && + echo D >fileD.t && + + cat >expect <<-\EOF && + M fileA.t + EOF + + echo fileA.t | git stash push --pathspec-from-file=- && + verify_expect +' + +test_expect_success '--pathspec-file-nul' ' + restore_checkpoint && + + # More files are written to make sure that git didnt ignore + # --pathspec-from-file, stashing everything + echo A >fileA.t && + echo B >fileB.t && + echo C >fileC.t && + echo D >fileD.t && + + cat >expect <<-\EOF && + M fileA.t + M fileB.t + EOF + + printf "fileA.t\0fileB.t\0" | git stash push --pathspec-from-file=- --pathspec-file-nul && + verify_expect +' + +test_expect_success 'only touches what was listed' ' + restore_checkpoint && + + # More files are written to make sure that git didnt ignore + # --pathspec-from-file, stashing everything + echo A >fileA.t && + echo B >fileB.t && + echo C >fileC.t && + echo D >fileD.t && + + cat >expect <<-\EOF && + M fileB.t + M fileC.t + EOF + + printf "fileB.t\nfileC.t\n" | git stash push --pathspec-from-file=- && + verify_expect +' + +test_expect_success 'error conditions' ' + restore_checkpoint && + echo A >fileA.t && + echo fileA.t >list && + + test_must_fail git stash push --pathspec-from-file=list --patch 2>err && + test_i18ngrep -e "--pathspec-from-file is incompatible with --patch" err && + + test_must_fail git stash push --pathspec-from-file=list -- fileA.t 2>err && + test_i18ngrep -e "--pathspec-from-file is incompatible with pathspec arguments" err && + + test_must_fail git stash push --pathspec-file-nul 2>err && + test_i18ngrep -e "--pathspec-file-nul requires --pathspec-from-file" err +' + +test_done From 62e7a6f7a1427f461e72ccd5acebc200d55747e0 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 20 Feb 2020 15:15:15 +0100 Subject: [PATCH 09/27] parse-options: add testcases for OPT_CMDMODE() Before modifying the implementation, ensure that general operation of OPT_CMDMODE() and detection of incompatible options are covered. Signed-off-by: Paolo Bonzini Signed-off-by: Junio C Hamano --- t/helper/test-parse-options.c | 2 ++ t/t0040-parse-options.sh | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c index af82db06ac..2051ce57db 100644 --- a/t/helper/test-parse-options.c +++ b/t/helper/test-parse-options.c @@ -121,6 +121,8 @@ int cmd__parse_options(int argc, const char **argv) OPT_INTEGER('j', NULL, &integer, "get a integer, too"), OPT_MAGNITUDE('m', "magnitude", &magnitude, "get a magnitude"), OPT_SET_INT(0, "set23", &integer, "set integer to 23", 23), + OPT_CMDMODE(0, "mode1", &integer, "set integer to 1 (cmdmode option)", 1), + OPT_CMDMODE(0, "mode2", &integer, "set integer to 2 (cmdmode option)", 2), OPT_CALLBACK('L', "length", &integer, "str", "get length of ", length_callback), OPT_FILENAME('F', "file", &file, "set file to "), diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh index 705a136ed9..5c91464094 100755 --- a/t/t0040-parse-options.sh +++ b/t/t0040-parse-options.sh @@ -23,6 +23,8 @@ usage: test-tool parse-options -j get a integer, too -m, --magnitude get a magnitude --set23 set integer to 23 + --mode1 set integer to 1 (cmdmode option) + --mode2 set integer to 2 (cmdmode option) -L, --length get length of -F, --file set file to @@ -324,6 +326,22 @@ test_expect_success 'OPT_NEGBIT() works' ' test-tool parse-options --expect="boolean: 6" -bb --no-neg-or4 ' +test_expect_success 'OPT_CMDMODE() works' ' + test-tool parse-options --expect="integer: 1" --mode1 +' + +test_expect_success 'OPT_CMDMODE() detects incompatibility' ' + test_must_fail test-tool parse-options --mode1 --mode2 >output 2>output.err && + test_must_be_empty output && + test_i18ngrep "incompatible with --mode" output.err +' + +test_expect_success 'OPT_CMDMODE() detects incompatibility with something else' ' + test_must_fail test-tool parse-options --set23 --mode2 >output 2>output.err && + test_must_be_empty output && + test_i18ngrep "incompatible with something else" output.err +' + test_expect_success 'OPT_COUNTUP() with PARSE_OPT_NODASH works' ' test-tool parse-options --expect="boolean: 6" + + + + + + ' From bc8620b44094e9fb6fc20d141678f9b845c19193 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 20 Feb 2020 15:15:16 +0100 Subject: [PATCH 10/27] parse-options: convert "command mode" to a flag OPTION_CMDMODE is essentially OPTION_SET_INT plus an extra check that the variable had not set before. In order to allow custom processing of the option, for example a "command mode" option that also has an argument, it would be nice to use OPTION_CALLBACK and not have to rewrite the extra check on incompatible options. In other words, making the processing of the option orthogonal to the "only one of these" behavior provided by OPTION_CMDMODE. Add a new flag that takes care of the check, and modify OPT_CMDMODE to use it together with OPTION_SET_INT. The new flag still requires that the option value points to an int, but any OPTION_* value can be specified as long as it does not require a non-int type for opt->value. Signed-off-by: Paolo Bonzini Signed-off-by: Junio C Hamano --- parse-options.c | 20 +++++++++----------- parse-options.h | 8 ++++---- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/parse-options.c b/parse-options.c index 60fae3ad21..c51be1860f 100644 --- a/parse-options.c +++ b/parse-options.c @@ -61,7 +61,7 @@ static enum parse_opt_result opt_command_mode_error( */ for (that = all_opts; that->type != OPTION_END; that++) { if (that == opt || - that->type != OPTION_CMDMODE || + !(that->flags & PARSE_OPT_CMDMODE) || that->value != opt->value || that->defval != *(int *)opt->value) continue; @@ -95,6 +95,14 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p, if (!(flags & OPT_SHORT) && p->opt && (opt->flags & PARSE_OPT_NOARG)) return error(_("%s takes no value"), optname(opt, flags)); + /* + * Giving the same mode option twice, although unnecessary, + * is not a grave error, so let it pass. + */ + if ((opt->flags & PARSE_OPT_CMDMODE) && + *(int *)opt->value && *(int *)opt->value != opt->defval) + return opt_command_mode_error(opt, all_opts, flags); + switch (opt->type) { case OPTION_LOWLEVEL_CALLBACK: return opt->ll_callback(p, opt, NULL, unset); @@ -130,16 +138,6 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p, *(int *)opt->value = unset ? 0 : opt->defval; return 0; - case OPTION_CMDMODE: - /* - * Giving the same mode option twice, although is unnecessary, - * is not a grave error, so let it pass. - */ - if (*(int *)opt->value && *(int *)opt->value != opt->defval) - return opt_command_mode_error(opt, all_opts, flags); - *(int *)opt->value = opt->defval; - return 0; - case OPTION_STRING: if (unset) *(const char **)opt->value = NULL; diff --git a/parse-options.h b/parse-options.h index fdc0c1cb97..05bf8245de 100644 --- a/parse-options.h +++ b/parse-options.h @@ -18,7 +18,6 @@ enum parse_opt_type { OPTION_BITOP, OPTION_COUNTUP, OPTION_SET_INT, - OPTION_CMDMODE, /* options with arguments (usually) */ OPTION_STRING, OPTION_INTEGER, @@ -47,7 +46,8 @@ enum parse_opt_option_flags { PARSE_OPT_LITERAL_ARGHELP = 64, PARSE_OPT_SHELL_EVAL = 256, PARSE_OPT_NOCOMPLETE = 512, - PARSE_OPT_COMP_ARG = 1024 + PARSE_OPT_COMP_ARG = 1024, + PARSE_OPT_CMDMODE = 2048 }; enum parse_opt_result { @@ -168,8 +168,8 @@ struct option { #define OPT_BOOL(s, l, v, h) OPT_BOOL_F(s, l, v, h, 0) #define OPT_HIDDEN_BOOL(s, l, v, h) { OPTION_SET_INT, (s), (l), (v), NULL, \ (h), PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 1} -#define OPT_CMDMODE(s, l, v, h, i) { OPTION_CMDMODE, (s), (l), (v), NULL, \ - (h), PARSE_OPT_NOARG|PARSE_OPT_NONEG, NULL, (i) } +#define OPT_CMDMODE(s, l, v, h, i) { OPTION_SET_INT, (s), (l), (v), NULL, \ + (h), PARSE_OPT_CMDMODE|PARSE_OPT_NOARG|PARSE_OPT_NONEG, NULL, (i) } #define OPT_INTEGER(s, l, v, h) OPT_INTEGER_F(s, l, v, h, 0) #define OPT_MAGNITUDE(s, l, v, h) { OPTION_MAGNITUDE, (s), (l), (v), \ N_("n"), (h), PARSE_OPT_NONEG } From e8ef1e8d6edd835fcea7f88d2740d756a1353190 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 20 Feb 2020 15:15:17 +0100 Subject: [PATCH 11/27] am: convert "resume" variable to a struct This will allow stashing the submode of --show-current-patch from a callback function. Using a struct will allow accessing both fields from outside cmd_am (through container_of). Signed-off-by: Paolo Bonzini Signed-off-by: Junio C Hamano --- builtin/am.c | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 8181c2aef3..bd3cda8bec 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -2118,7 +2118,7 @@ static int parse_opt_patchformat(const struct option *opt, const char *arg, int return 0; } -enum resume_mode { +enum resume_type { RESUME_FALSE = 0, RESUME_APPLY, RESUME_RESOLVED, @@ -2128,6 +2128,10 @@ enum resume_mode { RESUME_SHOW_PATCH }; +struct resume_mode { + enum resume_type mode; +}; + static int git_am_config(const char *k, const char *v, void *cb) { int status; @@ -2145,7 +2149,7 @@ int cmd_am(int argc, const char **argv, const char *prefix) int binary = -1; int keep_cr = -1; int patch_format = PATCH_FORMAT_UNKNOWN; - enum resume_mode resume = RESUME_FALSE; + struct resume_mode resume = { .mode = RESUME_FALSE }; int in_progress; int ret = 0; @@ -2214,22 +2218,22 @@ int cmd_am(int argc, const char **argv, const char *prefix) PARSE_OPT_NOARG), OPT_STRING(0, "resolvemsg", &state.resolvemsg, NULL, N_("override error message when patch failure occurs")), - OPT_CMDMODE(0, "continue", &resume, + OPT_CMDMODE(0, "continue", &resume.mode, N_("continue applying patches after resolving a conflict"), RESUME_RESOLVED), - OPT_CMDMODE('r', "resolved", &resume, + OPT_CMDMODE('r', "resolved", &resume.mode, N_("synonyms for --continue"), RESUME_RESOLVED), - OPT_CMDMODE(0, "skip", &resume, + OPT_CMDMODE(0, "skip", &resume.mode, N_("skip the current patch"), RESUME_SKIP), - OPT_CMDMODE(0, "abort", &resume, + OPT_CMDMODE(0, "abort", &resume.mode, N_("restore the original branch and abort the patching operation."), RESUME_ABORT), - OPT_CMDMODE(0, "quit", &resume, + OPT_CMDMODE(0, "quit", &resume.mode, N_("abort the patching operation but keep HEAD where it is."), RESUME_QUIT), - OPT_CMDMODE(0, "show-current-patch", &resume, + OPT_CMDMODE(0, "show-current-patch", &resume.mode, N_("show the patch being applied."), RESUME_SHOW_PATCH), OPT_BOOL(0, "committer-date-is-author-date", @@ -2281,12 +2285,12 @@ int cmd_am(int argc, const char **argv, const char *prefix) * intend to feed us a patch but wanted to continue * unattended. */ - if (argc || (resume == RESUME_FALSE && !isatty(0))) + if (argc || (resume.mode == RESUME_FALSE && !isatty(0))) die(_("previous rebase directory %s still exists but mbox given."), state.dir); - if (resume == RESUME_FALSE) - resume = RESUME_APPLY; + if (resume.mode == RESUME_FALSE) + resume.mode = RESUME_APPLY; if (state.signoff == SIGNOFF_EXPLICIT) am_append_signoff(&state); @@ -2300,7 +2304,7 @@ int cmd_am(int argc, const char **argv, const char *prefix) * stray directories. */ if (file_exists(state.dir) && !state.rebasing) { - if (resume == RESUME_ABORT || resume == RESUME_QUIT) { + if (resume.mode == RESUME_ABORT || resume.mode == RESUME_QUIT) { am_destroy(&state); am_state_release(&state); return 0; @@ -2311,7 +2315,7 @@ int cmd_am(int argc, const char **argv, const char *prefix) state.dir); } - if (resume) + if (resume.mode) die(_("Resolve operation not in progress, we are not resuming.")); for (i = 0; i < argc; i++) { @@ -2329,7 +2333,7 @@ int cmd_am(int argc, const char **argv, const char *prefix) argv_array_clear(&paths); } - switch (resume) { + switch (resume.mode) { case RESUME_FALSE: am_run(&state, 0); break; From f3b4822899dc4ddca688de285f01e1d12aeb33b6 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 20 Feb 2020 15:15:18 +0100 Subject: [PATCH 12/27] am: support --show-current-patch=raw as a synonym for--show-current-patch When "git am --show-current-patch" was added in commit 984913a210 ("am: add --show-current-patch", 2018-02-12), "git am" started recommending it as a replacement for .git/rebase-merge/patch. Unfortunately the suggestion is somewhat misguided; for example, the output "git am --show-current-patch" cannot be passed to "git apply" if it is encoded as quoted-printable or base64. To simplify worktree operations and to avoid that users poke into .git, it would be better if "git am" also provided a mode that copies .git/rebase-merge/patch to stdout. One possibility could be to have completely separate options, introducing for example --show-current-message (for .git/rebase-apply/NNNN) and --show-current-diff (for .git/rebase-apply/patch), while possibly deprecating --show-current-patch. That would even remove the need for the first two patches in the series. However, the long common prefix would have prevented using an abbreviated option such as "--show". Therefore, I chose instead to add a string argument to --show-current-patch. The new argument is optional, so that "git am --show-current-patch"'s behavior remains backwards-compatible. The next choice to make is how to handle multiple --show-current-patch options. Right now, something like "git am --abort --show-current-patch" is rejected, and the previous suggestion would likewise have naturally rejected a command line like git am --show-current-message --show-current-diff Therefore, I decided to also reject for example git am --show-current-patch=diff --show-current-patch=raw In other words the whole of --show-current-patch=xxx (including the optional argument) is treated as the command mode. I found this to be more consistent and intuitive, even though it differs from the usual "last one wins" semantics of the git command line. Add the code to parse submodes based on the above design, where for now "raw" is the only valid submode. "raw" prints the full e-mail message just like "git am --show-current-patch". Signed-off-by: Paolo Bonzini Signed-off-by: Junio C Hamano --- Documentation/git-am.txt | 9 ++-- builtin/am.c | 59 +++++++++++++++++++++++--- contrib/completion/git-completion.bash | 5 +++ t/t4150-am.sh | 10 +++++ 4 files changed, 73 insertions(+), 10 deletions(-) diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt index 11ca61b00b..590b711536 100644 --- a/Documentation/git-am.txt +++ b/Documentation/git-am.txt @@ -16,7 +16,7 @@ SYNOPSIS [--exclude=] [--include=] [--reject] [-q | --quiet] [--[no-]scissors] [-S[]] [--patch-format=] [( | )...] -'git am' (--continue | --skip | --abort | --quit | --show-current-patch) +'git am' (--continue | --skip | --abort | --quit | --show-current-patch[=raw]) DESCRIPTION ----------- @@ -176,9 +176,10 @@ default. You can use `--no-utf8` to override this. Abort the patching operation but keep HEAD and the index untouched. ---show-current-patch:: - Show the entire e-mail message "git am" has stopped at, because - of conflicts. +--show-current-patch[=raw]:: + Show the raw contents of the e-mail message at which `git am` + has stopped due to conflicts. The argument must be omitted or + `raw`. DISCUSSION ---------- diff --git a/builtin/am.c b/builtin/am.c index bd3cda8bec..54b04da86d 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -81,6 +81,10 @@ enum signoff_type { SIGNOFF_EXPLICIT /* --signoff was set on the command-line */ }; +enum show_patch_type { + SHOW_PATCH_RAW = 0, +}; + struct am_state { /* state directory path */ char *dir; @@ -2061,7 +2065,7 @@ static void am_abort(struct am_state *state) am_destroy(state); } -static int show_patch(struct am_state *state) +static int show_patch(struct am_state *state, enum show_patch_type sub_mode) { struct strbuf sb = STRBUF_INIT; const char *patch_path; @@ -2078,7 +2082,14 @@ static int show_patch(struct am_state *state) return ret; } - patch_path = am_path(state, msgnum(state)); + switch (sub_mode) { + case SHOW_PATCH_RAW: + patch_path = am_path(state, msgnum(state)); + break; + default: + BUG("invalid mode for --show-current-patch"); + } + len = strbuf_read_file(&sb, patch_path, 0); if (len < 0) die_errno(_("failed to read '%s'"), patch_path); @@ -2130,8 +2141,42 @@ enum resume_type { struct resume_mode { enum resume_type mode; + enum show_patch_type sub_mode; }; +static int parse_opt_show_current_patch(const struct option *opt, const char *arg, int unset) +{ + int *opt_value = opt->value; + struct resume_mode *resume = container_of(opt_value, struct resume_mode, mode); + + /* + * Please update $__git_showcurrentpatch in git-completion.bash + * when you add new options + */ + const char *valid_modes[] = { + [SHOW_PATCH_RAW] = "raw" + }; + int new_value = SHOW_PATCH_RAW; + + if (arg) { + for (new_value = 0; new_value < ARRAY_SIZE(valid_modes); new_value++) { + if (!strcmp(arg, valid_modes[new_value])) + break; + } + if (new_value >= ARRAY_SIZE(valid_modes)) + return error(_("Invalid value for --show-current-patch: %s"), arg); + } + + if (resume->mode == RESUME_SHOW_PATCH && new_value != resume->sub_mode) + return error(_("--show-current-patch=%s is incompatible with " + "--show-current-patch=%s"), + arg, valid_modes[resume->sub_mode]); + + resume->mode = RESUME_SHOW_PATCH; + resume->sub_mode = new_value; + return 0; +} + static int git_am_config(const char *k, const char *v, void *cb) { int status; @@ -2233,9 +2278,11 @@ int cmd_am(int argc, const char **argv, const char *prefix) OPT_CMDMODE(0, "quit", &resume.mode, N_("abort the patching operation but keep HEAD where it is."), RESUME_QUIT), - OPT_CMDMODE(0, "show-current-patch", &resume.mode, - N_("show the patch being applied."), - RESUME_SHOW_PATCH), + { OPTION_CALLBACK, 0, "show-current-patch", &resume.mode, + "raw", + N_("show the patch being applied"), + PARSE_OPT_CMDMODE | PARSE_OPT_OPTARG | PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP, + parse_opt_show_current_patch, RESUME_SHOW_PATCH }, OPT_BOOL(0, "committer-date-is-author-date", &state.committer_date_is_author_date, N_("lie about committer date")), @@ -2354,7 +2401,7 @@ int cmd_am(int argc, const char **argv, const char *prefix) am_destroy(&state); break; case RESUME_SHOW_PATCH: - ret = show_patch(&state); + ret = show_patch(&state, resume.sub_mode); break; default: BUG("invalid resume value"); diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index e4d9ff4a95..2673fc162c 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1180,6 +1180,7 @@ __git_count_arguments () __git_whitespacelist="nowarn warn error error-all fix" __git_patchformat="mbox stgit stgit-series hg mboxrd" +__git_showcurrentpatch="raw" __git_am_inprogress_options="--skip --continue --resolved --abort --quit --show-current-patch" _git_am () @@ -1198,6 +1199,10 @@ _git_am () __gitcomp "$__git_patchformat" "" "${cur##--patch-format=}" return ;; + --show-current-patch=*) + __gitcomp "$__git_showcurrentpatch" "" "${cur##--show-current-patch=}" + return + ;; --*) __gitcomp_builtin am "" \ "$__git_am_inprogress_options" diff --git a/t/t4150-am.sh b/t/t4150-am.sh index 4f1e24ecbe..afe456e75e 100755 --- a/t/t4150-am.sh +++ b/t/t4150-am.sh @@ -666,6 +666,16 @@ test_expect_success 'am --show-current-patch' ' test_cmp .git/rebase-apply/0001 actual.patch ' +test_expect_success 'am --show-current-patch=raw' ' + git am --show-current-patch=raw >actual.patch && + test_cmp .git/rebase-apply/0001 actual.patch +' + +test_expect_success 'am accepts repeated --show-current-patch' ' + git am --show-current-patch --show-current-patch=raw >actual.patch && + test_cmp .git/rebase-apply/0001 actual.patch +' + test_expect_success 'am --skip works' ' echo goodbye >expected && git am --skip && From aa416b22ea73321aba94c3c96db0491fb25ea7ea Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 20 Feb 2020 15:15:19 +0100 Subject: [PATCH 13/27] am: support --show-current-patch=diff to retrieve .git/rebase-apply/patch When "git am --show-current-patch" was added in commit 984913a210 ("am: add --show-current-patch", 2018-02-12), "git am" started recommending it as a replacement for .git/rebase-merge/patch. Unfortunately the suggestion is somewhat misguided; for example, the output of "git am --show-current-patch" cannot be passed to "git apply" if it is encoded as quoted-printable or base64. Add a new mode to "git am --show-current-patch" in order to straighten the suggestion. Reported-by: J. Bruce Fields Signed-off-by: Paolo Bonzini Signed-off-by: Junio C Hamano --- Documentation/git-am.txt | 11 ++++++----- builtin/am.c | 9 +++++++-- contrib/completion/git-completion.bash | 2 +- t/t4150-am.sh | 10 ++++++++++ 4 files changed, 24 insertions(+), 8 deletions(-) diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt index 590b711536..ab5754e05d 100644 --- a/Documentation/git-am.txt +++ b/Documentation/git-am.txt @@ -16,7 +16,7 @@ SYNOPSIS [--exclude=] [--include=] [--reject] [-q | --quiet] [--[no-]scissors] [-S[]] [--patch-format=] [( | )...] -'git am' (--continue | --skip | --abort | --quit | --show-current-patch[=raw]) +'git am' (--continue | --skip | --abort | --quit | --show-current-patch[=(diff|raw)]) DESCRIPTION ----------- @@ -176,10 +176,11 @@ default. You can use `--no-utf8` to override this. Abort the patching operation but keep HEAD and the index untouched. ---show-current-patch[=raw]:: - Show the raw contents of the e-mail message at which `git am` - has stopped due to conflicts. The argument must be omitted or - `raw`. +--show-current-patch[=(diff|raw)]:: + Show the message at which `git am` has stopped due to + conflicts. If `raw` is specified, show the raw contents of + the e-mail message; if `diff`, show the diff portion only. + Defaults to `raw`. DISCUSSION ---------- diff --git a/builtin/am.c b/builtin/am.c index 54b04da86d..e3dfd93c25 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -83,6 +83,7 @@ enum signoff_type { enum show_patch_type { SHOW_PATCH_RAW = 0, + SHOW_PATCH_DIFF = 1, }; struct am_state { @@ -1767,7 +1768,7 @@ static void am_run(struct am_state *state, int resume) linelen(state->msg), state->msg); if (advice_amworkdir) - advise(_("Use 'git am --show-current-patch' to see the failed patch")); + advise(_("Use 'git am --show-current-patch=diff' to see the failed patch")); die_user_resolve(state); } @@ -2086,6 +2087,9 @@ static int show_patch(struct am_state *state, enum show_patch_type sub_mode) case SHOW_PATCH_RAW: patch_path = am_path(state, msgnum(state)); break; + case SHOW_PATCH_DIFF: + patch_path = am_path(state, "patch"); + break; default: BUG("invalid mode for --show-current-patch"); } @@ -2154,6 +2158,7 @@ static int parse_opt_show_current_patch(const struct option *opt, const char *ar * when you add new options */ const char *valid_modes[] = { + [SHOW_PATCH_DIFF] = "diff", [SHOW_PATCH_RAW] = "raw" }; int new_value = SHOW_PATCH_RAW; @@ -2279,7 +2284,7 @@ int cmd_am(int argc, const char **argv, const char *prefix) N_("abort the patching operation but keep HEAD where it is."), RESUME_QUIT), { OPTION_CALLBACK, 0, "show-current-patch", &resume.mode, - "raw", + "(diff|raw)", N_("show the patch being applied"), PARSE_OPT_CMDMODE | PARSE_OPT_OPTARG | PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP, parse_opt_show_current_patch, RESUME_SHOW_PATCH }, diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 2673fc162c..b33c23b717 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1180,7 +1180,7 @@ __git_count_arguments () __git_whitespacelist="nowarn warn error error-all fix" __git_patchformat="mbox stgit stgit-series hg mboxrd" -__git_showcurrentpatch="raw" +__git_showcurrentpatch="diff raw" __git_am_inprogress_options="--skip --continue --resolved --abort --quit --show-current-patch" _git_am () diff --git a/t/t4150-am.sh b/t/t4150-am.sh index afe456e75e..cb45271457 100755 --- a/t/t4150-am.sh +++ b/t/t4150-am.sh @@ -671,11 +671,21 @@ test_expect_success 'am --show-current-patch=raw' ' test_cmp .git/rebase-apply/0001 actual.patch ' +test_expect_success 'am --show-current-patch=diff' ' + git am --show-current-patch=diff >actual.patch && + test_cmp .git/rebase-apply/patch actual.patch +' + test_expect_success 'am accepts repeated --show-current-patch' ' git am --show-current-patch --show-current-patch=raw >actual.patch && test_cmp .git/rebase-apply/0001 actual.patch ' +test_expect_success 'am detects incompatible --show-current-patch' ' + test_must_fail git am --show-current-patch=raw --show-current-patch=diff && + test_must_fail git am --show-current-patch --show-current-patch=diff +' + test_expect_success 'am --skip works' ' echo goodbye >expected && git am --skip && From a51d9e8f07652ded830cb50187be74c1709816d8 Mon Sep 17 00:00:00 2001 From: Rasmus Jonsson Date: Sun, 23 Feb 2020 01:50:49 +0100 Subject: [PATCH 14/27] t1050: replace test -f with test_path_is_file Use test_path_is_file() instead of 'test -f' for better debugging information. Signed-off-by: Rasmus Jonsson Signed-off-by: Junio C Hamano --- t/t1050-large.sh | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/t/t1050-large.sh b/t/t1050-large.sh index d3b2adb28b..184b479a21 100755 --- a/t/t1050-large.sh +++ b/t/t1050-large.sh @@ -53,7 +53,8 @@ test_expect_success 'add a large file or two' ' for p in .git/objects/pack/pack-*.pack do count=$(( $count + 1 )) - if test -f "$p" && idx=${p%.pack}.idx && test -f "$idx" + if test_path_is_file "$p" && + idx=${p%.pack}.idx && test_path_is_file "$idx" then continue fi @@ -65,7 +66,7 @@ test_expect_success 'add a large file or two' ' test $cnt = 2 && for l in .git/objects/??/?????????????????????????????????????? do - test -f "$l" || continue + test_path_is_file "$l" || continue bad=t done && test -z "$bad" && @@ -76,7 +77,8 @@ test_expect_success 'add a large file or two' ' for p in .git/objects/pack/pack-*.pack do count=$(( $count + 1 )) - if test -f "$p" && idx=${p%.pack}.idx && test -f "$idx" + if test_path_is_file "$p" && + idx=${p%.pack}.idx && test_path_is_file "$idx" then continue fi @@ -111,7 +113,7 @@ test_expect_success 'packsize limit' ' count=0 && for pi in .git/objects/pack/pack-*.idx do - test -f "$pi" && count=$(( $count + 1 )) + test_path_is_file "$pi" && count=$(( $count + 1 )) done && test $count = 2 && From fd0bc175576d74faf5c7bfe626c09cfea884cc6a Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 21 Feb 2020 12:15:45 -0800 Subject: [PATCH 15/27] completion: add diff --color-moved[-ws] These options are available since git v2.15, but somehow eluded from the completion script. Note that while --color-moved-ws= accepts comma-separated list of values, there is no (easy?) way to make it work with completion (see e.g. [1]). [1]: https://github.com/scop/bash-completion/issues/240 Acked-by: Matheus Tavares Bernardino Signed-off-by: Kir Kolyshkin Signed-off-by: Junio C Hamano --- contrib/completion/git-completion.bash | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index e4d9ff4a95..e3187c3d92 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1470,9 +1470,16 @@ __git_diff_algorithms="myers minimal patience histogram" __git_diff_submodule_formats="diff log short" +__git_color_moved_opts="no default plain blocks zebra dimmed-zebra" + +__git_color_moved_ws_opts="no ignore-space-at-eol ignore-space-change + ignore-all-space allow-indentation-change" + __git_diff_common_options="--stat --numstat --shortstat --summary --patch-with-stat --name-only --name-status --color --no-color --color-words --no-renames --check + --color-moved --color-moved= --no-color-moved + --color-moved-ws= --no-color-moved-ws --full-index --binary --abbrev --diff-filter= --find-copies-harder --ignore-cr-at-eol --text --ignore-space-at-eol --ignore-space-change @@ -1503,6 +1510,14 @@ _git_diff () __gitcomp "$__git_diff_submodule_formats" "" "${cur##--submodule=}" return ;; + --color-moved=*) + __gitcomp "$__git_color_moved_opts" "" "${cur##--color-moved=}" + return + ;; + --color-moved-ws=*) + __gitcomp "$__git_color_moved_ws_opts" "" "${cur##--color-moved-ws=}" + return + ;; --*) __gitcomp "--cached --staged --pickaxe-all --pickaxe-regex --base --ours --theirs --no-index From 802050400a51270bd2b4a13e410412693975c766 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 27 Feb 2020 00:05:05 +0000 Subject: [PATCH 16/27] merge-recursive: apply collision handling unification to recursive case In the en/merge-path-collision topic (see commit ac193e0e0aa5, "Merge branch 'en/merge-path-collision'", 2019-01-04), all the "file collision" conflict types were modified for consistency. In particular, rename/add, rename/rename(2to1) and each rename/add piece of a rename/rename(1to2)/add[/add] conflict were made to behave like add/add conflicts have always been handled. However, this consistency was not enforced when opt->priv->call_depth > 0 for rename/rename conflicts. Update rename/rename(1to2) and rename/rename(2to1) conflicts in the recursive case to also be consistent. As an added bonus, this simplifies the code considerably. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 152 +++++++++--------------------- t/t6036-recursive-corner-cases.sh | 39 ++++++-- 2 files changed, 77 insertions(+), 114 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index aee1769a7a..3728b3f659 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1557,35 +1557,6 @@ static int handle_file_collision(struct merge_options *opt, b, a); } - /* - * In the recursive case, we just opt to undo renames - */ - if (opt->priv->call_depth && (prev_path1 || prev_path2)) { - /* Put first file (a->oid, a->mode) in its original spot */ - if (prev_path1) { - if (update_file(opt, 1, a, prev_path1)) - return -1; - } else { - if (update_file(opt, 1, a, collide_path)) - return -1; - } - - /* Put second file (b->oid, b->mode) in its original spot */ - if (prev_path2) { - if (update_file(opt, 1, b, prev_path2)) - return -1; - } else { - if (update_file(opt, 1, b, collide_path)) - return -1; - } - - /* Don't leave something at collision path if unrenaming both */ - if (prev_path1 && prev_path2) - remove_file(opt, 1, collide_path, 0); - - return 0; - } - /* Remove rename sources if rename/add or rename/rename(2to1) */ if (prev_path1) remove_file(opt, 1, prev_path1, @@ -1746,85 +1717,56 @@ static int handle_rename_rename_1to2(struct merge_options *opt, return -1; free(path_desc); - if (opt->priv->call_depth) { - /* - * FIXME: For rename/add-source conflicts (if we could detect - * such), this is wrong. We should instead find a unique - * pathname and then either rename the add-source file to that - * unique path, or use that unique path instead of src here. - */ - if (update_file(opt, 0, &mfi.blob, o->path)) + if (opt->priv->call_depth) + remove_file_from_index(opt->repo->index, o->path); + + /* + * For each destination path, we need to see if there is a + * rename/add collision. If not, we can write the file out + * to the specified location. + */ + add = &ci->ren1->dst_entry->stages[flip_stage(2)]; + if (is_valid(add)) { + add->path = mfi.blob.path = a->path; + if (handle_file_collision(opt, a->path, + NULL, NULL, + ci->ren1->branch, + ci->ren2->branch, + &mfi.blob, add) < 0) return -1; - - /* - * Above, we put the merged content at the merge-base's - * path. Now we usually need to delete both a->path and - * b->path. However, the rename on each side of the merge - * could also be involved in a rename/add conflict. In - * such cases, we should keep the added file around, - * resolving the conflict at that path in its favor. - */ - add = &ci->ren1->dst_entry->stages[flip_stage(2)]; - if (is_valid(add)) { - if (update_file(opt, 0, add, a->path)) - return -1; - } - else - remove_file_from_index(opt->repo->index, a->path); - add = &ci->ren2->dst_entry->stages[flip_stage(3)]; - if (is_valid(add)) { - if (update_file(opt, 0, add, b->path)) - return -1; - } - else - remove_file_from_index(opt->repo->index, b->path); } else { - /* - * For each destination path, we need to see if there is a - * rename/add collision. If not, we can write the file out - * to the specified location. - */ - add = &ci->ren1->dst_entry->stages[flip_stage(2)]; - if (is_valid(add)) { - add->path = mfi.blob.path = a->path; - if (handle_file_collision(opt, a->path, - NULL, NULL, - ci->ren1->branch, - ci->ren2->branch, - &mfi.blob, add) < 0) - return -1; - } else { - char *new_path = find_path_for_conflict(opt, a->path, - ci->ren1->branch, - ci->ren2->branch); - if (update_file(opt, 0, &mfi.blob, - new_path ? new_path : a->path)) - return -1; - free(new_path); - if (update_stages(opt, a->path, NULL, a, NULL)) - return -1; - } + char *new_path = find_path_for_conflict(opt, a->path, + ci->ren1->branch, + ci->ren2->branch); + if (update_file(opt, 0, &mfi.blob, + new_path ? new_path : a->path)) + return -1; + free(new_path); + if (!opt->priv->call_depth && + update_stages(opt, a->path, NULL, a, NULL)) + return -1; + } - add = &ci->ren2->dst_entry->stages[flip_stage(3)]; - if (is_valid(add)) { - add->path = mfi.blob.path = b->path; - if (handle_file_collision(opt, b->path, - NULL, NULL, - ci->ren1->branch, - ci->ren2->branch, - add, &mfi.blob) < 0) - return -1; - } else { - char *new_path = find_path_for_conflict(opt, b->path, - ci->ren2->branch, - ci->ren1->branch); - if (update_file(opt, 0, &mfi.blob, - new_path ? new_path : b->path)) - return -1; - free(new_path); - if (update_stages(opt, b->path, NULL, NULL, b)) - return -1; - } + add = &ci->ren2->dst_entry->stages[flip_stage(3)]; + if (is_valid(add)) { + add->path = mfi.blob.path = b->path; + if (handle_file_collision(opt, b->path, + NULL, NULL, + ci->ren1->branch, + ci->ren2->branch, + add, &mfi.blob) < 0) + return -1; + } else { + char *new_path = find_path_for_conflict(opt, b->path, + ci->ren2->branch, + ci->ren1->branch); + if (update_file(opt, 0, &mfi.blob, + new_path ? new_path : b->path)) + return -1; + free(new_path); + if (!opt->priv->call_depth && + update_stages(opt, b->path, NULL, NULL, b)) + return -1; } return 0; diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh index 7d73afdcda..b3bf462617 100755 --- a/t/t6036-recursive-corner-cases.sh +++ b/t/t6036-recursive-corner-cases.sh @@ -60,9 +60,9 @@ test_expect_success 'merge simple rename+criss-cross with no modifications' ' test_must_fail git merge -s recursive R2^0 && git ls-files -s >out && - test_line_count = 2 out && + test_line_count = 5 out && git ls-files -u >out && - test_line_count = 2 out && + test_line_count = 3 out && git ls-files -o >out && test_line_count = 1 out && @@ -133,9 +133,9 @@ test_expect_success 'merge criss-cross + rename merges with basic modification' test_must_fail git merge -s recursive R2^0 && git ls-files -s >out && - test_line_count = 2 out && + test_line_count = 5 out && git ls-files -u >out && - test_line_count = 2 out && + test_line_count = 3 out && git ls-files -o >out && test_line_count = 1 out && @@ -218,8 +218,18 @@ test_expect_success 'git detects differently handled merges conflict' ' git ls-files -o >out && test_line_count = 1 out && - git rev-parse >expect \ - C:new_a D:new_a E:new_a && + git cat-file -p C:new_a >ours && + git cat-file -p C:a >theirs && + >empty && + test_must_fail git merge-file \ + -L "Temporary merge branch 1" \ + -L "" \ + -L "Temporary merge branch 2" \ + ours empty theirs && + sed -e "s/^\([<=>]\)/\1\1\1/" ours >ours-tweaked && + git hash-object ours-tweaked >expect && + git rev-parse >>expect \ + D:new_a E:new_a && git rev-parse >actual \ :1:new_a :2:new_a :3:new_a && test_cmp expect actual && @@ -257,7 +267,8 @@ test_expect_success 'git detects differently handled merges conflict, swapped' ' ctime=$(git log --no-walk --date=raw --format=%cd C | awk "{print \$1}") && newctime=$(($btime+1)) && git fast-export --no-data --all | sed -e s/$ctime/$newctime/ | git fast-import --force --quiet && - # End of differences; rest is copy-paste of last test + # End of most differences; rest is copy-paste of last test, + # other than swapping C:a and C:new_a due to order switch git checkout D^0 && test_must_fail git merge -s recursive E^0 && @@ -269,8 +280,18 @@ test_expect_success 'git detects differently handled merges conflict, swapped' ' git ls-files -o >out && test_line_count = 1 out && - git rev-parse >expect \ - C:new_a D:new_a E:new_a && + git cat-file -p C:a >ours && + git cat-file -p C:new_a >theirs && + >empty && + test_must_fail git merge-file \ + -L "Temporary merge branch 1" \ + -L "" \ + -L "Temporary merge branch 2" \ + ours empty theirs && + sed -e "s/^\([<=>]\)/\1\1\1/" ours >ours-tweaked && + git hash-object ours-tweaked >expect && + git rev-parse >>expect \ + D:new_a E:new_a && git rev-parse >actual \ :1:new_a :2:new_a :3:new_a && test_cmp expect actual && From 42d180dd01d5cc2017e6001d8d9dc2b3ee496713 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 27 Feb 2020 00:14:20 +0000 Subject: [PATCH 17/27] t602[1236], t6034: modernize test formatting Indent code, and include it inside test_expect* blocks. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- t/t6021-merge-criss-cross.sh | 127 +++----- t/t6022-merge-rename.sh | 313 +++++++++--------- t/t6023-merge-file.sh | 556 +++++++++++++++++--------------- t/t6026-merge-attr.sh | 46 +-- t/t6034-merge-rename-nocruft.sh | 132 ++++---- 5 files changed, 585 insertions(+), 589 deletions(-) diff --git a/t/t6021-merge-criss-cross.sh b/t/t6021-merge-criss-cross.sh index d254e020b6..9d5e992878 100755 --- a/t/t6021-merge-criss-cross.sh +++ b/t/t6021-merge-criss-cross.sh @@ -10,87 +10,58 @@ test_description='Test criss-cross merge' . ./test-lib.sh -test_expect_success 'prepare repository' \ -'echo "1 -2 -3 -4 -5 -6 -7 -8 -9" > file && -git add file && -git commit -m "Initial commit" file && -git branch A && -git branch B && -git checkout A && -echo "1 -2 -3 -4 -5 -6 -7 -8 changed in B8, branch A -9" > file && -git commit -m "B8" file && -git checkout B && -echo "1 -2 -3 changed in C3, branch B -4 -5 -6 -7 -8 -9 -" > file && -git commit -m "C3" file && -git branch C3 && -git merge -m "pre E3 merge" A && -echo "1 -2 -3 changed in E3, branch B. New file size -4 -5 -6 -7 -8 changed in B8, branch A -9 -" > file && -git commit -m "E3" file && -git checkout A && -git merge -m "pre D8 merge" C3 && -echo "1 -2 -3 changed in C3, branch B -4 -5 -6 -7 -8 changed in D8, branch A. New file size 2 -9" > file && -git commit -m D8 file' +test_expect_success 'prepare repository' ' + test_write_lines 1 2 3 4 5 6 7 8 9 >file && + git add file && + git commit -m "Initial commit" file && -test_expect_success 'Criss-cross merge' 'git merge -m "final merge" B' + git branch A && + git branch B && + git checkout A && -cat > file-expect <file && + git commit -m "B8" file && + git checkout B && -test_expect_success 'Criss-cross merge result' 'cmp file file-expect' + test_write_lines 1 2 "3 changed in C3, branch B" 4 5 6 7 8 9 >file && + git commit -m "C3" file && + git branch C3 && -test_expect_success 'Criss-cross merge fails (-s resolve)' \ -'git reset --hard A^ && -test_must_fail git merge -s resolve -m "final merge" B' + git merge -m "pre E3 merge" A && + + test_write_lines 1 2 "3 changed in E3, branch B. New file size" 4 5 6 7 "8 changed in B8, branch A" 9 >file && + git commit -m "E3" file && + + git checkout A && + git merge -m "pre D8 merge" C3 && + test_write_lines 1 2 "3 changed in C3, branch B" 4 5 6 7 "8 changed in D8, branch A. New file size 2" 9 >file && + + git commit -m D8 file +' + +test_expect_success 'Criss-cross merge' ' + git merge -m "final merge" B +' + +test_expect_success 'Criss-cross merge result' ' + cat <<-\EOF >file-expect && + 1 + 2 + 3 changed in E3, branch B. New file size + 4 + 5 + 6 + 7 + 8 changed in D8, branch A. New file size 2 + 9 + EOF + + test_cmp file-expect file +' + +test_expect_success 'Criss-cross merge fails (-s resolve)' ' + git reset --hard A^ && + test_must_fail git merge -s resolve -m "final merge" B +' test_done diff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh index 53cc9b2ffb..1e34e1f48b 100755 --- a/t/t6022-merge-rename.sh +++ b/t/t6022-merge-rename.sh @@ -8,94 +8,94 @@ modify () { mv "$2.x" "$2" } -test_expect_success setup \ +test_expect_success 'setup' ' + cat >A <<-\EOF && + a aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + b bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + c cccccccccccccccccccccccccccccccccccccccccccccccc + d dddddddddddddddddddddddddddddddddddddddddddddddd + e eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee + f ffffffffffffffffffffffffffffffffffffffffffffffff + g gggggggggggggggggggggggggggggggggggggggggggggggg + h hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh + i iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii + j jjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjj + k kkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkk + l llllllllllllllllllllllllllllllllllllllllllllllll + m mmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmm + n nnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnn + o oooooooooooooooooooooooooooooooooooooooooooooooo + EOF + + cat >M <<-\EOF && + A AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA + B BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB + C CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC + D DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD + E EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE + F FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF + G GGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGG + H HHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHH + I IIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIII + J JJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJ + K KKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKK + L LLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLL + M MMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMM + N NNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNN + O OOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOO + EOF + + git add A M && + git commit -m "initial has A and M" && + git branch white && + git branch red && + git branch blue && + git branch yellow && + git branch change && + git branch change+rename && + + sed -e "/^g /s/.*/g : master changes a line/" A+ && + mv A+ A && + git commit -a -m "master updates A" && + + git checkout yellow && + rm -f M && + git commit -a -m "yellow removes M" && + + git checkout white && + sed -e "/^g /s/.*/g : white changes a line/" B && + sed -e "/^G /s/.*/G : colored branch changes a line/" N && + rm -f A M && + git update-index --add --remove A B M N && + git commit -m "white renames A->B, M->N" && + + git checkout red && + sed -e "/^g /s/.*/g : red changes a line/" B && + sed -e "/^G /s/.*/G : colored branch changes a line/" N && + rm -f A M && + git update-index --add --remove A B M N && + git commit -m "red renames A->B, M->N" && + + git checkout blue && + sed -e "/^g /s/.*/g : blue changes a line/" C && + sed -e "/^G /s/.*/G : colored branch changes a line/" N && + rm -f A M && + git update-index --add --remove A C M N && + git commit -m "blue renames A->C, M->N" && + + git checkout change && + sed -e "/^g /s/.*/g : changed line/" A+ && + mv A+ A && + git commit -q -a -m "changed" && + + git checkout change+rename && + sed -e "/^g /s/.*/g : changed line/" B && + rm A && + git update-index --add B && + git commit -q -a -m "changed and renamed" && + + git checkout master ' -cat >A <<\EOF && -a aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa -b bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb -c cccccccccccccccccccccccccccccccccccccccccccccccc -d dddddddddddddddddddddddddddddddddddddddddddddddd -e eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee -f ffffffffffffffffffffffffffffffffffffffffffffffff -g gggggggggggggggggggggggggggggggggggggggggggggggg -h hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh -i iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii -j jjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjj -k kkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkk -l llllllllllllllllllllllllllllllllllllllllllllllll -m mmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmm -n nnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnn -o oooooooooooooooooooooooooooooooooooooooooooooooo -EOF - -cat >M <<\EOF && -A AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA -B BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB -C CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC -D DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD -E EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE -F FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF -G GGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGG -H HHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHH -I IIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIII -J JJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJ -K KKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKK -L LLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLL -M MMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMM -N NNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNN -O OOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOO -EOF - -git add A M && -git commit -m "initial has A and M" && -git branch white && -git branch red && -git branch blue && -git branch yellow && -git branch change && -git branch change+rename && - -sed -e "/^g /s/.*/g : master changes a line/" A+ && -mv A+ A && -git commit -a -m "master updates A" && - -git checkout yellow && -rm -f M && -git commit -a -m "yellow removes M" && - -git checkout white && -sed -e "/^g /s/.*/g : white changes a line/" B && -sed -e "/^G /s/.*/G : colored branch changes a line/" N && -rm -f A M && -git update-index --add --remove A B M N && -git commit -m "white renames A->B, M->N" && - -git checkout red && -sed -e "/^g /s/.*/g : red changes a line/" B && -sed -e "/^G /s/.*/G : colored branch changes a line/" N && -rm -f A M && -git update-index --add --remove A B M N && -git commit -m "red renames A->B, M->N" && - -git checkout blue && -sed -e "/^g /s/.*/g : blue changes a line/" C && -sed -e "/^G /s/.*/G : colored branch changes a line/" N && -rm -f A M && -git update-index --add --remove A C M N && -git commit -m "blue renames A->C, M->N" && - -git checkout change && -sed -e "/^g /s/.*/g : changed line/" A+ && -mv A+ A && -git commit -q -a -m "changed" && - -git checkout change+rename && -sed -e "/^g /s/.*/g : changed line/" B && -rm A && -git update-index --add B && -git commit -q -a -m "changed and renamed" && - -git checkout master' test_expect_success 'pull renaming branch into unrenaming one' \ ' @@ -288,14 +288,15 @@ test_expect_success 'setup for rename + d/f conflicts' ' git commit -m "Conflicting change" ' -printf "1\n2\n3\n4\n5555\n6\n7\n8\n9\n10\n11\n" >expected - test_expect_success 'Rename+D/F conflict; renamed file merges + dir not in way' ' git reset --hard && git checkout -q renamed-file-has-no-conflicts^0 && + git merge --strategy=recursive dir-not-in-way && + git diff --quiet && test -f dir && + printf "1\n2\n3\n4\n5555\n6\n7\n8\n9\n10\n11\n" >expected && test_cmp expected dir ' @@ -342,24 +343,6 @@ test_expect_success 'Same as previous, but merged other way' ' test_cmp expected dir~renamed-file-has-no-conflicts ' -cat >expected <<\EOF && -1 -2 -3 -4 -5 -6 -7 -8 -9 -10 -<<<<<<< HEAD:dir -12 -======= -11 ->>>>>>> dir-not-in-way:sub/file -EOF - test_expect_success 'Rename+D/F conflict; renamed file cannot merge, dir not in way' ' git reset --hard && rm -rf dir~* && @@ -373,6 +356,23 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge, dir not in test_must_fail git diff --cached --quiet && test -f dir && + cat >expected <<-\EOF && + 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 10 + <<<<<<< HEAD:dir + 12 + ======= + 11 + >>>>>>> dir-not-in-way:sub/file + EOF test_cmp expected dir ' @@ -396,24 +396,6 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge and dir in t test_cmp expected dir~HEAD ' -cat >expected <<\EOF && -1 -2 -3 -4 -5 -6 -7 -8 -9 -10 -<<<<<<< HEAD:sub/file -11 -======= -12 ->>>>>>> renamed-file-has-conflicts:dir -EOF - test_expect_success 'Same as previous, but merged other way' ' git reset --hard && rm -rf dir~* && @@ -429,6 +411,23 @@ test_expect_success 'Same as previous, but merged other way' ' test -f dir/file-in-the-way && test -f dir~renamed-file-has-conflicts && + cat >expected <<-\EOF && + 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 10 + <<<<<<< HEAD:sub/file + 11 + ======= + 12 + >>>>>>> renamed-file-has-conflicts:dir + EOF test_cmp expected dir~renamed-file-has-conflicts ' @@ -810,48 +809,48 @@ test_expect_success 'setup for use of extended merge markers' ' git commit -mC ' -cat >expected <<\EOF && -1 -2 -3 -4 -5 -6 -7 -8 -<<<<<<< HEAD:renamed_file -9 -======= -8.5 ->>>>>>> master^0:original_file -EOF - test_expect_success 'merge master into rename has correct extended markers' ' git checkout rename^0 && test_must_fail git merge -s recursive master^0 && + + cat >expected <<-\EOF && + 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + <<<<<<< HEAD:renamed_file + 9 + ======= + 8.5 + >>>>>>> master^0:original_file + EOF test_cmp expected renamed_file ' -cat >expected <<\EOF && -1 -2 -3 -4 -5 -6 -7 -8 -<<<<<<< HEAD:original_file -8.5 -======= -9 ->>>>>>> rename^0:renamed_file -EOF - test_expect_success 'merge rename into master has correct extended markers' ' git reset --hard && git checkout master^0 && test_must_fail git merge -s recursive rename^0 && + + cat >expected <<-\EOF && + 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + <<<<<<< HEAD:original_file + 8.5 + ======= + 9 + >>>>>>> rename^0:renamed_file + EOF test_cmp expected renamed_file ' diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh index 51ee887a77..2f421d967a 100755 --- a/t/t6023-merge-file.sh +++ b/t/t6023-merge-file.sh @@ -3,56 +3,59 @@ test_description='RCS merge replacement: merge-file' . ./test-lib.sh -cat > orig.txt << EOF -Dominus regit me, -et nihil mihi deerit. -In loco pascuae ibi me collocavit, -super aquam refectionis educavit me; -animam meam convertit, -deduxit me super semitas jusitiae, -propter nomen suum. -EOF +test_expect_success 'setup' ' + cat >orig.txt <<-\EOF && + Dominus regit me, + et nihil mihi deerit. + In loco pascuae ibi me collocavit, + super aquam refectionis educavit me; + animam meam convertit, + deduxit me super semitas jusitiae, + propter nomen suum. + EOF -cat > new1.txt << EOF -Dominus regit me, -et nihil mihi deerit. -In loco pascuae ibi me collocavit, -super aquam refectionis educavit me; -animam meam convertit, -deduxit me super semitas jusitiae, -propter nomen suum. -Nam et si ambulavero in medio umbrae mortis, -non timebo mala, quoniam tu mecum es: -virga tua et baculus tuus ipsa me consolata sunt. -EOF + cat >new1.txt <<-\EOF && + Dominus regit me, + et nihil mihi deerit. + In loco pascuae ibi me collocavit, + super aquam refectionis educavit me; + animam meam convertit, + deduxit me super semitas jusitiae, + propter nomen suum. + Nam et si ambulavero in medio umbrae mortis, + non timebo mala, quoniam tu mecum es: + virga tua et baculus tuus ipsa me consolata sunt. + EOF -cat > new2.txt << EOF -Dominus regit me, et nihil mihi deerit. -In loco pascuae ibi me collocavit, -super aquam refectionis educavit me; -animam meam convertit, -deduxit me super semitas jusitiae, -propter nomen suum. -EOF + cat >new2.txt <<-\EOF && + Dominus regit me, et nihil mihi deerit. + In loco pascuae ibi me collocavit, + super aquam refectionis educavit me; + animam meam convertit, + deduxit me super semitas jusitiae, + propter nomen suum. + EOF -cat > new3.txt << EOF -DOMINUS regit me, -et nihil mihi deerit. -In loco pascuae ibi me collocavit, -super aquam refectionis educavit me; -animam meam convertit, -deduxit me super semitas jusitiae, -propter nomen suum. -EOF + cat >new3.txt <<-\EOF && + DOMINUS regit me, + et nihil mihi deerit. + In loco pascuae ibi me collocavit, + super aquam refectionis educavit me; + animam meam convertit, + deduxit me super semitas jusitiae, + propter nomen suum. + EOF -cat > new4.txt << EOF -Dominus regit me, et nihil mihi deerit. -In loco pascuae ibi me collocavit, -super aquam refectionis educavit me; -animam meam convertit, -deduxit me super semitas jusitiae, -EOF -printf "propter nomen suum." >> new4.txt + cat >new4.txt <<-\EOF && + Dominus regit me, et nihil mihi deerit. + In loco pascuae ibi me collocavit, + super aquam refectionis educavit me; + animam meam convertit, + deduxit me super semitas jusitiae, + EOF + + printf "propter nomen suum." >>new4.txt +' test_expect_success 'merge with no changes' ' cp orig.txt test.txt && @@ -60,9 +63,10 @@ test_expect_success 'merge with no changes' ' test_cmp test.txt orig.txt ' -cp new1.txt test.txt -test_expect_success "merge without conflict" \ - "git merge-file test.txt orig.txt new2.txt" +test_expect_success "merge without conflict" ' + cp new1.txt test.txt && + git merge-file test.txt orig.txt new2.txt +' test_expect_success 'works in subdirectory' ' mkdir dir && @@ -73,151 +77,176 @@ test_expect_success 'works in subdirectory' ' test_path_is_missing a.txt ' -cp new1.txt test.txt -test_expect_success "merge without conflict (--quiet)" \ - "git merge-file --quiet test.txt orig.txt new2.txt" +test_expect_success "merge without conflict (--quiet)" ' + cp new1.txt test.txt && + git merge-file --quiet test.txt orig.txt new2.txt +' -cp new1.txt test2.txt -test_expect_failure "merge without conflict (missing LF at EOF)" \ - "git merge-file test2.txt orig.txt new4.txt" +test_expect_failure "merge without conflict (missing LF at EOF)" ' + cp new1.txt test2.txt && + git merge-file test2.txt orig.txt new4.txt +' -test_expect_failure "merge result added missing LF" \ - "test_cmp test.txt test2.txt" +test_expect_failure "merge result added missing LF" ' + test_cmp test.txt test2.txt +' -cp new4.txt test3.txt -test_expect_success "merge without conflict (missing LF at EOF, away from change in the other file)" \ - "git merge-file --quiet test3.txt new2.txt new3.txt" +test_expect_success "merge without conflict (missing LF at EOF, away from change in the other file)" ' + cp new4.txt test3.txt && + git merge-file --quiet test3.txt new2.txt new3.txt +' -cat > expect.txt << EOF -DOMINUS regit me, -et nihil mihi deerit. -In loco pascuae ibi me collocavit, -super aquam refectionis educavit me; -animam meam convertit, -deduxit me super semitas jusitiae, -EOF -printf "propter nomen suum." >> expect.txt +test_expect_success "merge does not add LF away of change" ' + cat >expect.txt <<-\EOF && + DOMINUS regit me, + et nihil mihi deerit. + In loco pascuae ibi me collocavit, + super aquam refectionis educavit me; + animam meam convertit, + deduxit me super semitas jusitiae, + EOF + printf "propter nomen suum." >>expect.txt && -test_expect_success "merge does not add LF away of change" \ - "test_cmp expect.txt test3.txt" + test_cmp expect.txt test3.txt +' -cp test.txt backup.txt -test_expect_success "merge with conflicts" \ - "test_must_fail git merge-file test.txt orig.txt new3.txt" +test_expect_success "merge with conflicts" ' + cp test.txt backup.txt && + test_must_fail git merge-file test.txt orig.txt new3.txt +' -cat > expect.txt << EOF -<<<<<<< test.txt -Dominus regit me, et nihil mihi deerit. -======= -DOMINUS regit me, -et nihil mihi deerit. ->>>>>>> new3.txt -In loco pascuae ibi me collocavit, -super aquam refectionis educavit me; -animam meam convertit, -deduxit me super semitas jusitiae, -propter nomen suum. -Nam et si ambulavero in medio umbrae mortis, -non timebo mala, quoniam tu mecum es: -virga tua et baculus tuus ipsa me consolata sunt. -EOF +test_expect_success "expected conflict markers" ' + cat >expect.txt <<-\EOF && + <<<<<<< test.txt + Dominus regit me, et nihil mihi deerit. + ======= + DOMINUS regit me, + et nihil mihi deerit. + >>>>>>> new3.txt + In loco pascuae ibi me collocavit, + super aquam refectionis educavit me; + animam meam convertit, + deduxit me super semitas jusitiae, + propter nomen suum. + Nam et si ambulavero in medio umbrae mortis, + non timebo mala, quoniam tu mecum es: + virga tua et baculus tuus ipsa me consolata sunt. + EOF -test_expect_success "expected conflict markers" "test_cmp expect.txt test.txt" + test_cmp expect.txt test.txt +' -cp backup.txt test.txt +test_expect_success "merge conflicting with --ours" ' + cp backup.txt test.txt && -cat > expect.txt << EOF -Dominus regit me, et nihil mihi deerit. -In loco pascuae ibi me collocavit, -super aquam refectionis educavit me; -animam meam convertit, -deduxit me super semitas jusitiae, -propter nomen suum. -Nam et si ambulavero in medio umbrae mortis, -non timebo mala, quoniam tu mecum es: -virga tua et baculus tuus ipsa me consolata sunt. -EOF -test_expect_success "merge conflicting with --ours" \ - "git merge-file --ours test.txt orig.txt new3.txt && test_cmp expect.txt test.txt" -cp backup.txt test.txt + cat >expect.txt <<-\EOF && + Dominus regit me, et nihil mihi deerit. + In loco pascuae ibi me collocavit, + super aquam refectionis educavit me; + animam meam convertit, + deduxit me super semitas jusitiae, + propter nomen suum. + Nam et si ambulavero in medio umbrae mortis, + non timebo mala, quoniam tu mecum es: + virga tua et baculus tuus ipsa me consolata sunt. + EOF -cat > expect.txt << EOF -DOMINUS regit me, -et nihil mihi deerit. -In loco pascuae ibi me collocavit, -super aquam refectionis educavit me; -animam meam convertit, -deduxit me super semitas jusitiae, -propter nomen suum. -Nam et si ambulavero in medio umbrae mortis, -non timebo mala, quoniam tu mecum es: -virga tua et baculus tuus ipsa me consolata sunt. -EOF -test_expect_success "merge conflicting with --theirs" \ - "git merge-file --theirs test.txt orig.txt new3.txt && test_cmp expect.txt test.txt" -cp backup.txt test.txt + git merge-file --ours test.txt orig.txt new3.txt && + test_cmp expect.txt test.txt +' -cat > expect.txt << EOF -Dominus regit me, et nihil mihi deerit. -DOMINUS regit me, -et nihil mihi deerit. -In loco pascuae ibi me collocavit, -super aquam refectionis educavit me; -animam meam convertit, -deduxit me super semitas jusitiae, -propter nomen suum. -Nam et si ambulavero in medio umbrae mortis, -non timebo mala, quoniam tu mecum es: -virga tua et baculus tuus ipsa me consolata sunt. -EOF -test_expect_success "merge conflicting with --union" \ - "git merge-file --union test.txt orig.txt new3.txt && test_cmp expect.txt test.txt" -cp backup.txt test.txt +test_expect_success "merge conflicting with --theirs" ' + cp backup.txt test.txt && -test_expect_success "merge with conflicts, using -L" \ - "test_must_fail git merge-file -L 1 -L 2 test.txt orig.txt new3.txt" + cat >expect.txt <<-\EOF && + DOMINUS regit me, + et nihil mihi deerit. + In loco pascuae ibi me collocavit, + super aquam refectionis educavit me; + animam meam convertit, + deduxit me super semitas jusitiae, + propter nomen suum. + Nam et si ambulavero in medio umbrae mortis, + non timebo mala, quoniam tu mecum es: + virga tua et baculus tuus ipsa me consolata sunt. + EOF -cat > expect.txt << EOF -<<<<<<< 1 -Dominus regit me, et nihil mihi deerit. -======= -DOMINUS regit me, -et nihil mihi deerit. ->>>>>>> new3.txt -In loco pascuae ibi me collocavit, -super aquam refectionis educavit me; -animam meam convertit, -deduxit me super semitas jusitiae, -propter nomen suum. -Nam et si ambulavero in medio umbrae mortis, -non timebo mala, quoniam tu mecum es: -virga tua et baculus tuus ipsa me consolata sunt. -EOF + git merge-file --theirs test.txt orig.txt new3.txt && + test_cmp expect.txt test.txt +' -test_expect_success "expected conflict markers, with -L" \ - "test_cmp expect.txt test.txt" +test_expect_success "merge conflicting with --union" ' + cp backup.txt test.txt && -sed "s/ tu / TU /" < new1.txt > new5.txt -test_expect_success "conflict in removed tail" \ - "test_must_fail git merge-file -p orig.txt new1.txt new5.txt > out" + cat >expect.txt <<-\EOF && + Dominus regit me, et nihil mihi deerit. + DOMINUS regit me, + et nihil mihi deerit. + In loco pascuae ibi me collocavit, + super aquam refectionis educavit me; + animam meam convertit, + deduxit me super semitas jusitiae, + propter nomen suum. + Nam et si ambulavero in medio umbrae mortis, + non timebo mala, quoniam tu mecum es: + virga tua et baculus tuus ipsa me consolata sunt. + EOF -cat > expect << EOF -Dominus regit me, -et nihil mihi deerit. -In loco pascuae ibi me collocavit, -super aquam refectionis educavit me; -animam meam convertit, -deduxit me super semitas jusitiae, -propter nomen suum. -<<<<<<< orig.txt -======= -Nam et si ambulavero in medio umbrae mortis, -non timebo mala, quoniam TU mecum es: -virga tua et baculus tuus ipsa me consolata sunt. ->>>>>>> new5.txt -EOF + git merge-file --union test.txt orig.txt new3.txt && + test_cmp expect.txt test.txt +' -test_expect_success "expected conflict markers" "test_cmp expect out" +test_expect_success "merge with conflicts, using -L" ' + cp backup.txt test.txt && + + test_must_fail git merge-file -L 1 -L 2 test.txt orig.txt new3.txt +' + +test_expect_success "expected conflict markers, with -L" ' + cat >expect.txt <<-\EOF && + <<<<<<< 1 + Dominus regit me, et nihil mihi deerit. + ======= + DOMINUS regit me, + et nihil mihi deerit. + >>>>>>> new3.txt + In loco pascuae ibi me collocavit, + super aquam refectionis educavit me; + animam meam convertit, + deduxit me super semitas jusitiae, + propter nomen suum. + Nam et si ambulavero in medio umbrae mortis, + non timebo mala, quoniam tu mecum es: + virga tua et baculus tuus ipsa me consolata sunt. + EOF + + test_cmp expect.txt test.txt +' + +test_expect_success "conflict in removed tail" ' + sed "s/ tu / TU /" new5.txt && + test_must_fail git merge-file -p orig.txt new1.txt new5.txt >out +' + +test_expect_success "expected conflict markers" ' + cat >expect <<-\EOF && + Dominus regit me, + et nihil mihi deerit. + In loco pascuae ibi me collocavit, + super aquam refectionis educavit me; + animam meam convertit, + deduxit me super semitas jusitiae, + propter nomen suum. + <<<<<<< orig.txt + ======= + Nam et si ambulavero in medio umbrae mortis, + non timebo mala, quoniam TU mecum es: + virga tua et baculus tuus ipsa me consolata sunt. + >>>>>>> new5.txt + EOF + + test_cmp expect out +' test_expect_success 'binary files cannot be merged' ' test_must_fail git merge-file -p \ @@ -225,59 +254,55 @@ test_expect_success 'binary files cannot be merged' ' grep "Cannot merge binary files" merge.err ' -sed -e "s/deerit.\$/deerit;/" -e "s/me;\$/me./" < new5.txt > new6.txt -sed -e "s/deerit.\$/deerit,/" -e "s/me;\$/me,/" < new5.txt > new7.txt - test_expect_success 'MERGE_ZEALOUS simplifies non-conflicts' ' + sed -e "s/deerit.\$/deerit;/" -e "s/me;\$/me./" new6.txt && + sed -e "s/deerit.\$/deerit,/" -e "s/me;\$/me,/" new7.txt && test_must_fail git merge-file -p new6.txt new5.txt new7.txt > output && - test 1 = $(grep ======= < output | wc -l) - + test 1 = $(grep ======= new8.txt -sed -e 's/deerit./&%%%%/' -e "s/locavit,/locavit --/" < new7.txt | tr '%' '\012' > new9.txt - test_expect_success 'ZEALOUS_ALNUM' ' + sed -e "s/deerit./&%%%%/" -e "s/locavit,/locavit;/" new8.txt && + sed -e "s/deerit./&%%%%/" -e "s/locavit,/locavit --/" new9.txt && test_must_fail git merge-file -p \ - new8.txt new5.txt new9.txt > merge.out && - test 1 = $(grep ======= < merge.out | wc -l) - + new8.txt new5.txt new9.txt >merge.out && + test 1 = $(grep ======= expect <<\EOF -Dominus regit me, -<<<<<<< new8.txt -et nihil mihi deerit; - - - - -In loco pascuae ibi me collocavit; -super aquam refectionis educavit me. -||||||| new5.txt -et nihil mihi deerit. -In loco pascuae ibi me collocavit, -super aquam refectionis educavit me; -======= -et nihil mihi deerit, - - - - -In loco pascuae ibi me collocavit -- -super aquam refectionis educavit me, ->>>>>>> new9.txt -animam meam convertit, -deduxit me super semitas jusitiae, -propter nomen suum. -Nam et si ambulavero in medio umbrae mortis, -non timebo mala, quoniam TU mecum es: -virga tua et baculus tuus ipsa me consolata sunt. -EOF - test_expect_success '"diff3 -m" style output (1)' ' + cat >expect <<-\EOF && + Dominus regit me, + <<<<<<< new8.txt + et nihil mihi deerit; + + + + + In loco pascuae ibi me collocavit; + super aquam refectionis educavit me. + ||||||| new5.txt + et nihil mihi deerit. + In loco pascuae ibi me collocavit, + super aquam refectionis educavit me; + ======= + et nihil mihi deerit, + + + + + In loco pascuae ibi me collocavit -- + super aquam refectionis educavit me, + >>>>>>> new9.txt + animam meam convertit, + deduxit me super semitas jusitiae, + propter nomen suum. + Nam et si ambulavero in medio umbrae mortis, + non timebo mala, quoniam TU mecum es: + virga tua et baculus tuus ipsa me consolata sunt. + EOF + test_must_fail git merge-file -p --diff3 \ new8.txt new5.txt new9.txt >actual && test_cmp expect actual @@ -290,61 +315,64 @@ test_expect_success '"diff3 -m" style output (2)' ' test_cmp expect actual ' -cat >expect <<\EOF -Dominus regit me, -<<<<<<<<<< new8.txt -et nihil mihi deerit; - - - - -In loco pascuae ibi me collocavit; -super aquam refectionis educavit me. -|||||||||| new5.txt -et nihil mihi deerit. -In loco pascuae ibi me collocavit, -super aquam refectionis educavit me; -========== -et nihil mihi deerit, - - - - -In loco pascuae ibi me collocavit -- -super aquam refectionis educavit me, ->>>>>>>>>> new9.txt -animam meam convertit, -deduxit me super semitas jusitiae, -propter nomen suum. -Nam et si ambulavero in medio umbrae mortis, -non timebo mala, quoniam TU mecum es: -virga tua et baculus tuus ipsa me consolata sunt. -EOF - test_expect_success 'marker size' ' + cat >expect <<-\EOF && + Dominus regit me, + <<<<<<<<<< new8.txt + et nihil mihi deerit; + + + + + In loco pascuae ibi me collocavit; + super aquam refectionis educavit me. + |||||||||| new5.txt + et nihil mihi deerit. + In loco pascuae ibi me collocavit, + super aquam refectionis educavit me; + ========== + et nihil mihi deerit, + + + + + In loco pascuae ibi me collocavit -- + super aquam refectionis educavit me, + >>>>>>>>>> new9.txt + animam meam convertit, + deduxit me super semitas jusitiae, + propter nomen suum. + Nam et si ambulavero in medio umbrae mortis, + non timebo mala, quoniam TU mecum es: + virga tua et baculus tuus ipsa me consolata sunt. + EOF + test_must_fail git merge-file -p --marker-size=10 \ new8.txt new5.txt new9.txt >actual && test_cmp expect actual ' -printf "line1\nline2\nline3" >nolf-orig.txt -printf "line1\nline2\nline3x" >nolf-diff1.txt -printf "line1\nline2\nline3y" >nolf-diff2.txt +test_expect_success 'conflict at EOF without LF resolved by --ours' ' + printf "line1\nline2\nline3" >nolf-orig.txt && + printf "line1\nline2\nline3x" >nolf-diff1.txt && + printf "line1\nline2\nline3y" >nolf-diff2.txt && -test_expect_success 'conflict at EOF without LF resolved by --ours' \ - 'git merge-file -p --ours nolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt && - printf "line1\nline2\nline3x" >expect.txt && - test_cmp expect.txt output.txt' + git merge-file -p --ours nolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt && + printf "line1\nline2\nline3x" >expect.txt && + test_cmp expect.txt output.txt +' -test_expect_success 'conflict at EOF without LF resolved by --theirs' \ - 'git merge-file -p --theirs nolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt && - printf "line1\nline2\nline3y" >expect.txt && - test_cmp expect.txt output.txt' +test_expect_success 'conflict at EOF without LF resolved by --theirs' ' + git merge-file -p --theirs nolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt && + printf "line1\nline2\nline3y" >expect.txt && + test_cmp expect.txt output.txt +' -test_expect_success 'conflict at EOF without LF resolved by --union' \ - 'git merge-file -p --union nolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt && - printf "line1\nline2\nline3x\nline3y" >expect.txt && - test_cmp expect.txt output.txt' +test_expect_success 'conflict at EOF without LF resolved by --union' ' + git merge-file -p --union nolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt && + printf "line1\nline2\nline3x\nline3y" >expect.txt && + test_cmp expect.txt output.txt +' test_expect_success 'conflict sections match existing line endings' ' printf "1\\r\\n2\\r\\n3" >crlf-orig.txt && diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh index 8f9b48a493..5900358ce9 100755 --- a/t/t6026-merge-attr.sh +++ b/t/t6026-merge-attr.sh @@ -32,7 +32,29 @@ test_expect_success setup ' test_tick && git commit -m Side && - git tag anchor + git tag anchor && + + cat >./custom-merge <<-\EOF && + #!/bin/sh + + orig="$1" ours="$2" theirs="$3" exit="$4" path=$5 + ( + echo "orig is $orig" + echo "ours is $ours" + echo "theirs is $theirs" + echo "path is $path" + echo "=== orig ===" + cat "$orig" + echo "=== ours ===" + cat "$ours" + echo "=== theirs ===" + cat "$theirs" + ) >"$ours+" + cat "$ours+" >"$ours" + rm -f "$ours+" + exit "$exit" + EOF + chmod +x ./custom-merge ' test_expect_success merge ' @@ -82,28 +104,6 @@ test_expect_success 'retry the merge with longer context' ' grep "<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<" actual ' -cat >./custom-merge <<\EOF -#!/bin/sh - -orig="$1" ours="$2" theirs="$3" exit="$4" path=$5 -( - echo "orig is $orig" - echo "ours is $ours" - echo "theirs is $theirs" - echo "path is $path" - echo "=== orig ===" - cat "$orig" - echo "=== ours ===" - cat "$ours" - echo "=== theirs ===" - cat "$theirs" -) >"$ours+" -cat "$ours+" >"$ours" -rm -f "$ours+" -exit "$exit" -EOF -chmod +x ./custom-merge - test_expect_success 'custom merge backend' ' echo "* merge=union" >.gitattributes && diff --git a/t/t6034-merge-rename-nocruft.sh b/t/t6034-merge-rename-nocruft.sh index 89871aa5b0..a25e730460 100755 --- a/t/t6034-merge-rename-nocruft.sh +++ b/t/t6034-merge-rename-nocruft.sh @@ -3,74 +3,73 @@ test_description='Merge-recursive merging renames' . ./test-lib.sh -test_expect_success setup \ +test_expect_success 'setup' ' + cat >A <<-\EOF && + a aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + b bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + c cccccccccccccccccccccccccccccccccccccccccccccccc + d dddddddddddddddddddddddddddddddddddddddddddddddd + e eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee + f ffffffffffffffffffffffffffffffffffffffffffffffff + g gggggggggggggggggggggggggggggggggggggggggggggggg + h hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh + i iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii + j jjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjj + k kkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkk + l llllllllllllllllllllllllllllllllllllllllllllllll + m mmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmm + n nnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnn + o oooooooooooooooooooooooooooooooooooooooooooooooo + EOF + + cat >M <<-\EOF && + A AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA + B BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB + C CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC + D DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD + E EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE + F FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF + G GGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGG + H HHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHH + I IIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIII + J JJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJ + K KKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKK + L LLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLL + M MMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMM + N NNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNN + O OOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOO + EOF + + git add A M && + git commit -m "initial has A and M" && + git branch white && + git branch red && + git branch blue && + + git checkout white && + sed -e "/^g /s/.*/g : white changes a line/" B && + sed -e "/^G /s/.*/G : colored branch changes a line/" N && + rm -f A M && + git update-index --add --remove A B M N && + git commit -m "white renames A->B, M->N" && + + git checkout red && + echo created by red >R && + git update-index --add R && + git commit -m "red creates R" && + + git checkout blue && + sed -e "/^o /s/.*/g : blue changes a line/" B && + rm -f A && + mv B A && + git update-index A && + git commit -m "blue modify A" && + + git checkout master ' -cat >A <<\EOF && -a aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa -b bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb -c cccccccccccccccccccccccccccccccccccccccccccccccc -d dddddddddddddddddddddddddddddddddddddddddddddddd -e eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee -f ffffffffffffffffffffffffffffffffffffffffffffffff -g gggggggggggggggggggggggggggggggggggggggggggggggg -h hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh -i iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii -j jjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjj -k kkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkk -l llllllllllllllllllllllllllllllllllllllllllllllll -m mmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmm -n nnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnn -o oooooooooooooooooooooooooooooooooooooooooooooooo -EOF - -cat >M <<\EOF && -A AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA -B BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB -C CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC -D DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD -E EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE -F FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF -G GGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGG -H HHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHH -I IIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIII -J JJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJJ -K KKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKK -L LLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLL -M MMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMM -N NNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNN -O OOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOO -EOF - -git add A M && -git commit -m "initial has A and M" && -git branch white && -git branch red && -git branch blue && - -git checkout white && -sed -e "/^g /s/.*/g : white changes a line/" B && -sed -e "/^G /s/.*/G : colored branch changes a line/" N && -rm -f A M && -git update-index --add --remove A B M N && -git commit -m "white renames A->B, M->N" && - -git checkout red && -echo created by red >R && -git update-index --add R && -git commit -m "red creates R" && - -git checkout blue && -sed -e "/^o /s/.*/g : blue changes a line/" B && -rm -f A && -mv B A && -git update-index A && -git commit -m "blue modify A" && - -git checkout master' # This test broke in 65ac6e9c3f47807cb603af07a6a9e1a43bc119ae -test_expect_success 'merge white into red (A->B,M->N)' \ -' +test_expect_success 'merge white into red (A->B,M->N)' ' git checkout -b red-white red && git merge white && git write-tree && @@ -82,8 +81,7 @@ test_expect_success 'merge white into red (A->B,M->N)' \ ' # This test broke in 8371234ecaaf6e14fe3f2082a855eff1bbd79ae9 -test_expect_success 'merge blue into white (A->B, mod A, A untracked)' \ -' +test_expect_success 'merge blue into white (A->B, mod A, A untracked)' ' git checkout -b white-blue white && echo dirty >A && git merge blue && From b821ca788bee258fb2364722dafe0b60a3219863 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 27 Feb 2020 00:14:21 +0000 Subject: [PATCH 18/27] t6020, t6022, t6035: update merge tests to use test helper functions Make use of test_path_is_file, test_write_lines, and similar helpers in these old test files. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- t/t6020-merge-df.sh | 12 ++++---- t/t6022-merge-rename.sh | 54 ++++++++++++++++----------------- t/t6035-merge-dir-to-symlink.sh | 28 ++++++++--------- 3 files changed, 47 insertions(+), 47 deletions(-) diff --git a/t/t6020-merge-df.sh b/t/t6020-merge-df.sh index 46b506b3b7..cf662068da 100755 --- a/t/t6020-merge-df.sh +++ b/t/t6020-merge-df.sh @@ -83,9 +83,9 @@ test_expect_success 'modify/delete + directory/file conflict' ' test 4 -eq $(git ls-files -u | wc -l) && test 1 -eq $(git ls-files -o | wc -l) && - test -f letters/file && - test -f letters.txt && - test -f letters~modify + test_path_is_file letters/file && + test_path_is_file letters.txt && + test_path_is_file letters~modify ' test_expect_success 'modify/delete + directory/file conflict; other way' ' @@ -99,9 +99,9 @@ test_expect_success 'modify/delete + directory/file conflict; other way' ' test 4 -eq $(git ls-files -u | wc -l) && test 1 -eq $(git ls-files -o | wc -l) && - test -f letters/file && - test -f letters.txt && - test -f letters~HEAD + test_path_is_file letters/file && + test_path_is_file letters.txt && + test_path_is_file letters~HEAD ' test_done diff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh index 1e34e1f48b..6f196aaf27 100755 --- a/t/t6022-merge-rename.sh +++ b/t/t6022-merge-rename.sh @@ -295,8 +295,8 @@ test_expect_success 'Rename+D/F conflict; renamed file merges + dir not in way' git merge --strategy=recursive dir-not-in-way && git diff --quiet && - test -f dir && - printf "1\n2\n3\n4\n5555\n6\n7\n8\n9\n10\n11\n" >expected && + test_path_is_file dir && + test_write_lines 1 2 3 4 5555 6 7 8 9 10 11 >expected && test_cmp expected dir ' @@ -316,8 +316,8 @@ test_expect_success 'Rename+D/F conflict; renamed file merges but dir in way' ' test_must_fail git diff --quiet && test_must_fail git diff --cached --quiet && - test -f dir/file-in-the-way && - test -f dir~HEAD && + test_path_is_file dir/file-in-the-way && + test_path_is_file dir~HEAD && test_cmp expected dir~HEAD ' @@ -338,8 +338,8 @@ test_expect_success 'Same as previous, but merged other way' ' test_must_fail git diff --quiet && test_must_fail git diff --cached --quiet && - test -f dir/file-in-the-way && - test -f dir~renamed-file-has-no-conflicts && + test_path_is_file dir/file-in-the-way && + test_path_is_file dir~renamed-file-has-no-conflicts && test_cmp expected dir~renamed-file-has-no-conflicts ' @@ -355,7 +355,7 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge, dir not in test_must_fail git diff --quiet && test_must_fail git diff --cached --quiet && - test -f dir && + test_path_is_file dir && cat >expected <<-\EOF && 1 2 @@ -391,8 +391,8 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge and dir in t test_must_fail git diff --quiet && test_must_fail git diff --cached --quiet && - test -f dir/file-in-the-way && - test -f dir~HEAD && + test_path_is_file dir/file-in-the-way && + test_path_is_file dir~HEAD && test_cmp expected dir~HEAD ' @@ -409,8 +409,8 @@ test_expect_success 'Same as previous, but merged other way' ' test_must_fail git diff --quiet && test_must_fail git diff --cached --quiet && - test -f dir/file-in-the-way && - test -f dir~renamed-file-has-conflicts && + test_path_is_file dir/file-in-the-way && + test_path_is_file dir~renamed-file-has-conflicts && cat >expected <<-\EOF && 1 2 @@ -463,9 +463,9 @@ test_expect_success 'both rename source and destination involved in D/F conflict test_must_fail git diff --quiet && - test -f destdir/foo && - test -f one && - test -f destdir~HEAD && + test_path_is_file destdir/foo && + test_path_is_file one && + test_path_is_file destdir~HEAD && test "stuff" = "$(cat destdir~HEAD)" ' @@ -506,9 +506,9 @@ test_expect_success 'pair rename to parent of other (D/F conflicts) w/ untracked test 4 -eq $(find . | grep -v .git | wc -l) && - test -d one && - test -f one~rename-two && - test -f two && + test_path_is_dir one && + test_path_is_file one~rename-two && + test_path_is_file two && test "other" = $(cat one~rename-two) && test "stuff" = $(cat two) ' @@ -526,8 +526,8 @@ test_expect_success 'pair rename to parent of other (D/F conflicts) w/ clean sta test 3 -eq $(find . | grep -v .git | wc -l) && - test -f one && - test -f two && + test_path_is_file one && + test_path_is_file two && test "other" = $(cat one) && test "stuff" = $(cat two) ' @@ -567,11 +567,11 @@ test_expect_success 'check handling of differently renamed file with D/F conflic test 1 -eq "$(git ls-files -u original | wc -l)" && test 2 -eq "$(git ls-files -o | wc -l)" && - test -f one/file && - test -f two/file && - test -f one~HEAD && - test -f two~second-rename && - ! test -f original + test_path_is_file one/file && + test_path_is_file two/file && + test_path_is_file one~HEAD && + test_path_is_file two~second-rename && + test_path_is_missing original ' test_expect_success 'setup rename one file to two; directories moving out of the way' ' @@ -606,9 +606,9 @@ test_expect_success 'check handling of differently renamed file with D/F conflic test 1 -eq "$(git ls-files -u original | wc -l)" && test 0 -eq "$(git ls-files -o | wc -l)" && - test -f one && - test -f two && - ! test -f original + test_path_is_file one && + test_path_is_file two && + test_path_is_missing original ' test_expect_success 'setup avoid unnecessary update, normal rename' ' diff --git a/t/t6035-merge-dir-to-symlink.sh b/t/t6035-merge-dir-to-symlink.sh index 9324ea4416..2eddcc7664 100755 --- a/t/t6035-merge-dir-to-symlink.sh +++ b/t/t6035-merge-dir-to-symlink.sh @@ -31,19 +31,19 @@ test_expect_success 'a/b-2/c/d is kept when clobbering symlink b' ' git rm --cached a/b && git commit -m "untracked symlink remains" && git checkout -f start^0 && - test -f a/b-2/c/d + test_path_is_file a/b-2/c/d ' test_expect_success 'checkout should not have deleted a/b-2/c/d' ' git checkout HEAD^0 && git reset --hard master && git checkout start^0 && - test -f a/b-2/c/d + test_path_is_file a/b-2/c/d ' test_expect_success 'setup for merge test' ' git reset --hard && - test -f a/b-2/c/d && + test_path_is_file a/b-2/c/d && echo x > a/x && git add a/x && git commit -m x && @@ -54,7 +54,7 @@ test_expect_success 'Handle D/F conflict, do not lose a/b-2/c/d in merge (resolv git reset --hard && git checkout baseline^0 && git merge -s resolve master && - test -f a/b-2/c/d + test_path_is_file a/b-2/c/d ' test_expect_success SYMLINKS 'a/b was resolved as symlink' ' @@ -65,7 +65,7 @@ test_expect_success 'Handle D/F conflict, do not lose a/b-2/c/d in merge (recurs git reset --hard && git checkout baseline^0 && git merge -s recursive master && - test -f a/b-2/c/d + test_path_is_file a/b-2/c/d ' test_expect_success SYMLINKS 'a/b was resolved as symlink' ' @@ -76,7 +76,7 @@ test_expect_success 'Handle F/D conflict, do not lose a/b-2/c/d in merge (resolv git reset --hard && git checkout master^0 && git merge -s resolve baseline^0 && - test -f a/b-2/c/d + test_path_is_file a/b-2/c/d ' test_expect_success SYMLINKS 'a/b was resolved as symlink' ' @@ -87,7 +87,7 @@ test_expect_success 'Handle F/D conflict, do not lose a/b-2/c/d in merge (recurs git reset --hard && git checkout master^0 && git merge -s recursive baseline^0 && - test -f a/b-2/c/d + test_path_is_file a/b-2/c/d ' test_expect_success SYMLINKS 'a/b was resolved as symlink' ' @@ -99,8 +99,8 @@ test_expect_failure 'do not lose untracked in merge (resolve)' ' git checkout baseline^0 && >a/b/c/e && test_must_fail git merge -s resolve master && - test -f a/b/c/e && - test -f a/b-2/c/d + test_path_is_file a/b/c/e && + test_path_is_file a/b-2/c/d ' test_expect_success 'do not lose untracked in merge (recursive)' ' @@ -108,8 +108,8 @@ test_expect_success 'do not lose untracked in merge (recursive)' ' git checkout baseline^0 && >a/b/c/e && test_must_fail git merge -s recursive master && - test -f a/b/c/e && - test -f a/b-2/c/d + test_path_is_file a/b/c/e && + test_path_is_file a/b-2/c/d ' test_expect_success 'do not lose modifications in merge (resolve)' ' @@ -140,7 +140,7 @@ test_expect_success 'merge should not have D/F conflicts (resolve)' ' git reset --hard && git checkout baseline^0 && git merge -s resolve test2 && - test -f a/b/c/d + test_path_is_file a/b/c/d ' test_expect_success SYMLINKS 'a/b-2 was resolved as symlink' ' @@ -151,7 +151,7 @@ test_expect_success 'merge should not have D/F conflicts (recursive)' ' git reset --hard && git checkout baseline^0 && git merge -s recursive test2 && - test -f a/b/c/d + test_path_is_file a/b/c/d ' test_expect_success SYMLINKS 'a/b-2 was resolved as symlink' ' @@ -162,7 +162,7 @@ test_expect_success 'merge should not have F/D conflicts (recursive)' ' git reset --hard && git checkout -b foo test2 && git merge -s recursive baseline^0 && - test -f a/b/c/d + test_path_is_file a/b/c/d ' test_expect_success SYMLINKS 'a/b-2 was resolved as symlink' ' From d5bb92ecede53ee8e22c13bc5d86b6a2bc620bb7 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 27 Feb 2020 00:14:22 +0000 Subject: [PATCH 19/27] t3035: prefer test_must_fail to bash negation for git commands Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- t/t3035-merge-sparse.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t3035-merge-sparse.sh b/t/t3035-merge-sparse.sh index c4b4a94324..74562e1235 100755 --- a/t/t3035-merge-sparse.sh +++ b/t/t3035-merge-sparse.sh @@ -28,7 +28,7 @@ test_expect_success 'setup' ' git config core.sparseCheckout true && echo "/checked-out" >.git/info/sparse-checkout && git reset --hard && - ! git merge theirs + test_must_fail git merge theirs ' test_expect_success 'reset --hard works after the conflict' ' @@ -42,7 +42,7 @@ test_expect_success 'is reset properly' ' ' test_expect_success 'setup: conflict back' ' - ! git merge theirs + test_must_fail git merge theirs ' test_expect_success 'Merge abort works after the conflict' ' From 9f697ded88286804683ab2988abed23b4c63f530 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 27 Feb 2020 00:14:23 +0000 Subject: [PATCH 20/27] t6022, t6046: test expected behavior instead of testing a proxy for it In t6022, we were testing for file being overwritten (or not) based on an output message instead of checking for the file being overwritten. Since we can check for the file being overwritten via mtime updates, check that instead. In t6046, we were largely checking for both the expected behavior and a proxy for it, which is unnecessary. The calls to test-tool also were a bit cryptic. Make them a little clearer. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- t/t6022-merge-rename.sh | 15 ++++- t/t6046-merge-skip-unneeded-updates.sh | 89 +++++++++++++++++--------- 2 files changed, 70 insertions(+), 34 deletions(-) diff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh index 6f196aaf27..d97cf48495 100755 --- a/t/t6022-merge-rename.sh +++ b/t/t6022-merge-rename.sh @@ -242,12 +242,23 @@ test_expect_success 'merge of identical changes in a renamed file' ' rm -f A M N && git reset --hard && git checkout change+rename && + + test-tool chmtime =31337 B && + test-tool chmtime --get B >old-mtime && GIT_MERGE_VERBOSITY=3 git merge change >out && - test_i18ngrep "^Skipped B" out && + + test-tool chmtime --get B >new-mtime && + test_cmp old-mtime new-mtime && + git reset --hard HEAD^ && git checkout change && + + test-tool chmtime =-1 M && + test-tool chmtime --get M >old-mtime && GIT_MERGE_VERBOSITY=3 git merge change+rename >out && - test_i18ngrep ! "^Skipped B" out + + test-tool chmtime --get B >new-mtime && + test $(cat old-mtime) -lt $(cat new-mtime) ' test_expect_success 'setup for rename + d/f conflicts' ' diff --git a/t/t6046-merge-skip-unneeded-updates.sh b/t/t6046-merge-skip-unneeded-updates.sh index b7e4669832..962030ecdb 100755 --- a/t/t6046-merge-skip-unneeded-updates.sh +++ b/t/t6046-merge-skip-unneeded-updates.sh @@ -71,16 +71,16 @@ test_expect_success '1a-L: Modify(A)/Modify(B), change on B subset of A' ' git checkout A^0 && - test-tool chmtime =31337 b && - test-tool chmtime -v +0 b >expected-mtime && + test-tool chmtime =-1 b && + test-tool chmtime --get b >old-mtime && GIT_MERGE_VERBOSITY=3 git merge -s recursive B^0 >out 2>err && - test_i18ngrep "Skipped b" out && test_must_be_empty err && - test-tool chmtime -v +0 b >actual-mtime && - test_cmp expected-mtime actual-mtime && + # Make sure b was NOT updated + test-tool chmtime --get b >new-mtime && + test_cmp old-mtime new-mtime && git ls-files -s >index_files && test_line_count = 1 index_files && @@ -102,9 +102,14 @@ test_expect_success '1a-R: Modify(A)/Modify(B), change on B subset of A' ' git checkout B^0 && + test-tool chmtime =-1 b && + test-tool chmtime --get b >old-mtime && GIT_MERGE_VERBOSITY=3 git merge -s recursive A^0 >out 2>err && - test_i18ngrep "Auto-merging b" out && + # Make sure b WAS updated + test-tool chmtime --get b >new-mtime && + test $(cat old-mtime) -lt $(cat new-mtime) && + test_must_be_empty err && git ls-files -s >index_files && @@ -165,10 +170,10 @@ test_expect_success '2a-L: Modify/rename, merge into modify side' ' git checkout A^0 && + test_path_is_missing c && GIT_MERGE_VERBOSITY=3 git merge -s recursive B^0 >out 2>err && - test_i18ngrep ! "Skipped c" out && - test_must_be_empty err && + test_path_is_file c && git ls-files -s >index_files && test_line_count = 1 index_files && @@ -193,9 +198,14 @@ test_expect_success '2a-R: Modify/rename, merge into rename side' ' git checkout B^0 && + test-tool chmtime =-1 c && + test-tool chmtime --get c >old-mtime && GIT_MERGE_VERBOSITY=3 git merge -s recursive A^0 >out 2>err && - test_i18ngrep ! "Skipped c" out && + # Make sure c WAS updated + test-tool chmtime --get c >new-mtime && + test $(cat old-mtime) -lt $(cat new-mtime) && + test_must_be_empty err && git ls-files -s >index_files && @@ -256,16 +266,15 @@ test_expect_success '2b-L: Rename+Mod(A)/Mod(B), B mods subset of A' ' git checkout A^0 && - test-tool chmtime =31337 c && - test-tool chmtime -v +0 c >expected-mtime && - + test-tool chmtime =-1 c && + test-tool chmtime --get c >old-mtime && GIT_MERGE_VERBOSITY=3 git merge -s recursive B^0 >out 2>err && - test_i18ngrep "Skipped c" out && test_must_be_empty err && - test-tool chmtime -v +0 c >actual-mtime && - test_cmp expected-mtime actual-mtime && + # Make sure c WAS updated + test-tool chmtime --get c >new-mtime && + test_cmp old-mtime new-mtime && git ls-files -s >index_files && test_line_count = 1 index_files && @@ -290,9 +299,12 @@ test_expect_success '2b-R: Rename+Mod(A)/Mod(B), B mods subset of A' ' git checkout B^0 && + test_path_is_missing c && GIT_MERGE_VERBOSITY=3 git merge -s recursive A^0 >out 2>err && - test_i18ngrep "Auto-merging c" out && + # Make sure c now present (and thus was updated) + test_path_is_file c && + test_must_be_empty err && git ls-files -s >index_files && @@ -361,13 +373,18 @@ test_expect_success '2c: Modify b & add c VS rename b->c' ' git checkout A^0 && + test-tool chmtime =-1 c && + test-tool chmtime --get c >old-mtime && GIT_MERGE_VERBOSITY=3 && export GIT_MERGE_VERBOSITY && test_must_fail git merge -s recursive B^0 >out 2>err && test_i18ngrep "CONFLICT (rename/add): Rename b->c" out && - test_i18ngrep ! "Skipped c" out && - test_must_be_empty err + test_must_be_empty err && + + # Make sure c WAS updated + test-tool chmtime --get c >new-mtime && + test $(cat old-mtime) -lt $(cat new-mtime) # FIXME: rename/add conflicts are horribly broken right now; # when I get back to my patch series fixing it and @@ -460,11 +477,13 @@ test_expect_success '3a-L: bq_1->foo/bq_2 on A, foo/->bar/ on B' ' git checkout A^0 && + test_path_is_missing bar/bq && GIT_MERGE_VERBOSITY=3 git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err && - test_i18ngrep ! "Skipped bar/bq" out && test_must_be_empty err && + test_path_is_file bar/bq && + git ls-files -s >index_files && test_line_count = 2 index_files && @@ -488,11 +507,13 @@ test_expect_success '3a-R: bq_1->foo/bq_2 on A, foo/->bar/ on B' ' git checkout B^0 && + test_path_is_missing bar/bq && GIT_MERGE_VERBOSITY=3 git -c merge.directoryRenames=true merge -s recursive A^0 >out 2>err && - test_i18ngrep ! "Skipped bar/bq" out && test_must_be_empty err && + test_path_is_file bar/bq && + git ls-files -s >index_files && test_line_count = 2 index_files && @@ -552,11 +573,13 @@ test_expect_success '3b-L: bq_1->foo/bq_2 on A, foo/->bar/ on B' ' git checkout A^0 && + test_path_is_missing bar/bq && GIT_MERGE_VERBOSITY=3 git -c merge.directoryRenames=true merge -s recursive B^0 >out 2>err && - test_i18ngrep ! "Skipped bar/bq" out && test_must_be_empty err && + test_path_is_file bar/bq && + git ls-files -s >index_files && test_line_count = 2 index_files && @@ -580,11 +603,13 @@ test_expect_success '3b-R: bq_1->foo/bq_2 on A, foo/->bar/ on B' ' git checkout B^0 && + test_path_is_missing bar/bq && GIT_MERGE_VERBOSITY=3 git -c merge.directoryRenames=true merge -s recursive A^0 >out 2>err && - test_i18ngrep ! "Skipped bar/bq" out && test_must_be_empty err && + test_path_is_file bar/bq && + git ls-files -s >index_files && test_line_count = 2 index_files && @@ -654,16 +679,16 @@ test_expect_failure '4a: Change on A, change on B subset of A, dirty mods presen git checkout A^0 && echo "File rewritten" >b && - test-tool chmtime =31337 b && - test-tool chmtime -v +0 b >expected-mtime && + test-tool chmtime =-1 b && + test-tool chmtime --get b >old-mtime && GIT_MERGE_VERBOSITY=3 git merge -s recursive B^0 >out 2>err && - test_i18ngrep "Skipped b" out && test_must_be_empty err && - test-tool chmtime -v +0 b >actual-mtime && - test_cmp expected-mtime actual-mtime && + # Make sure b was NOT updated + test-tool chmtime --get b >new-mtime && + test_cmp old-mtime new-mtime && git ls-files -s >index_files && test_line_count = 1 index_files && @@ -722,16 +747,16 @@ test_expect_success '4b: Rename+Mod(A)/Mod(B), change on B subset of A, dirty mo git checkout A^0 && echo "File rewritten" >c && - test-tool chmtime =31337 c && - test-tool chmtime -v +0 c >expected-mtime && + test-tool chmtime =-1 c && + test-tool chmtime --get c >old-mtime && GIT_MERGE_VERBOSITY=3 git merge -s recursive B^0 >out 2>err && - test_i18ngrep "Skipped c" out && test_must_be_empty err && - test-tool chmtime -v +0 c >actual-mtime && - test_cmp expected-mtime actual-mtime && + # Make sure c was NOT updated + test-tool chmtime --get c >new-mtime && + test_cmp old-mtime new-mtime && git ls-files -s >index_files && test_line_count = 1 index_files && From 65bf820d0ee050ba6a0c8908ff1d5f58d7c7131e Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 27 Feb 2020 00:14:24 +0000 Subject: [PATCH 21/27] t6020: new test with interleaved lexicographic ordering of directories If a repository has two files: foo/bar/baz foo/bar-2/baz then a simple lexicographic ordering of files and directories shows ... foo/bar foo/bar-2 foo/bar/baz ... and the appearance of foo/bar-2 between foo/bar and foo/bar/baz can trip up some codepaths. Add a test to catch such cases. t6020 might be a slight misfit since this testcase does not test any kind of file/directory conflict. However, it is similar in spirit to some tests (4-6) already in t6020 that check cases where a *file* sorted between a directory and the files underneath that directory. This testcase differs in that now there is a *directory* that sorts in the middle. Although merge-recursive currently has no problems with this simple testcase, I discovered that it's very possible to accidentally mess it up. Further, we have no other merge or cherry-pick or rebase testcases in the entire testsuite that cover such a case, so I felt like it would be a worthwhile addition to the testsuite. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- t/t6020-merge-df.sh | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/t/t6020-merge-df.sh b/t/t6020-merge-df.sh index cf662068da..400a4cd139 100755 --- a/t/t6020-merge-df.sh +++ b/t/t6020-merge-df.sh @@ -104,4 +104,47 @@ test_expect_success 'modify/delete + directory/file conflict; other way' ' test_path_is_file letters~HEAD ' +test_expect_success 'Simple merge in repo with interesting pathnames' ' + # Simple lexicographic ordering of files and directories would be: + # foo + # foo/bar + # foo/bar-2 + # foo/bar/baz + # foo/bar-2/baz + # The fact that foo/bar-2 appears between foo/bar and foo/bar/baz + # can trip up some codepaths, and is the point of this test. + test_create_repo name-ordering && + ( + cd name-ordering && + + mkdir -p foo/bar && + mkdir -p foo/bar-2 && + >foo/bar/baz && + >foo/bar-2/baz && + git add . && + git commit -m initial && + + git branch main && + git branch other && + + git checkout other && + echo other >foo/bar-2/baz && + git add -u && + git commit -m other && + + git checkout main && + echo main >foo/bar/baz && + git add -u && + git commit -m main && + + git merge other && + git ls-files -s >out && + test_line_count = 2 out && + git rev-parse :0:foo/bar/baz :0:foo/bar-2/baz >actual && + git rev-parse HEAD~1:foo/bar/baz other:foo/bar-2/baz >expect && + test_cmp expect actual + ) + +' + test_done From 94f4d01932279c419844aa708bec31a26056bc6b Mon Sep 17 00:00:00 2001 From: Alexandr Miloslavskiy Date: Mon, 17 Feb 2020 18:01:26 +0000 Subject: [PATCH 22/27] mingw: workaround for hangs when sending STDIN Explanation ----------- The problem here is flawed `poll()` implementation. When it tries to see if pipe can be written without blocking, it eventually calls `NtQueryInformationFile()` and tests `WriteQuotaAvailable`. However, the meaning of quota was misunderstood. The value of quota is reduced when either some data was written to a pipe, *or* there is a pending read on the pipe. Therefore, if there is a pending read of size >= than the pipe's buffer size, poll() will think that pipe is not writable and will hang forever, usually that means deadlocking both pipe users. I have studied the problem and found that Windows pipes track two values: `QuotaUsed` and `BytesInQueue`. The code in `poll()` apparently wants to know `BytesInQueue` instead of quota. Unfortunately, `BytesInQueue` can only be requested from read end of the pipe, while `poll()` receives write end. The git's implementation of `poll()` was copied from gnulib, which also contains a flawed implementation up to today. I also had a look at implementation in cygwin, which is also broken in a subtle way. It uses this code in `pipe_data_available()`: fpli.WriteQuotaAvailable = (fpli.OutboundQuota - fpli.ReadDataAvailable) However, `ReadDataAvailable` always returns 0 for the write end of the pipe, turning the code into an obfuscated version of returning pipe's total buffer size, which I guess will in turn have `poll()` always say that pipe is writable. The commit that introduced the code doesn't say anything about this change, so it could be some debugging code that slipped in. These are the typical sizes used in git: 0x2000 - default read size in `strbuf_read()` 0x1000 - default read size in CRT, used by `strbuf_getwholeline()` 0x2000 - pipe buffer size in compat\mingw.c As a consequence, as soon as child process uses `strbuf_read()`, `poll()` in parent process will hang forever, deadlocking both processes. This results in two observable behaviors: 1) If parent process begins sending STDIN quickly (and usually that's the case), then first `poll()` will succeed and first block will go through. MAX_IO_SIZE_DEFAULT is 8MB, so if STDIN exceeds 8MB, then it will deadlock. 2) If parent process waits a little bit for any reason (including OS scheduler) and child is first to issue `strbuf_read()`, then it will deadlock immediately even on small STDINs. The problem is illustrated by `git stash push`, which will currently read the entire patch into memory and then send it to `git apply` via STDIN. If patch exceeds 8MB, git hangs on Windows. Possible solutions ------------------ 1) Somehow obtain `BytesInQueue` instead of `QuotaUsed` I did a pretty thorough search and didn't find any ways to obtain the value from write end of the pipe. 2) Also give read end of the pipe to `poll()` That can be done, but it will probably invite some dirty code, because `poll()` * can accept multiple pipes at once * can accept things that are not pipes * is expected to have a well known signature. 3) Make `poll()` always reply "writable" for write end of the pipe Afterall it seems that cygwin (accidentally?) does that for years. Also, it should be noted that `pump_io_round()` writes 8MB blocks, completely ignoring the fact that pipe's buffer size is only 8KB, which means that pipe gets clogged many times during that single write. This may invite a deadlock, if child's STDERR/STDOUT gets clogged while it's trying to deal with 8MB of STDIN. Such deadlocks could be defeated with writing less than pipe's buffer size per round, and always reading everything from STDOUT/STDERR before starting next round. Therefore, making `poll()` always reply "writable" shouldn't cause any new issues or block any future solutions. 4) Increase the size of the pipe's buffer The difference between `BytesInQueue` and `QuotaUsed` is the size of pending reads. Therefore, if buffer is bigger than size of reads, `poll()` won't hang so easily. However, I found that for example `strbuf_read()` will get more and more hungry as it reads large inputs, eventually surpassing any reasonable pipe buffer size. Chosen solution --------------- Make `poll()` always reply "writable" for write end of the pipe. Hopefully one day someone will find a way to implement it properly. Reproduction ------------ printf "%8388608s" X >large_file.txt git stash push --include-untracked -- large_file.txt I have decided not to include this as test to avoid slowing down the test suite. I don't expect the specific problem to come back, and chances are that `git stash push` will be reworked to avoid sending the entire patch via STDIN. Signed-off-by: Alexandr Miloslavskiy Signed-off-by: Junio C Hamano --- compat/poll/poll.c | 31 +++---------------------------- 1 file changed, 3 insertions(+), 28 deletions(-) diff --git a/compat/poll/poll.c b/compat/poll/poll.c index 0e95dd493c..afa6d24584 100644 --- a/compat/poll/poll.c +++ b/compat/poll/poll.c @@ -139,22 +139,10 @@ win32_compute_revents (HANDLE h, int *p_sought) INPUT_RECORD *irbuffer; DWORD avail, nbuffer; BOOL bRet; - IO_STATUS_BLOCK iosb; - FILE_PIPE_LOCAL_INFORMATION fpli; - static PNtQueryInformationFile NtQueryInformationFile; - static BOOL once_only; switch (GetFileType (h)) { case FILE_TYPE_PIPE: - if (!once_only) - { - NtQueryInformationFile = (PNtQueryInformationFile)(void (*)(void)) - GetProcAddress (GetModuleHandleW (L"ntdll.dll"), - "NtQueryInformationFile"); - once_only = TRUE; - } - happened = 0; if (PeekNamedPipe (h, NULL, 0, NULL, &avail, NULL) != 0) { @@ -166,22 +154,9 @@ win32_compute_revents (HANDLE h, int *p_sought) else { - /* It was the write-end of the pipe. Check if it is writable. - If NtQueryInformationFile fails, optimistically assume the pipe is - writable. This could happen on Win9x, where NtQueryInformationFile - is not available, or if we inherit a pipe that doesn't permit - FILE_READ_ATTRIBUTES access on the write end (I think this should - not happen since WinXP SP2; WINE seems fine too). Otherwise, - ensure that enough space is available for atomic writes. */ - memset (&iosb, 0, sizeof (iosb)); - memset (&fpli, 0, sizeof (fpli)); - - if (!NtQueryInformationFile - || NtQueryInformationFile (h, &iosb, &fpli, sizeof (fpli), - FilePipeLocalInformation) - || fpli.WriteQuotaAvailable >= PIPE_BUF - || (fpli.OutboundQuota < PIPE_BUF && - fpli.WriteQuotaAvailable == fpli.OutboundQuota)) + /* It was the write-end of the pipe. Unfortunately there is no + reliable way of knowing if it can be written without blocking. + Just say that it's all good. */ happened |= *p_sought & (POLLOUT | POLLWRNORM | POLLWRBAND); } return happened; From 7daf4f2ac7b8fdddf6d8e128dc30e7daec7abad2 Mon Sep 17 00:00:00 2001 From: Ralf Thielow Date: Thu, 27 Feb 2020 20:25:30 +0000 Subject: [PATCH 23/27] rebase-interactive.c: silence format-zero-length warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes the following warnings: rebase-interactive.c: In function ‘edit_todo_list’: rebase-interactive.c:137:38: warning: zero-length gnu_printf format string [-Wformat-zero-length] write_file(rebase_path_dropped(), ""); rebase-interactive.c:144:37: warning: zero-length gnu_printf format string [-Wformat-zero-length] write_file(rebase_path_dropped(), ""); Signed-off-by: Ralf Thielow Signed-off-by: Junio C Hamano --- rebase-interactive.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rebase-interactive.c b/rebase-interactive.c index ac001dea58..0a4572e67e 100644 --- a/rebase-interactive.c +++ b/rebase-interactive.c @@ -134,14 +134,14 @@ int edit_todo_list(struct repository *r, struct todo_list *todo_list, if (incorrect) { if (todo_list_check_against_backup(r, new_todo)) { - write_file(rebase_path_dropped(), ""); + write_file(rebase_path_dropped(), "%s", ""); return -4; } if (incorrect > 0) unlink(rebase_path_dropped()); } else if (todo_list_check(todo_list, new_todo)) { - write_file(rebase_path_dropped(), ""); + write_file(rebase_path_dropped(), "%s", ""); return -4; } From 7329d94be72654f7f211e1bfa5ce3dd6fd2dc1fa Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 27 Feb 2020 18:54:45 -0500 Subject: [PATCH 24/27] config.mak.dev: re-enable -Wformat-zero-length We recently triggered some -Wformat-zero-length warnings in the code, but no developers noticed because we suppress that warning in builds with the DEVELOPER=1 Makefile knob set. But we _don't_ suppress them in a non-developer build (and they're part of -Wall). So even though non-developers probably aren't using -Werror, they see the annoying warnings when they build. We've had back and forth discussion over the years on whether this warning is useful or not. In most cases we've seen, it's not true that the call is a mistake, since we're using its side effects (like adding a newline status_printf_ln()) or writing an empty string to a destination which is handled by the function (as in write_file()). And so we end up working around it in the source by passing ("%s", ""). There's more discussion in the subthread starting at: https://lore.kernel.org/git/xmqqtwaod7ly.fsf@gitster.mtv.corp.google.com/ The short of it is that we probably can't just disable the warning for everybody because of portability issues. And ignoring it for developers puts us in the situation we're in now, where non-dev builds are annoyed. Since the workaround is both rarely needed and fairly straight-forward, let's just commit to doing it as necessary, and re-enable the warning. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- config.mak.dev | 1 - 1 file changed, 1 deletion(-) diff --git a/config.mak.dev b/config.mak.dev index bf1f3fcdee..89b218d11a 100644 --- a/config.mak.dev +++ b/config.mak.dev @@ -9,7 +9,6 @@ endif DEVELOPER_CFLAGS += -Wall DEVELOPER_CFLAGS += -Wdeclaration-after-statement DEVELOPER_CFLAGS += -Wformat-security -DEVELOPER_CFLAGS += -Wno-format-zero-length DEVELOPER_CFLAGS += -Wold-style-definition DEVELOPER_CFLAGS += -Woverflow DEVELOPER_CFLAGS += -Wpointer-arith From 237a28173feffc9bdec5aa669aa17fba1ac7c279 Mon Sep 17 00:00:00 2001 From: Harald van Dijk Date: Sat, 29 Feb 2020 13:07:57 +0000 Subject: [PATCH 25/27] show_one_mergetag: print non-parent in hex form. When a mergetag names a non-parent, which can occur after a shallow clone, its hash was previously printed as raw data. Print it in hex form instead. Signed-off-by: Harald van Dijk Signed-off-by: Junio C Hamano --- log-tree.c | 2 +- t/t4202-log.sh | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/log-tree.c b/log-tree.c index 1e56df62a7..f02343a461 100644 --- a/log-tree.c +++ b/log-tree.c @@ -514,7 +514,7 @@ static int show_one_mergetag(struct commit *commit, "merged tag '%s'\n", tag->tag); else if ((nth = which_parent(&tag->tagged->oid, commit)) < 0) strbuf_addf(&verify_message, "tag %s names a non-parent %s\n", - tag->tag, tag->tagged->oid.hash); + tag->tag, oid_to_hex(&tag->tagged->oid)); else strbuf_addf(&verify_message, "parent #%d, tagged '%s'\n", nth + 1, tag->tag); diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 819c24d10e..eecac1801b 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -1596,6 +1596,26 @@ test_expect_success GPG 'log --graph --show-signature for merged tag' ' grep "^| | gpg: Good signature" actual ' +test_expect_success GPG 'log --graph --show-signature for merged tag in shallow clone' ' + test_when_finished "git reset --hard && git checkout master" && + git checkout -b plain-shallow master && + echo aaa >bar && + git add bar && + git commit -m bar_commit && + git checkout --detach master && + echo bbb >baz && + git add baz && + git commit -m baz_commit && + git tag -s -m signed_tag_msg signed_tag_shallow && + hash=$(git rev-parse HEAD) && + git checkout plain-shallow && + git merge --no-ff -m msg signed_tag_shallow && + git clone --depth 1 --no-local . shallow && + test_when_finished "rm -rf shallow" && + git -C shallow log --graph --show-signature -n1 plain-shallow >actual && + grep "tag signed_tag_shallow names a non-parent $hash" actual +' + test_expect_success GPGSM 'log --graph --show-signature for merged tag x509' ' test_when_finished "git reset --hard && git checkout master" && test_config gpg.format x509 && From 7655b4119d49844e6ebc62da571e5f18528dbde8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Tue, 3 Mar 2020 21:55:34 +0100 Subject: [PATCH 26/27] remote-curl: show progress for fetches over dumb HTTP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fetching over dumb HTTP transport doesn't show any progress, even with the option --progress. If the connection is slow or there is a lot of data to get then this can take a long time while the user is left to wonder if git got stuck. We don't know the number of objects to fetch at the outset, but we can count the ones we got. Show an open-ended progress indicator based on that number if the user asked for it. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- remote-curl.c | 1 + walker.c | 13 ++++++++++++- walker.h | 1 + 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/remote-curl.c b/remote-curl.c index 350d92a074..3ce85b6d5a 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -1026,6 +1026,7 @@ static int fetch_dumb(int nr_heads, struct ref **to_fetch) walker = get_http_walker(url.buf); walker->get_verbosely = options.verbosity >= 3; + walker->get_progress = options.progress; walker->get_recover = 0; ret = walker_fetch(walker, nr_heads, targets, NULL, NULL); walker_free(walker); diff --git a/walker.c b/walker.c index 06cd2bd569..6a579b230c 100644 --- a/walker.c +++ b/walker.c @@ -8,6 +8,7 @@ #include "tag.h" #include "blob.h" #include "refs.h" +#include "progress.h" static struct object_id current_commit_oid; @@ -162,6 +163,11 @@ static int process(struct walker *walker, struct object *obj) static int loop(struct walker *walker) { struct object_list *elem; + struct progress *progress = NULL; + uint64_t nr = 0; + + if (walker->get_progress) + progress = start_delayed_progress(_("Fetching objects"), 0); while (process_queue) { struct object *obj = process_queue->item; @@ -176,15 +182,20 @@ static int loop(struct walker *walker) */ if (! (obj->flags & TO_SCAN)) { if (walker->fetch(walker, obj->oid.hash)) { + stop_progress(&progress); report_missing(obj); return -1; } } if (!obj->type) parse_object(the_repository, &obj->oid); - if (process_object(walker, obj)) + if (process_object(walker, obj)) { + stop_progress(&progress); return -1; + } + display_progress(progress, ++nr); } + stop_progress(&progress); return 0; } diff --git a/walker.h b/walker.h index 6d8ae00e5b..d40b016bab 100644 --- a/walker.h +++ b/walker.h @@ -10,6 +10,7 @@ struct walker { int (*fetch)(struct walker *, unsigned char *sha1); void (*cleanup)(struct walker *); int get_verbosely; + int get_progress; int get_recover; int corrupt_object_found; From b4374e96c84ed9394fed363973eb540da308ed4f Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 9 Mar 2020 11:20:59 -0700 Subject: [PATCH 27/27] Git 2.26-rc1 Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.26.0.txt | 27 +++++++++++++++++++++++++++ GIT-VERSION-GEN | 2 +- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/Documentation/RelNotes/2.26.0.txt b/Documentation/RelNotes/2.26.0.txt index 1ca76ac8d0..13c2684a19 100644 --- a/Documentation/RelNotes/2.26.0.txt +++ b/Documentation/RelNotes/2.26.0.txt @@ -47,6 +47,18 @@ UI, Workflows & Features * "git clone --recurse-submodules --single-branch" now uses the same single-branch option when cloning the submodules. + * "git rm" and "git stash" learns the new "--pathspec-from-file" + option. + + * "git am --short-current-patch" is a way to show the piece of e-mail + for the stopped step, which is not suitable to directly feed "git + apply" (it is designed to be a good "git am" input). It learned a + new option to show only the patch part. + + * Handling of conflicting renames in merge-recursive have further + been made consistent with how existing codepaths try to mimic what + is done to add/add conflicts. + Performance, Internal Implementation, Development Support etc. @@ -288,6 +300,18 @@ Fixes since v2.25 reverted. (merge 0106b1d4be hi/gpg-use-check-signature later to maint). + * MinGW's poll() emulation has been improved. + (merge 94f4d01932 am/mingw-poll-fix later to maint). + + * "git show" and others gave an object name in raw format in its + error output, which has been corrected to give it in hex. + (merge 237a28173f hd/show-one-mergetag-fix later to maint). + + * "git fetch" over HTTP walker protocol did not show any progress + output. We inherently do not know how much work remains, but still + we can show something not to bore users. + (merge 7655b4119d rs/show-progress-in-dumb-http-fetch later to maint). + * Other code cleanup, docfix, build fix, etc. (merge 26f924d50e en/simplify-check-updates-in-unpack-trees later to maint). (merge d0d0a357a1 am/update-pathspec-f-f-tests later to maint). @@ -315,3 +339,6 @@ Fixes since v2.25 (merge 240fc04f81 ag/rebase-remove-redundant-code later to maint). (merge 7f487ce062 js/ci-windows-update later to maint). (merge d68ce906c7 rs/commit-graph-code-simplification later to maint). + (merge a51d9e8f07 rj/t1050-use-test-path-is-file later to maint). + (merge fd0bc17557 kk/complete-diff-color-moved later to maint). + (merge 65bf820d0e en/test-cleanup later to maint). diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index da14713427..41c2d7c885 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v2.26.0-rc0 +DEF_VER=v2.26.0-rc1 LF=' '