Hi, Andy Good to discuss/synchronize with you in details around this issue. > From: Andy Shevchenko [mailto:andy.shevchenko@xxxxxxxxx] > Subject: Re: Fix in ACPICA tools broke cross compilation of tools/power/acpi > > 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 > > 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. The result depends on where you start your O= command. In your previous report, you preferred tools folder. While you reported failures against my old patch which allows O= command from tools/power/acpi Can you tell me your current directory in this case? > > > 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) \ > + CROSS_COMPILE="$(TARGET_CROSS)" \ > + DESTDIR="$(TARGET_DIR)" \ > + tools/$${t}_$(1); \ > + done > > So. there is no magic here. So it seems you stay in objtree to do this test. Which is not handled by the previous commit. That commit only handles make in "tools" folder. > > Can you add this simple test to your test cases? The problem is the buggy descend macro, which has some conflicts against the needs of putting the new include folder to objtree. The descend macro is used in tools/Makefile, and in order to avoid using export (which looks very bad in Makefiles), we reused it in tools/power/acpi/Makefile. acpidbg acpidump ec: FORCE $(call descend,tools/$@,all) ... It worked previously because we have magic in tools/power/acpi/Makefile to work the descend macro around. The new acpi makefiles actually uses different output folder from different current directory and different O= assignments. While we need a stationary directory for the new include folder to be used with -I parameter. If the new include is in "srctree" folders, there will be no problem, however it pollutes srctree. (That's why I asked your need - do you want to put the include folder to objtree. But you didn't reply.) If the new include is in "objtree" folders, I'm not able to find a way to make it stationary because of above magic. The problem is in tools/scripts/Makefile.include related to objtree: ifneq ($(O),) ifeq ($(origin O), command line) dummy := $(if $(shell test -d $(O) || echo $(O)),$(error O=$(O) does not exist),) ABSOLUTE_O := $(shell cd $(O) ; pwd) OUTPUT := $(ABSOLUTE_O)/$(if $(subdir),$(subdir)/) COMMAND_O := O=$(ABSOLUTE_O) ifeq ($(objtree),) objtree := $(O) endif endif endif # check that the output directory actually exists ifneq ($(OUTPUT),) OUTDIR := $(shell cd $(OUTPUT) && /bin/pwd) $(if $(OUTDIR),, $(error output directory "$(OUTPUT)" does not exist)) Endif 1. When we stay in "tools" folder, and O is omit. Descend invoked from tools/Makefile causes: objtree=tools. 2. When we stay in "tools" folder, and O is "/tmp". Descend invoked from tools/Makefile causes: objtree=/tmp. 3. When we stay in "tools/power/acpi" folder, and O is omit. Descend invoked from tools/power/acpi/Makefile causes: objtree=tools/power/acpi. 4. When we stay in "tools/power/acpi" folder, and O is omit. Descend invoked from tools/power/acpi/Makefile causes: objtree=/tmp. 5. Now you want to stay in O folder, and O is /tmp. Descend invoked from tools/Makefile causes: objtree=/tmp. So if we want to put the include folder to objtree, what should it be? For example, if we put include folder in objtree/power/acpi/include, the it probably will fail case 4. Another example is, if we put include folder in objtree/include, then "make clean" probably will delete the source tree in case 1. The O= handling in tools/scripts/Makefile.include is the root cause of the issue reported by you, IMO. Do you have suggestions about how can we get all cases working while still can put include folder in objtree? I'll also try a different approach to see if the puzzle can be solved. Thanks and best regards Lv ��.n��������+%������w��{.n�����{�����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
