On 9 March 2018 at 11:09, Daniel Stone <dan...@fooishbar.org> wrote:
> Hi Emil,
>
> On 9 March 2018 at 10:59, Emil Velikov <emil.l.veli...@gmail.com> wrote:
>> On 9 March 2018 at 09:47, Daniel Stone <dan...@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
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
