Thread (3 messages) flat view 3 messages, 1 author, 2016-06-15
DORMANTno replies

Revision v1 of 4 in this series.

Revisions (4)
  1. v1 [diff vs current]
  2. v1 [diff vs current]
  3. v1 current
  4. v1 [diff vs current]

[PATCH 0/2] Port `git submodule init` from shell to C

From: Stefan Beller <hidden>
Date: 2016-06-15 23:08:44
Subsystem: the rest · Maintainer: Linus Torvalds

This applies on top of origin/sb/submodule-parallel-update.
By having the `init` functionality in C, we can reference it easier
from other parts in the code.

Thanks for the reviews a long time ago!

Junio,
  I fixed the NEEDSWORK comment for relative_url
  and thought about the memory allocation
Duy,
  I fixed the i18n issues introduced.
Johannes,
  I took $PWD instead of absolute paths. According to your last message, this
  may still break on Windows for 
test_submodule_relative_url "(null)" "$PWD/home2/../remote" "../bundle1" "$PWD/home2/../bundle1"

There are however no further tests starting with /.

Thanks,
Stefan

Stefan Beller (2):
  submodule: port resolve_relative_url from shell to C
  submodule: port init from shell to C

 builtin/submodule--helper.c | 317 +++++++++++++++++++++++++++++++++++++++++++-
 git-submodule.sh            | 118 +----------------
 submodule.c                 |  21 +++
 submodule.h                 |   1 +
 t/t0060-path-utils.sh       |  43 ++++++
 5 files changed, 385 insertions(+), 115 deletions(-)

Diff to origin/sb/submodule-init:
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 213af2e..d942463 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -22,7 +22,7 @@ static char *get_default_remote(void)
 	const char *refname = resolve_ref_unsafe("HEAD", 0, sha1, &flag);
 
 	if (!refname)
-		die("No such ref: HEAD");
+		die(_("No such ref: %s"), "HEAD");
 
 	/* detached HEAD */
 	if (!strcmp(refname, "HEAD"))
