[pacman-dev] [PATCH 1/2] skip unneeded parsing

[jjacky]

On 01/09/12 19:40, Dan McGee wrote:
> On Mon, Jan 9, 2012 at 12:09 PM, jjacky <i.am.jack.mail at gmail.com> wrote:
>> parsing config file is a two-steps process, one for
>> "options" and one for the repos. in each of those,
>> there's no need to parse the other section(s), so we'll
>> skip it. (most notably, when parsing for "options" it
>> would read all included files for repos, while just
>> ignoring everything in them)
>>
>> Signed-off-by: Olivier Brunel <i.am.jack.mail at gmail.com>
> 
> I think this will fail hard in the following scenario:
> 
> pacman.conf follows:
> -----
> Include /etc/pacman.d/shared-options.conf
> 
> [repo]
> ....
> -----
> 
> Include directives can occur *anywhere*, not just inside sections.

Sorry, I'm back on this (because I work on something that needs to parse
pacman.conf)

First off, actually your example was invalid, as there cannot be
directives outside of any section, pacman would even throw a syntax
error ("All directives must belong to a section").

What is true, however, is that inside an included file new sections can
be opened. So as I said last time, if a file included in "options" was
to define some repos, my patch would fail (since any & all Include
directive in repos would be ignored during options pass, and vice versa).

However, I feel like I should ask: is it really the expected behavior?
That pacman allows/supports sections being defined inside included files?

Because this could lead to trouble; for instance, w/ this:

-- pacman.conf -------
[options]
Include = /etc/pacman.d/shared.conf
DBPath = /opt/pacman/
----------------------

-- shared.conf -------
[core]
Server = http//foo.bar/
----------------------

Then by the time pacman gets to the DBPath directive, it thinks the
current section is "core" and therefore will *not* apply that directive.
That doesn't seem very right to me?

In fact, this would result in a warning: "directive 'DBPath' in section
'core' not recognized."

One could also imagine that servers get added to the wrong repo with the
same kind of situation. Only then, there would be no warnings!

Is this really the expected behavior?


A fix for this could be to use a new struct section_t when parsing an
included file, so as to not "mess up" the current one. (And my patch
would still be invalid, for the same reason it is now.)

Or, to ignore sections in included file, and going with the idea that
all directives in an included file belong to the section where the
Include directive was in.
(In which case I think my patch could work, since e.g. during the
options pass there would be no need to process Include directives in
repos, and vice versa.)

I also feel the later might make things clearer, but I'm sure there
might be usage cases I'm not thinking of, where it might be good to
define sections inside included files?

Any ideas/thoughts on this?
-jjacky


>> ---
>>  src/pacman/conf.c |    6 ++++++
>>  1 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/pacman/conf.c b/src/pacman/conf.c
>> index 7ba2791..09749ea 100644
>> --- a/src/pacman/conf.c
>> +++ b/src/pacman/conf.c
>> @@ -755,6 +755,12 @@ static int _parseconfig(const char *file, struct
>> section_t *section,
>>                        continue;
>>                }
>>
>> +               /* skip unnecessary parsing */
>> +               if ( (parse_options && !section->is_options)
>> +                               || (!parse_options && section->is_options) )
>> {
>> +                       continue;
>> +               }
>> +
>>                /* directive */
>>                /* strsep modifies the 'line' string: 'key \0 value' */
>>                key = line;
>> --
>> 1.7.8.3
>>
>