[PATCH 0/2] Git-Config Parsing during scan-path

[PATCH 0/2] Git-Config Parsing during scan-path

[@ 2012-10-07 21:23 UTC (permalink / raw)]

Some time ago, Jamie sent a patch to this list that allows to parse the git
config on scan-path [1]. The main use-case is a nice integration with gitolite
instead of the 'one workaround per option' as it is handled now
(enable-gitweb-*).

As Jason had asked to resend non-applied patches, I took the chance to port the
patch to the current master and to update the documentation.

Changes wrt the old patch:

* Use "cgit" as gitconfig section instead of "repo" -- the latter is too generic
imho and might lead to problems in the future.

* Removed some obscure Apache hint that seemed to be out of context.

* Update example gitolite.conf to recent gitolite-3 syntax.

* Pointed out that it works perfectly fine together with the (now) existing
'enable-gitweb-*' stuff.

[1] http://hjemli.net/pipermail/cgit/2011-November/000414.html

- Ren?

Ren? 'Necoro' Neumann (2):
  Read "cgit.*" settings from gitconfig.
  Documentation for the gitconfig sourcing.

 cgit.c       |    3 +++
 cgit.h       |    1 +
 cgitrc.5.txt |   47 +++++++++++++++++++++++++++++++++++++++++++++++
 scan-tree.c  |   19 +++++++++++++++----
 4 files changed, 66 insertions(+), 4 deletions(-)

-- 
1.7.8.6

[PATCH 1/2] Read "cgit.*" settings from gitconfig.

[@ 2012-10-07 21:23 UTC (permalink / raw)]

Allow to override cgit settings directly using git config -- this
is especially useful for gitolite, as it works without having to export
multiple options by hand.

The gitconfig section is "cgit", in contrast to the normal "repo" key used
throughout normal cgit. This is to avoid possible future name clashes.

Patch based on: http://hjemli.net/pipermail/cgit/2011-November/000428.html
---
 cgit.c      |    3 +++
 cgit.h      |    1 +
 scan-tree.c |   19 +++++++++++++++----
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/cgit.c b/cgit.c
index 1ec02e7..910bf1d 100644
--- a/cgit.c
+++ b/cgit.c
@@ -183,6 +183,8 @@ void config_cb(const char *name, const char *value)
 		ctx.cfg.enable_subject_links = atoi(value);
 	else if (!strcmp(name, "enable-tree-linenumbers"))
 		ctx.cfg.enable_tree_linenumbers = atoi(value);
+	else if (!strcmp(name, "enable-git-config"))
+		ctx.cfg.enable_git_config = atoi(value);
 	else if (!strcmp(name, "max-stats"))
 		ctx.cfg.max_stats = cgit_find_stats_period(value, NULL);
 	else if (!strcmp(name, "cache-size"))
@@ -348,6 +350,7 @@ static void prepare_context(struct cgit_context *ctx)
 	ctx->cfg.enable_gitweb_section = 1;
 	ctx->cfg.enable_http_clone = 1;
 	ctx->cfg.enable_tree_linenumbers = 1;
+	ctx->cfg.enable_git_config = 0;
 	ctx->cfg.max_repo_count = 50;
 	ctx->cfg.max_commit_count = 50;
 	ctx->cfg.max_lock_attempts = 5;
diff --git a/cgit.h b/cgit.h
index 79ba7ad..4ac6f80 100644
--- a/cgit.h
+++ b/cgit.h
@@ -211,6 +211,7 @@ struct cgit_config {
 	int enable_remote_branches;
 	int enable_subject_links;
 	int enable_tree_linenumbers;
+	int enable_git_config;
 	int local_time;
 	int max_atom_items;
 	int max_repo_count;
diff --git a/scan-tree.c b/scan-tree.c
index 6d1941e..7e711d5 100644
--- a/scan-tree.c
+++ b/scan-tree.c
@@ -67,7 +67,14 @@ static int gitweb_config(const char *key, const char *value, void *cb)
 	return 0;
 }
 
