{"thread":{"id":"43011","subject":"[PATCH] git mv: do not keep slash in `git mv dir non-existing-dir/`","startedAt":"2016-08-05T14:41:26Z","lastAt":"2016-08-05T16:18:33Z","messageCount":4,"participants":["Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"293191","messageId":"e0415c55e9bc651e7fa9f5f7717b4f6e44eb9ce1.1470407827.git.johannes.schindelin@gmx.de","threadId":"43011","inReplyTo":null,"subject":"[PATCH] git mv: do not keep slash in `git mv dir non-existing-dir/`","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-08-05T14:41:12Z","receivedAt":"2016-08-05T14:41:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"When calling `rename(\"dir\", \"non-existing-dir/\")` on Linux, it silently\nsucceeds, stripping the trailing slash of the second argument.\n\nThis is all good and dandy but this behavior disagrees with the specs at\n\nhttp://pubs.opengroup.org/onlinepubs/9699919799/functions/rename.html\n\nthat state clearly regarding the 2nd parameter (called `new`):\n\n\tIf the `new` argument does not resolve to an existing directory\n\tentry for a file of type directory and the `new` argument\n\tcontains at least one non- character and ends with one\n\tor more trailing characters after all symbolic links\n\thave been processed, `rename()` shall fail.\n\nOf course, we would like `git mv dir non-existing-dir/` to succeed (and\nrename the directory \"dir\" to \"non-existing-dir\"). Let's be extra\ncareful to remove the trailing slash in that case.\n\nThis lets t7001-mv.sh pass in Bash on Windows.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tThere is unfortunately another problem with Git running in Bash on\n\tWindows: the credential cache daemon somehow gets stuck. I guess\n\tthis is some timing issue, in more than one way: right now, I do\n\tnot really have time to pursue this further.\n\nPublished-As: https://github.com/dscho/git/releases/tag/bash-on-windows-v1\nFetch-It-Via: git fetch https://github.com/dscho/git bash-on-windows-v1\n\n builtin/mv.c | 11 +++++++----\n 1 file changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex a201426..446a316 100644\n--- a/builtin/mv.c\n+++ b/builtin/mv.c\n@@ -104,7 +104,7 @@ static int index_range_of_same_dir(const char *src, int length,\n \n int cmd_mv(int argc, const char **argv, const char *prefix)\n {\n-\tint i, gitmodules_modified = 0;\n+\tint i, flags, gitmodules_modified = 0;\n \tint verbose = 0, show_only = 0, force = 0, ignore_errors = 0;\n \tstruct option builtin_mv_options[] = {\n \t\tOPT__VERBOSE(&verbose, N_(\"be verbose\")),\n@@ -134,10 +134,13 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \tmodes = xcalloc(argc, sizeof(enum update_mode));\n \t/*\n \t * Keep trailing slash, needed to let\n-\t * \"git mv file no-such-dir/\" error out.\n+\t * \"git mv file no-such-dir/\" error out, except in the case\n+\t * \"git mv directory no-such-dir/\".\n \t */\n-\tdest_path = internal_copy_pathspec(prefix, argv + argc, 1,\n-\t\t\t\t\t KEEP_TRAILING_SLASH);\n+\tflags = KEEP_TRAILING_SLASH;\n+\tif (argc == 1 && is_directory(argv[0]) && !is_directory(argv[1]))\n+\t\tflags = 0;\n+\tdest_path = internal_copy_pathspec(prefix, argv + argc, 1, flags);\n \tsubmodule_gitfile = xcalloc(argc, sizeof(char *));\n \n \tif (dest_path[0][0] == '\\0')\n-- \n2.9.0.281.g286a8d9\n\nbase-commit: c6b0597e9ac7277e148e2fd4d7615ac6e0bfb661\n"},{"id":"293202","messageId":"xmqqtwezxlf2.fsf@gitster.mtv.corp.google.com","threadId":"43011","inReplyTo":"e0415c55e9bc651e7fa9f5f7717b4f6e44eb9ce1.1470407827.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH] git mv: do not keep slash in `git mv dir non-existing-dir/`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-05T15:52:49Z","receivedAt":"2016-08-05T15:52:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> When calling `rename(\"dir\", \"non-existing-dir/\")` on Linux, it silently\n> succeeds, stripping the trailing slash of the second argument.\n>\n> This is all good and dandy but this behavior disagrees with the specs at\n>\n> http://pubs.opengroup.org/onlinepubs/9699919799/functions/rename.html\n>\n> that state clearly regarding the 2nd parameter (called `new`):\n>\n> \tIf the `new` argument does not resolve to an existing directory\n> \tentry for a file of type directory and the `new` argument\n> \tcontains at least one non- character and ends with one\n> \tor more trailing characters after all symbolic links\n> \thave been processed, `rename()` shall fail.\n\nI agree with all of the above. But\n\n> Of course, we would like `git mv dir non-existing-dir/` to succeed (and\n> rename the directory \"dir\" to \"non-existing-dir\").\n\nI do not think I want that. When I say \"mv A B/\", I want it to fail\nif I made a typo for B; the trailing slash after B is an explicit\nstatement \"I expect B to exist and I want A to appear at B/A\".\n\nCurrent Git behaviour on Linux seems to allow \"git mv dir no-such-dir/\"\nbut \"dir\" is renamed to \"no-such-dir\", which fails two expectations,\nand I think this is broken. If Windows port does not share this\nbreakage, that is a good thing. We should fix Git behaviour on Linux\ninstead, I would think.\n"},{"id":"293203","messageId":"alpine.DEB.2.20.1608051754080.5786@virtualbox","threadId":"43011","inReplyTo":"xmqqtwezxlf2.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] git mv: do not keep slash in `git mv dir non-existing-dir/`","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-08-05T15:58:25Z","receivedAt":"2016-08-05T15:58:49Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Fri, 5 Aug 2016, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > When calling `rename(\"dir\", \"non-existing-dir/\")` on Linux, it silently\n> > succeeds, stripping the trailing slash of the second argument.\n> >\n> > This is all good and dandy but this behavior disagrees with the specs at\n> >\n> > http://pubs.opengroup.org/onlinepubs/9699919799/functions/rename.html\n> >\n> > that state clearly regarding the 2nd parameter (called `new`):\n> >\n> > \tIf the `new` argument does not resolve to an existing directory\n> > \tentry for a file of type directory and the `new` argument\n> > \tcontains at least one non- character and ends with one\n> > \tor more trailing characters after all symbolic links\n> > \thave been processed, `rename()` shall fail.\n> \n> I agree with all of the above. But\n> \n> > Of course, we would like `git mv dir non-existing-dir/` to succeed (and\n> > rename the directory \"dir\" to \"non-existing-dir\").\n> \n> I do not think I want that. When I say \"mv A B/\", I want it to fail\n> if I made a typo for B; the trailing slash after B is an explicit\n> statement \"I expect B to exist and I want A to appear at B/A\".\n\nPlease note that t7001 *specifically* tests for the opposite of what you\nwant, then ;-)\n\n\thttps://github.com/git/git/blob/v2.9.2/t/t7001-mv.sh#L79-L80\n\n> Current Git behaviour on Linux seems to allow \"git mv dir no-such-dir/\"\n> but \"dir\" is renamed to \"no-such-dir\", which fails two expectations,\n> and I think this is broken. If Windows port does not share this\n> breakage, that is a good thing. We should fix Git behaviour on Linux\n> instead, I would think.\n\nTo be precise, Git for Windows displays the same behavior as Git on Linux,\nbecause rename(\"dir\", \"no-such-dir/\") succeeds.\n\nThe breakage fixed by this here patch happens when running plain Linux Git\nin \"Bash on Windows\" (i.e. Bash on Ubuntu on Windows, the new Linux\nsubsystem of Windows, allowing to run unmodified Linux binaries on Windows\nwithout the need for a Virtual Machine).\n\nCiao,\nDscho\n"},{"id":"293206","messageId":"xmqqlh0bxk8d.fsf@gitster.mtv.corp.google.com","threadId":"43011","inReplyTo":"alpine.DEB.2.20.1608051754080.5786@virtualbox","subject":"Re: [PATCH] git mv: do not keep slash in `git mv dir non-existing-dir/`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-05T16:18:26Z","receivedAt":"2016-08-05T16:18:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Please note that t7001 *specifically* tests for the opposite of what you\n> want, then ;-)\n\nYes, I know. I am torn.\n\nIt seems \"mv A B/\" (not \"git mv\") when B does not exist does the\nsame \"wrong\" thing, so the existing behaviour is at least consistent\nwith the command line tools on Linux, and more importantly, existing\nusers of \"git mv A B/\" have expected it to ignore the trailing\nslash for a long time, so let's not change the expected behaviour.\n\nThanks.\n\n\n\n\n\n\n\n"}]}