[Mesa-dev] [PATCH 3/5] Android: kill off {MESA_}ANDROID_VERSION defines

[Tapani Pälli]

On 03/20/2017 07:33 PM, Emil Velikov wrote:
> On 20 March 2017 at 17:22, Rob Herring <robh at kernel.org> wrote:
>> On Mon, Mar 20, 2017 at 10:51 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>> Nit:
>>>
>>> Subject: Android: kill off {MESA_}ANDROID_VERSION defines aka Android
>>> 4.1 and older
>>>
>>> On 20 March 2017 at 01:33, Rob Herring <robh at kernel.org> wrote:
>>>> The Android version defines are only needed for versions less than 4.2
>>>> which aren't really supported or tested.
>>>>
>>>> Signed-off-by: Rob Herring <robh at kernel.org>
>>>> ---
>>>>  Android.common.mk                       |  2 --
>>>>  Android.mk                              |  1 -
>>>>  src/egl/Android.mk                      |  5 +----
>>>>  src/egl/drivers/dri2/egl_dri2.h         |  9 +--------
>>>>  src/egl/drivers/dri2/platform_android.c | 15 ---------------
>>>>  5 files changed, 2 insertions(+), 30 deletions(-)
>>>>
>>>> diff --git a/Android.common.mk b/Android.common.mk
>>>> index 89b1061eb100..7431b1d3dc5b 100644
>>>> --- a/Android.common.mk
>>>> +++ b/Android.common.mk
>>>> @@ -30,7 +30,6 @@ LOCAL_C_INCLUDES += \
>>>>         $(MESA_TOP)/include
>>>>
>>>>  MESA_VERSION := $(shell cat $(MESA_TOP)/VERSION)
>>>> -# define ANDROID_VERSION (e.g., 4.0.x => 0x0400)
>>>>  LOCAL_CFLAGS += \
>>>>         -Wno-unused-parameter \
>>>>         -Wno-date-time \
>>>> @@ -40,7 +39,6 @@ LOCAL_CFLAGS += \
>>>>         -Wno-mismatched-tags \
>>>>         -DPACKAGE_VERSION=\"$(MESA_VERSION)\" \
>>>>         -DPACKAGE_BUGREPORT=\"https://bugs.freedesktop.org/enter_bug.cgi?product=Mesa\" \
>>> Drop the trailing \
>>
>> That's not a line I'm changing though.
>>
>>>> --- a/src/egl/Android.mk
>>>> +++ b/src/egl/Android.mk
>>>> @@ -55,10 +55,7 @@ LOCAL_SHARED_LIBRARIES := \
>>>>         libhardware \
>>>>         liblog \
>>>>         libcutils \
>>>> -
>>>> -ifeq ($(shell echo "$(MESA_ANDROID_VERSION) >= 4.2" | bc),1)
>>>> -LOCAL_SHARED_LIBRARIES += libsync
>>>> -endif
>>>> +       libsync \
>>> Ditto
>>
>> Then you have to change 2 lines whenever a new item is added. I'm just
>> be consistent with what is above.
>>
>> I prefer the way it is, but not enough to argue about. Let's decide
>> "the one true way" and I'll change the Android.mk files tree wide to
>> be consistent.
>>
> It's a bug that we had these trailing \ in the first place. If you'd
> want to go tree-wide and fix them, please do - that will be amazing !

Probably not a bug but just inconsistent usage. I know people that 
prefer to use this style since then it's 'cleaner'. When adding new 
entries from git perspective you are not touching other lines and it's 
easier to hunt down which change added some particular entry. 
Conceptually this is same thing as leaving a ',' in last slot of enum 
declaration just in case something will be added later (we have these in 
code).

> Since you were touching these lines one might as well fix these
> instances. Keeping it separate is fine if you prefer that.
>
> Thanks
> Emil
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>