Plan for libweston backend configuration?

[Bryce Harrington]

On Sat, Feb 27, 2016 at 01:56:53AM +0100, Benoit Gschwind wrote:
> Hello Bryce,
> 
> Thanks for the summarization, I have few comment that I share here.
> 
> Le 26/02/2016 22:37, Bryce Harrington a écrit :
> > To followup Pekka's recent libweston thread, here's the next actions it
> > looks like we should take?
> > 
> >    a.  Revert 5ffbfffa
> >    b.  Land https://patchwork.freedesktop.org/patch/67547/, which covers
> >        the drm-backend.  (Is this patch proposal good as is, or would it
> >        benefit from any additional review?)
> >    c.  Defer the two alternative options for now
> >  	   https://patchwork.freedesktop.org/patch/73206/
> >        https://patchwork.freedesktop.org/patch/[73035,73036,73037,73038,73039]
> >    d.  Review/update wayland-backend and x11-backend to comply
> >    	   https://patchwork.freedesktop.org/patch/74553/
> > 	   https://patchwork.freedesktop.org/patch/74504/
> > 
> > This establishes Giulio's "Well Defined Structs" approach for
> > configuring libweston backends.  This uses versioned structs for
> > communicating parameters with the backends.
> > 
> > If no one raises an objection to this plan, I can tackle (a), (b) and
> > (c) myself directly.  For (d), offhand it appears they at least need to
> > add the structure versioning support, but might be suitable to consider
> > landing after that?
> 
> (a), (b), (c) and (d) are fine for me.

> I would add an action to sort weston API function (i.e. function that
> are exposed to user(developer)) from functions that must be kept
> hidden. For this action I think we should start a libweston.h (or what
> ever name that look good) and the related libweston.a . Currently
> implementing a compositor from weston look durty, some low level
> actions require to use libwayland directly and "link" them to
> weston. Another issue is that user(developer) can use any weston
> internal API and some of those functions may be unsafe.

It sounds like this would be suitable to do as a followup patchset?  I'm
sure it'll deserve some additional review and discussion.

For the header file naming, in my /usr/include, the "libfoo.h" naming
style is less often used than something like "libfoo/foo.h".  For
example, 'libdrm/drm.h'.  Plain old foo.h seems to be the most common,
although there's probably value in disambiguating libweston from weston.

In addition to the header file, api docs would be nice too.

> I do not know well where the border are but we should start to draft one
> in the same manner we started with backends.
> 
> 
> <side topic>
> I waiting for your review for (d) and I expect to provide patch for
> others back-end soon (i.e. maybe once per week until all testable
> back-end are done).

Will take a look today or tomorrow.
 
> Also note that I didn't added versionning header for two main reasons:
>  1. I do not know which best versionning method I should use, is one
> incremental number enough ?
>  2. I do not know how the user(developer) is expected to fill it properly.
> 
> Once I get answer to this both questions I will be happy to updates
> previous patch :)

Is pq's feedback sufficient for progressing on this now?

> </side topic>
> 
> 
> 
> > 
> > 
> > ---
> > A couple of more sophisticated solutions were proposed and evaluated,
> > but it feels like our requirements here are modest, and so simpler is
> > probably better.  Things aren't cast in stone here and we can always
> > move to something more advanced later if conditions warrant it.
> > 
> > In mind of this, here are the assumptions that are leading us to this
> > choice.  In the future if these assumptions fail to hold adequately,
> > then this design should be re-evaluated.
> > 
> > 1.  libweston has no stability promises, and won't for a long time, and
> >     it is and will remain parallel installable.  We are freely able to
> >     redo tomorrow any bad decisions we make today.
> > 
> > 2.  New options will be appended to the the struct, which will avoid ABI
> >     breaks.
> > 
> > 3.  The struct is versioned, so if we do need to break ABI for some
> >     reason, we can and backends can thereby check and verify their
> >     version.
> > 
> > 4.  Configuration needs by the backends are on the tame side, mostly
> >     just involving basic data types (strings, ints, etc.)
> 
> This assumption (4) is already wrong, back-end at the moment needs what
> I consider complex setup, in particular to define outputs list or the
> output_setup callback in drm-backend.

I figured this would be the first violated.  Lists of basic types
(e.g. list of strings) is probably not that big of a deal.  Or are these
output lists containers of output structs?

Anyway, we can probably hand wave one or two special cases here without
too much worry.

> > 5.  Backend configuration is internal to the backend.  End users won't
> >     be exposed to it, and only backend developers will need to tinker
> >     with these things.
> > 
> > 6.  Additions of or changes to configuration parameter definitions are
> >     going to be done by developers who are either part of the Wayland
> >     development community, or will be sending all of their changes to
> >     the community.  
> > 
> > 7.  All libweston backends will be living in the weston repository.  We
> >     do not provide support for third-party backends.
> > 
> > It's probably non-fatal if we periodically violate one or two of these
> > assumptions, but if we get beyond that it should be a cue to revisit our
> > approach.
> > 
> > 
> > So far, two other alternatives were considered; here are the key design
> > differences:
> > 
> > A.  Opaque Structs
> > 	https://patchwork.freedesktop.org/patch/73206/
> > 
> > 	This still uses structs for sharing configuration parameters but the
> > 	structs are hidden as internal details.  The backend instead uses
> > 	function calls to fill in parameters.
> > 
> > 	While this does decouple things a bit and avoid ABI breakge on
> > 	structure definitions, this requires coding and maintaining an array
> > 	of backend-specific functions, which bring their own API breakage
> > 	potentials.
> > 
> > 	This is essentially an incremental enhancement of the "Well Defined
> > 	Structs" scheme, so would be straightforward to upgrade to later.
> > 
> > 
> > B.  Getter/Setters
> > 	https://patchwork.freedesktop.org/patch/[73035,73036,73037,73038,73039]
> > 
> > 	Establishes a key/value backend config API, with separate
> > 	getter/setter calls for different config item data types.  Backends
> > 	then call these getters to retrieve configuration variables.
> > 	This system includes provision for sections and default values.
> > 
> > 	This is certainly a much more flexible system, but many of the same
> > 	problems will exist when configuration parameters change.  Just that
> > 	instead of erroring at the API/ABI layer it breaks at the
> > 	parser/configuration layer.  Shifting to this lower level means
> > 	errors may get detected later on, or may be missed entirely, where a
> > 	broken struct will be more highly visible.
> > 
> > Alternative A might be worth considering if we start seeing
> > proliferation of backend options, or if the configuration settings start
> > going beyond basic types.
> > 
> > Alternative B could become more convenient if we need to expose options
> > to end-users or if we start seeing third-party backends.
> > 
> > Bryce
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> > 
> 
> I agree with all uncommented parts. I have also comments about the last
> part about alternatives, but I can't find a good way and strong
> arguments to expose them. Thus, I will agree with what is written and do
> not write down my immature comments; but, maybe, when they will be more
> mature, I will share them. Please don't assume that those immature
> comments are pro-A comments.

Thanks,
Bryce

> Best regards
> 
> --
> Benoit Gschwind