+static int cgit_repo_config(const char *key, const char *value, void *cb)
+{
+	if (!prefixcmp(key, "cgit.")) {
+		config_fn(repo, key + 5, value);
+	}
 
+	return 0;
+}
 
 static char *xstrrchr(char *s, char *from, int c)
 {
@@ -166,10 +173,14 @@ static void add_repo(const char *base, const char *path, repo_config_fn fn)
 		}
 	}
 
-	p = fmt("%s/cgitrc", path);
-	if (!stat(p, &st)) {
-		config_fn = fn;
-		parse_configfile(xstrdup(p), &repo_config);
+	config_fn = fn;
+	if (ctx.cfg.enable_git_config) {
+		git_config_from_file(cgit_repo_config, fmt("%s/config", path), NULL);
+	} else {
+		p = fmt("%s/cgitrc", path);
+		if (!stat(p, &st)) {
+			parse_configfile(xstrdup(p), &repo_config);
+		}
 	}
 
 	free(rel);
-- 
1.7.8.6

[PATCH 2/2] Documentation for the gitconfig sourcing.

[@ 2012-10-07 21:23 UTC (permalink / raw)]

Document the 'enable-git-config' option and give an example of a a gitolite
configuration.
---
 cgitrc.5.txt |   47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/cgitrc.5.txt b/cgitrc.5.txt
index 902fff3..52c33e3 100644
--- a/cgitrc.5.txt
+++ b/cgitrc.5.txt
@@ -163,6 +163,13 @@ enable-tree-linenumbers::
 	Flag which, when set to "1", will make cgit generate linenumber links
 	for plaintext blobs printed in the tree view. Default value: "1".
 
+enable-git-config::
+    Flag which, when set to "1", will allow cgit to use git config to set
+    any repo specific settings. Please read <<repo-settings>> to learn more
+    about which settings are available. This option is used in conjunction with
+    "scan-path" to override _repo._ settings. Please read <<git-config>> to
+    learn how to integrate with gitolite.  Default value: "0".
+
 favicon::
 	Url used as link to a shortcut icon for cgit. If specified, it is
 	suggested to use the value "/favicon.ico" since certain browsers will
@@ -394,6 +401,7 @@ virtual-root::
 	NOTE: cgit has recently learned how to use PATH_INFO to achieve the
 	same kind of virtual urls, so this option will probably be deprecated.
 
+[[repo-settings]]
 REPOSITORY SETTINGS
 -------------------
 repo.about-filter::
@@ -509,6 +517,45 @@ options are only acknowledged in repo-specific config files when
 Note: the "repo." prefix is dropped from the option names in repo-specific
 config files, e.g. "repo.desc" becomes "desc".
 
+[[git-config]]
+REPOSITORY-SPECIFIC GIT CONFIG
+------------------------------
+When "scan-path" is used to auto-discover repositories "enable-git-config" will
+allow cgit to look at git config to obtain repo specific settings.
+This is especially helpful for tools like gitolite, that allow setting git
+config values, but do not define their own interface for cgit. Please read more
+about http://sitaramc.github.com/gitolite/git-config.html[gitolite repo specific 
+commands here].
+
+Note: If the "enable-gitweb-*" options are set to "1", the corresponding gitweb
+settings are also used (in the example below: "owner", "desc", "category"). In
+case of conflicts the "cgit." setting is preferred.
+
+.Example gitolite.conf
+......
+repo aproject
+    RW+ = bob
+    config cgit.section = test
+    config cgit.logo = /cgit-data/logo.png
+    config cgit.owner = "Bob McPerson"
+    config cgit.desc =  "Bar description"
+
+repo project_with_gitweb_settings
+    RW+ = bob
+    config cgit.defbranch = development
+    owner = "Bob McPerson"
+    desc = "Foo description"
+    category = applications
+......
+
+[IMPORTANT]
+Unlike cgitrc, we're looking for repo settings with prefix "cgit.". That is, all
+options named 'repo.XY' above have to be called 'cgit.XY'.
+Remember to edit _.gitolite.rc_ and add the following:
+.......
+$GIT_CONFIG_KEYS = "cgit\..*"
+......
+
 
 FILTER API
 ----------
-- 
1.7.8.6

