RE: Fix in ACPICA tools broke cross compilation of tools/power/acpi

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi, Rafael

> From: rjwysocki@xxxxxxxxx [mailto:rjwysocki@xxxxxxxxx] On Behalf Of Rafael J. Wysocki
> Subject: Re: Fix in ACPICA tools broke cross compilation of tools/power/acpi
> 
> On Wed, Nov 16, 2016 at 9:43 AM, Zheng, Lv <lv.zheng@xxxxxxxxx> wrote:
> > Hi, Andy
> >
> >> From: Andy Shevchenko [mailto:andy.shevchenko@xxxxxxxxx]
> >> Subject: Re: Fix in ACPICA tools broke cross compilation of tools/power/acpi
> >>
> >> On Tue, Nov 15, 2016 at 5:01 PM, Andy Shevchenko
> >> <andy.shevchenko@xxxxxxxxx> wrote:
> >>
> >> > The below + several runs (need to serialize makefile, by default it
> >> > races install vs. build)  helped eventually.
> >>
> >> Isn't better just to revert patch I mention and continue from working case?
> >
> > It's not possible to revert that patch.
> 
> It surely is possible to revert things, but it may not be a good idea.
> So I think you wanted to say that reverting this particular one would
> not be a good idea.

Yes, it's not a good idea.
The cleanup is correct.

> 
> > 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.

> 
> > While this case is in fact a special case, bugs in tools/Makefile are triggered (about OUT/SRC).
> 
> I'm not sure what you mean.

I'm wrong here.
The only issue triggered is:
The following line in the acenv.h need to be commented out:
#include <signal.h>

> 
> > OTOH, this case is actually not a very useful case.
> 
> In your opinion I guess?

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.

Thanks and best regards
Lv
��.n��������+%������w��{.n�����{�����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f




[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux