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