[PATCH 0/2] Set section via git config

[PATCH 0/2] Set section via git config

[jamie.couture]

From: Jamie Couture <jamie.couture at gmail.com>

Sorry for the noise.  I can see my attachments, but some people can not.
Trying again.

Third time's a charm?

Jamie Couture (2):
  avoid memory leak
  Add feature: obtain repo section from git config

 cgit.c       |    2 ++
 cgit.h       |    1 +
 cgitrc.5.txt |    8 ++++++++
 scan-tree.c  |   12 ++++++++++++
 4 files changed, 23 insertions(+), 0 deletions(-)

-- 
1.7.5.4

[PATCH 1/2] avoid memory leak

[jamie.couture]

From: Jamie Couture <jamie.couture at gmail.com>

free rel when we're done
---
 scan-tree.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/scan-tree.c b/scan-tree.c
index 627af1b..3f579ec 100644
--- a/scan-tree.c
+++ b/scan-tree.c
@@ -155,6 +155,8 @@ static void add_repo(const char *base, const char *path, repo_config_fn fn)
 		config_fn = fn;
 		parse_configfile(xstrdup(p), &repo_config);
 	}
+
+	free(rel);
 }
 
 static void scan_path(const char *base, const char *path, repo_config_fn fn)
-- 
1.7.5.4

[PATCH 2/2] Add feature: obtain repo section from git config

[jamie.couture]

From: Jamie Couture <jamie.couture at gmail.com>

---
 cgit.c       |    2 ++
 cgit.h       |    1 +
 cgitrc.5.txt |    8 ++++++++
 scan-tree.c  |   10 ++++++++++
 4 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/cgit.c b/cgit.c
index f4dd6ef..6a51bec 100644
--- a/cgit.c
+++ b/cgit.c
@@ -207,6 +207,8 @@ void config_cb(const char *name, const char *value)
 		ctx.cfg.scan_hidden_path = atoi(value);
 	else if (!strcmp(name, "section-from-path"))
 		ctx.cfg.section_from_path = atoi(value);
+	else if (!strcmp(name, "section-from-repo-config"))
+		ctx.cfg.section_from_repo_config = atoi(value);
 	else if (!strcmp(name, "source-filter"))
 		ctx.cfg.source_filter = new_filter(value, 1);
 	else if (!strcmp(name, "summary-log"))
diff --git a/cgit.h b/cgit.h
index b5f00fc..14e8243 100644
--- a/cgit.h
+++ b/cgit.h
@@ -214,6 +214,7 @@ struct cgit_config {
 	int remove_suffix;
 	int scan_hidden_path;
 	int section_from_path;
+	int section_from_repo_config;
 	int snapshots;
 	int summary_branches;
 	int summary_log;
diff --git a/cgitrc.5.txt b/cgitrc.5.txt
index c3698a6..a78d226 100644
--- a/cgitrc.5.txt
+++ b/cgitrc.5.txt
@@ -301,6 +301,14 @@ section-from-path::
 	If negative, cgit will discard the specified number of path elements
 	above the repo directory. Default value: 0.
 
+section-from-repo-config::
+	If set to "1" obtain the section name from git config. The expected config
+	section.key that is used is "cgit.section".
+	Ex: $ git config cgit.section mysection
+	An alternative to section-from-path, and will not check git config if
+	section-from-path is set. See also: scan-path.  This must be defined prior
+	to scan-path.
+
 side-by-side-diffs::
 	If set to "1" shows side-by-side diffs instead of unidiffs per
 	default. Default value: "0".
diff --git a/scan-tree.c b/scan-tree.c
index 3f579ec..3bbc4bc 100644
--- a/scan-tree.c
+++ b/scan-tree.c
@@ -61,6 +61,13 @@ static int git_owner_config(const char *key, const char *value, void *cb)
 	return 0;
 }
 
+static int git_section_config(const char *key, const char *value, void *cb)
+{
+	if (!strcmp(key, "cgit.section"))
+		repo->section = xstrdup(value);
+	return 0;
+}
+
 static char *xstrrchr(char *s, char *from, int c)
 {
 	while (from >= s && *from != c)
@@ -149,6 +156,9 @@ static void add_repo(const char *base, const char *path, repo_config_fn fn)
 			}
 		}
 	}
+	else if (ctx.cfg.section_from_repo_config) {
+		git_config_from_file(git_section_config, fmt("%s/config", path), NULL);
+	}
 
 	p = fmt("%s/cgitrc", path);
 	if (!stat(p, &st)) {
-- 
1.7.5.4

[PATCH 1/2] avoid memory leak

[larsh]

On Fri, Jun 03, 2011 at 07:21:01PM -0400, Jamie Couture wrote:
> --- a/scan-tree.c
> +++ b/scan-tree.c
> @@ -155,6 +155,8 @@ static void add_repo(const char *base, const char *path, repo_config_fn fn)
>  		config_fn = fn;
>  		parse_configfile(xstrdup(p), &repo_config);
>  	}
> +
> +	free(rel);
>  }
>  
>  static void scan_path(const char *base, const char *path, repo_config_fn fn)

