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

[Ilia Mirkin]

On Tue, Dec 13, 2016 at 12:33 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> 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

And really --with-platforms is --with-egl-platforms +
--with-vulkan-platforms (which I realize doesn't presently exist).

And the reason you don't want to add it is that you're afraid someone
will do something like

--with-egl-platforms=drm --with-vulkan-platforms=x11

and it's a pain to make it so that EGL only has drm and vulkan only
has x11? And eventually one might also want --with-vdpau-platforms,
--with-vaapi-platforms, --with-whoknowswhat-platforms, and you don't
want to keep adding them, since the chances that you *really* want
those to be different for legitimate reasons is ... low.

Is that right?

Thanks,

  -ilia