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