[Mesa-dev] [RFC][PATCH 3/5] CleanSpec.mk: Remove HOST_OUT_release

[Emil Velikov]

On 25 July 2018 at 21:20, John Stultz <john.stultz at linaro.org> wrote:
> On Wed, Jul 25, 2018 at 1:06 PM, Dan Willemsen <dwillemsen at google.com> wrote:
>> On Wed, Jul 25, 2018 at 12:35 PM John Stultz <john.stultz at linaro.org> wrote:
>>>
>>> On Wed, Jul 25, 2018 at 5:36 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>> > On 25 July 2018 at 00:21, John Stultz <john.stultz at linaro.org> wrote:
>>> >> From: Dan Willemsen <dwillemsen at google.com>
>>> >>
>>> >> This is a forward port of a patch from the AOSP/master tree:
>>> >> https://android.googlesource.com/platform/external/mesa3d/+/bd633f11de0c6ac1ed333a28344c74fd9898df9e%5E%21/
>>> >>
>>> >> Which replaces HOST_OUT_release with HOST_OUT
>>> >>
>>> > What's wrong with HOST_OUT_release? If it is something that got
>>> > deprecated, we could:
>>> >  a) add a comment above it (deprecated since Android ...)
>>> >  b) add the HOST_OUT alongside the existing HOST_OUT_release
>>>
>>> Seems to be the case since:
>>> https://android.googlesource.com/platform/build/+/d6ed368fde0609742540b5da6d2e8a2a19b2c0eb%5E%21/
>>>
>>> Dan/Alistair: Any objections to option b?
>>
>> You can't add side-by-side, since new lines must be added at the end of the
>> file (the makefiles keep track of which lines were run by index, not their
>> contents). But when that variable isn't set, it would try to remove
>> "/*/EXECUTABLES/...
>> You could do something like:
>>
>> ifdef HOST_OUT_release
>> ...
>> else
>> ...
>> endif
>>
>> But it was already incorrect to use $(HOST_OUT_release) here, $(HOST_OUT) will
>> be set properly for whether the current build that's being cleaned during
>> incrementals is using host debug or release builds (it was incredibly uncommon
>> to use a debug host build, there was never a shortcut to use that, you had to
>> set an environment variable manually, and it was rarely if ever tested).
>
> Emil: Is this sufficient justification to drop HOST_OUT_release then?
>
Seems reasonable. With the explanation split into proper sentences and
included to the commit message, the patch is
Reviewed-by: Emil Velikov <emil.velikov at collabora.com>

-Emil