On Wed, 2008-08-27 at 12:31 +0100, Daniel P. Berrange wrote:
On Tue, Aug 26, 2008 at 09:40:41PM +0000, David Lutterkort wrote:
> On Tue, 2008-08-26 at 21:05 +0100, Daniel P. Berrange wrote:
>
> > + let empty = [ label "empty" . del /[ \t]*\n/ "" ]
>
> Do you really care that empty entries show up in the tree ? If you don't
> want to see them, you can remove the 'label' from the above, and
> possibly also the '[ .. ]'.
Ok, this gets fun. I don't particularly want the label or tree node.
I can remove the label OK, but that gives anonymous tree nods.
Thinking about this some more, anonymous (or labelled) nodes are
probably your best bet here, because:
If I remove the [...], then I end up with
test -x /usr/bin/augparse && /usr/bin/augparse -I . test_libvirtd.aug
./libvirtd.aug:63.3-.43:Failed to compile lns
./libvirtd.aug:63.13-.43:exception: ambiguous tree iteration
'ca_file/' can be split into
'|=|ca_file/'
and
'ca_file/|=|'
Iterated lens: ./libvirtd.aug:63.15-.39
test_libvirtd.aug:253.8-.20:Could not load module Libvirtd for Libvirtd.lns
test_libvirtd.aug:253.8-.20:Undefined variable Libvirtd.lns
test_libvirtd.aug: error: Loading failed
And I'm utterly stuck on what this means / how to fix it. I didn't
expect removing the square brackets to change the DFA.
The error message is admittedly obtuse (though I am not sure how to
improve it, suggestions welcome)
What Augeas is telling you (or trying to, atleast) is that when it goes
to write a tree back into the file (that's why it mentions 'tree
iteration') it has a choice, and doesn't know what to do in the lens on
line 63, columns 3 - 43, which is the toplevel 'lns' lens:
let lns = ( record | comment | empty ) *
The '|=|' mark in the error message shows where Augeas thinks it could
split a tree with a single node 'ca_file': either into (no node,
cafile/) or (cafile/, no node) .. what makes this error message doubly
heinous is that there's no good notation for 'no node' - it's the empty
string in the error message.
The issue is that when it sees a tree with a single node 'ca_file' it
doesn't know whether to process that as a single 'record' or as a
'record . empty' or 'empty . record', since all of these match the tree
with a single 'ca_file' entry, simply because the 'empty' lens now
leaves no traces in the tree that it had been used.
When you define 'empty' as '[ eol ]', Augeas knows how to process that
tree since every time 'empty' should be used, there must be a node in
the tree with a NULL label, so a single tree node 'ca_file' can only be
processed by using 'record', and not 'record . empty' or similar.
As I said, the simplest way around this is to define 'empty' as
'[ eol ]' - you could also try to change things so that empty lines are
pulled into record or comment entries, but that's probably more trouble
than it's worth.
> I've been thinking that we need to have some function that
reads a file
> and returns a string so we don't have to clutter tests with these long
> strings. But for now, that's what it is :(
Yes, that would be very nice - and/or supporting use of <<HERE documents
for strings, so I don't have to escape " throughout the embedded config
file
One more note on this: to get started, pulling in a whole config file
and checking that that gets processed in a reasonable way is deifnitely
the best thing to do. Over time, as you make changes to the file format
and/or fix bugs in the lens, you're probably better off writing small
tests that check for one particular thing (the test_shellvars.aug in
Augeas hg is a good example for this)
I put a ticket into Trac for this[1]
One other useful improvement would be for the test suite to only
print
out sections of the tree which differ. In debugging this it printed
out a 400 line 'Expected' tree structure, and then a 400 line 'Actual'
tree structure and wanted me to play "Where's Waldo" to find the typo.
It would be good to trim the leading and trailing nodes which are the
same
Yeah, I've run into this before, too. Yet another ticket[2]
> All in all, very nice, and I am really glad that upstream is
shipping a
> lens :)
Just discovered one minor issue. The Fedora RPM guidelines require that
an RPM either own a directory, or require an RPM that owns it, and two
RPMs aren't allowed to own the same directory. I don't want/need to
depend on Augeas directly, but equally I need someone to own the
/usr/share/augeas/lens directory. Wonder if that directory should be put
into the 'filesystem' RPM ?
Yeah, that's the only feasible solution. I'll need to figure out what
the process for that is.
David
[1]
http://fedorahosted.org/augeas/ticket/10
[2]
http://fedorahosted.org/augeas/ticket/11