clone url generation issue

clone url generation issue

[jugg]

I use gitolite to manage my repositories, and per my gitolite configuration
the general clone url for all of my repositories is:

git at example.com:<project>

My cgit instance is such that a particular project is browsed at:

http://example.com/git/<project>/

As such, I have the following in my cgitrc:

logo=/git/cgit.png
css=/git/cgit.css
remove-suffix=1
virtual-root=/git
project-list=/var/lib/gitolite/projects.list
scan-path=/srv/gitolite
clone-prefix=git at example.com:

However, with this setup, the clone url shown on each project is incorrect,
like so:

git at example.com:/<project>

Notice the erroneous forward slash being inserted between the clone-prefix
and project name.

Have I configured something incorrectly, or is this setup not supported?

Thanks,

chris

clone url generation issue

[plenz]

Hi!

* chris <jugg at hotmail.com> [2011-05-04 13:56]:
> I use gitolite to manage my repositories, and per my gitolite
> configuration the general clone url for all of my repositories is:
> 
> git at example.com:<project>

This is a short syntax supported by git because it resembles scp
usage, where you also do user at host:path. However, Git equally supports
the ssh://user at host/path syntax, which might be a better approach
because you explicitly state the protocol to use (ssh) so that a
browser may infer the appropriate program to call.

> clone-prefix=git at example.com:

> However, with this setup, the clone url shown on each project is incorrect,
> like so:
> 
> git at example.com:/<project>
> 
> Notice the erroneous forward slash being inserted between the
> clone-prefix and project name.

As far as I can see the print_url() function is responsible
(ui-summary.c), which does a simple fmt("%s/%s", base, suffix). So
you'd have to do some patching there in order to support that short
notion you're using.

I think it's best to use the long ssh:// syntax instead.

Julius

clone url generation issue

[jugg]

Julius Plenz <plenz at ...> writes:
> * chris <jugg at ...> [2011-05-04 13:56]:
> > git at ...:/<project>
> > 
> > Notice the erroneous forward slash being inserted between the
> > clone-prefix and project name.
> 
> As far as I can see the print_url() function is responsible
> (ui-summary.c), which does a simple fmt("%s/%s", base, suffix). So
> you'd have to do some patching there in order to support that short
> notion you're using.

From my reading of the code, that concatenation is only used when a url-prefix 
is defined.  In which case, simply changing the line from:

  base = fmt("%s/%s", base, suffix);

to

  base = fmt("%s%s", base, suffix);

allowing the forward slash to be specified as part of the prefix insetad.

> I think it's best to use the long ssh:// syntax instead.

Yes, unfortunately prior to integrating cgit, our repository urls have been 
using the shorthand format.

Thanks for the feedback and pointer to the relevant code.

chris

clone url generation issue

[jugg]

chris <jugg at ...> writes:
> From my reading of the code, that concatenation is only used when a url-prefix 
> is defined.

I mean clone-prefix of course.

Examples:

clone-prefix=ssh://git at example.com/

or

clone-prefix=git at example.com:

chris

clone url generation issue

[hjemli]

On Fri, May 6, 2011 at 06:03, chris <jugg at hotmail.com> wrote:
> Julius Plenz <plenz at ...> writes:
>> * chris <jugg at ...> [2011-05-04 13:56]:
>> > git at ...:/<project>
>> >
>> > Notice the erroneous forward slash being inserted between the
>> > clone-prefix and project name.
>>
>> As far as I can see the print_url() function is responsible
>> (ui-summary.c), which does a simple fmt("%s/%s", base, suffix). So
>> you'd have to do some patching there in order to support that short
>> notion you're using.
>
> From my reading of the code, that concatenation is only used when a url-prefix
> is defined. ?In which case, simply changing the line from:
>
> ?base = fmt("%s/%s", base, suffix);
>
> to
>
> ?base = fmt("%s%s", base, suffix);
>
> allowing the forward slash to be specified as part of the prefix insetad.
>