Thanks. Can I add 'Signed-off by' from you on this?

--
larsh

[PATCH 2/2] Add feature: obtain repo section from git config

[larsh]

On Fri, Jun 03, 2011 at 07:21:02PM -0400, Jamie Couture wrote:
> +section-from-repo-config::
> +	If set to "1" obtain the section name from git config. The expected config
> +	section.key that is used is "cgit.section".
> +	Ex: $ git config cgit.section mysection
> +	An alternative to section-from-path, and will not check git config if
> +	section-from-path is set. See also: scan-path.  This must be defined prior
> +	to scan-path.
> +

* What value is added by $GITDIR/config compared to $GITDIR/cgitrc?
* If we want to support reading repo-config from $GITDIR/config, why not
  implement all the options supported by a $GITDIR/cgitrc?

--
larsh

[PATCH 2/2] Add feature: obtain repo section from git config

[mailings]

On 06/06/2011 07:13 PM, larsh at hjemli.net wrote:
> On Fri, Jun 03, 2011 at 07:21:02PM -0400, Jamie Couture wrote:
>> +section-from-repo-config::
>> +	If set to "1" obtain the section name from git config. The expected config
>> +	section.key that is used is "cgit.section".
>> +	Ex: $ git config cgit.section mysection
>> +	An alternative to section-from-path, and will not check git config if
>> +	section-from-path is set. See also: scan-path.  This must be defined prior
>> +	to scan-path.
>> +
> 
> * What value is added by $GITDIR/config compared to $GITDIR/cgitrc?
> * If we want to support reading repo-config from $GITDIR/config, why not
>   implement all the options supported by a $GITDIR/cgitrc?
> 

I thought about this too.

It seems attractive but to me is mixing concerns: git and cgit are two
different tools (although closely tied). Having cgit store (part of) its
configuration in git configuration files is not good architecture,
unwise and fragile since it make the cgit configuration directly
dependent on git configuration. cgit can't change it's configuration
format since it has to follow git's and once git changes its format cgit
immediately breaks.

I'd prefer not doing this (everything in $GITDIR/config). I think it's
better to have $GITDIR/cgitrc files that hold the repo settings.

There is this setting called repo.path though that then is kind of an
annoyance to set and update. If we always use $GITDIR/cgitrc files then
the repo.path setting could be automatically deduced by cgit.


-- 
Ferry Huberts

[PATCH 2/2] Add feature: obtain repo section from git config

[larsh]

On Mon, Jun 06, 2011 at 07:38:13PM +0200, Ferry Huberts wrote:
> On 06/06/2011 07:13 PM, larsh at hjemli.net wrote:
> > * If we want to support reading repo-config from $GITDIR/config, why not
> >   implement all the options supported by a $GITDIR/cgitrc?
> 
> I'd prefer not doing this (everything in $GITDIR/config). I think it's
> better to have $GITDIR/cgitrc files that hold the repo settings.

I agree.


> There is this setting called repo.path though that then is kind of an
> annoyance to set and update. If we always use $GITDIR/cgitrc files then
> the repo.path setting could be automatically deduced by cgit.