[PATCH 1/2] Read "cgit.*" settings from gitconfig.

[jamie.couture]

I'm fine with this.

The reason for choosing 'repo.' as a prefix was to avoid any confusion for
cgit users: cgitrc uses 'repo.' namespace but gitolite big-config uses
'cgit.'

However, using cgit. prefix in the gitolite big-config file makes more
sense; I had changed it to 'cigt.' in my own code but didn't push changes
back upstream in  hopes that it would be brought up as a discussion with
Lars et al.

Should I re-submit these patches? I'm okay with Ren?'s changes.

On Sun, Oct 7, 2012 at 5:23 PM, Ren? 'Necoro' Neumann <necoro at necoro.net>wrote:

> Allow to override cgit settings directly using git config -- this
> is especially useful for gitolite, as it works without having to export
> multiple options by hand.
>
> The gitconfig section is "cgit", in contrast to the normal "repo" key used
> throughout normal cgit. This is to avoid possible future name clashes.
>
> Patch based on: http://hjemli.net/pipermail/cgit/2011-November/000428.html
> ---
>  cgit.c      |    3 +++
>  cgit.h      |    1 +
>  scan-tree.c |   19 +++++++++++++++----
>  3 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/cgit.c b/cgit.c
> index 1ec02e7..910bf1d 100644
> --- a/cgit.c
> +++ b/cgit.c
> @@ -183,6 +183,8 @@ void config_cb(const char *name, const char *value)
>                 ctx.cfg.enable_subject_links = atoi(value);
>         else if (!strcmp(name, "enable-tree-linenumbers"))
>                 ctx.cfg.enable_tree_linenumbers = atoi(value);
> +       else if (!strcmp(name, "enable-git-config"))
> +               ctx.cfg.enable_git_config = atoi(value);
>         else if (!strcmp(name, "max-stats"))
>                 ctx.cfg.max_stats = cgit_find_stats_period(value, NULL);
>         else if (!strcmp(name, "cache-size"))
> @@ -348,6 +350,7 @@ static void prepare_context(struct cgit_context *ctx)
>         ctx->cfg.enable_gitweb_section = 1;
>         ctx->cfg.enable_http_clone = 1;
>         ctx->cfg.enable_tree_linenumbers = 1;
> +       ctx->cfg.enable_git_config = 0;
>         ctx->cfg.max_repo_count = 50;
>         ctx->cfg.max_commit_count = 50;
>         ctx->cfg.max_lock_attempts = 5;
> diff --git a/cgit.h b/cgit.h
> index 79ba7ad..4ac6f80 100644
> --- a/cgit.h
> +++ b/cgit.h
> @@ -211,6 +211,7 @@ struct cgit_config {
>         int enable_remote_branches;
>         int enable_subject_links;
>         int enable_tree_linenumbers;
> +       int enable_git_config;
>         int local_time;
>         int max_atom_items;
>         int max_repo_count;
> diff --git a/scan-tree.c b/scan-tree.c
> index 6d1941e..7e711d5 100644
> --- a/scan-tree.c
> +++ b/scan-tree.c
> @@ -67,7 +67,14 @@ static int gitweb_config(const char *key, const char
> *value, void *cb)
>         return 0;
>  }
>
> +static int cgit_repo_config(const char *key, const char *value, void *cb)
> +{
> +       if (!prefixcmp(key, "cgit.")) {
> +               config_fn(repo, key + 5, value);
> +       }
>
> +       return 0;
> +}
>
>  static char *xstrrchr(char *s, char *from, int c)
>  {
> @@ -166,10 +173,14 @@ static void add_repo(const char *base, const char
> *path, repo_config_fn fn)
>                 }
>         }
>
> -       p = fmt("%s/cgitrc", path);
> -       if (!stat(p, &st)) {
> -               config_fn = fn;
> -               parse_configfile(xstrdup(p), &repo_config);
> +       config_fn = fn;
> +       if (ctx.cfg.enable_git_config) {
> +               git_config_from_file(cgit_repo_config, fmt("%s/config",
> path), NULL);
> +       } else {
> +               p = fmt("%s/cgitrc", path);
> +               if (!stat(p, &st)) {
> +                       parse_configfile(xstrdup(p), &repo_config);
> +               }
>         }
>
>         free(rel);
> --
> 1.7.8.6
>
>

[PATCH 1/2] Read "cgit.*" settings from gitconfig.

[Jason]

On Mon, Oct 8, 2012 at 5:14 PM, Jamie Couture <jamie.couture at gmail.com> wrote:
> Should I re-submit these patches? I'm okay with Ren?'s changes.

Submit yours too, and I'll take a look at both.

[PATCH 1/2] Read "cgit.*" settings from gitconfig.

[@ 2012-10-08 15:24 UTC (permalink / raw)]

I didn't do any changes in the C-code (besides the "repo" -> "cgit"). So
except changes in the surroundings (=porting to master), there is
nothing new here.

It's the documentation I changed :) (I note, that Jamie still speaks of
"gitolite bigconfig", which is a term not existing anymore in (new)
gitolite-3).