Such a change would correctly fix your problem, but it would break
existing installations. I think a better solution is to introduce a
new setting, clone-url, which is adopted by each repo during
config/scanning and which supports expansion of environment variables
like $CGIT_REPO_NAME. Maybe something like this untested (and
gmail-mangled) patch:

diff --git a/cgit.c b/cgit.c
index 6be3754..b651843 100644
--- a/cgit.c
+++ b/cgit.c
@@ -244,6 +244,8 @@ void config_cb(const char *name, const char *value)
 		ctx.cfg.robots = xstrdup(value);
 	else if (!strcmp(name, "clone-prefix"))
 		ctx.cfg.clone_prefix = xstrdup(value);
+	else if (!strcmp(name, "clone-url"))
+		ctx.cfg.clone_url = xstrdup(value);
 	else if (!strcmp(name, "local-time"))
 		ctx.cfg.local_time = atoi(value);
 	else if (!prefixcmp(name, "mimetype."))
@@ -463,6 +465,7 @@ static int prepare_repo_cmd(struct cgit_context *ctx)
 		cgit_print_docend();
 		return 1;
 	}
+	cgit_prepare_env(ctx->repo);
 	return 0;
 }

diff --git a/cgit.h b/cgit.h
index caa9d8e..21f4815 100644
--- a/cgit.h
+++ b/cgit.h
@@ -165,6 +165,7 @@ struct cgit_config {
 	char *agefile;
 	char *cache_root;
 	char *clone_prefix;
+	char *clone_url;
 	char *css;
 	char *favicon;
 	char *footer;
@@ -319,6 +320,8 @@ extern const char *cgit_repobasename(const char *reponame);

 extern int cgit_parse_snapshots_mask(const char *str);

+extern void cgit_prepare_env(struct cgit_repo *repo);
+
 extern int cgit_open_filter(struct cgit_filter *filter, struct
cgit_repo * repo);
 extern int cgit_close_filter(struct cgit_filter *filter);

diff --git a/shared.c b/shared.c
index be2ae59..af1f403 100644
--- a/shared.c
+++ b/shared.c
@@ -66,6 +66,7 @@ struct cgit_repo *cgit_add_repo(const char *url)
 	ret->max_stats = ctx.cfg.max_stats;
 	ret->module_link = ctx.cfg.module_link;
 	ret->readme = ctx.cfg.readme;
+	ret->clone_url = ctx.cfg.clone_url;
 	ret->mtime = -1;
 	ret->about_filter = ctx.cfg.about_filter;
 	ret->commit_filter = ctx.cfg.commit_filter;
@@ -374,7 +375,8 @@ typedef struct {
 	char * value;
 } cgit_env_var;

-static void prepare_env(struct cgit_repo * repo) {
+void cgit_prepare_env(struct cgit_repo *repo)
+{
 	cgit_env_var env_vars[] = {
 		{ .name = "CGIT_REPO_URL", .value = repo->url },
 		{ .name = "CGIT_REPO_NAME", .value = repo->name },
@@ -406,8 +408,6 @@ int cgit_open_filter(struct cgit_filter *filter,
struct cgit_repo * repo)
 		close(filter->pipe_fh[1]);
 		chk_non_negative(dup2(filter->pipe_fh[0], STDIN_FILENO),
 			"Unable to use pipe as STDIN");
-		if (repo)
-			prepare_env(repo);
 		execvp(filter->cmd, filter->argv);
 		die("Unable to exec subprocess %s: %s (%d)", filter->cmd,
 			strerror(errno), errno);
diff --git a/ui-summary.c b/ui-summary.c
index 1e9a1b6..d8a745a 100644
--- a/ui-summary.c
+++ b/ui-summary.c
@@ -62,7 +62,7 @@ void cgit_print_summary()
 			       NULL, NULL, 0, 0);
 	}
 	if (ctx.repo->clone_url)
-		print_urls(ctx.repo->clone_url, NULL);
+		print_urls(expand_macros(ctx.repo->clone_url), NULL);
 	else if (ctx.cfg.clone_prefix)
 		print_urls(ctx.cfg.clone_prefix, ctx.repo->url);
 	html("</table>");

-- 
larsh