From: Stefan Beller <sbeller@google.com>
To: gitster@pobox.com, j6t@kdbg.org, Johannes.Schindelin@gmx.de
Cc: git@vger.kernel.org, venv21@gmail.com, dennis@kaarsemaker.net,
jrnieder@gmail.com, Stefan Beller <sbeller@google.com>
Subject: [PATCH 3/3] submodule--helper: normalize funny urls
Date: Fri, 21 Oct 2016 16:59:39 -0700 [thread overview]
Message-ID: <20161021235939.20792-4-sbeller@google.com> (raw)
In-Reply-To: <20161021235939.20792-1-sbeller@google.com>
The remote URL for the submodule can be specified relative
to the URL of the superproject in .gitmodules. A top-level
git://site.xz/toplevel.git can specify in its .gitmodules
[submodule "sub"]
url = ../submodule.git
path = sub
to say that git://site.xz/submodule.git is where the
submodule bound at its "sub/" is found.
However, when the toplevel is cloned like this:
git clone git://site.xz/toplevel.git/. top
i.e. the toplevel specifies its URL with trailing "/.", the
code set the URL to git://site.xz/toplevel.git/submodule.git
for the submodule, which is nonsense. This was because the
logic failed to treat trailing "/." any differently from
trailing "/<anything-without-slash>" when resolving a
relative URL "../<something>" off of it. Stripping "/." at
the end does *not* take you one level up, even though
stripping "/<anything-without-slash>" does!
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.
The test 3600 needs a slight adaption as well, and was not covered by
the previous patch, because the setup of the submodule in this test
is different than the "clone . super" pattern that was fixed in the
previous patch.
The url configured for the submodule path submod is "./.", which
gets normalized with this patch, such that the nested submodule
'subsubmod' doesn't resolve its path properly via '../'.
In later tests we do not need the url any more as testing of
removal of the config including url happens in early tests, so the
easieast fix for that test is to just make the submodule its own
authoritative source of truth by removing the remote url.
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
builtin/submodule--helper.c | 48 +++++++++++++++++++++++++++++++++------------
t/t0060-path-utils.sh | 11 +++++++----
t/t3600-rm.sh | 1 +
3 files changed, 44 insertions(+), 16 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 4beeda5..21e2e2a 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);
+
+ for (;;) {
+ if (len > 1 && is_dir_sep(url[len - 2]) && url[len - 1] == '.') {
+ url[len-2] = '\0';
+ len -= 2;
+ continue;
+ }
+ if (len > 0 && is_dir_sep(url[len-1])) {
+ url[len-1] = '\0';
+ len--;
+ continue;
+ }
+
+ break;
+ }
+
+ if (len_)
+ *len_ = len;
+}
+
/*
* The `url` argument is the URL that navigates to the submodule origin
* repo. When relative, this URL is relative to the superproject origin
@@ -93,14 +116,16 @@ static int chop_last_dir(char **remoteurl, int is_relative)
* the superproject working tree otherwise.
*
* NEEDSWORK: This works incorrectly on the domain and protocol part.
- * remote_url url outcome expectation
- * http://a.com/b ../c http://a.com/c as is
- * http://a.com/b/ ../c http://a.com/c same as previous line, but
- * ignore trailing slash in url
- * http://a.com/b ../../c http://c error out
- * http://a.com/b ../../../c http:/c error out
- * http://a.com/b ../../../../c http:c error out
- * http://a.com/b ../../../../../c .:c error out
+ * remote_url url outcome expectation
+ * http://a.com/b ../c http://a.com/c as is
+ * http://a.com/b/ ../c http://a.com/c same as previous line, but
+ * ignore trailing '/' in url
+ * http://a.com/b/. ../c http://a.com/c same as previous line, but
+ * ignore trailing '/.' in url
+ * http://a.com/b ../../c http://c error out
+ * http://a.com/b ../../../c http:/c error out
+ * http://a.com/b ../../../../c http:c error out
+ * http://a.com/b ../../../../../c .:c error out
* NEEDSWORK: Given how chop_last_dir() works, this function is broken
* when a local part has a colon in its path component, too.
*/
@@ -115,8 +140,7 @@ static char *relative_url(const char *remote_url,
struct strbuf sb = STRBUF_INIT;
size_t len = strlen(remoteurl);
- if (is_dir_sep(remoteurl[len-1]))
- remoteurl[len-1] = '\0';
+ strip_url_ending(remoteurl, &len);
if (!url_is_local_not_ssh(remoteurl) || is_absolute_path(remoteurl))
is_relative = 0;
@@ -149,10 +173,10 @@ static char *relative_url(const char *remote_url,
}
strbuf_reset(&sb);
strbuf_addf(&sb, "%s%s%s", remoteurl, colonsep ? ":" : "/", url);
- if (ends_with(url, "/"))
- strbuf_setlen(&sb, sb.len - 1);
free(remoteurl);
+ strip_url_ending(sb.buf, &sb.len);
+
if (starts_with_dot_slash(sb.buf))
out = xstrdup(sb.buf + 2);
else
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 25b48e5..e154e5f 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -329,14 +329,17 @@ test_submodule_relative_url "(null)" "./foo" "../submodule" "submodule"
test_submodule_relative_url "(null)" "//somewhere else/repo" "../subrepo" "//somewhere else/subrepo"
test_submodule_relative_url "(null)" "$PWD/subsuper_update_r" "../subsubsuper_update_r" "$(pwd)/subsubsuper_update_r"
test_submodule_relative_url "(null)" "$PWD/super_update_r2" "../subsuper_update_r" "$(pwd)/subsuper_update_r"
-test_submodule_relative_url "(null)" "$PWD/." "../." "$(pwd)/."
-test_submodule_relative_url "(null)" "$PWD" "./." "$(pwd)/."
+test_submodule_relative_url "(null)" "$PWD/sub/." "../." "$(pwd)"
+test_submodule_relative_url "(null)" "$PWD/sub/./." "../." "$(pwd)"
+test_submodule_relative_url "(null)" "$PWD/sub/.////././/./." "../." "$(pwd)"
+test_submodule_relative_url "(null)" "$PWD" "./." "$(pwd)"
test_submodule_relative_url "(null)" "$PWD/addtest" "../repo" "$(pwd)/repo"
test_submodule_relative_url "(null)" "$PWD" "./å äö" "$(pwd)/å äö"
-test_submodule_relative_url "(null)" "$PWD/." "../submodule" "$(pwd)/submodule"
+test_submodule_relative_url "(null)" "$PWD/sub" "../submodule" "$(pwd)/submodule"
+test_submodule_relative_url "(null)" "$PWD/sub/." "../submodule" "$(pwd)/submodule"
test_submodule_relative_url "(null)" "$PWD/submodule" "../submodule" "$(pwd)/submodule"
test_submodule_relative_url "(null)" "$PWD/home2/../remote" "../bundle1" "$(pwd)/home2/../bundle1"
-test_submodule_relative_url "(null)" "$PWD/submodule_update_repo" "./." "$(pwd)/submodule_update_repo/."
+test_submodule_relative_url "(null)" "$PWD/submodule_update_repo" "./." "$(pwd)/submodule_update_repo"
test_submodule_relative_url "(null)" "file:///tmp/repo" "../subrepo" "file:///tmp/subrepo"
test_submodule_relative_url "(null)" "foo/bar" "../submodule" "foo/submodule"
test_submodule_relative_url "(null)" "foo" "../submodule" "submodule"
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index d046d98..7839ccd 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -615,6 +615,7 @@ test_expect_success 'setup subsubmodule' '
git reset --hard &&
git submodule update &&
(cd submod &&
+ git config --unset remote.origin.url &&
git update-index --add --cacheinfo 160000 $(git rev-parse HEAD) subsubmod &&
git config -f .gitmodules submodule.sub.url ../. &&
git config -f .gitmodules submodule.sub.path subsubmod &&
--
2.10.1.507.g2a9098a
prev parent reply other threads:[~2016-10-22 0:00 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-21 23:59 [PATCH 0/3] Fix submodule url issues Stefan Beller
2016-10-21 23:59 ` [PATCH 1/3] t7506: fix diff order arguments in test_cmp Stefan Beller
2016-10-21 23:59 ` [PATCH 2/3] submodule tests: replace cloning from . by "$(pwd)" Stefan Beller
2016-10-22 7:09 ` Johannes Sixt
2016-10-22 7:33 ` Junio C Hamano
2016-10-22 20:46 ` Stefan Beller
2016-10-23 10:14 ` Johannes Sixt
2016-10-24 17:46 ` Junio C Hamano
2016-10-24 19:10 ` Stefan Beller
2016-10-24 19:47 ` Johannes Sixt
2016-10-21 23:59 ` Stefan Beller [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161021235939.20792-4-sbeller@google.com \
--to=sbeller@google.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=dennis@kaarsemaker.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
--cc=jrnieder@gmail.com \
--cc=venv21@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).