- Ren?

Am 08.10.2012 17:19, schrieb Jason A. Donenfeld:
> On Mon, Oct 8, 2012 at 5:14 PM, Jamie Couture <jamie.couture at gmail.com> wrote:
>> Should I re-submit these patches? I'm okay with Ren?'s changes.
> 
> Submit yours too, and I'll take a look at both.
> 
> _______________________________________________
> cgit mailing list
> cgit at hjemli.net
> http://hjemli.net/mailman/listinfo/cgit
> 

[PATCH 1/2] Read "cgit.*" settings from gitconfig.

[Jason]

On Mon, Oct 8, 2012 at 5:24 PM, Ren? Neumann <lists at necoro.eu> wrote:
> I didn't do any changes in the C-code (besides the "repo" -> "cgit"). So
> except changes in the surroundings (=porting to master), there is
> nothing new here.
>
> It's the documentation I changed :) (I note, that Jamie still speaks of
> "gitolite bigconfig", which is a term not existing anymore in (new)
> gitolite-3).

Yes yes, I understand. Send the doc changes?

[PATCH 0/2] Git-Config Parsing during scan-path

[Jason]

Hi Ren?,

I've merged these changes into a branch in the repo:
http://git.zx2c4.com/cgit/log/?h=rn/gitconfig

I'd like to mull over them for a few days and not to hastily merge
them into master for a couple of reasons.

- Currently the section and description tags are taken care of using
gitweb.category and gitweb.description, just fine using these two
commits in master:
http://git.zx2c4.com/cgit/commit/?id=fc9181ff3d3ebbe0159871f6a49438e60bb17f58
http://git.zx2c4.com/cgit/commit/?id=b56be4ba3a942dd1978fe4bfecd9afc35aab0027
So one set of patches or the other does duplicate some functionality.

- There are certain advantages of keeping with the gitweb.* namespace,
as it promotes compatibility and unifies a web-oriented configuration
namespace. "When things are in the gitweb area, it means they should
work with webpages that use git for data." I'm not sure I like the
idea of a general cgit namespace, and then cherry picking special
gitweb.* items.

- git_config_from_file should only be called once. This patch series
duplicates the call. Things should be unified into one callback
function.

- The two patches listed above work with the gitolite default, which is nice.

- I don't like having the uber gitolite-specific documentation in
there. It seems like a general solution would be more optimal. On the
other hand, since cgit is so commonly used alongside gitolite, we
could creep gradually in this direction, thought I don't think this
was ever Lars' intention.

What are your thoughts?

Jason

[PATCH 0/2] Git-Config Parsing during scan-path

[@ 2012-10-09 9:30 UTC (permalink / raw)]

Hi Jason,

> I've merged these changes into a branch in the repo:
> http://git.zx2c4.com/cgit/log/?h=rn/gitconfig

Thanks.

> - Currently the section and description tags are taken care of using
> gitweb.category and gitweb.description, just fine using these two
> commits in master:
> http://git.zx2c4.com/cgit/commit/?id=fc9181ff3d3ebbe0159871f6a49438e60bb17f58
> http://git.zx2c4.com/cgit/commit/?id=b56be4ba3a942dd1978fe4bfecd9afc35aab0027
> So one set of patches or the other does duplicate some functionality.

