On Wed, Jan 13, 2016 at 1:15 PM, Stefan Beller <sbeller@xxxxxxxxxx> wrote:
> Later on we want to deprecate the `git submodule init` command and make
> it implicit in other submodule commands. As these other commands are
> written in C already, we'd need the init functionality in C, too.
> The `resolve_relative_url` function is a rather large part of that init
> functionality, so start by porting this function to C.
>
> As I was porting the functionality I noticed some odds with the inputs.
> To fully understand the situation I added some logging to the function
> temporarily to capture all calls to the function throughout the test
> suite. Duplicates have been removed and all unique testing inputs have
> been recorded into t0060.
>
> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
> ---
No time presently for a proper review, so just a few superficial comments...
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> @@ -9,6 +9,193 @@
> +static int resolve_relative_url(int argc, const char **argv, const char *prefix)
> +{
> + char *remoteurl = NULL;
> + char *remote = get_default_remote();
> + const char *up_path = NULL;
> + char *res;
> + const char *url;
> + struct strbuf sb = STRBUF_INIT;
> +
> + if (argc != 2 && argc != 3)
> + die("BUG: resolve_relative_url only accepts one or two arguments");
This is diagnosing incorrect arguments, so:
s/resolve_relative_url/resolve-relative-url/
Also, I'm not convinced that this deserves a "BUG:" prefix, as this is
now a user-accessible command (even though it's plumbing).
> + url = argv[1];
> + strbuf_addf(&sb, "remote.%s.url", remote);
> + free(remote);
> +
> + if (git_config_get_string(sb.buf, &remoteurl))
> + /* the repository is its own authoritative upstream */
> + remoteurl = xgetcwd();
> +
> + if (argc == 3)
> + up_path = argv[2];
> +
> + res = relative_url(remoteurl, url, up_path);
> + printf("%s\n", res);
> +
> + free(res);
> + return 0;
> +}
> +
> +static int resolve_relative_url_test(int argc, const char **argv, const char *prefix)
> +{
> + char *remoteurl, *res;
> + const char *up_path, *url;
> +
> + if (argc != 4)
> + die("BUG: resolve_relative_url only accepts three arguments: <up_path> <remoteurl> <url>");
s/resolve_relative_url/resolve-relative-url/
Ditto observation about "BUG:" prefix.
> + up_path = argv[1];
> + remoteurl = xstrdup(argv[2]);
> + url = argv[3];
> +
> + if (!strcmp(up_path, "(null)"))
> + up_path = NULL;
> +
> + res = relative_url(remoteurl, url, up_path);
> + printf("%s\n", res);
This could be:
puts(res);
though I don't care strongly.
> + free(res);
> + return 0;
> +}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html