Re: [PATCH] git mv: do not keep slash in `git mv dir non-existing-dir/`
Johannes Schindelin writes: > Please note that t7001 *specifically* tests for the opposite of what you > want, then ;-) Yes, I know. I am torn. It seems "mv A B/" (not "git mv") when B does not exist does the same "wrong" thing, so the existing behaviour is at least consistent with the command line tools on Linux, and more importantly, existing users of "git mv A B/" have expected it to ignore the trailing slash for a long time, so let's not change the expected behaviour. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git mv: do not keep slash in `git mv dir non-existing-dir/`
Hi Junio,
On Fri, 5 Aug 2016, Junio C Hamano wrote:
> Johannes Schindelin writes:
>
> > When calling `rename("dir", "non-existing-dir/")` on Linux, it silently
> > succeeds, stripping the trailing slash of the second argument.
> >
> > This is all good and dandy but this behavior disagrees with the specs at
> >
> > http://pubs.opengroup.org/onlinepubs/9699919799/functions/rename.html
> >
> > that state clearly regarding the 2nd parameter (called `new`):
> >
> > If the `new` argument does not resolve to an existing directory
> > entry for a file of type directory and the `new` argument
> > contains at least one non- character and ends with one
> > or more trailing characters after all symbolic links
> > have been processed, `rename()` shall fail.
>
> I agree with all of the above. But
>
> > Of course, we would like `git mv dir non-existing-dir/` to succeed (and
> > rename the directory "dir" to "non-existing-dir").
>
> I do not think I want that. When I say "mv A B/", I want it to fail
> if I made a typo for B; the trailing slash after B is an explicit
> statement "I expect B to exist and I want A to appear at B/A".
Please note that t7001 *specifically* tests for the opposite of what you
want, then ;-)
https://github.com/git/git/blob/v2.9.2/t/t7001-mv.sh#L79-L80
> Current Git behaviour on Linux seems to allow "git mv dir no-such-dir/"
> but "dir" is renamed to "no-such-dir", which fails two expectations,
> and I think this is broken. If Windows port does not share this
> breakage, that is a good thing. We should fix Git behaviour on Linux
> instead, I would think.
To be precise, Git for Windows displays the same behavior as Git on Linux,
because rename("dir", "no-such-dir/") succeeds.
The breakage fixed by this here patch happens when running plain Linux Git
in "Bash on Windows" (i.e. Bash on Ubuntu on Windows, the new Linux
subsystem of Windows, allowing to run unmodified Linux binaries on Windows
without the need for a Virtual Machine).
Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git mv: do not keep slash in `git mv dir non-existing-dir/`
Johannes Schindelin writes:
> When calling `rename("dir", "non-existing-dir/")` on Linux, it silently
> succeeds, stripping the trailing slash of the second argument.
>
> This is all good and dandy but this behavior disagrees with the specs at
>
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/rename.html
>
> that state clearly regarding the 2nd parameter (called `new`):
>
> If the `new` argument does not resolve to an existing directory
> entry for a file of type directory and the `new` argument
> contains at least one non- character and ends with one
> or more trailing characters after all symbolic links
> have been processed, `rename()` shall fail.
I agree with all of the above. But
> Of course, we would like `git mv dir non-existing-dir/` to succeed (and
> rename the directory "dir" to "non-existing-dir").
I do not think I want that. When I say "mv A B/", I want it to fail
if I made a typo for B; the trailing slash after B is an explicit
statement "I expect B to exist and I want A to appear at B/A".
Current Git behaviour on Linux seems to allow "git mv dir no-such-dir/"
but "dir" is renamed to "no-such-dir", which fails two expectations,
and I think this is broken. If Windows port does not share this
breakage, that is a good thing. We should fix Git behaviour on Linux
instead, I would think.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git mv: do not keep slash in `git mv dir non-existing-dir/`
When calling `rename("dir", "non-existing-dir/")` on Linux, it silently
succeeds, stripping the trailing slash of the second argument.
This is all good and dandy but this behavior disagrees with the specs at
http://pubs.opengroup.org/onlinepubs/9699919799/functions/rename.html
that state clearly regarding the 2nd parameter (called `new`):
If the `new` argument does not resolve to an existing directory
entry for a file of type directory and the `new` argument
contains at least one non- character and ends with one
or more trailing characters after all symbolic links
have been processed, `rename()` shall fail.
Of course, we would like `git mv dir non-existing-dir/` to succeed (and
rename the directory "dir" to "non-existing-dir"). Let's be extra
careful to remove the trailing slash in that case.
This lets t7001-mv.sh pass in Bash on Windows.
Signed-off-by: Johannes Schindelin
---
There is unfortunately another problem with Git running in Bash on
Windows: the credential cache daemon somehow gets stuck. I guess
this is some timing issue, in more than one way: right now, I do
not really have time to pursue this further.
Published-As: https://github.com/dscho/git/releases/tag/bash-on-windows-v1
Fetch-It-Via: git fetch https://github.com/dscho/git bash-on-windows-v1
builtin/mv.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/builtin/mv.c b/builtin/mv.c
index a201426..446a316 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -104,7 +104,7 @@ static int index_range_of_same_dir(const char *src, int
length,
int cmd_mv(int argc, const char **argv, const char *prefix)
{
- int i, gitmodules_modified = 0;
+ int i, flags, gitmodules_modified = 0;
int verbose = 0, show_only = 0, force = 0, ignore_errors = 0;
struct option builtin_mv_options[] = {
OPT__VERBOSE(&verbose, N_("be verbose")),
@@ -134,10 +134,13 @@ int cmd_mv(int argc, const char **argv, const char
*prefix)
modes = xcalloc(argc, sizeof(enum update_mode));
/*
* Keep trailing slash, needed to let
-* "git mv file no-such-dir/" error out.
+* "git mv file no-such-dir/" error out, except in the case
+* "git mv directory no-such-dir/".
*/
- dest_path = internal_copy_pathspec(prefix, argv + argc, 1,
- KEEP_TRAILING_SLASH);
+ flags = KEEP_TRAILING_SLASH;
+ if (argc == 1 && is_directory(argv[0]) && !is_directory(argv[1]))
+ flags = 0;
+ dest_path = internal_copy_pathspec(prefix, argv + argc, 1, flags);
submodule_gitfile = xcalloc(argc, sizeof(char *));
if (dest_path[0][0] == '\0')
--
2.9.0.281.g286a8d9
base-commit: c6b0597e9ac7277e148e2fd4d7615ac6e0bfb661
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