That's true. My original intention was: Use the gitweb stuff 90% of the
time -- and the cgit ones if there is no appropriate gitweb equivalent.
Gitweb only has three useful keys -- and those are already mapped.
One other might be "gitweb.snaphot" to be mapped to "repo.snapshots".

But anything else (like repo.defbranch) cannot be set with the current
mechanisms. Currently people are using git-hooks to generate cgitrc and
stuff -- which to me personally feels more like hacking then real
support :).

So I would vote for keeping both sets of patches: The one above to have
a simple standard -- and the new one to allow more fine-grained cgit
control if someone needs it.

> - There are certain advantages of keeping with the gitweb.* namespace,
> as it promotes compatibility and unifies a web-oriented configuration
> namespace. "When things are in the gitweb area, it means they should
> work with webpages that use git for data." I'm not sure I like the
> idea of a general cgit namespace, and then cherry picking special
> gitweb.* items.

Well -- sticking with the gitweb namespace alone has the same
consequences I mentioned above: Either we stick with the already defined
keys there -- thereby limiting what can easily be configured. Or we add
new keys to it which might lead to problems in the future. Also there
might be keys in gitweb, that have slightly different semantics or
namings from the counterparts in cgit (see for instance 'snapshot' vs
'snapshots').

It certainly depends on what you see the 'gitweb' category to be -- is
it 'all web for git' or 'the single product "gitweb"'?

> - git_config_from_file should only be called once. This patch series
> duplicates the call. Things should be unified into one callback
> function.

I'll fix this (hopefully this evening).

> - The two patches listed above work with the gitolite default, which is nice.

So does the new patch. It does not hinder you to use the
gitweb-variables. (Again: I'd recommend to keep both patch sets).

> - I don't like having the uber gitolite-specific documentation in
> there. It seems like a general solution would be more optimal.

I don't oppose removing most of it. I think the remark to set gitconfig
values (plus perhaps a link to the appropriate gitolite doc) is
self-explanatory. Also, if it hits 'master', I would send a
documentation patch to gitolite, adding a cgit section to their
'External Tools' page.

- Ren?

[PATCH 0/2] Git-Config Parsing during scan-path

[jamie.couture]

On Mon, Oct 8, 2012 at 5:27 PM, Jason A. Donenfeld <Jason at zx2c4.com> wrote:

> Hi Ren?,
>
> I've merged these changes into a branch in the repo:
> http://git.zx2c4.com/cgit/log/?h=rn/gitconfig
>
> I'd like to mull over them for a few days and not to hastily merge
> them into master for a couple of reasons.
>
> - Currently the section and description tags are taken care of using
> gitweb.category and gitweb.description, just fine using these two
> commits in master:
>
> http://git.zx2c4.com/cgit/commit/?id=fc9181ff3d3ebbe0159871f6a49438e60bb17f58
>
> http://git.zx2c4.com/cgit/commit/?id=b56be4ba3a942dd1978fe4bfecd9afc35aab0027
> So one set of patches or the other does duplicate some functionality.
>
> - There are certain advantages of keeping with the gitweb.* namespace,
> as it promotes compatibility and unifies a web-oriented configuration
> namespace. "When things are in the gitweb area, it means they should
> work with webpages that use git for data." I'm not sure I like the
> idea of a general cgit namespace, and then cherry picking special
> gitweb.* items.
>

I'm okay with it being referred to as gitweb.* section that can configure
cgit - but maybe that might be confusing, again, due to there being a
different product under the same name.  I'm also okay with it being a cgit.
namespace - as it is aid in configuring cgit.  I do think it's necessary
since we pick something to be consistent, which is why I initially chose
'repo.' as it was the cgitrc configuration terminology to avoid confusing
cgit users.

I mostly use this for section / defbranch and about - it's quite helpful.


>
> - git_config_from_file should only be called once. This patch series
> duplicates the call. Things should be unified into one callback
> function.
>
> No problem; this was only done originally since it was a quick patch to
help support those who wanted the feature.

- The two patches listed above work with the gitolite default, which is
> nice.
>
> - I don't like having the uber gitolite-specific documentation in
> there. It seems like a general solution would be more optimal. On the
> other hand, since cgit is so commonly used alongside gitolite, we
> could creep gradually in this direction, thought I don't think this
> was ever Lars' intention.
>
>
What are your thoughts?
>
> Jason
>
> _______________________________________________
> cgit mailing list
> cgit at hjemli.net
> http://hjemli.net/mailman/listinfo/cgit
>

[PATCH 0/2] Git-Config Parsing during scan-path

[Jason]

On Tue, Oct 9, 2012 at 11:30 AM, Ren? Neumann <lists at necoro.eu> wrote:
> That's true. My original intention was: Use the gitweb stuff 90% of the
> time -- and the cgit ones if there is no appropriate gitweb equivalent.
> Gitweb only has three useful keys -- and those are already mapped.
> One other might be "gitweb.snaphot" to be mapped to "repo.snapshots".
>
> But anything else (like repo.defbranch) cannot be set with the current
> mechanisms. Currently people are using git-hooks to generate cgitrc and
> stuff -- which to me personally feels more like hacking then real
> support :).


