# [PATCH 0/3] Add `-p' option to `git-mv', inspired by `mkdir' 11 messages from 2023-10-09 to 2023-10-31. Participants: Hugo Sales, Junio C Hamano, Dragan Simic. Thread: https://gitlist.dev/t/60335 ## Hugo Sales, 2023-10-09 23:34 Subject: [PATCH 0/3] Add `-p' option to `git-mv', inspired by `mkdir' Message-ID: <20231009233458.1371351-1-hugo@hsal.es> URL: https://gitlist.dev/e/20231009233458.1371351-1-hugo%40hsal.es ``` Allow passing a new `-p'/`--parents' option to `git-mv' making it so missing directories in the given target path get created if they don't exist. This allows running `git mv -p foo bar/quux/' to first create the `bar/' and `bar/quux/' directories if they don't exist, and then move `foo' to `bar/quux/foo'. This is inspired by `mkdir -p foo/bar/quux/' which will create the `foo/', `foo/bar/', and `foo/bar/quux/' directories if they don't exist. Hugo Sales (3): mv: Add -p option to create parent directories mv: Add tests for new -p flag mv: Add documentation for new `-p' flag Documentation/git-mv.txt | 12 ++++-- builtin/mv.c | 27 +++++++++----- t/t7009-mv-parents.sh | 79 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 105 insertions(+), 13 deletions(-) create mode 100755 t/t7009-mv-parents.sh base-commit: 3a06386e314565108ad56a9bdb8f7b80ac52fb69 -- 2.42.0 ``` ## Hugo Sales, 2023-10-09 23:34 Subject: [PATCH 2/3] mv: Add tests for new -p flag Message-ID: <20231009233458.1371351-3-hugo@hsal.es> URL: https://gitlist.dev/e/20231009233458.1371351-3-hugo%40hsal.es In-Reply-To: <20231009233458.1371351-1-hugo@hsal.es> ``` Signed-off-by: Hugo Sales --- t/t7009-mv-parents.sh | 79 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100755 t/t7009-mv-parents.sh diff --git a/t/t7009-mv-parents.sh b/t/t7009-mv-parents.sh new file mode 100755 index 0000000000..cc1d9dae08 --- /dev/null +++ b/t/t7009-mv-parents.sh @@ -0,0 +1,79 @@ +#!/bin/sh + +test_description='git mv -p' +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME + +. ./test-lib.sh + +test_expect_success 'mv fails to move a file if the target directory does not exist' ' + echo test >test1 && + git add test1 && + test_must_fail git mv test1 foo/ +' + +test_expect_success 'mv fails to move multiple files if the target directory does not exist' ' + echo test >test2-1 && + echo test >test2-2 && + git add test2-1 test2-2 && + test_must_fail git mv test2-1 test2-2 foo/ +' + +test_expect_success 'mv fails to move a file if the target refers to a file in a directory that does not exist' ' + echo test >test3 && + git add test3 && + test_must_fail git mv test3 foo/test3.txt +' + +test_expect_success 'mv succeeds to move a file even if the target directory does not exist' ' + echo test >test4 && + git add test4 && + git commit -m test4-commit1 && + git mv -p test4 dir4/ && + git commit -m test4-commit2 && + git diff-tree -r -M --name-status HEAD^ HEAD >test4-actual && + grep "^R100..*test4..*dir4/test4" test4-actual +' + +test_expect_success 'mv succeeds to move multiple files even if the target directory does not exist' ' + echo test >test5-1 && + echo test >test5-2 && + git add test5-1 test5-2 && + git commit -m test5-commit1 && + git mv -p test5-1 test5-2 dir5/ && + git commit -m test5-commit2 && + git diff-tree -r -M --name-status HEAD^ HEAD >test5-actual && + grep -e "^R100..*test5-1..*dir5/test5-1" -e "^R100..*test5-2..*dir5/test5-2" test5-actual +' + +test_expect_success 'mv succeeds to move a file even if the target refers to a file in a directory that does not exist' ' + echo test >test6 && + git add test6 && + git commit -m test6-commmit-1 && + git mv -p test6 dir6/test6.txt && + git commit -m test6-commit2 && + git diff-tree -r -M --name-status HEAD^ HEAD >test6-actual && + grep "^R100..*test6..*dir6/test6.txt" test6-actual +' + +test_expect_success 'mv succeeds to move a file even if the target refers to a file in a directory inside a directory that does not exist' ' + echo test >test7 && + git add test7 && + git commit -m test7-commit1 && + git mv -p test7 dir7/dir7/test7.txt && + git commit -m test7-commit2 && + git diff-tree -r -M --name-status HEAD^ HEAD >test7-actual && + grep "^R100..*test7..*dir7/dir7/test7.txt" test7-actual +' + +test_expect_success 'mv succeeds to move a file even if the target refers to a file in a directory inside a directory inside a directory that does not exist' ' + echo test >test8 && + git add test8 && + git commit -m test8-commit1 && + git mv -p test8 dir8/dir8/dir8/test8.txt && + git commit -m test8-commit2 && + git diff-tree -r -M --name-status HEAD^ HEAD >test8-actual && + grep "^R100..*test8..*dir8/dir8/dir8/test8.txt" test8-actual +' + +test_done -- 2.42.0 ``` ## Hugo Sales, 2023-10-09 23:34 Subject: [PATCH 3/3] mv: Add documentation for new `-p' flag Message-ID: <20231009233458.1371351-4-hugo@hsal.es> URL: https://gitlist.dev/e/20231009233458.1371351-4-hugo%40hsal.es In-Reply-To: <20231009233458.1371351-1-hugo@hsal.es> ``` Signed-off-by: Hugo Sales --- Documentation/git-mv.txt | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/Documentation/git-mv.txt b/Documentation/git-mv.txt index fb0220fd18..70c1e9572c 100644 --- a/Documentation/git-mv.txt +++ b/Documentation/git-mv.txt @@ -15,13 +15,14 @@ DESCRIPTION ----------- Move or rename a file, directory or symlink. - git mv [-v] [-f] [-n] [-k] - git mv [-v] [-f] [-n] [-k] ... + git mv [-v] [-f] [-n] [-k] [-p] + git mv [-v] [-f] [-n] [-k] [-p] ... In the first form, it renames , which must exist and be either a file, symlink or directory, to . -In the second form, the last argument has to be an existing -directory; the given sources will be moved into this directory. +In the second form, the last argument refers to a directory, which has +to exist unless -p is specified, in which case it gets created; the +given sources will be moved into this directory. The index is updated after successful completion, but the change must still be committed. @@ -39,6 +40,9 @@ OPTIONS -n:: --dry-run:: Do nothing; only show what would happen +-p:: +--parents:: + Create missing parent directories in the or path. Similar to `mkdir -p', all missing leading directories are created. -v:: --verbose:: -- 2.42.0 ``` ## Hugo Sales, 2023-10-09 23:34 Subject: [PATCH 1/3] mv: Add -p option to create parent directories Message-ID: <20231009233458.1371351-2-hugo@hsal.es> URL: https://gitlist.dev/e/20231009233458.1371351-2-hugo%40hsal.es In-Reply-To: <20231009233458.1371351-1-hugo@hsal.es> ``` Inspired by "mkdir -p", this patch allows specifying a "-p" or "--parents" flag which will create all non-existent directories in the destination path before renaming the file. This allows the user to not have to run two commands to move files to a new directory. Signed-off-by: Hugo Sales --- builtin/mv.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/builtin/mv.c b/builtin/mv.c index c596515ad0..5d64d86179 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -168,7 +168,7 @@ static int empty_dir_has_sparse_contents(const char *name) int cmd_mv(int argc, const char **argv, const char *prefix) { int i, flags, gitmodules_modified = 0; - int verbose = 0, show_only = 0, force = 0, ignore_errors = 0, ignore_sparse = 0; + int verbose = 0, show_only = 0, force = 0, ignore_errors = 0, ignore_sparse = 0, create_parents = 0; struct option builtin_mv_options[] = { OPT__VERBOSE(&verbose, N_("be verbose")), OPT__DRY_RUN(&show_only, N_("dry run")), @@ -176,6 +176,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix) PARSE_OPT_NOCOMPLETE), OPT_BOOL('k', NULL, &ignore_errors, N_("skip move/rename errors")), OPT_BOOL(0, "sparse", &ignore_sparse, N_("allow updating entries outside of the sparse-checkout cone")), + OPT_BOOL('p', "parents", &create_parents, N_("create missing parent directories")), OPT_END(), }; const char **source, **destination, **dest_path, **submodule_gitfile; @@ -220,8 +221,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix) if (dest_path[0][0] == '\0') /* special case: "." was normalized to "" */ destination = internal_prefix_pathspec(dest_path[0], argv, argc, DUP_BASENAME); - else if (!lstat(dest_path[0], &st) && - S_ISDIR(st.st_mode)) { + else if (create_parents || + (!lstat(dest_path[0], &st) && S_ISDIR(st.st_mode))) { destination = internal_prefix_pathspec(dst_w_slash, argv, argc, DUP_BASENAME); } else { if (!path_in_sparse_checkout(dst_w_slash, &the_index) && @@ -381,7 +382,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix) bad = _("multiple sources for the same target"); goto act_on_entry; } - if (is_dir_sep(dst[strlen(dst) - 1])) { + + if (!create_parents && is_dir_sep(dst[strlen(dst) - 1])) { bad = _("destination directory does not exist"); goto act_on_entry; } @@ -459,11 +461,18 @@ int cmd_mv(int argc, const char **argv, const char *prefix) if (show_only) continue; if (!(mode & (INDEX | SPARSE | SKIP_WORKTREE_DIR)) && - !(dst_mode & (SKIP_WORKTREE_DIR | SPARSE)) && - rename(src, dst) < 0) { - if (ignore_errors) - continue; - die_errno(_("renaming '%s' failed"), src); + !(dst_mode & (SKIP_WORKTREE_DIR | SPARSE))) { + if (create_parents && safe_create_leading_directories_const(dst) < 0) { + if (ignore_errors) + continue; + die_errno(_("creating parent directories for '%s' failed"), dst); + } + + if (rename(src, dst) < 0) { + if (ignore_errors) + continue; + die_errno(_("renaming '%s' failed"), src); + } } if (submodule_gitfile[i]) { if (!update_path_in_gitmodules(src, dst)) -- 2.42.0 ``` ## Junio C Hamano, 2023-10-10 00:39 Subject: Re: [PATCH 0/3] Add `-p' option to `git-mv', inspired by `mkdir' Message-ID: URL: https://gitlist.dev/e/xmqq1qe3wbt1.fsf%40gitster.g In-Reply-To: <20231009233458.1371351-1-hugo@hsal.es> ``` Hugo Sales writes: > Allow passing a new `-p'/`--parents' option to `git-mv' making it so > missing directories in the given target path get created if they don't > exist. This allows running `git mv -p foo bar/quux/' to first create > the `bar/' and `bar/quux/' directories if they don't exist, and then > move `foo' to `bar/quux/foo'. > > This is inspired by `mkdir -p foo/bar/quux/' which will create the > `foo/', `foo/bar/', and `foo/bar/quux/' directories if they don't > exist. I'll step back and ask a related design question without reading the patches (yet). Is there a reason why somebody benefits by us retaining the current behaviour, where $ git mv file no/such/dir/yet/file fails with "No such file or directory", and they are forced to say either $ mkdir -p no/such/dir/yet $ git mv file no/such/dir/yet/file or $ git mv -p file no/such/dir/yet/file Yes, what I am getting at is if it makes sense to just "fix" the behaviour of "git mv" about missing leading directories and do what the end-user wanted to do without requiring "-p". Regardless of the "do we require, or is it sifficient to imply, the '-p' option when we lack leading directories?" question, once we start "auto-creating" the leading directory hierarchy, one worrysome thing such a new feature introduces is an ambiguity and unexpected behaviour. Imagine there is no "no" directory (so nothing under it exists), and you did this (you do have a regular file "file"). $ git mv [-p] file no/such/dir/yet What should happen? Would we do the equivalent of $ mkdir -p no/such/dir && git mv file no/such/dir/yet or are we doing $ mkdir -p no/such/dir/yet && git mv file no/such/dir/yet/file Both are plausible, and "mkdir -p" does not have such a nasty ambiguity. That is what makes me unsure about the new feature (again, either with required "-p" or with implied "-p"). Thanks. ``` ## Junio C Hamano, 2023-10-10 00:59 Subject: Re: [PATCH 0/3] Add `-p' option to `git-mv', inspired by `mkdir' Message-ID: URL: https://gitlist.dev/e/xmqqttqzuwah.fsf%40gitster.g In-Reply-To: ``` Junio C Hamano writes: > Regardless of the "do we require, or is it sifficient to imply, the > '-p' option when we lack leading directories?" question, once we > start "auto-creating" the leading directory hierarchy, one worrysome > thing such a new feature introduces is an ambiguity and unexpected > behaviour. ... > Both are plausible, and "mkdir -p" does not have such a nasty > ambiguity. That is what makes me unsure about the new feature > (again, either with required "-p" or with implied "-p"). I checked what POSIX, who does give "-p" to mkdir(1), has with mv(1), and they do not have "-p". Neither GNU, which tends to add such "usability" enhancements on top of what everybody else has. And I think they are being very sensible. By not adding such an option to "mv", they can sidestep the unnecessary ambiguity, and it isn't too bad to do a "mkdir -p" separately before doing such a "mv" into a (potential) new directory. So, let's not do this patch. Thanks. ``` ## Hugo Sales, 2023-10-11 12:33 Subject: Re: [PATCH 0/3] Add `-p' option to `git-mv', inspired by `mkdir' Message-ID: <1384513657.119681.1697027599941@office.mailbox.org> URL: https://gitlist.dev/e/1384513657.119681.1697027599941%40office.mailbox.org In-Reply-To: ``` > On 10/10/2023 12:39 AM GMT Junio C Hamano wrote: > > Is there a reason why somebody benefits by us retaining the current > behaviour, where > > $ git mv file no/such/dir/yet/file > > fails with "No such file or directory", and they are forced to say > either > > $ mkdir -p no/such/dir/yet > $ git mv file no/such/dir/yet/file > > or > > $ git mv -p file no/such/dir/yet/file Somewhat, as it could be a typo, so if you actually want to create a new folder, you have to be explicit. I wouldn't be opposed to removing the need for the flag, but if we did, I think we should warn the user that a new directory was created. > Imagine there is no "no" directory (so nothing under it > exists), and you did this (you do have a regular file "file"). > > $ git mv [-p] file no/such/dir/yet > > What should happen? Would we do the equivalent of > > $ mkdir -p no/such/dir && git mv file no/such/dir/yet > > or are we doing > > $ mkdir -p no/such/dir/yet && git mv file no/such/dir/yet/file > > Both are plausible, and "mkdir -p" does not have such a nasty > ambiguity. That is what makes me unsure about the new feature > (again, either with required "-p" or with implied "-p"). I think the ambiguity is resolved by the inclusion of lack thereof of a trailing `/`. This is what the implementation already does, too, before this patch. So $ git mv [-p] file no/such/dir/yet would be the same as $ mkdir -p no/such/dir && git mv file no/such/dir/yet and $ git mv [-p] file no/such/dir/yet/ would be the same as $ mkdir -p no/such/dir/yet && git mv file no/such/dir/yet/ Moving `file` to `no/such/dir/yet/file` Thanks for the feedback, Hugo P.S. Apologies for the duplicated email, Junio, I did a Reply instead of a Reply All ``` ## Junio C Hamano, 2023-10-31 04:30 Subject: Re: [PATCH 0/3] Add `-p' option to `git-mv', inspired by `mkdir' Message-ID: URL: https://gitlist.dev/e/xmqqlebj4dnt.fsf%40gitster.g In-Reply-To: <1384513657.119681.1697027599941@office.mailbox.org> ``` Hugo Sales writes: >> Both are plausible, and "mkdir -p" does not have such a nasty >> ambiguity. That is what makes me unsure about the new feature >> (again, either with required "-p" or with implied "-p"). > > I think the ambiguity is resolved by the inclusion of lack thereof > of a trailing `/`. The question is not if we can come up with a rule that the user can use to disambiguate. It is if the user will find that such a rule is a naturally acceptable way to disambiguate. When both of git mv file there/exists/such/a/directory git mv file there/exists/such/a/directory/ create "there/exists/such/a/directory/file" and removes "file", with or without a trailing slash, "you should add a slash if you want a directory, and otherwise you should not add a slash" is a rather arbitrary rule. Let's not go there. I still view the downside more grave than having to occasionally do "mkdir". ``` ## Dragan Simic, 2023-10-31 04:54 Subject: Re: [PATCH 0/3] Add `-p' option to `git-mv', inspired by `mkdir' Message-ID: URL: https://gitlist.dev/e/ffbb04b363938e4a487906134ce4f3c6%40manjaro.org In-Reply-To: ``` On 2023-10-31 05:30, Junio C Hamano wrote: > Hugo Sales writes: > >>> Both are plausible, and "mkdir -p" does not have such a nasty >>> ambiguity. That is what makes me unsure about the new feature >>> (again, either with required "-p" or with implied "-p"). >> >> I think the ambiguity is resolved by the inclusion of lack thereof >> of a trailing `/`. > > The question is not if we can come up with a rule that the user can > use to disambiguate. It is if the user will find that such a rule > is a naturally acceptable way to disambiguate. > > When both of > > git mv file there/exists/such/a/directory > git mv file there/exists/such/a/directory/ > > create "there/exists/such/a/directory/file" and removes "file", with > or without a trailing slash, "you should add a slash if you want a > directory, and otherwise you should not add a slash" is a rather > arbitrary rule. Let's not go there. I still view the downside more > grave than having to occasionally do "mkdir". Please note that the above-described git-mv operation succeeds with no trailing slash if "there/exists/such/a/directory" doesn't already exist as a directory, and creates "there/exists/such/a/directory" as a file. With the trailing slash, the git-mv operation succeeds only if "there/exists/such/a/directory" already exists as a directory, and fails otherwise. A quite similar ambiguity exists in cp(1) in mv(1), which is also resolved by the use of the trailing slash character. However, I've encountered only one person aware of that disambiguation, and in cp(1) only, but in the "I always include the trailing slash" way, without actually understanding it fully. Maybe I need to encounter more people, I don't know. ``` ## Junio C Hamano, 2023-10-31 05:58 Subject: Re: [PATCH 0/3] Add `-p' option to `git-mv', inspired by `mkdir' Message-ID: URL: https://gitlist.dev/e/xmqq5y2n49la.fsf%40gitster.g In-Reply-To: ``` Dragan Simic writes: > A quite similar ambiguity exists in cp(1) in mv(1), which is also > resolved by the use of the trailing slash character. However, I've > encountered only one person aware of that disambiguation, and in cp(1) > only, but in the "I always include the trailing slash" way, without > actually understanding it fully. Maybe I need to encounter more > people, I don't know. If the majority of (perhaps new) users you already know find such disambiguation method unfamiliar, that already is a good anecdata without any need for you to meet more people to tell us that it is not a very easy-to-understand thing for them, no? ``` ## Dragan Simic, 2023-10-31 06:38 Subject: Re: [PATCH 0/3] Add `-p' option to `git-mv', inspired by `mkdir' Message-ID: <245b1be0fcee3ade2d477fa1378647b8@manjaro.org> URL: https://gitlist.dev/e/245b1be0fcee3ade2d477fa1378647b8%40manjaro.org In-Reply-To: ``` On 2023-10-31 06:58, Junio C Hamano wrote: > Dragan Simic writes: > >> A quite similar ambiguity exists in cp(1) in mv(1), which is also >> resolved by the use of the trailing slash character. However, I've >> encountered only one person aware of that disambiguation, and in cp(1) >> only, but in the "I always include the trailing slash" way, without >> actually understanding it fully. Maybe I need to encounter more >> people, I don't know. > > If the majority of (perhaps new) users you already know find such > disambiguation method unfamiliar, that already is a good anecdata > without any need for you to meet more people to tell us that it is > not a very easy-to-understand thing for them, no? Quite frankly, I'm divided there. On the one hand, you're right that this disambiguation method is a bit confusing and it's absolutely not very well known. On the other hand, it's already out there in the wild, including git, and it's actually quite useful when used properly. If I had to vote, I'd give my vote to embracing this disambiguation method, but only with good documentation and some kind of education through an article or two. I believe that proper education simply isn't present, or at least not in a user-friendly manner, which should be corrected, regardless of the new "git mv -p" feature being accepted or not. ```