Re: [Mesa-dev] [PATCH] android: amd/common: fix sid_tables.h generation rules

2017-08-11 Thread Mauro Rossi
2017-08-11 20:06 GMT+02:00 Mauro Rossi :
> 2017-08-11 17:25 GMT+02:00 Rob Herring :
>> On Fri, Aug 11, 2017 at 10:10 AM, Mauro Rossi  wrote:
>>> 2017-08-11 16:23 GMT+02:00 Rob Herring :
 On Fri, Aug 11, 2017 at 9:02 AM, Mauro Rossi  wrote:
> Current generation rules rely on LOCAL_PATH variable,
> which may be undefined when dependencies are expanded;
> move to using MESA_TOP variable to define sid_tables.py script path

 I count roughly 67 occurrences of pointing to python scripts using
 LOCAL_PATH. Presumably they all need to be fixed or this isn't really
 the problem.

> Fixes the following building error:
>
> external/mesa/src/gallium/drivers/radeonsi/si_debug.c:30:10: fatal error: 
> 'sid_tables.h' file not found
>  ^
> 1 error generated.
>
> Fixes: 730574c58e "android: amd/common: add support for 
> libmesa_amd_common"

 Why do I not see this error?
>>>
>>>
>>> I was also suprised to see the error,
>>> it started to appear persistently when building nougat-x86 from scratch.
>>>
>>> As a similar case I saw this one:
>>> https://cgit.freedesktop.org/mesa/mesa/commit/?id=c1a29e104cc585ad3219b12d09f532a129d68dad
>>>
>>> and in general I empirically saw it is unsafe to use $(LOCAL_PATH) in
>>> generated files dependencies rules.
>>> Chih-Wei may know better the reason.
>>
>> The discussion on this concluded that LOCAL_PATH as a rule dependency
>> is okay. LOCAL_PATH in the recipe for the rule is not.
>

Hi Rob,

It is as you said, the patch is not necessary

Sorry, I suspect I've been affected by a failing hard drive
M.

>
>>
>>>
>>> Added him in Cc:
>>>
>>> Mauro
>>>
>>> PS: If it is a false positive and not needed in 17.2 and mesa-dev
>>> please Rob, Chih-Wei just tell me,
>>> but in any case 17.1 branch requires to add:
>>>
>>> +LOCAL_STATIC_LIBRARIES := libmesa_amd_common
>>>
>>> in 
>>> https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/drivers/radeonsi/Android.mk?h=17.1
>>
>> Not sure, but probably needed as radeonsi was not in good shape in 17.1.
>>
>> Rob

Now I cross-checked and the additional static dependendency is not needed either
Also mesa-stable can be dropped and does not need changes
M.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] android: amd/common: fix sid_tables.h generation rules

2017-08-11 Thread Rob Herring
On Fri, Aug 11, 2017 at 10:10 AM, Mauro Rossi  wrote:
> 2017-08-11 16:23 GMT+02:00 Rob Herring :
>> On Fri, Aug 11, 2017 at 9:02 AM, Mauro Rossi  wrote:
>>> Current generation rules rely on LOCAL_PATH variable,
>>> which may be undefined when dependencies are expanded;
>>> move to using MESA_TOP variable to define sid_tables.py script path
>>
>> I count roughly 67 occurrences of pointing to python scripts using
>> LOCAL_PATH. Presumably they all need to be fixed or this isn't really
>> the problem.
>>
>>> Fixes the following building error:
>>>
>>> external/mesa/src/gallium/drivers/radeonsi/si_debug.c:30:10: fatal error: 
>>> 'sid_tables.h' file not found
>>>  ^
>>> 1 error generated.
>>>
>>> Fixes: 730574c58e "android: amd/common: add support for libmesa_amd_common"
>>
>> Why do I not see this error?
>
>
> I was also suprised to see the error,
> it started to appear persistently when building nougat-x86 from scratch.
>
> As a similar case I saw this one:
> https://cgit.freedesktop.org/mesa/mesa/commit/?id=c1a29e104cc585ad3219b12d09f532a129d68dad
>
> and in general I empirically saw it is unsafe to use $(LOCAL_PATH) in
> generated files dependencies rules.
> Chih-Wei may know better the reason.

The discussion on this concluded that LOCAL_PATH as a rule dependency
is okay. LOCAL_PATH in the recipe for the rule is not.

>
> Added him in Cc:
>
> Mauro
>
> PS: If it is a false positive and not needed in 17.2 and mesa-dev
> please Rob, Chih-Wei just tell me,
> but in any case 17.1 branch requires to add:
>
> +LOCAL_STATIC_LIBRARIES := libmesa_amd_common
>
> in 
> https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/drivers/radeonsi/Android.mk?h=17.1

Not sure, but probably needed as radeonsi was not in good shape in 17.1.

Rob
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] android: amd/common: fix sid_tables.h generation rules