I am sort of leaning toward this -- just adding gitweb.defbranch, and
calling it a day.

> Well -- sticking with the gitweb namespace alone has the same
> consequences I mentioned above: Either we stick with the already defined
> keys there -- thereby limiting what can easily be configured. Or we add
> new keys to it which might lead to problems in the future. Also there
> might be keys in gitweb, that have slightly different semantics or
> namings from the counterparts in cgit (see for instance 'snapshot' vs
> 'snapshots').

Yea, you might be right. So let's say we do two namespaces, and it
looks like this:

gitweb.description -->
gitweb.category --> section
gitweb.owner -->
gitweb.defbranch -->

And we put these under a setting "gitweb gitconfig".

And then:

cgit.* -->

And this goes under a different setting "cgit gitconfig".



Alternatively, we just have one option, "use gitconfig settings", that maps:

gitweb.description -->
gitweb.category --> section
gitweb.owner -->
cgit.* -->


Which of these alternatives do you like better?




> I don't oppose removing most of it. I think the remark to set gitconfig
> values (plus perhaps a link to the appropriate gitolite doc) is
> self-explanatory. Also, if it hits 'master', I would send a
> documentation patch to gitolite, adding a cgit section to their
> 'External Tools' page.

Don't forget, also, about the cgit wiki!

http://git.zx2c4.com/cgit/about/
http://git.zx2c4.com/cgit/about/faq
http://git.zx2c4.com/cgit/tree/?h=wiki

[PATCH 0/2] Git-Config Parsing during scan-path

[@ 2012-10-09 12:07 UTC (permalink / raw)]

Am 09.10.2012 13:40, schrieb Jason A. Donenfeld:
> Alternatively, we just have one option, "use gitconfig settings", that maps:
> 
> gitweb.description -->
> gitweb.category --> section
> gitweb.owner -->
> cgit.* -->
> 
> 
> Which of these alternatives do you like better?

This one. Perhaps one should see it this way:

Putting stuff in a [cgit] category is the per-repository cgitrc taken to
the next level (i.e. use well-known git-stuff instead of homebrewn file
format). And understanding gitweb-keys is just a goody :).

> Don't forget, also, about the cgit wiki!

Ah. Hadn't thought that this is actually intended as a wiki, but that
'wiki' is now the web2.x-term for a general homepage :D.

- Ren?

[PATCH 0/2] Git-Config Parsing during scan-path

[Jason]

On Tue, Oct 9, 2012 at 2:07 PM, Ren? Neumann <lists at necoro.eu> wrote:
> This one. Perhaps one should see it this way:
>
> Putting stuff in a [cgit] category is the per-repository cgitrc taken to
> the next level (i.e. use well-known git-stuff instead of homebrewn file
> format). And understanding gitweb-keys is just a goody :).

I dig that. I've started working on something here:
http://git.zx2c4.com/cgit/commit/?h=jd/gitconfig

It doesn't work at the moment, but things are happening.