For repo-specific cgitrc-files, repo.path (and repo.url) are automatically
set (see http://hjemli.net/git/cgit/tree/cgitrc.5.txt#n446).

--
larsh

[PATCH 2/2] Add feature: obtain repo section from git config

[mailings]

On 06/06/2011 07:48 PM, larsh at hjemli.net wrote:
> On Mon, Jun 06, 2011 at 07:38:13PM +0200, Ferry Huberts wrote:
>> On 06/06/2011 07:13 PM, larsh at hjemli.net wrote:
>>> * If we want to support reading repo-config from $GITDIR/config, why not
>>>   implement all the options supported by a $GITDIR/cgitrc?
>>
>> I'd prefer not doing this (everything in $GITDIR/config). I think it's
>> better to have $GITDIR/cgitrc files that hold the repo settings.
> 
> I agree.
> 
> 
>> There is this setting called repo.path though that then is kind of an
>> annoyance to set and update. If we always use $GITDIR/cgitrc files then
>> the repo.path setting could be automatically deduced by cgit.
> 
> For repo-specific cgitrc-files, repo.path (and repo.url) are automatically
> set (see http://hjemli.net/git/cgit/tree/cgitrc.5.txt#n446).
> 

yeah, but that is dependent on scan-path which I don't use and don't
want to use.

-- 
Ferry Huberts

[PATCH 2/2] Add feature: obtain repo section from git config

[jamie.couture]

On 11-06-06 01:38 PM, Ferry Huberts wrote:
> On 06/06/2011 07:13 PM, larsh at hjemli.net wrote:
>> On Fri, Jun 03, 2011 at 07:21:02PM -0400, Jamie Couture wrote:
>>> +section-from-repo-config::
>>> +	If set to "1" obtain the section name from git config. The expected config
>>> +	section.key that is used is "cgit.section".
>>> +	Ex: $ git config cgit.section mysection
>>> +	An alternative to section-from-path, and will not check git config if
>>> +	section-from-path is set. See also: scan-path.  This must be defined prior
>>> +	to scan-path.
>>> +
>> * What value is added by $GITDIR/config compared to $GITDIR/cgitrc?
>> * If we want to support reading repo-config from $GITDIR/config, why not
>>    implement all the options supported by a $GITDIR/cgitrc?
>>
> I thought about this too.
>
> It seems attractive but to me is mixing concerns: git and cgit are two
> different tools (although closely tied). Having cgit store (part of) its
> configuration in git configuration files is not good architecture,
> unwise and fragile since it make the cgit configuration directly
> dependent on git configuration. cgit can't change it's configuration
> format since it has to follow git's and once git changes its format cgit
> immediately breaks.
The motivation was more about being lazy for those who use scan-path to 
pick up repositories, and only serves to help the presentation / 
separation of sections in the front-end, but is by no means easier to 
maintain.  I agree that mixing configuration is clumsy.

In my case, I was using gitoilte + cgit.  Perhaps I overlooked a feature 
of gitolite to create repositories based on some path, say:
parent/{section1, ..., sectionN}/actual_project.git (the 
section-from-path feature should have been used in this case). Instead 
everything is living as children from a common parent, which is how I 
currently have it setup.

> I'd prefer not doing this (everything in $GITDIR/config). I think it's
> better to have $GITDIR/cgitrc files that hold the repo settings.
>
> There is this setting called repo.path though that then is kind of an
> annoyance to set and update. If we always use $GITDIR/cgitrc files then
> the repo.path setting could be automatically deduced by cgit.
>
I was trying to do as little touching of cgitrc as possible, with 
respect to updating / maintaining repository information.

Thanks for the consideration; I appreciate the feedback.


Jamie Couture

[PATCH 2/2] Add feature: obtain repo section from git config

[mailings]

On 06/06/2011 08:31 PM, Jamie Couture wrote:
> On 11-06-06 01:38 PM, Ferry Huberts wrote:
>> On 06/06/2011 07:13 PM, larsh at hjemli.net wrote:
>>> On Fri, Jun 03, 2011 at 07:21:02PM -0400, Jamie Couture wrote:
>>>> +section-from-repo-config::
>>>> +    If set to "1" obtain the section name from git config. The
>>>> expected config
>>>> +    section.key that is used is "cgit.section".
>>>> +    Ex: $ git config cgit.section mysection
>>>> +    An alternative to section-from-path, and will not check git
>>>> config if
>>>> +    section-from-path is set. See also: scan-path.  This must be
>>>> defined prior
>>>> +    to scan-path.
>>>> +
>>> * What value is added by $GITDIR/config compared to $GITDIR/cgitrc?
>>> * If we want to support reading repo-config from $GITDIR/config, why not
>>>    implement all the options supported by a $GITDIR/cgitrc?
>>>
>> I thought about this too.
>>
>> It seems attractive but to me is mixing concerns: git and cgit are two
>> different tools (although closely tied). Having cgit store (part of) its
>> configuration in git configuration files is not good architecture,
>> unwise and fragile since it make the cgit configuration directly
>> dependent on git configuration. cgit can't change it's configuration
>> format since it has to follow git's and once git changes its format cgit
>> immediately breaks.
> The motivation was more about being lazy for those who use scan-path to
> pick up repositories, and only serves to help the presentation /
> separation of sections in the front-end, but is by no means easier to
> maintain.  I agree that mixing configuration is clumsy.
> 
> In my case, I was using gitoilte + cgit.  Perhaps I overlooked a feature
> of gitolite to create repositories based on some path, say:
> parent/{section1, ..., sectionN}/actual_project.git (the
> section-from-path feature should have been used in this case). Instead
> everything is living as children from a common parent, which is how I
> currently have it setup.
> 
>> I'd prefer not doing this (everything in $GITDIR/config). I think it's
>> better to have $GITDIR/cgitrc files that hold the repo settings.
>>
>> There is this setting called repo.path though that then is kind of an
>> annoyance to set and update. If we always use $GITDIR/cgitrc files then
>> the repo.path setting could be automatically deduced by cgit.
>>
> I was trying to do as little touching of cgitrc as possible, with
> respect to updating / maintaining repository information.
> 


You could use something like I do:
cgitrc includes a file called 'sections'
the sections file includes all separate section files.
a separate section file includes all separate repo cgitrc files

this makes every include a single line to maintain

this allows me to quickly change repos from section without touching the
cgitrc. also allows me to quickly change section descriptions without
touching cgitrc. and keeps cgit repo config together with the git repo
and easy to maintain

-- 
Ferry Huberts

[PATCH 1/2] avoid memory leak

[jamie.couture]

free rel when we're done
---
 scan-tree.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

[PATCH 1/2] avoid memory leak

[mailings]

On 06/03/2011 01:40 AM, jamie.couture at gmail.com wrote:
> 
> free rel when we're done
> ---
>  scan-tree.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> 
> 
> 
> _______________________________________________
> cgit mailing list
> cgit at hjemli.net
> http://hjemli.net/mailman/listinfo/cgit

i see no patches...

-- 
Ferry Huberts

[PATCH 1/2] avoid memory leak

[jamie.couture]

free rel when we're done
---
 scan-tree.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)