On Tue, Aug 04, 2015 at 09:31:18AM +0200, Sebastian Schuberth wrote:

> On Tue, Aug 4, 2015 at 6:34 AM, Lukas Fleischer <lfleisc...@lfos.de> wrote:
> 
> > I am currently on vacation and cannot bisect or debug this but I am
> > pretty confident that this patch changes the behaviour of directory name
> > guessing. With Git 2.4.6, cloning http://foo.bar/foo.git/ results in a
> > directory named foo and with Git 2.5.0, the resulting directory is
> > called foo.git.
> >
> > Note how the end variable is decreased when the repository name ends
> > with a slash but that isn't taken into account when simply using
> > strip_suffix() later...
> >
> > Is this intended?
> 
> I did not intend this change in behavior, and I can confirm that
> reverting my patch restores the original behavior. Thanks for bringing
> this to my attention, I'll work on a patch.

I think this regression is in v2.4.8, as well. We should be able to use
a running "len" instead of the "end" pointer in the earlier part, and
then use strip_suffix_mem later (to strip from our already-reduced
length, rather than the full NUL-terminated string). Like this:

diff --git a/builtin/clone.c b/builtin/clone.c
index 303a3a7..4b61e4c 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -146,20 +146,19 @@ static char *get_repo_path(const char *repo, int 
*is_bundle)
 
 static char *guess_dir_name(const char *repo, int is_bundle, int is_bare)
 {
-       const char *end = repo + strlen(repo), *start;
-       size_t len;
+       const char *start;
+       size_t len = strlen(repo);
        char *dir;
 
        /*
         * Strip trailing spaces, slashes and /.git
         */
-       while (repo < end && (is_dir_sep(end[-1]) || isspace(end[-1])))
-               end--;
-       if (end - repo > 5 && is_dir_sep(end[-5]) &&
-           !strncmp(end - 4, ".git", 4)) {
-               end -= 5;
-               while (repo < end && is_dir_sep(end[-1]))
-                       end--;
+       while (len > 0 && (is_dir_sep(repo[len-1]) || isspace(repo[len-1])))
+               len--;
+       if (len > 5 && is_dir_sep(repo[len-5]) &&
+           strip_suffix_mem(repo, &len, ".git")) {
+               while (len > 0 && is_dir_sep(repo[len-1]))
+                       len--;
        }
 
        /*
@@ -167,14 +166,14 @@ static char *guess_dir_name(const char *repo, int 
is_bundle, int is_bare)
         * the form  "remote.example.com:foo.git", i.e. no slash
         * in the directory part.
         */
-       start = end;
+       start = repo + len;
        while (repo < start && !is_dir_sep(start[-1]) && start[-1] != ':')
                start--;
 
        /*
         * Strip .{bundle,git}.
         */
-       strip_suffix(start, is_bundle ? ".bundle" : ".git" , &len);
+       strip_suffix_mem(start, &len, is_bundle ? ".bundle" : ".git");
 
        if (is_bare)
                dir = xstrfmt("%.*s.git", (int)len, start);
@@ -187,6 +186,7 @@ static char *guess_dir_name(const char *repo, int 
is_bundle, int is_bare)
        if (*dir) {
                char *out = dir;
                int prev_space = 1 /* strip leading whitespace */;
+               const char *end;
                for (end = dir; *end; ++end) {
                        char ch = *end;
                        if ((unsigned char)ch < '\x20')

Sadly we cannot just `strip_suffix_mem(repo, &len, "/.git"))` in the
earlier code, as we have to account for multiple directory separators. I
believe the above code does the right thing, though. I haven't looked at
how badly it interacts with the other guess_dir_name work from Patrick
Steinhardt that has been going on, though.

-Peff
--
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

Reply via email to