StGIT and repo-config

StGIT and repo-config

[Yann Dirson]

I'm trying to get StGIT to work with git's config information.
Currently, the stgit.config stuff uses a generic ConfigParser, which
I'm not sure is adequate for the work:

- it requires to forge section names like 'branch "foo"'
- it is only a reader and I'm not sure it is easy to turn it to a
writer.  It has a set() method, but it appears to only act on
in-memory stuff.
- since it parses the file at startup, it would not see any change
done from stgit by calling repo-config.
- the canonical way to access the info is git-repo-config.

I'd think this would be sufficient to rewrite stgit.config.
No objection ?

Best regards,
-- 
Yann.

Re: StGIT and repo-config

[Catalin Marinas]

On 25/01/07, Yann Dirson <ydirson@altern.org> wrote:
> I'm trying to get StGIT to work with git's config information.
> Currently, the stgit.config stuff uses a generic ConfigParser, which
> I'm not sure is adequate for the work:

There are some drawbacks indeed and it currently reads the configs and
transforms them slightly before passing them to ConfigParser. StGIT
initially had its own config files (git config came a bit later).

It would be good to have this re-written. It is even better to use
something like config.get('user.name') rather than config.get('user',
'name') as in ConfigParser.

What I'd like it to have is a single initial call to git-repo-config
--list (in config_setup) and all the options cached in a dictionary
(so that .git/config options would override the global ~/.gitconfig
ones). The dictionary should also be pre-populated with the default
values (only in memory, not going to the config file on disk).

For the options setting, it shouldn't probably need to use --global.
As above, these options should be cached in the dictionary as well.

-- 
Catalin

Re: StGIT and repo-config

[Johannes Schindelin]

Hi,

On Thu, 25 Jan 2007, Catalin Marinas wrote:

> What I'd like it to have is a single initial call to git-repo-config 
> --list (in config_setup) and all the options cached in a dictionary (so 
> that .git/config options would override the global ~/.gitconfig ones). 
> The dictionary should also be pre-populated with the default values 
> (only in memory, not going to the config file on disk).

There was another thread, a few days ago, where Jakub proposed an 
alternative config parser in Perl. Eric Wong even wrote a rather large 
patch to automatically generate _source_ code for Perl, Python and Ruby.

I then proposed to have a simple --dump option to repo-config, which 
outputs NUL separated key/value pairs (substituting "true" for keys 
without -- not with empty! -- values). But somehow the discussion petered 
out before anything came out of it.

The most important point (to me) which came out of the discussion: It is 
not at all easy, or straight-forward, to handle multi-vars, i.e. multiple 
values for the same key.

Ciao,
Dscho

Re: StGIT and repo-config

[Yann Dirson]

On Fri, Jan 26, 2007 at 12:58:38AM +0100, Johannes Schindelin wrote:
> On Thu, 25 Jan 2007, Catalin Marinas wrote:
> > What I'd like it to have is a single initial call to git-repo-config 
> > --list (in config_setup) and all the options cached in a dictionary (so 
> > that .git/config options would override the global ~/.gitconfig ones). 
> > The dictionary should also be pre-populated with the default values 
> > (only in memory, not going to the config file on disk).

Pre-filling with default value has a drawback: it will create lots of
useless entries, especially for per-branch settings.  I'd rather let
the accessors return the default value when needed.  We can still
group all defaults in a single dictionnary.


> I then proposed to have a simple --dump option to repo-config, which 
> outputs NUL separated key/value pairs (substituting "true" for keys 
> without -- not with empty! -- values). But somehow the discussion petered 
> out before anything came out of it.

Even if that was to be done in git, it would surely be post-1.5, so we
need another way to do things in the meantime.


> The most important point (to me) which came out of the discussion: It is 
> not at all easy, or straight-forward, to handle multi-vars, i.e. multiple 
> values for the same key.

Right, at least for filling a dictionnary.  We would need to declare
multi-valued parameters as such, which basically means we must only
try to read those config items we know about, which has all sorts of
consequences for config.py :)

It would seem reasonable to start without a cache dictionnary, at
least for now.  After all, there are not so many config items to know
about in a single stgit run, so IIMHO we're only going to notice a
difference for the time needed to run the testsuite.

Best regards,
-- 
Yann.

Re: StGIT and repo-config

[Catalin Marinas]

On 26/01/07, Yann Dirson <ydirson@altern.org> wrote:
> On Fri, Jan 26, 2007 at 12:58:38AM +0100, Johannes Schindelin wrote:
> > On Thu, 25 Jan 2007, Catalin Marinas wrote:
> > > The dictionary should also be pre-populated with the default values
> > > (only in memory, not going to the config file on disk).
>
> Pre-filling with default value has a drawback: it will create lots of
> useless entries, especially for per-branch settings.  I'd rather let
> the accessors return the default value when needed.  We can still
> group all defaults in a single dictionnary.

What I meant is that it should pre-fill only with the default entries
needed by StGIT (probably around 10, see stgit.config.config_setup()).
The rest should be filled in when reading the config files.

> > The most important point (to me) which came out of the discussion: It is
> > not at all easy, or straight-forward, to handle multi-vars, i.e. multiple
> > values for the same key.
>
> Right, at least for filling a dictionnary.  We would need to declare
> multi-valued parameters as such, which basically means we must only
> try to read those config items we know about, which has all sorts of
> consequences for config.py :)

But are multi-vars in the same file currently used? There is a clear
situation when a key is written in both the global and the
per-repository config files. From the StGIT point of view, it should
only care about the most specific one, i.e. the one in the per-repo
config and this would be the last one listed by git-repo-config (and
therefore overriding any preceding dictionary entry).

