On Mon, Dec 12, 2016 at 10:58 PM, Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
> On Thu, Nov 17, 2016 at 5:10 AM, Zheng, Lv <lv.zheng@xxxxxxxxx> wrote:
>>>
>>> > It contains correct header inclusion cleanups.
>>>
>>> Cleanups are not a good enough reason for breaking builds.
>>
>> It doesn't break, build is OK in my environment.
>
> You know, before you did that first clean up for MSVC it worked for me.
>
> Now I got the following
I serialized make in my example to be make -j1 and got the following
DESCEND power/acpi
DESCEND tools/acpidbg
CC tools/acpidbg/acpidbg.o
Assembler messages:
Fatal error: can't create .../power/acpi/tools/acpidbg/acpidbg.o: No
such file or dir
ectory
../../Makefile.rules:26: recipe for target
'.../power/acpi/tools/acpidbg/acpidbg.o' f
ailed
As you may already notice from the below there is a wrong folder used.
The hierarchy is
% ls -R tools/power/
tools/power/:
acpi x86
tools/power/acpi:
tools
tools/power/acpi/tools:
acpidbg acpidump ec
tools/power/acpi/tools/acpidbg:
tools/power/acpi/tools/acpidump:
tools/power/acpi/tools/ec:
tools/power/x86:
turbostat
>
> DESCEND power/acpi
> DESCEND tools/acpidbg
> DESCEND tools/acpidump
> DESCEND tools/ec
> MKDIR include
> INST acpidbg
> MKDIR include
> INST ec
> MKDIR include
> CP include
> CP include
> /usr/bin/install: cannot stat '.../power/acpi/acpidbg': No such file
> or directory
> INST acpidump
> ../../Makefile.rules:41: recipe for target 'install-tools' failed
> make[6]: *** [install-tools] Error 1
> /usr/bin/install: make[6]: *** Waiting for unfinished jobs....
> cannot stat '.../power/acpi/ec': No such file or directory
> ../../Makefile.rules:41: recipe for target 'install-tools' failed
> make[6]: *** [install-tools] Error 1
> make[6]: *** Waiting for unfinished jobs....
> INST acpidump.8
> CP include
> Makefile:23: recipe for target 'acpidbg_install' failed
> make[5]: *** [acpidbg_install] Error 2
> make[5]: *** Waiting for unfinished jobs....
> /usr/bin/install: cannot stat '.../power/acpi/acpidump': No such file
> or directory
> ../../Makefile.rules:41: recipe for target 'install-tools' failed
> make[6]: *** [install-tools] Error 1
> make[6]: *** Waiting for unfinished jobs....
> Makefile:23: recipe for target 'ec_install' failed
> make[5]: *** [ec_install] Error 2
> Makefile:23: recipe for target 'acpidump_install' failed
> make[5]: *** [acpidump_install] Error 2
> Makefile:95: recipe for target 'acpi_install' failed
> make[4]: *** [acpi_install] Error 2
>
> .../Makefile:1613: recipe for target 'tools/acpi_install' failed
>
>>>> Just to notice that above Makefile comes from the source tree not the O= variant
> The rest of ... related to O=/my/output/path.
>
> make[3]: *** [tools/acpi_install] Error 2
> Makefile:150: recipe for target 'sub-make' failed
> make[2]: *** [sub-make] Error 2
> Makefile:24: recipe for target '__sub-make' failed
> make[1]: *** [__sub-make] Error 2
>
> So, it doesn't even try to build anything.
>
>> Yes. IMO,
>> The above breakage looks much more strange than the commit itself.
>> It's better to make the build successful without commenting this line out.
>>
>> Such kind of regressions bothered me a lot.
>> It seems we can easily fell into such old traps.
>>
>> If I cannot fix the new regression in time, I will sure suggest to revert breakage commit before a better solution can be offered after investigation.
>> However, for this issue:
>> 1. There is actually no serious breakage (only toolchains built with incompliant kernel headers)
>> 2. We have fixed the issue, but the fix need to be improved.
>> So reverting the correct commit isn't a good idea.
>> Reverting it may trigger more serious issues because of wrong header inclusion orders.
>> Let me improve it.
>>
>>>
>>> > We can see many tools build broken for cross-compilers, and tools/power/acpi is not the only one.
>>>
>>> I guess the issue is that it was not broken before and it is broken
>>> after your changes. That's quite a bit of a difference.
>>
>> IMO, the non-updated toolchains/or ACPICA's sX/uX definitions are actually buggy while the commit isn't.
>>
>>>
>>> > We are just to improve it.
>>>
>>> That's nice, but see above.
>>
>> Yes, we have fixed it.
>> And Yisheng Xie has feedback that this issue has been fixed.
>>
>> However, the v1 fix contains problem:
>> There is one line dependency not correct:
>> acpidbg acpidump ec: include/acpi FORCE
>> I failed to correct it in v2/v3:
>> $(OUTPUT)$(TOOL): $(KERNEL_INCLUDE) $(toolobjs) FORCE
>> Which was criticized by Andy, complaining new breaks for parallel builds.
>> Now in v4, it is corrected:
>> $(objdir)%.o: %.c $(KERNEL_INCLUDE)
>>
>> However, in v2/v3, I cleaned up OUTPUT related code and changed its behavior of build with "O=".
>> In v2/v3, build "O=" from tools/power/acpi folder passes, but build from tools folder fails.
>> In v4, build "O=" from tools folder passes, but build from tools/power/acpi folder fails.
>>
>> There doesn't seem to be a solution to make both cases working.
>> Because the "descend" macro in tools/scripts/Makefile.include cannot handle both cases.
>> Stopping using "descend" macro in tools/power/acpi/Makefile could be the only possible fix...
>> So we only need one of them working and IMO v4 is suitable for Andy's needs.
>>
>> As a conclusion, v4 should be OK now.
>
> See above.
>
> This is how make started. $(KERNEL_TOOLS_TARGET) is a list of folders
> under tools (for now just "acpi gpio turbostat"). $(1) either
> "install" or "clean".
> KERNEL_SRC the output folder of kernel build (it might be the same as
> sources, but not my case, I used O=). The rest from the list are
> compiled and installed nicely. This *was* the case for acpi tools.
>
> + $(Q)for t in $(KERNEL_TOOLS_TARGET); do \
> + $(MAKE) -C $(KERNEL_SRC) \
$(MAKE) is something like make -jXX here
> + CROSS_COMPILE="$(TARGET_CROSS)" \
> + DESTDIR="$(TARGET_DIR)" \
> + tools/$${t}_$(1); \
> + done
>
> So. there is no magic here.
>
> Can you add this simple test to your test cases?
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
