[Mesa-maintainers] [Mesa-dev] [PATCH 06/28] configure: rename --with-{egl-, }platforms

[Emil Velikov]

On 13 December 2016 at 16:35, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Tue, Dec 13, 2016 at 11:30 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> On 13 December 2016 at 16:19, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>> On Tue, Dec 13, 2016 at 11:14 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>>> On 13 December 2016 at 16:09, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>>>> On Tue, Dec 13, 2016 at 11:08 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>>>>> On 13 December 2016 at 15:25, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>>>>>> On Thu, Dec 8, 2016 at 2:21 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>>>>>>> From: Emil Velikov <emil.velikov at collabora.com>
>>>>>>>>
>>>>>>>> Since day 1, Vulkan has depended on --with-egl-platforms to select the
>>>>>>>> platforms build.
>>>>>>>>
>>>>>>>> With earlier commits, we've attributed for that internally by renaming
>>>>>>>> the [internal] conditionals in our build. At the same time having the
>>>>>>>> --enable-egl and --with-egl-platforms dependency for Vulkan makes no
>>>>>>>> sense.
>>>>>>>>
>>>>>>>> Since where' on the bridge we want to make the conditional generic, thus
>>>>>>>> others (such as the gallium VL targets) can honour the respective
>>>>>>>> platforms.
>>>>>>>>
>>>>>>>> Cc: mesa-maintainers at lists.freedesktop.org
>>>>>>>> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>>>>>>>> ---
>>>>>>>> Should we keep the old configure and error out when anyone provides a
>>>>>>>> non-default value ? This way they'll have direct feedback how to move
>>>>>>>> forward. Otherwise they'll likely miss the warning message.
>>>>>>>
>>>>>>> I can't tell what problem this is solving. OTOH this is creating the
>>>>>>> problem that everyone's configure lines will need fixing.
>>>>>> This need fixing (builder intervention) either way, I'm afraid. Question is
>>>>>>  a) should we expect people to read the release notes, or
>>>>>>  b) get their attention with an error message, providing an example
>>>>>> in order to do the right thing.
>>>>>
>>>>> (c) leave things alone if this isn't fixing anything other than aesthetics.
>>>> It's not aesthetics... atm if you want just Vulkan you still need
>>>> --enable-egl --with-egl-platforms=x11,wayland*.
>>>> Otherwise it won't have any winsys integration... Not sure if it'll
>>>> build/link properly even :-(
>>>
>>> So error the build and say you don't support that configuration if you
>>> enable vulkan and don't enable any winsys stuff. Or make it so that
>>> --enable-egl sets the default value of --with-egl-platforms to
>>> whatever. Don't break everyone's builds, bisects, etc.
>>>
>> Sounds like you've not looked at the lovely stuff we have in there. I
>> welcome you to do so.
>>
>> That aside:
>> Your suggestions contradict each other - on one hand you're saying
>> "error out" in a what is buggy yet valid usecase. On the other hand
>> you're against AC_MSG_ERROR since it will break X/Y usecase.
>
> I thought you said it didn't currently work. If it doesn't work, or is
> unsupportable, error out. If it currently works, and is supportable -
> keep it working.
>
If it works is by sheer magic. Personally I would stay away from
calling it "supported".
Furthermore close to nobody has any knowledge of the

>>
>> Also "everyone" is a mild overstatement, but I guess we all know that ;-)
>
> Perhaps I'm misunderstanding. This will break everyone that has
> --with-egl-platforms in their configure line, no? If not, disregard my
> comments.
>
> I imagine that nearly everyone that does development on mesa has that
> flag in their configure line. That means any bisect will require
> changing the configure line as you cross this commit on the bisect.
> This is very annoying. Sometimes it's required, but it shouldn't be
> done lightly.
>
Agreed we shouldn't do these lightly and as mentioned a few times
months ago "if we have a flag day we can fix the world" sadly we
cannot.

> So I'll ask again because it's still unclear to me - what *problem* is
> this fixing? I.e. what currently doesn't work that you are trying to
> make work?
>
Ok let me try again, starting with the mandatory backstory:
[Note: a fair bit of this is in the cover letter... still]

Currently there's no clear way to select which platform you want to
use for Vulkan - one "inherits" it from the EGL platform selection.
At the same time you do _not_ need EGL to have Vulkan. Thus as you
disable EGL (and therefore its platform selection) you'll end up with
Vulkan driver(s) without _any_ winsys support - not something that's
intuitive and/or you'd would want most of the time.

Side note: Vulkan requires DRI3 with X11, yet it's missing the check
or any of the dependency tracking/management. Thus it fails at build
or link time - depending on how lucky you get.
This has been fixed as standalone patch(es) which should be ok for stable.

Now combine that with the fact that we want a similar logic for the VL
targets (omx, vaapi and maybe vdpau/xvmc/other one day) you'll end up
with the following options
 a) provide a --with-foo-platforms=...
Here we'll end up with [even greather] combinatoric explosion,
hundreds of lines of build glue and nearly impossible to test/ensure
that mesa isn't constantly broken.
There is the part of "building EGL w/o wayland while Vulkan with
wayland" is the style of thing that we do _not_ support [for example
gallium drivers building/using graw/gallivm], nor something people
[normally] go for.

 b) or, have a single --with-platforms=
Giving us a uniform way to control the winsys/platforms. It is _very_
annoying about the rename, but it handles a lot more nuisances in a
brief and easy to understand manner.
I doubt many builders will be excited about reading and managing
another 3-4 new configure toggles...

A workaround which just make to mind:
 - keep --with-egl-platforms around and it's heuristics (which will
also be applied for --with-platforms).
 - keep the --with-egl-platforms decisions for EGL only, and let one
override it via --with-platforms. Warn as the latter happens.
 - warn for the deprecation of --with-egl-platforms

This gives us:
 - old (consistent) behaviour for EGL
 - definition of the current undefined behaviour for Vulkan
 - definition for VL targets and any future ones.
 - everybody's scripts keep working... on the premise that they'll see
the warning and update before things are removed (2...20 releases?)

How does this sound ?

Thanks
Emil
P.S. For the future please point out "what do you mean with 'some
quoted text'" rather than saying "what's the problem".