[Mesa-dev] [PATCH 2/2] android: fix spirv_info generation

[Emil Velikov]

On 20 July 2017 at 11:02, Chih-Wei Huang <cwhuang at android-x86.org> wrote:
> 2017-07-19 15:12 GMT+08:00 Tapani Pälli <tapani.palli at intel.com>:
>> Depending on build order, LOCAL_PATH maybe set or not (and can't
>> be trusted to have assumed path), change modifies all occurences
>> of LOCAL_PATH as locally defined COMPILER_PATH instead.
>>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>>  src/compiler/Android.nir.gen.mk | 38 ++++++++++++++++++++------------------
>>  1 file changed, 20 insertions(+), 18 deletions(-)
>>
>> diff --git a/src/compiler/Android.nir.gen.mk b/src/compiler/Android.nir.gen.mk
>> index 4507ac4..81511de 100644
>> --- a/src/compiler/Android.nir.gen.mk
>> +++ b/src/compiler/Android.nir.gen.mk
>> @@ -27,6 +27,8 @@ ifeq ($(LOCAL_MODULE_CLASS),)
>>  LOCAL_MODULE_CLASS := STATIC_LIBRARIES
>>  endif
>>
>> +COMPILER_PATH := $(MESA_TOP)/src/compiler
>> +
>>  intermediates := $(call local-generated-sources-dir)
>>
>>  LOCAL_SRC_FILES := $(LOCAL_SRC_FILES)
>> @@ -48,48 +50,48 @@ MESA_GEN_NIR_H := $(addprefix $(call local-generated-sources-dir)/, \
>>         nir/nir_opcodes.h \
>>         nir/nir_builder_opcodes.h)
>>
>> -nir_builder_opcodes_gen := $(LOCAL_PATH)/nir/nir_builder_opcodes_h.py
>> +nir_builder_opcodes_gen := $(COMPILER_PATH)/nir/nir_builder_opcodes_h.py
>>  nir_builder_opcodes_deps := \
>> -       $(LOCAL_PATH)/nir/nir_opcodes.py \
>> -       $(LOCAL_PATH)/nir/nir_builder_opcodes_h.py
>> +       $(COMPILER_PATH)/nir/nir_opcodes.py \
>> +       $(COMPILER_PATH)/nir/nir_builder_opcodes_h.py
>>
>>  $(intermediates)/nir/nir_builder_opcodes.h: $(nir_builder_opcodes_deps)
>>         @mkdir -p $(dir $@)
>>         $(hide) $(MESA_PYTHON2) $(nir_builder_opcodes_gen) $< > $@
>>
>> -nir_constant_expressions_gen := $(LOCAL_PATH)/nir/nir_constant_expressions.py
>> +nir_constant_expressions_gen := $(COMPILER_PATH)/nir/nir_constant_expressions.py
>>  nir_constant_expressions_deps := \
>> -       $(LOCAL_PATH)/nir/nir_opcodes.py \
>> -       $(LOCAL_PATH)/nir/nir_constant_expressions.py
>> +       $(COMPILER_PATH)/nir/nir_opcodes.py \
>> +       $(COMPILER_PATH)/nir/nir_constant_expressions.py
>>
>>  $(intermediates)/nir/nir_constant_expressions.c: $(nir_constant_expressions_deps)
>>         @mkdir -p $(dir $@)
>>         $(hide) $(MESA_PYTHON2) $(nir_constant_expressions_gen) $< > $@
>>
>> -nir_opcodes_h_gen := $(LOCAL_PATH)/nir/nir_opcodes_h.py
>> +nir_opcodes_h_gen := $(COMPILER_PATH)/nir/nir_opcodes_h.py
>>  nir_opcodes_h_deps := \
>> -       $(LOCAL_PATH)/nir/nir_opcodes.py \
>> -       $(LOCAL_PATH)/nir/nir_opcodes_h.py
>> +       $(COMPILER_PATH)/nir/nir_opcodes.py \
>> +       $(COMPILER_PATH)/nir/nir_opcodes_h.py
>>
>>  $(intermediates)/nir/nir_opcodes.h: $(nir_opcodes_h_deps)
>>         @mkdir -p $(dir $@)
>>         $(hide) $(MESA_PYTHON2) $(nir_opcodes_h_gen) $< > $@
>>
>> -$(LOCAL_PATH)/nir/nir.h: $(intermediates)/nir/nir_opcodes.h
>> +$(COMPILER_PATH)/nir/nir.h: $(intermediates)/nir/nir_opcodes.h
>>
>> -nir_opcodes_c_gen := $(LOCAL_PATH)/nir/nir_opcodes_c.py
>> +nir_opcodes_c_gen := $(COMPILER_PATH)/nir/nir_opcodes_c.py
>>  nir_opcodes_c_deps := \
>> -       $(LOCAL_PATH)/nir/nir_opcodes.py \
>> -       $(LOCAL_PATH)/nir/nir_opcodes_c.py
>> +       $(COMPILER_PATH)/nir/nir_opcodes.py \
>> +       $(COMPILER_PATH)/nir/nir_opcodes_c.py
>>
>>  $(intermediates)/nir/nir_opcodes.c: $(nir_opcodes_c_deps)
>>         @mkdir -p $(dir $@)
>>         $(hide) $(MESA_PYTHON2) $(nir_opcodes_c_gen) $< > $@
>>
>> -nir_opt_algebraic_gen := $(LOCAL_PATH)/nir/nir_opt_algebraic.py
>> +nir_opt_algebraic_gen := $(COMPILER_PATH)/nir/nir_opt_algebraic.py
>>  nir_opt_algebraic_deps := \
>> -       $(LOCAL_PATH)/nir/nir_opt_algebraic.py \
>> -       $(LOCAL_PATH)/nir/nir_algebraic.py
>> +       $(COMPILER_PATH)/nir/nir_opt_algebraic.py \
>> +       $(COMPILER_PATH)/nir/nir_algebraic.py
>>
>>  $(intermediates)/nir/nir_opt_algebraic.c: $(nir_opt_algebraic_deps)
>>         @mkdir -p $(dir $@)
>> @@ -98,6 +100,6 @@ $(intermediates)/nir/nir_opt_algebraic.c: $(nir_opt_algebraic_deps)
>>  LOCAL_GENERATED_SOURCES += $(addprefix $(intermediates)/, \
>>         $(SPIRV_GENERATED_FILES))
>>
>> -$(intermediates)/spirv/spirv_info.c: $(LOCAL_PATH)/spirv/spirv_info_c.py $(LOCAL_PATH)/spirv/spirv.core.grammar.json
>> +$(intermediates)/spirv/spirv_info.c: $(COMPILER_PATH)/spirv/spirv_info_c.py $(COMPILER_PATH)/spirv/spirv.core.grammar.json
>>         @mkdir -p $(dir $@)
>> -       $(hide) $(MESA_PYTHON2) $(LOCAL_PATH)/spirv/spirv_info_c.py $(LOCAL_PATH)/spirv/spirv.core.grammar.json $@ || ($(RM) $@; false)
>> +       $(hide) $(MESA_PYTHON2) $(COMPILER_PATH)/spirv/spirv_info_c.py $(COMPILER_PATH)/spirv/spirv.core.grammar.json $@ || ($(RM) $@; false)
>
> OK. I see the real problem.
> The rules to build spirv_info.c are incorrectly
> to use $(LOCAL_PATH).
> Basically speaking, $(LOCAL_PATH) can't be used
> in the recipes[1] since it is always changing.
> When the recipe rules are executed its value
> is not you expected.
> (using it in targets and prerequisites is OK)
>
Thanks Chih-Wei.

To put it in other words:

Variables are expanded and used in the build rules _after_ the
makefile is read. And since LOCAL_PATH is reset (many times in dozens
of places) we're in a pickle.

Strictly speaking the Android folk should _not_ have mandated the
explicit prefix for either dependencies (srcdir/LOCAL_PATH) or
generated files (builddir/intermediates).
... just like any other build system does it - scons, autotools,
cmake, meson, waf(?).

Ship has sailed, hope they've did not make the same mistake with their
new system.

Thanks again.
Emil