romtool and makefile breakage

[Carl-Daniel Hailfinger]

On 04.04.2009 20:40, [e-mail] wrote:
> Author: stepan
> Date: 2009-04-04 20:40:46 +0200 (Sat, 04 Apr 2009)
> New Revision: 4068
>
> Modified:
>    trunk/coreboot-v2/src/arch/ppc/Config.lb
> Log:
> small workaround for romtool incompatibility with ppc ports
>
> Signed-off-by: Stefan Reinauer 
> Acked-by: Stefan Reinauer

No offense intended, but almost every commit intended to fix the build
since the introduction of romtool broke the build for some boards.

I'm very tempted to simply suggest reverting romtool until someone
figures out how to get a reliable and stable abuild.

I mean, look at the commits which tried to fix romtool breakage:
4049, 4054, 4061, 4062, 4067, 4068
and the build is still broken!

It seems the cause of the breakage is not understood.

Can we please require a full _parallel_ abuild on a
_multicore/multiprocessor_ machine for each "bugfix" before it is committed?

By the way, each of the make rules in the code snippet below is a pure
race condition:
> Modified: trunk/coreboot-v2/src/arch/ppc/Config.lb
>
> --- trunk/coreboot-v2/src/arch/ppc/Config.lb	2009-04-04 18:24:21 UTC (rev 4067)
> +++ trunk/coreboot-v2/src/arch/ppc/Config.lb	2009-04-04 18:40:46 UTC (rev 4068)
> @@ -1,10 +1,15 @@
>  ldscript init/ldscript.lb
>
> -makerule coreboot.rom 
> +makerule coreboot.strip 
>    depends	"coreboot" 
>    action	"cp $< $@"
>  end
>
> +makerule coreboot.rom
> depends	"coreboot.strip"
> action "cp $< $@"
>
> +end
>
> dir init
>  dir lib
>  dir boot

"cp" and "cat" should be banned in makefiles. They are not atomic and if
they touch any files specified in any dependency rule, they WILL
randomly BREAK parallel compilation (or just result in silently
corrupted images) and work without problems if compilation is not
parallelized. Since almost everybody doesn't run abuild in parallel mode
on a multicore machine (except our build-bot), the breakage happens only
on the build-bot.

I just checked our v2 makefiles and every single usage of cp and every
single usage of cat looks broken.

Solution:
If you really have to use cp and cat, make sure the destination file is
not mentioned in any dependency rule. Use temporary files as destination
files and rename (mv) them to the real destination file. Make sure the
temp file and the destination file are in the same directory

Example:
Broken rule:

makerule a
    depends	"b"
    action "cp $< $@"
end

Fixed rule:
makerule a
    depends	"b"
    action "cp $< $someuniquetempname"
    action "mv $someuniquetempname $@"
end

someuniquetempname must be created (in the same directory as the
destination file of the rule) with mkstemp (or mktemp, if mkstemp is
unavailable) in that rule (not globally, or you get another race
condition), but my make-fu is too weak for this.

And while we're on the topic of makefile race conditions, please note
that gcc is smart enough to create output files atomically, but almost
every other program (romcc, romtool, cat, cp) is not. Non-smart programs
MUST (in the RFC sense) always output to files not specified in any
dependency rule. Creating smart programs is hard.

Sorry for the lengthy mail, it's meant to be helpful.

Regards,
Carl-Daniel

[Patrick Georgi]

Am 05.04.2009 02:42, schrieb Carl-Daniel Hailfinger:
> No offense intended, but almost every commit intended to fix the build
> since the introduction of romtool broke the build for some boards.

As long as someone works on it, I think it's fine. If problems arise and 
no-one works on fixing them, that's a serious issue and code should be 
rolled back.
That's why this is a development repository. And people who fear that 
their local copy might break can look at the autobuilder.
> It seems the cause of the breakage is not understood.
>
> Can we please require a full _parallel_ abuild on a
> _multicore/multiprocessor_ machine for each "bugfix" before it is committed?

Doesn't necessarily help. Many of my patches were tested exactly that 
way, and they worked fine on _my_ box when built that way. What more 
should I do?
If you want an "tree-always-works" process, I can recommend aegis 
(http://aegis.sf.net)
> "cp" and "cat" should be banned in makefiles. They are not atomic and if
> [...]
>
> And while we're on the topic of makefile race conditions, please note
> that gcc is smart enough to create output files atomically, but almost
> every other program (romcc, romtool, cat, cp) is not. Non-smart programs
> MUST (in the RFC sense) always output to files not specified in any
> dependency rule. Creating smart programs is hard.

How about just banning make, if it doesn't manage such a trivial thing 
such as looking if the job make itself created finished, before assuming 
that the file is ready to be used?
This isn't rocket science.

Patrick

[Stefan Reinauer]

On 05.04.2009 2:42 Uhr, Carl-Daniel Hailfinger wrote:
> No offense intended, but almost every commit intended to fix the build
> since the introduction of romtool broke the build for some boards.
>
> I'm very tempted to simply suggest reverting romtool until someone
> figures out how to get a reliable and stable abuild.

Please accept my apologies. If someone else did what I did last night, I
would have been writing this mail, quite sure ;-)

We did test romtool/romfs over the last two weeks in a separate
repository, and it never exposed the issues we were seeing.
> I mean, look at the commits which tried to fix romtool breakage:
> 4049, 4054, 4061, 4062, 4067, 4068
> and the build is still broken!

It should all be fixed again now. (Missing: Some PPC stuff). The
revisions you list mostly fix different issues that did not show up in
our tests before.
Plus, the problem is we didn't have a ppc cross compiler installed on
our development abuild boxes)
> It seems the cause of the breakage is not understood.

The cause_s_ were quite simple. However, there were several.
> Can we please require a full _parallel_ abuild on a
> _multicore/multiprocessor_ machine for each "bugfix" before it is committed?
>
> Example:
> Broken rule:
>
> makerule a
>    depends	"b"
>    action "cp $< $@"
> end
>
> Fixed rule:
> makerule a
>    depends	"b"
>    action "cp $< $someuniquetempname"
>    action "mv $someuniquetempname $@"
> end

I have to agree with Patrick here: If make can't handle trivial stuff
like copying a file around without introducing a race condition or
adding severe workarounds, it is probably not the tool of our choice.
> And while we're on the topic of makefile race conditions, please note
> that gcc is smart enough to create output files atomically, but almost
> every other program (romcc, romtool, cat, cp) is not. Non-smart programs
> MUST (in the RFC sense) always output to files not specified in any
> dependency rule. Creating smart programs is hard.

I don't like a BIOS to be smart about Make processes. Autoconf and
libtool tried that, and failed. How could a BIOS ever succeed.

Maybe fixing GNU make instead would be an option, too? If make has a
production rule for a target, it MUST (in the RFC sense) never ever
execute a rule depending on that one, while the rule is still running.
That's just broken behavior, by design or implementation, and I don't
think our two layers of python on top of it should be fixing it. ;-)

Stefan