Hi Clay,
On 06/14/10 07:12 PM, [email protected] wrote:
[...]
Hi Jan,
My responses in-line. Thank you for the response!
-Clay
On Mon, 14 Jun 2010, Jan Damborsky wrote:
Hi Clay,
On 06/10/10 11:06 PM, [email protected] wrote:
Hi Jan,
I'm concerned about sc_conv.ksh, it would be good to add a
comment what it's doing exactly and is this expected to be run on
the AI client or AI server, at least for anyone stumbling across it.
I agree - I have added comment section to the beginning of the script -
could you please take a look at that comment and let me know if
it might appropriate clarify the purpose of this script ?
The updated webrev is available at
http://cr.opensolaris.org/~dambi/bug-15723-cr/
Delta webrev
http://cr.opensolaris.org/~dambi/bug-15723-cr-delta/
Your comment helps one understand the script quite a bit! Thank you.
I'm concerned that sc_conv.ksh is not applicable for running on the
server due to the concerns cited below.
As far as utilizing XSLT for SC manifest conversion, while I can see
that XSLT is the solution for manipulating XML files in CUD,
I am not convinced that moving to XSLT in sc_conv.ksh is the right thing
to do at this point. sc_conv.ksh is tied to the existing infrastructure
which is going to be removed and replaced with the new one, thus I am
not sure it is worth to invest that effort. Why not to introduce
XSLT integrated with new CUD effort after it is fully exercised ?
The main reason to use XSLT is that it is XML compliant.
I would accept that argument if other links of chain followed
that. However, this is apparently not the case - see AI client SC parser
which basically processes SC manifest in similar way sc_conv.ksh does.
For example, sc_conv.ksh breaks on the test case I provide called
"sc.xml"[1] which publish-manifest(1) will happily accept from an admin.
I believe that the whole chain (AI-server -> AI-client) has to be taken
into account -
i.e. if particular SC manifest is not valid from AI client point of
view, why admin
would publish such manifest on AI server ?
If publish-manifest(1) accepts something which does not validate on AI
client, then
it seems like a deficiency, as I believe that admin could expect that if
something
validates on AI server, it validates on AI client as well.
If sc_conv.ksh is only to be used on the AI client than regular
expressions and expecting a certain formatting to the XML is okay (as
publish-manifest should somewhat reorder the XML on output[2]) but
otherwise XML can be quite free-form with regards to whitespace and
value ordering and it seems sc_conv.ksh breaks in these cases.
See above.
[1]: test-case which sc_conv.ksh fails on
http://cr.opensolaris.org/~clayb/SC_using_XSLT/test_cases/sc.xml
But this manifest does not validate on AI client either even after
processed by publish-manifest, right ?
[2]: another test-case which sc_conv.ksh fails on (duplicate type and
name attributes for the description element) which is post
publish-manifest(1):
http://cr.opensolaris.org/~clayb/SC_using_XSLT/test_cases/default_reorder.xml
Unfortunately, I can't access this one.
At this point, I believe I spent more time on this topic than I feel was
reasonable. I will
basically go with XSLT solution, partially because I do not want to
ignore the effort
you invested when putting that script together. However, after giving it
a try
I can see some work is still needed in that area - I have just noticed
following:
* converted manifest is not valid from publish-manifest point of view
(the source was AI default manifest)
# installadm add -m /tmp/default.xml -n x-15723-141
# /tmp/update_sc.py -i /tmp/default.xml -o /tmp/m.xml
# installadm add -m /tmp/m.xml -n x-15723-141
Error: SC Manifest AI failed validation:
Start tag expected, '<' not found
* tags missing in source manifest are present in converted manifest
For instance, I have noticed that for 'hostname'. It is not present in
current default AI manifest, but it is added to converted manifest
with value set to empty string.
Such manifest will not validate on AI client, since 'value' with
empty strings are not allowed:
...
<AI Jun 15 12:37:43> Parsing system configuration manifest
<AI_E Jun 15 12:37:43> Property 'hostname' in system configuration
manifest is set to empty string which is invalid value.
If you do not want to configure this property, please remove it from SC
manifest.
<AI Jun 15 12:37:43> Automated Installation failed in parser module
<AI Jun 15 12:37:43> Invalid System Configuration manifest provided
The correct behavior is that tags missing in source manifest
should not be present in converted manifest.
If we can sort those issues out in reasonable amount of time, I will
go with your solution.
Thank you,
Jan
Also, looking at XSLT version, it is unnecessarily complex given
its goal. sc_conv.ksh is mostly targeted to the admins who will use it
to convert existing SC manifests. sc_conv.ksh in current form is
easy to understand which I believe is the advantage in case
some minor modifications are desired.
I see sc_conv's regular expressions to be very painful to catch
possible errors in (certainly I've picked on a few but I might be
missing more), versus reading the XSLT expressions from convert.xslt.
XSLT is a nice formalized way for dealing with XML versus regular
expressions which are highly nasty to correctly handle XML and quite
complex.
I don't believe folks should need to modify the scripts we provide but
I believe the extra verbosity of an XSLT stylesheet allows for far
more ease of modification, in the unlikely event it is necessary.
I can see that this might be a good exercise to give XSLT a try,
but I would like to treat that separately, since XSLT is not something
we have appropriately exercised yet, thus might constitute the risk
I believe is not worth to undergo in this particular case.
If it seems plausible, do you think I could file separate bug for this ?
I believe xsltproc(1) which can use convert.xslt -- part of libxml2
has had far more users than our code and further the W3C has ratified
XSLT 1.0 which we are here using; again the W3C XSLT standard has very
likely had far more users than any code we've written for slim_source.
Lastly, empirical testing shows the XSLT conversion to do as expected.
To be honest, I don't understand your concern about polluting our
gate with another shell script given the fact that it will be removed
when no longer needed (as soon as AI/DC manifest rework is integrated).
I would like to see us work towards our goals at all times and not
just occasionally. Code live far longer than expected more often than
not (e.g. why is libspmi still in our gate? ;) )
Please let me know what you think.
My parting thoughts would be that we should go with the expected,
industry technology now and not later. That I believe, would be XSLT.
[snip...]
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
