On Tue, Aug 04, 2015 at 09:31:18AM +0200, Sebastian Schuberth wrote:
> On Tue, Aug 4, 2015 at 6:34 AM, Lukas Fleischer <[email protected]> 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html