2017-08-11 Thread Mauro Rossi
2017-08-11 16:23 GMT+02:00 Rob Herring :
> On Fri, Aug 11, 2017 at 9:02 AM, Mauro Rossi  wrote:
>> Current generation rules rely on LOCAL_PATH variable,
>> which may be undefined when dependencies are expanded;
>> move to using MESA_TOP variable to define sid_tables.py script path
>
> I count roughly 67 occurrences of pointing to python scripts using
> LOCAL_PATH. Presumably they all need to be fixed or this isn't really
> the problem.
>
>> Fixes the following building error:
>>
>> external/mesa/src/gallium/drivers/radeonsi/si_debug.c:30:10: fatal error: 
>> 'sid_tables.h' file not found
>>  ^
>> 1 error generated.
>>
>> Fixes: 730574c58e "android: amd/common: add support for libmesa_amd_common"
>
> Why do I not see this error?


I was also suprised to see the error,
it started to appear persistently when building nougat-x86 from scratch.

As a similar case I saw this one:
https://cgit.freedesktop.org/mesa/mesa/commit/?id=c1a29e104cc585ad3219b12d09f532a129d68dad

and in general I empirically saw it is unsafe to use $(LOCAL_PATH) in
generated files dependencies rules.
Chih-Wei may know better the reason.

Added him in Cc:

Mauro

PS: If it is a false positive and not needed in 17.2 and mesa-dev
please Rob, Chih-Wei just tell me,
but in any case 17.1 branch requires to add:

+LOCAL_STATIC_LIBRARIES := libmesa_amd_common

in 
https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/drivers/radeonsi/Android.mk?h=17.1

>
>>
>> Cc: "17.1 17.2" 
>> ---
>>  src/amd/Android.common.mk | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/amd/Android.common.mk b/src/amd/Android.common.mk
>> index 7d08bfd31d..f4497ed639 100644
>> --- a/src/amd/Android.common.mk
>> +++ b/src/amd/Android.common.mk
>> @@ -44,7 +44,7 @@ LOCAL_GENERATED_SOURCES := $(addprefix $(intermediates)/, 
>> $(AMD_GENERATED_FILES)
>>  $(LOCAL_GENERATED_SOURCES): PRIVATE_PYTHON := $(MESA_PYTHON2)
>>  $(LOCAL_GENERATED_SOURCES): PRIVATE_CUSTOM_TOOL = $(PRIVATE_PYTHON) $^ > $@
>>
>> -$(intermediates)/common/sid_tables.h: $(LOCAL_PATH)/common/sid_tables.py 
>> $(MESA_TOP)/src/amd/common/sid.h
>> +$(intermediates)/common/sid_tables.h: 
>> $(MESA_TOP)/src/amd/common/sid_tables.py $(MESA_TOP)/src/amd/common/sid.h
>> $(transform-generated-source)
>>
>>  LOCAL_C_INCLUDES := \
>> --
>> 2.11.0
>>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] android: amd/common: fix sid_tables.h generation rules

2017-08-11 Thread Rob Herring
On Fri, Aug 11, 2017 at 9:02 AM, Mauro Rossi  wrote:
> Current generation rules rely on LOCAL_PATH variable,
> which may be undefined when dependencies are expanded;
> move to using MESA_TOP variable to define sid_tables.py script path

I count roughly 67 occurrences of pointing to python scripts using
LOCAL_PATH. Presumably they all need to be fixed or this isn't really
the problem.

> Fixes the following building error:
>
> external/mesa/src/gallium/drivers/radeonsi/si_debug.c:30:10: fatal error: 
> 'sid_tables.h' file not found
>  ^
> 1 error generated.
>
> Fixes: 730574c58e "android: amd/common: add support for libmesa_amd_common"

Why do I not see this error?

>
> Cc: "17.1 17.2" 
> ---
>  src/amd/Android.common.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/amd/Android.common.mk b/src/amd/Android.common.mk
> index 7d08bfd31d..f4497ed639 100644
> --- a/src/amd/Android.common.mk
> +++ b/src/amd/Android.common.mk
> @@ -44,7 +44,7 @@ LOCAL_GENERATED_SOURCES := $(addprefix $(intermediates)/, 
> $(AMD_GENERATED_FILES)
>  $(LOCAL_GENERATED_SOURCES): PRIVATE_PYTHON := $(MESA_PYTHON2)
>  $(LOCAL_GENERATED_SOURCES): PRIVATE_CUSTOM_TOOL = $(PRIVATE_PYTHON) $^ > $@
>
> -$(intermediates)/common/sid_tables.h: $(LOCAL_PATH)/common/sid_tables.py 
> $(MESA_TOP)/src/amd/common/sid.h
> +$(intermediates)/common/sid_tables.h: 
> $(MESA_TOP)/src/amd/common/sid_tables.py $(MESA_TOP)/src/amd/common/sid.h
> $(transform-generated-source)
>
>  LOCAL_C_INCLUDES := \
> --
> 2.11.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev