* feature-request: git "cp" like there is git mv. @ 2017-12-12 10:53 Simon Doodkin 2017-12-13 16:39 ` Johannes Schindelin ` (2 more replies) 0 siblings, 3 replies; 12+ messages in thread From: Simon Doodkin @ 2017-12-12 10:53 UTC (permalink / raw) To: git please develop a new feature, git "cp" like there is git mv tomovefile1 tofile2 (to save space). there is a solution in https://stackoverflow.com/a/44036771/466363 however, it is not single easy command. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: feature-request: git "cp" like there is git mv. 2017-12-12 10:53 feature-request: git "cp" like there is git mv Simon Doodkin @ 2017-12-13 16:39 ` Johannes Schindelin 2017-12-13 17:21 ` Randall S. Becker 2017-12-16 1:31 ` Jonathan Nieder 2017-12-18 0:28 ` Igor Djordjevic 2 siblings, 1 reply; 12+ messages in thread From: Johannes Schindelin @ 2017-12-13 16:39 UTC (permalink / raw) To: Simon Doodkin; +Cc: git Hi Simon, On Tue, 12 Dec 2017, Simon Doodkin wrote: > please develop a new feature, git "cp" like there is git mv tomovefile1 > tofile2 (to save space). > > there is a solution in https://stackoverflow.com/a/44036771/466363 > however, it is not single easy command. This is not how this project works. The idea is that it is Open Source, so that you can develop this feature yourself, and contribute a patch. Ciao, Johannes ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: feature-request: git "cp" like there is git mv. 2017-12-13 16:39 ` Johannes Schindelin @ 2017-12-13 17:21 ` Randall S. Becker 0 siblings, 0 replies; 12+ messages in thread From: Randall S. Becker @ 2017-12-13 17:21 UTC (permalink / raw) To: 'Johannes Schindelin', 'Simon Doodkin'; +Cc: git -----Original Message----- On December 13, 2017 11:40 AM Johannes Schindelin wrote: >On Tue, 12 Dec 2017, Simon Doodkin wrote: >> please develop a new feature, git "cp" like there is git mv >> tomovefile1 tofile2 (to save space). >> there is a solution in https://stackoverflow.com/a/44036771/466363 >> however, it is not single easy command. >This is not how this project works. The idea is that it is Open Source, so that you can develop this feature yourself, and contribute a patch. Agree with Johannes. Let's help though, to quantify the requirements so that Simon can get this right. I'm putting my tyrannical repository manager hat on here rather than developer so... Are you looking to have git cp copy the entire history of tomovefile1 to tofile2 or just copy the content of tomovefile1 to tofile2 and add and/or commit the file? In the latter, I see the convenience of this capability. Even so, a simple cp would copy the content and then you can commit it fairly easily. In the former, copying the entire history of a file inside the repository is going to potentially cause tofile2 to appear in old commits where prior to the git cp command the file was not present? In this situation, you are actually rewriting history and potentially impacting signed commits (which would no longer pass a signature check, I hope). Stitching repositories is sometimes done when repairs or reorganization is required, but I'm concerned that this is opening up a can of worms that breaks the atomicity of commits (particularly signed ones). What I don't want, for my own teams, is for members to think that git cp would be a harmless (unless it actually is) command, rather than a repair/reorg mechanism used for splitting apart a repository, or copying a file to a new project then splitting selectively. So, I'm obviously a bit confused about the goal. Simon: the stackoverflow post provides a few options on this command. Can you clarify which particular direction you are interest it? Cheers, Randall -- Brief whoami: NonStop&UNIX developer since approximately UNIX(421664400)/NonStop(211288444200000000) -- In my real life, I talk too much. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: feature-request: git "cp" like there is git mv. 2017-12-12 10:53 feature-request: git "cp" like there is git mv Simon Doodkin 2017-12-13 16:39 ` Johannes Schindelin @ 2017-12-16 1:31 ` Jonathan Nieder 2017-12-31 19:11 ` Stefan Moch 2017-12-18 0:28 ` Igor Djordjevic 2 siblings, 1 reply; 12+ messages in thread From: Jonathan Nieder @ 2017-12-16 1:31 UTC (permalink / raw) To: Simon Doodkin; +Cc: git Hi Simon, Simon Doodkin wrote: > please develop a new feature, git "cp" like there is git mv tomovefile1 tofile2 > (to save space). > > there is a solution in https://stackoverflow.com/a/44036771/466363 > however, it is not single easy command. This sounds like a reasonable thing to add. See builtin/mv.c for how "git mv" works if you're looking for inspiration. cmd_mv in that file looks rather long, so I'd also be happy if someone interested refactors to break it into multiple self-contained pieces for easier reading (git mostly follows https://www.kernel.org/doc/html/latest/process/coding-style.html#functions). Thanks, Jonathan ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: feature-request: git "cp" like there is git mv. 2017-12-16 1:31 ` Jonathan Nieder @ 2017-12-31 19:11 ` Stefan Moch 2017-12-31 19:11 ` [PATCH 1/2] Add test case for mv --dry-run to t7001-mv.sh Stefan Moch ` (2 more replies) 0 siblings, 3 replies; 12+ messages in thread From: Stefan Moch @ 2017-12-31 19:11 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Simon Doodkin, git * Jonathan Nieder <jrnieder@gmail.com> [2017-12-15T17:31:30-0800]: > This sounds like a reasonable thing to add. See builtin/mv.c for how > "git mv" works if you're looking for inspiration. > > cmd_mv in that file looks rather long, so I'd also be happy if someone > interested refactors to break it into multiple self-contained pieces > for easier reading (git mostly follows > https://www.kernel.org/doc/html/latest/process/coding-style.html#functions). I looked at builtin/mv.c and have a rough idea how to split it up to support both mv and cp commands. But first I noticed and removed a redundant check in cmd_mv, also added a test case to check if mv --dry-run does not move the file. Stefan ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] Add test case for mv --dry-run to t7001-mv.sh 2017-12-31 19:11 ` Stefan Moch @ 2017-12-31 19:11 ` Stefan Moch 2017-12-31 19:11 ` [PATCH 2/2] mv: remove unneeded 'if (!show_only)' Stefan Moch 2018-02-07 19:49 ` feature-request: git "cp" like there is git mv Junio C Hamano 2 siblings, 0 replies; 12+ messages in thread From: Stefan Moch @ 2017-12-31 19:11 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Simon Doodkin, git, Stefan Moch It checks if mv --dry-run does not move file. Signed-off-by: Stefan Moch <stefanmoch@mail.de> --- t/t7001-mv.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh index 6e5031f56..d4e6485a2 100755 --- a/t/t7001-mv.sh +++ b/t/t7001-mv.sh @@ -38,6 +38,12 @@ test_expect_success \ 'git diff-tree -r -M --name-status HEAD^ HEAD | \ grep "^R100..*path1/COPYING..*path0/COPYING"' +test_expect_success \ + 'mv --dry-run does not move file' \ + 'git mv -n path0/COPYING MOVED && + test -f path0/COPYING && + test ! -f MOVED' + test_expect_success \ 'checking -k on non-existing file' \ 'git mv -k idontexist path0' -- 2.14.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] mv: remove unneeded 'if (!show_only)' 2017-12-31 19:11 ` Stefan Moch 2017-12-31 19:11 ` [PATCH 1/2] Add test case for mv --dry-run to t7001-mv.sh Stefan Moch @ 2017-12-31 19:11 ` Stefan Moch 2018-02-07 19:49 ` feature-request: git "cp" like there is git mv Junio C Hamano 2 siblings, 0 replies; 12+ messages in thread From: Stefan Moch @ 2017-12-31 19:11 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Simon Doodkin, git, Stefan Moch Commit a127331cd (mv: allow moving nested submodules, 2016-04-19), introduced if (show_only) continue; in this for-loop before if (!show_only) which became redundant, because it is now always true. Signed-off-by: Stefan Moch <stefanmoch@mail.de> --- builtin/mv.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/builtin/mv.c b/builtin/mv.c index cf3684d90..8ce6a2ddd 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -286,8 +286,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix) pos = cache_name_pos(src, strlen(src)); assert(pos >= 0); - if (!show_only) - rename_cache_entry_at(pos, dst); + rename_cache_entry_at(pos, dst); } if (gitmodules_modified) -- 2.14.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: feature-request: git "cp" like there is git mv. 2017-12-31 19:11 ` Stefan Moch 2017-12-31 19:11 ` [PATCH 1/2] Add test case for mv --dry-run to t7001-mv.sh Stefan Moch 2017-12-31 19:11 ` [PATCH 2/2] mv: remove unneeded 'if (!show_only)' Stefan Moch @ 2018-02-07 19:49 ` Junio C Hamano 2018-02-07 20:27 ` Stefan Beller 2018-03-18 20:09 ` Stefan Moch 2 siblings, 2 replies; 12+ messages in thread From: Junio C Hamano @ 2018-02-07 19:49 UTC (permalink / raw) To: Stefan Moch; +Cc: Jonathan Nieder, Simon Doodkin, git, Stefan Beller Stefan Moch <stefanmoch@mail.de> writes: > * Jonathan Nieder <jrnieder@gmail.com> [2017-12-15T17:31:30-0800]: >> This sounds like a reasonable thing to add. See builtin/mv.c for how >> "git mv" works if you're looking for inspiration. >> >> cmd_mv in that file looks rather long, so I'd also be happy if someone >> interested refactors to break it into multiple self-contained pieces >> for easier reading (git mostly follows >> https://www.kernel.org/doc/html/latest/process/coding-style.html#functions). > > I looked at builtin/mv.c and have a rough idea how to split it > up to support both mv and cp commands. > > But first I noticed and removed a redundant check in cmd_mv, > also added a test case to check if mv --dry-run does not move > the file. I guess these two patches went unnoticed when posted at the end of last year. Reading them again, I think they are good changes. As a no-op clean-up of a127331c ("mv: allow moving nested submodules", 2016-04-19), the attached would also make sense, I would think. Thanks. builtin/mv.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/builtin/mv.c b/builtin/mv.c index 9662804d23..9cb07990fd 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -266,10 +266,11 @@ int cmd_mv(int argc, const char **argv, const char *prefix) const char *src = source[i], *dst = destination[i]; enum update_mode mode = modes[i]; int pos; - if (show_only || verbose) - printf(_("Renaming %s to %s\n"), src, dst); - if (show_only) + if (show_only) { + if (verbose) + printf(_("Renaming %s to %s\n"), src, dst); continue; + } if (mode != INDEX && rename(src, dst) < 0) { if (ignore_errors) continue; ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: feature-request: git "cp" like there is git mv. 2018-02-07 19:49 ` feature-request: git "cp" like there is git mv Junio C Hamano @ 2018-02-07 20:27 ` Stefan Beller 2018-03-18 20:09 ` Stefan Moch 1 sibling, 0 replies; 12+ messages in thread From: Stefan Beller @ 2018-02-07 20:27 UTC (permalink / raw) To: Junio C Hamano; +Cc: Stefan Moch, Jonathan Nieder, Simon Doodkin, git On Wed, Feb 7, 2018 at 11:49 AM, Junio C Hamano <gitster@pobox.com> wrote: > Stefan Moch <stefanmoch@mail.de> writes: > >> * Jonathan Nieder <jrnieder@gmail.com> [2017-12-15T17:31:30-0800]: >>> This sounds like a reasonable thing to add. See builtin/mv.c for how >>> "git mv" works if you're looking for inspiration. >>> >>> cmd_mv in that file looks rather long, so I'd also be happy if someone >>> interested refactors to break it into multiple self-contained pieces >>> for easier reading (git mostly follows >>> https://www.kernel.org/doc/html/latest/process/coding-style.html#functions). >> >> I looked at builtin/mv.c and have a rough idea how to split it >> up to support both mv and cp commands. >> >> But first I noticed and removed a redundant check in cmd_mv, >> also added a test case to check if mv --dry-run does not move >> the file. > > I guess these two patches went unnoticed when posted at the end of > last year. Reading them again, I think they are good changes. > > As a no-op clean-up of a127331c ("mv: allow moving nested > submodules", 2016-04-19), the attached would also make sense, I > would think. > > Thanks. > > builtin/mv.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/builtin/mv.c b/builtin/mv.c > index 9662804d23..9cb07990fd 100644 > --- a/builtin/mv.c > +++ b/builtin/mv.c > @@ -266,10 +266,11 @@ int cmd_mv(int argc, const char **argv, const char *prefix) > const char *src = source[i], *dst = destination[i]; > enum update_mode mode = modes[i]; > int pos; > - if (show_only || verbose) > - printf(_("Renaming %s to %s\n"), src, dst); > - if (show_only) > + if (show_only) { > + if (verbose) > + printf(_("Renaming %s to %s\n"), src, dst); > continue; > + } This is actually changing behavior to if (show_only && verbose) print(...) if show_only continue The second part is already there as is, only the printing behavior actually changes. So I might be missing the obvious here for the claim of no-op? Looking up further we have (line 177): if (show_only) printf(_("Checking rename of '%s' to '%s'\n"), src, dst); which prints regardless of verbosity. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: feature-request: git "cp" like there is git mv. 2018-02-07 19:49 ` feature-request: git "cp" like there is git mv Junio C Hamano 2018-02-07 20:27 ` Stefan Beller @ 2018-03-18 20:09 ` Stefan Moch 2018-03-19 20:03 ` Junio C Hamano 1 sibling, 1 reply; 12+ messages in thread From: Stefan Moch @ 2018-03-18 20:09 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jonathan Nieder, Simon Doodkin, git, Stefan Beller * Junio C Hamano <gitster@pobox.com> [2018-02-07T11:49:39-0800]: > Stefan Moch <stefanmoch@mail.de> writes: > > > * Jonathan Nieder <jrnieder@gmail.com> [2017-12-15T17:31:30-0800]: > >> This sounds like a reasonable thing to add. See builtin/mv.c for > >> how "git mv" works if you're looking for inspiration. > >> > >> cmd_mv in that file looks rather long, so I'd also be happy if > >> someone interested refactors to break it into multiple > >> self-contained pieces for easier reading (git mostly follows > >> https://www.kernel.org/doc/html/latest/process/coding-style.html#functions). > > > > I looked at builtin/mv.c and have a rough idea how to split it > > up to support both mv and cp commands. > > > > But first I noticed and removed a redundant check in cmd_mv, > > also added a test case to check if mv --dry-run does not move > > the file. > > I guess these two patches went unnoticed when posted at the end of > last year. Reading them again, I think they are good changes. Thanks. Are such redundant checks in general a pattern worth searching for and cleaning up globally? Or is this rather in the category of cleaning up only when noticed? > As a no-op clean-up of a127331c ("mv: allow moving nested > submodules", 2016-04-19), the attached would also make sense, I > would think. > > Thanks. > > builtin/mv.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/builtin/mv.c b/builtin/mv.c > index 9662804d23..9cb07990fd 100644 > --- a/builtin/mv.c > +++ b/builtin/mv.c > @@ -266,10 +266,11 @@ int cmd_mv(int argc, const char **argv, const > char *prefix) const char *src = source[i], *dst = destination[i]; > enum update_mode mode = modes[i]; > int pos; > - if (show_only || verbose) > - printf(_("Renaming %s to %s\n"), src, dst); > - if (show_only) > + if (show_only) { > + if (verbose) > + printf(_("Renaming %s to %s\n"), > src, dst); continue; > + } > if (mode != INDEX && rename(src, dst) < 0) { > if (ignore_errors) > continue; > As Stefan Beller already noted, this changes the printing behavior: <https://public-inbox.org/git/CAGZ79kbX4uhDpdp0kH=8+5tj_zLWZbtbMUb5WWtOeXWRQz8K3Q@mail.gmail.com/> See also the output of git mv -n git mv -n -v git mv -v without your patch: $ git mv -n 1 2 Checking rename of '1' to '2' Renaming 1 to 2 $ git mv -n -v 1 2 Checking rename of '1' to '2' Renaming 1 to 2 $ git mv -v 1 2 Renaming 1 to 2 and with your patch: $ git mv -n 1 2 Checking rename of '1' to '2' $ git mv -n -v 1 2 Checking rename of '1' to '2' Renaming 1 to 2 $ git mv -v 1 2 Having different outputs of “git mv -n” and “git mv -n -v” seems odd, but not necessarily wrong. However, “git mv -v” with no output at all, does not what the documentation says: -v, --verbose Report the names of files as they are moved. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: feature-request: git "cp" like there is git mv. 2018-03-18 20:09 ` Stefan Moch @ 2018-03-19 20:03 ` Junio C Hamano 0 siblings, 0 replies; 12+ messages in thread From: Junio C Hamano @ 2018-03-19 20:03 UTC (permalink / raw) To: Stefan Moch; +Cc: Jonathan Nieder, Simon Doodkin, git, Stefan Beller Stefan Moch <stefanmoch@mail.de> writes: > Are such redundant checks in general a pattern worth searching > for and cleaning up globally? Or is this rather in the category > of cleaning up only when noticed? A clean-up patch that is otherwise a no-op is still welcome as it will improve the health of the codebase, but they become hindrances if there are too many of them to consume the review bandwidth that would otherwise be better spent on other non no-op topics, and/or if they add too many merge conflicts with other non no-op topics in flight. The amount of such negative impact a no-op clean-up patch can have on the project does not depend on how the issue was discovered, so we do not even have to know if the issue was discovered by actively hunting or by noticing while working on a near-by area. It is possible that by actively looking for, you may end up producing more of the no-op clean-up patches and can more easily interfere with other topics, which we may need to discourge or at least ask you to slow down. On the other hand, issues discovered while working on a near-by area would typically not increase conflicts with other topics in flight over the conflicts that would be caused by that real work you were doing in a near-by area already, so in that sense, "only when noticed" is a practical way to avoid the clean-up fatigue. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: feature-request: git "cp" like there is git mv. 2017-12-12 10:53 feature-request: git "cp" like there is git mv Simon Doodkin 2017-12-13 16:39 ` Johannes Schindelin 2017-12-16 1:31 ` Jonathan Nieder @ 2017-12-18 0:28 ` Igor Djordjevic 2 siblings, 0 replies; 12+ messages in thread From: Igor Djordjevic @ 2017-12-18 0:28 UTC (permalink / raw) To: Simon Doodkin; +Cc: git Hi Simon, On 12/12/2017 11:53, Simon Doodkin wrote: > > please develop a new feature, git "cp" like there is git mv > tomovefile1 tofile2 (to save space). > > there is a solution in https://stackoverflow.com/a/44036771/466363 > however, it is not single easy command. While having `git cp` alongside `git mv` would make sense, I`m afraid that is not what you are really after, nor it would help in your case. Looking at referenced "Stack Overflow" post[1], it tries to address `git blame` not following "file copy", where it does "file rename". Proposed steps seem to be "solution" from your perspective, and while that may be absolutely valid and acceptable for your specific case, I would argue it`s the wrong approach in general - because `git blame` already supports what you want (just not by default), and making three additional, unneeded and possibly confusing commits (one being a merge), just to "bend" `git blame` to fit your (out of line?) usage expectations doesn`t seem right. I would say a better direction might be using `git blame` "-C[<num>]" option[2], where desired effect is achieved without any artificial history fiddling. Example being worth more than plain words, I`m providing a script[3] that demonstrates what I`m talking about :) Regards, Buga [1] https://stackoverflow.com/a/44036771/466363 [2] https://git-scm.com/docs/git-blame#git-blame--Cltnumgt [3] Demo script showing how using (multiple) "-C" option(s), with specified numeric value, can make `git blame` provide desired information, recognizing "file copy" operation (line copy, actually, but that is what we are really interested in, using `git blame`): --- >8 --- git init echo a >A echo b >>A echo c >>A git add A git commit -m "create file A" git mv A B git commit -m "rename file A -> B" # --- # (A) regular flow cp B C git add C git commit -m "copy file B -> C" # --- # --- # (B) proposed "solution", https://stackoverflow.com/a/44036771/466363 # git mv B C # git commit -n -m "rename file B -> C" # SAVED=`git rev-parse HEAD` # git reset --hard HEAD^ # git mv B B-temp # git commit -n -m "rename file B -> B-temp" # git merge $SAVED # This will generate conflicts # git commit -a -n --no-edit # Trivially resolved like this # git mv B-temp B # git commit -n -m "rename file B-temp -> B" # --- echo echo '(1) blames B back to A, as expected:' git blame -- B # git blame shows that file B has a history (back to file A)... echo echo '(2) blames C only, missing B and A:' git blame -- C # ... while file C doesn't have a history echo echo '(3) blames C back to A, as expected:' git blame -C -C3 -- C # git blame shows that file C has a history (back to file A) ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-03-19 20:03 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-12-12 10:53 feature-request: git "cp" like there is git mv Simon Doodkin 2017-12-13 16:39 ` Johannes Schindelin 2017-12-13 17:21 ` Randall S. Becker 2017-12-16 1:31 ` Jonathan Nieder 2017-12-31 19:11 ` Stefan Moch 2017-12-31 19:11 ` [PATCH 1/2] Add test case for mv --dry-run to t7001-mv.sh Stefan Moch 2017-12-31 19:11 ` [PATCH 2/2] mv: remove unneeded 'if (!show_only)' Stefan Moch 2018-02-07 19:49 ` feature-request: git "cp" like there is git mv Junio C Hamano 2018-02-07 20:27 ` Stefan Beller 2018-03-18 20:09 ` Stefan Moch 2018-03-19 20:03 ` Junio C Hamano 2017-12-18 0:28 ` Igor Djordjevic
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).