[Mesa-dev] [PATCH] gallium/targets: don't leave an empty target directory(ies)

[Emil Velikov]

On 2 March 2017 at 19:02, Matt Turner <mattst88 at gmail.com> wrote:
> I was just going through old bugs and found this. I asked you to look
> into this years ago, and then never did my part. Sorry for that.
>
Bth, I forgot about it as well :-\

> On Wed, Sep 2, 2015 at 1:34 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> Some drivers do not support certain targets - for example nouveau
>> doesn't do VAAPI, while freedreno doesn't do of the video backends.
>>
>> As such if we enter vdpau when building freedreno/ilo/etc, a vdpau/
>> folder will be created, empty library will be build and almost
>> immediately removed. Thus keeping an empty vdpau/ folder around.
>>
>> There are two ways to fix this.
>>
>>  * add substantial tracking in configure/makefiles so that we never end
>> up in targets/vdpau
>>  Downsides:
>> Error prone, as the configure checks and the 'include
>> gallium/drivers/foo/Automake.inc' can easily get out of sync.
>
> Yeah, seems difficult.
>
>>  * remove the folder, if empty, alongside the empty library.
>>  Downsides:
>> In the latter case vdpau/ might be empty before the mesa build has
>> started, yet we'll remove it either way.
>>
>> This patch implements the latter option, as the downside isn't that
>> significant, plus the patch is way shorter ;-)
>
> Definitely seems like the way to go.
>
>> Cc: Matt Turner <mattst88 at gmail.com>
>> Gentoo Bugzilla: https://bugs.gentoo.org/545230
>> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
>> ---
>>
>> Hi Matt, can you let me know if you're happy with the downside this
>> approach brings ? The alternative would involve part-reverting one or
>> two of your ealier patches.
>>
>> -Emil
>>
>>  src/gallium/targets/dri/Makefile.am   | 3 ++-
>>  src/gallium/targets/vdpau/Makefile.am | 3 ++-
>>  src/gallium/targets/xvmc/Makefile.am  | 3 ++-
>>  3 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/gallium/targets/dri/Makefile.am b/src/gallium/targets/dri/Makefile.am
>> index 7f945d1..77ea8c8 100644
>> --- a/src/gallium/targets/dri/Makefile.am
>> +++ b/src/gallium/targets/dri/Makefile.am
>> @@ -130,7 +130,8 @@ install-data-hook:
>>                 ln -f $(DESTDIR)$(dridir)/gallium_dri.so                \
>>                       $(DESTDIR)$(dridir)/$${i}_dri.so;                 \
>>         done;                                                           \
>> -       $(RM) $(DESTDIR)$(dridir)/gallium_dri.*
>> +       $(RM) $(DESTDIR)$(dridir)/gallium_dri.*;                        \
>> +       test x$(TARGET_DRIVERS) != x || $(RM) -d $(DESTDIR)$(dridir) &>/dev/null || true
>
> I think you just want test -n $(TARGET_DRIVERS)?
>
> I'm not sure if the || true is necessary, but doesn't matter either way.
>
Yes -n should be fine, I'll need to double-check on the "|| true" part.

> Thanks,
>
> Reviewed-by: Matt Turner <mattst88 at gmail.com>

Thanks
Emil