Thread (1 message) 1 message, 1 author, 2016-10-18

Re: [PATCH] submodule--helper: normalize funny urls

From: Junio C Hamano <hidden>
Date: 2016-10-18 19:49:49

Stefan Beller [off-list ref] writes:
Some users may rely on this by always cloning with '/.' and having
an additional '../' in the relative path for the submodule, and this
patch breaks them. So why introduce this patch?

The fix in c12922024 (submodule: ignore trailing slash on superproject
URL, 2016-10-10) and prior discussion revealed, that Git and Git
for Windows treat URLs differently, as currently Git for Windows
strips off a trailing dot from paths when calling a Git binary
unlike when running a shell. Which means Git for Windows is already
doing the right thing for the case mentioned above, but it would fail
our current tests, that test for the broken behavior and it would
confuse users working across platforms. So we'd rather fix it
in Git to ignore any of these trailing no ops in the path properly.

We never produce the URLs with a trailing '/.' in Git ourselves,
they come to us, because the user used it as the URL for cloning
a superproject. Normalize these paths.

Helped-by: Junio C Hamano [off-list ref]
Signed-off-by: Stefan Beller <redacted>
---

 * reworded the commit message, taken from Junio, but added more explanation
   why we want to introduce this patch. 
The additional explanation is very good.
quoted hunk
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 260f46f..ac03cb3 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -76,6 +76,29 @@ static int chop_last_dir(char **remoteurl, int is_relative)
 	return 0;
 }
 
+static void strip_url_ending(char *url, size_t *_len)
+{
+	size_t len = _len ? *_len : strlen(url);
Stare at our codebase and you'd notice that we avoid using names
with leading underscore deliberately and use trailing one instead
when we name a throw-away name like this.  Let's do the same here.
I.e.

	static void strip_url_ending(char *url, size_t *len_)
	{
		size_t len = len_ ? *len_ : strlen(url);
+	for (;;) {
+		if (len > 1 && is_dir_sep(url[len-2]) && url[len-1] == '.') {
+			url[len-2] = '\0';
"len-1" and "len-2" are usually spelled with SP on both sides of
binary operators.
+			len -= 2;
+			continue;
+		}
+		if (len > 0 && is_dir_sep(url[len-1])) {
+			url[len-1] = '\0';
+			len --;
And post-decrement sticks to whatever it is decrementing without SP.
+			continue;
+		}
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help