[PATCH wayland v2 1/4] Revert "wayland-egl-symbols-check: pass the DSO name via the build system"

[Emil Velikov]

On 9 March 2018 at 11:09, Daniel Stone <daniel at fooishbar.org> wrote:
> Hi Emil,
>
> On 9 March 2018 at 10:59, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> On 9 March 2018 at 09:47, Daniel Stone <daniel at fooishbar.org> wrote:
>>> Patches 2-4 look fine and I'm happy to merge them with my review, but
>>> could you please explain some more about this patch? I very much like
>>> keeping details of the build system (specifically its internal build
>>> paths) in the build system itself and not in the script. I was
>>> assuming something in 2-4 needed this revert to be applied, but
>>> couldn't see anything. Is there something I'm missing?
>>
>> There is one word to describe it - compromise:
>>
>>  - above all, the internal path is a 'dummy' fallback. anyone can
>> provide the binary name as an argument
>> $ .../wayland-egl-symbols-check .../libwayland-egl.so
>>  - since we have a fallback - a plain .../wayland-egl-symbols-check
>> will work most of the time
>
> That makes sense, running it from the build root. Is that just because
> 'make check' is slow, or? (sanity-test is really slow.)
>
Short back story: I was playing with OBS and getting the build
artefacts (the contents of a failing test) was a pain.
Admittedly, there may be another way to handle that, although in general:

Passing the file as argument makes debugging a lot quicker/easier.

>>  - handling env. variables (as opposed to arguments) is a pain with meson
>
> Hm, not really. You just add an 'env' argument when declaring the test, e.g.:
>     egl_sym_check = find_program('wayland-egl-symbols-check')
>     test_egl_syms = test('egl-symbols', egl_sym_check, env: [
> 'WAYLAND_EGL_LIB=@0@'.format(lib_wayland_egl) ])
>
Once you add NM and potentially others, it does get tiny bit messier.

>>  - handling arguments (as opposed to env. variable) is a pain with
>> current testing scheme
>
> Yeah, that doesn't work.
>
>>  - the original code is shorter
>>
>> Hope you find at least some of those reasonable.
>
> It's fair enough. I'm just trying to find out the balance of these:
> for instance, if it's no problem to add environment variables with
> Meson, do you still want to push it for reason #1, or?
>
Yes, #1 is perhaps the biggest reason behind the patch.

Thanks
Emil