> It would seem reasonable to start without a cache dictionnary, at
> least for now.  After all, there are not so many config items to know
> about in a single stgit run, so IIMHO we're only going to notice a
> difference for the time needed to run the testsuite.

I'd like to avoid calling git-repo-config for every config option,
even if this wouldn't be noticeable (initially, but the tool would
evolve). If we don't care about multi-vars, as I said above, a
dictionary cache would make sense.

-- 
Catalin

Re: StGIT and repo-config

[Johannes Schindelin]

Hi,

On Fri, 26 Jan 2007, Yann Dirson wrote:

> On Fri, Jan 26, 2007 at 12:58:38AM +0100, Johannes Schindelin wrote:
>
> > I then proposed to have a simple --dump option to repo-config, which 
> > outputs NUL separated key/value pairs (substituting "true" for keys 
> > without -- not with empty! -- values). But somehow the discussion petered 
> > out before anything came out of it.
> 
> Even if that was to be done in git, it would surely be post-1.5, so we
> need another way to do things in the meantime.

I don't think it is too late. If it is simple enough, and stuck behind a 
command line option (not affecting the rest of repo-config), it is safe 
enough to include.

> > The most important point (to me) which came out of the discussion: It is 
> > not at all easy, or straight-forward, to handle multi-vars, i.e. multiple 
> > values for the same key.
> 
> Right, at least for filling a dictionnary.  We would need to declare
> multi-valued parameters as such, which basically means we must only
> try to read those config items we know about, which has all sorts of
> consequences for config.py :)

Well, we have only two multi-valued keys at the moment (AFAIK): 
remote.<branch>.fetch and remove.<branch>.push.

Even if there are more, they are hardly interesting to StGIT, right? So 
why not just pretend that they don't exist, by making a dictionary, 
putting each new key/value pair into it one by one? (Forgetting all but 
the last value for multi-values...)

> It would seem reasonable to start without a cache dictionnary, at least 
> for now.  After all, there are not so many config items to know about in 
> a single stgit run, so IIMHO we're only going to notice a difference for 
> the time needed to run the testsuite.

The point is: it is very easy and short to just stash everything into a 
dictionary, and retrieve it from there.

Ciao,
Dscho

Re: StGIT and repo-config

[Yann Dirson]

On Sat, Jan 27, 2007 at 12:12:36AM +0100, Johannes Schindelin wrote:
> > Even if that was to be done in git, it would surely be post-1.5, so we
> > need another way to do things in the meantime.
> 
> I don't think it is too late. If it is simple enough, and stuck behind a 
> command line option (not affecting the rest of repo-config), it is safe 
> enough to include.

That would be great.

> > > The most important point (to me) which came out of the discussion: It is 
> > > not at all easy, or straight-forward, to handle multi-vars, i.e. multiple 
> > > values for the same key.
> > 
> > Right, at least for filling a dictionnary.  We would need to declare
> > multi-valued parameters as such, which basically means we must only
> > try to read those config items we know about, which has all sorts of
> > consequences for config.py :)
> 
> Well, we have only two multi-valued keys at the moment (AFAIK): 
> remote.<branch>.fetch and remove.<branch>.push.
> 
> Even if there are more, they are hardly interesting to StGIT, right?

Not completely right.  It is precisely remote.<branch>.fetch I'm
needing, to be able to locate which remote a parent branch belongs to
when creating a stack.  It may be that the problem is git-remote being
too limited, and if we implement the functionnality at that level,
then maybe we can ignore those config settings.  But maybe not, since
I'd like stack creation to record a remote.<stack>.fetch entry.

> So why not just pretend that they don't exist, by making a
> dictionary, putting each new key/value pair into it one by one? 
> (Forgetting all but the last value for multi-values...)

IMHO, that'd be the easiest way to shoot ourselves in the foot :)

> > It would seem reasonable to start without a cache dictionnary, at least 
> > for now.  After all, there are not so many config items to know about in 
> > a single stgit run, so IIMHO we're only going to notice a difference for 
> > the time needed to run the testsuite.
> 
> The point is: it is very easy and short to just stash everything into a 
> dictionary, and retrieve it from there.

OTOH, it is equally easy (if not easier) to call "git repo-config
--get" or --get-all when needed :)

Best regards,
-- 
Yann.

Re: StGIT and repo-config

[Catalin Marinas]

On 27/01/07, Yann Dirson <ydirson@altern.org> wrote:
> On Sat, Jan 27, 2007 at 12:12:36AM +0100, Johannes Schindelin wrote:
> > Well, we have only two multi-valued keys at the moment (AFAIK):
> > remote.<branch>.fetch and remove.<branch>.push.
> >
> > Even if there are more, they are hardly interesting to StGIT, right?
>
> Not completely right.  It is precisely remote.<branch>.fetch I'm
> needing, to be able to locate which remote a parent branch belongs to
> when creating a stack.

You could have a dictionary with list of values instead of just one
overriding the former. The config object can have 2 methods -
get_option(), returning config_dict[option][-1] (i.e. always the last,
most specific option) and get_moption() returning the whole list. The
code requiring the options should know which method to call for either
getting a single value or multiple values.

> > The point is: it is very easy and short to just stash everything into a
> > dictionary, and retrieve it from there.
>
> OTOH, it is equally easy (if not easier) to call "git repo-config
> --get" or --get-all when needed :)

It is possible that we'll end up calling it too many times when
working on huge series (a very good benchmark is the -mm kernel
series, it really shows performance bottlenecks, mainly caused by GIT
operations for writing the index).

You could also cache the values only when asked for an option the
first time but given the relatively small size the config files, there
isn't any performance impact in caching all the options at once.

-- 
Catalin