[RFCv2 00/46] RFC: Generate parsexml/formatbuf functions based on directives

[Daniel P. Berrangé]

On Mon, Dec 07, 2020 at 05:11:42PM +0800, Shi Lei wrote:
> On 2020-12-05 at 02:55, DanielP. Berrangé wrote:
> >On Fri, Sep 04, 2020 at 11:34:52AM +0800, Shi Lei wrote:
> >> V1 here: [https://www.redhat.com/archives/libvir-list/2020-June/msg00357.html]
> >>
> >> Differ from V1:
> >>
> >>   * Move the generator into scripts/xmlgen and rename it 'xmlgen'.
> >>
> >>   * Declare virXMLChildNode and virXMLChildNodeSet in libvirt_private.syms.
> >>
> >>   * Replace VIR_FREE with g_free and VIR_ALLOC[_N] with g_new0.
> >>
> >>   * Adjust virReportError to avoid unnecessary translation.
> >>
> >>   * Remove the macro VIR_USED and use G_GNUC_UNUSED to declare arguments.
> >>
> >>   * When parsing string member, assign value to it directly instead of
> >>     using middle variable.
> >>
> >>   * Don't set libclang_path. Just use python-clang's default setting.
> >>
> >>   * Use virEscapeString for escaping xml characters.
> >>
> >>   * Enable directive 'genformat' with a parameter to support separation mode.
> >>
> >>   * Add directive 'xmlswitch' and 'xmlgroup' to support discriminated unions.
> >>
> >>   * Allow directive 'array' and 'specified' to carry with a parameter,
> >>     which specifies its counterpart explicitly.
> >>
> >>   * Enable directive 'xmlattr' with path.
> >>
> >>   * Add directive 'formatflag' and 'formathook'.
> >
> >I'm very sorry for the extremely long delay in reviewing this new
> >series.
> >
> >Overall I like this series and would like to see how we can focus on
> >getting at least part of it merged.  It is interesting to see how it
> >is forcing a separation of the parsing and validation of data. I
> >also like that it is making the code more consistent in style.
> >
> >The main blocker I think is that we need to temporarily commit the
> >generated files to git, and not run the generator during normal
> >builds, until we ditch RHEL-7 support in April 2021.  This should
> >be quite easy to deal with.
> >
> >The next thing is that wierd need to insert a "_NONE" field in to
> >all the enums. I'm not understanding what reqiures this, but I think
> >we need to come up with a different solution to whatever the problem
> >is.  It looks like this only affects the DomainGraphics conversion
> >in your series. So we could start by only merging the Network conversion
> >patches, until we figure out a solution to avoid adding _NONE fields.
> >
> >That's it for the main blockers to merge from my POV.
> >
> >After that I think a priority is to get some test coverage of the
> >generator script, and to add the docs about the how the generator
> >annotations work. This can be done after the initial merge.
> >
> >Then there's a large ongoing work to actually convert everything,
> >but there's no rush to finish that, as you've shown that we can
> >do it incrementally.
> >
> >So if you want to re-post an update of the series with the few
> >blocking items addressed, then personally I'd look at trying to
> >merge that unless other people have objections they want to
> >raise that can't be solved after merging the initial support.
> >
> >Regards,
> >Daniel
> >--
> 
> Thanks for reviewing this series.
> 
> For the first blocker, I think we can wait until April 2021, since we have spent a lot time on it.
> 
> According to your comment, I think I can make some preparations for the initial merge during
> these 4~5 months.
> 
> (1) Implement some testcases for the generator to make sure its proper functions in the future.
> 
> (2) Add some docs to explain and demonstrate this generator. My English is not well, so I will
> try my best to do it and I may need a bit long time.
> 
> (3) For the 'default' item of enums, I think we can add an extra directive to direct the generator
> how to generate code for enum, as I mentioned in the last email. Or we can continue to talk about
> other solutions.
> 
> (4) Next series will only include conversion about network, so that we can focus on the docs,
> testcases and this generator itself.
> 
> I plan to finish these jobs and post the next series in the next month, which will be still a RFC.
> So you and other people can have sufficent time to review and comment it and I will have
> sufficent time to improve it :-)

Ok, if you're happy to take the time until April, that's fine with me too.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|