@@ -94,12 +94,14 @@ 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          correct
- * http://a.com/b  ../c             http://a.com/c   yes
- * http://a.com/b  ../../c          http://c         no (domain should be kept)
- * http://a.com/b  ../../../c       http:/c          no
- * http://a.com/b  ../../../../c    http:c           no
- * http://a.com/b  ../../../../../c    .:c           no
+ * remote_url      url              outcome          expectation
+ * http://a.com/b  ../c             http://a.com/c   as is
+ * 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.
  */
 static char *relative_url(const char *remote_url,
 				const char *url,
@@ -146,6 +148,7 @@ static char *relative_url(const char *remote_url,
 	}
 	strbuf_reset(&sb);
 	strbuf_addf(&sb, "%s%s%s", remoteurl, colonsep ? ":" : "/", url);
+	free(remoteurl);
 
 	if (starts_with_dot_slash(sb.buf))
 		out = xstrdup(sb.buf + 2);
@@ -153,7 +156,6 @@ static char *relative_url(const char *remote_url,
 		out = xstrdup(sb.buf);
 	strbuf_reset(&sb);
 
-	free(remoteurl);
 	if (!up_path || !is_relative)
 		return out;
 
@@ -299,7 +301,7 @@ static int module_init(int argc, const char **argv, const char *prefix)
 		OPT_STRING(0, "prefix", &prefix,
 			   N_("path"),
 			   N_("alternative anchor for relative paths")),
-		OPT__QUIET(&quiet, "Suppress output for initialzing a submodule"),
+		OPT__QUIET(&quiet, N_("Suppress output for initializing a submodule")),
 		OPT_END()
 	};
 
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 8a1579c..579c1fa 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -293,35 +293,36 @@ test_git_path GIT_COMMON_DIR=bar config                   bar/config
 test_git_path GIT_COMMON_DIR=bar packed-refs              bar/packed-refs
 test_git_path GIT_COMMON_DIR=bar shallow                  bar/shallow
 
-test_submodule_relative_url "(null)" "../foo/bar" "../sub/a/b/c" "../foo/sub/a/b/c"
+test_submodule_relative_url "../" "../foo" "../submodule" "../../submodule"
+test_submodule_relative_url "../" "../foo/bar" "../submodule" "../../foo/submodule"
+test_submodule_relative_url "../" "../foo/submodule" "../submodule" "../../foo/submodule"
+test_submodule_relative_url "../" "./foo" "../submodule" "../submodule"
+test_submodule_relative_url "../" "./foo/bar" "../submodule" "../foo/submodule"
 test_submodule_relative_url "../../../" "../foo/bar" "../sub/a/b/c" "../../../../foo/sub/a/b/c"
+test_submodule_relative_url "../" "$PWD/addtest" "../repo" "$PWD/repo"
+test_submodule_relative_url "../" "foo/bar" "../submodule" "../foo/submodule"
+test_submodule_relative_url "../" "foo" "../submodule" "../submodule"
+
+test_submodule_relative_url "(null)" "../foo/bar" "../sub/a/b/c" "../foo/sub/a/b/c"
 test_submodule_relative_url "(null)" "../foo/bar" "../submodule" "../foo/submodule"
-test_submodule_relative_url "../" "../foo/bar" "../submodule" "../../foo/submodule"
 test_submodule_relative_url "(null)" "../foo/submodule" "../submodule" "../foo/submodule"
-test_submodule_relative_url "../" "../foo/submodule" "../submodule" "../../foo/submodule"
 test_submodule_relative_url "(null)" "../foo" "../submodule" "../submodule"
-test_submodule_relative_url "../" "../foo" "../submodule" "../../submodule"
 test_submodule_relative_url "(null)" "./foo/bar" "../submodule" "foo/submodule"
-test_submodule_relative_url "../" "./foo/bar" "../submodule" "../foo/submodule"
 test_submodule_relative_url "(null)" "./foo" "../submodule" "submodule"
-test_submodule_relative_url "../" "./foo" "../submodule" "../submodule"
 test_submodule_relative_url "(null)" "//somewhere else/repo" "../subrepo" "//somewhere else/subrepo"
-test_submodule_relative_url "(null)" "/u//trash directory.t7406-submodule-update/subsuper_update_r" "../subsubsuper_update_r" "/u//trash directory.t7406-submodule-update/subsubsuper_update_r"
-test_submodule_relative_url "(null)" "/u//trash directory.t7406-submodule-update/super_update_r2" "../subsuper_update_r" "/u//trash directory.t7406-submodule-update/subsuper_update_r"
-test_submodule_relative_url "(null)" "/u/trash directory.t3600-rm/." "../." "/u/trash directory.t3600-rm/."
-test_submodule_relative_url "(null)" "/u/trash directory.t3600-rm" "./." "/u/trash directory.t3600-rm/."
-test_submodule_relative_url "(null)" "/u/trash directory.t7400-submodule-basic/addtest" "../repo" "/u/trash directory.t7400-submodule-basic/repo"
-test_submodule_relative_url "../" "/u/trash directory.t7400-submodule-basic/addtest" "../repo" "/u/trash directory.t7400-submodule-basic/repo"
-test_submodule_relative_url "(null)" "/u/trash directory.t7400-submodule-basic" "./å äö" "/u/trash directory.t7400-submodule-basic/å äö"
-test_submodule_relative_url "(null)" "/u/trash directory.t7403-submodule-sync/." "../submodule" "/u/trash directory.t7403-submodule-sync/submodule"
-test_submodule_relative_url "(null)" "/u/trash directory.t7407-submodule-foreach/submodule" "../submodule" "/u/trash directory.t7407-submodule-foreach/submodule"
-test_submodule_relative_url "(null)" "/u/trash directory.t7409-submodule-detached-worktree/home2/../remote" "../bundle1" "/u/trash directory.t7409-submodule-detached-worktree/home2/../bundle1"
-test_submodule_relative_url "(null)" "/u/trash directory.t7613-merge-submodule/submodule_update_repo" "./." "/u/trash directory.t7613-merge-submodule/submodule_update_repo/."
+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/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/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)" "file:///tmp/repo" "../subrepo" "file:///tmp/subrepo"
 test_submodule_relative_url "(null)" "foo/bar" "../submodule" "foo/submodule"
-test_submodule_relative_url "../" "foo/bar" "../submodule" "../foo/submodule"
 test_submodule_relative_url "(null)" "foo" "../submodule" "submodule"
-test_submodule_relative_url "../" "foo" "../submodule" "../submodule"
 test_submodule_relative_url "(null)" "helper:://hostname/repo" "../subrepo" "helper:://hostname/subrepo"
 test_submodule_relative_url "(null)" "ssh://hostname/repo" "../subrepo" "ssh://hostname/subrepo"
 test_submodule_relative_url "(null)" "ssh://hostname:22/repo" "../subrepo" "ssh://hostname:22/subrepo"

-- 
2.7.0.rc0.46.g8f16ed4.dirty
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help