All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Yann E. MORIN" <yann.morin.1998@free.fr>
To: James Hilliard <james.hilliard1@gmail.com>
Cc: Fabrice Fontaine <fontaine.fabrice@gmail.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH 1/1] package/bearssl: disable parallel build
Date: Sun, 16 Oct 2022 18:05:05 +0200	[thread overview]
Message-ID: <20221016160505.GK2503@scaer> (raw)
In-Reply-To: <CADvTj4rBi3uu2bXJy=Tp76NV6Yjk7--EtDOm0kmxi47XOVsQ=Q@mail.gmail.com>

James, All,

+Thomas, Peter, Arnout

On 2022-10-16 11:44 -0400, James Hilliard spake thusly:
> On Sun, Oct 16, 2022 at 3:51 AM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> > On 2022-10-15 19:21 -0400, James Hilliard spake thusly:
[--SNIP--]
> > > Yes, all my autobuilders were recently changed to use make master(will be
> > > make version 4.4 once released) with top level --shuffle=random.
> > By disabling parallel build because you are using ana as yet unreleased
> > version, we remove the parallel build for everyone, which is not very
> > nice at all.
> Failures with shuffle mode indicate that the package is not parallel compatible
> on existing make releases as well, it just makes those bugs more visible.

I never said they were not, and I even said (further on) that it *is*
interesting to find such bugs.

However, the behaviour so far has been ordering of prerequisites (and
goals!), and for some packages, that ordering was enough to work in a
parallel make.

For example, the bearsll parallel issue is caused by the ordering so
the mkdir is very early. Yes, it is incorrect, but it happens to always
work in practice. And yes it is easy to fix, here's a patch:

    diff -durN a/mk/mkrules.sh b/mk/mkrules.sh
    --- a/mk/mkrules.sh	2018-08-14 22:41:54.000000000 +0200
    +++ b/mk/mkrules.sh	2022-10-16 17:54:22.282069851 +0200
    @@ -531,23 +531,23 @@
     (for f in $coresrc ; do
     	b="$(basename "$f" .c)\$O"
     	g="$(escsep "$f")"
    -	printf '\n$(OBJDIR)$P%s: %s $(HEADERSPRIV)\n\t$(CC) $(CFLAGS) $(INCFLAGS) $(CCOUT)$(OBJDIR)$P%s %s\n' "$b" "$g" "$b" "$g"
    +	printf '\n$(OBJDIR)$P%s: $(OBJDIR) %s $(HEADERSPRIV)\n\t$(CC) $(CFLAGS) $(INCFLAGS) $(CCOUT)$(OBJDIR)$P%s %s\n' "$b" "$g" "$b" "$g"
     done
     
     for f in $toolssrc ; do
     	b="$(basename "$f" .c)\$O"
     	g="$(escsep "$f")"
    -	printf '\n$(OBJDIR)$P%s: %s $(HEADERSTOOLS)\n\t$(CC) $(CFLAGS) $(INCFLAGS) $(CCOUT)$(OBJDIR)$P%s %s\n' "$b" "$g" "$b" "$g"
    +	printf '\n$(OBJDIR)$P%s: $(OBJDIR) %s $(HEADERSTOOLS)\n\t$(CC) $(CFLAGS) $(INCFLAGS) $(CCOUT)$(OBJDIR)$P%s %s\n' "$b" "$g" "$b" "$g"
     done
     
     for f in $testcryptosrc $testspeedsrc ; do
     	b="$(basename "$f" .c)\$O"
     	g="$(escsep "$f")"
    -	printf '\n$(OBJDIR)$P%s: %s $(HEADERSPRIV)\n\t$(CC) $(CFLAGS) $(INCFLAGS) $(CCOUT)$(OBJDIR)$P%s %s\n' "$b" "$g" "$b" "$g"
    +	printf '\n$(OBJDIR)$P%s: $(OBJDIR) %s $(HEADERSPRIV)\n\t$(CC) $(CFLAGS) $(INCFLAGS) $(CCOUT)$(OBJDIR)$P%s %s\n' "$b" "$g" "$b" "$g"
     done
     
     for f in $testx509src ; do
     	b="$(basename "$f" .c)\$O"
     	g="$(escsep "$f")"
    -	printf '\n$(OBJDIR)$P%s: %s $(HEADERSPRIV)\n\t$(CC) $(CFLAGS) $(INCFLAGS) -DSRCDIRNAME=".." $(CCOUT)$(OBJDIR)$P%s %s\n' "$b" "$g" "$b" "$g"
    +	printf '\n$(OBJDIR)$P%s: $(OBJDIR) %s $(HEADERSPRIV)\n\t$(CC) $(CFLAGS) $(INCFLAGS) -DSRCDIRNAME=".." $(CCOUT)$(OBJDIR)$P%s %s\n' "$b" "$g" "$b" "$g"
     done) >> Rules.mk
    Binary files a/mk/.mkrules.sh.swp and b/mk/.mkrules.sh.swp differ

And then adding a CONFIGURE_CMDS that runs mk/mkrules.sh to regenerate
the Makefile.

> I'm running shuffle with -j1 because that way it's obvious in the logs where the
> failure is getting hit, otherwise these bugs tend to be transient and
> difficult to
> trace as they otherwise are timing dependent.

Yes, this is very interesting to find such bugs, but disabling parallel
build in packages is not nice when they can be (easily) fixed.

> > Notes:
> >   - it *is* interesting to find those bugs;
> >   - it is sad that the default mode for --shuffle will be random, not
> >     none.
> Default is equivalent to none,

Yeah, I noticed the hard way after having my patch rejected in makem as
you may have noticed. :-/

> I modified my autobuilder runner to pass
> --shuffle=random.

Again, that is very interesting, but please, please, conordinate with
the rest of us when you do such changes, because that is very weird, as
Fabrice noted, to suddenly get build failures out of the blue when
nothing seemed to have otherwise changed.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

  reply	other threads:[~2022-10-16 16:05 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-15  0:58 [Buildroot] [PATCH 1/1] package/bearssl: disable parallel build James Hilliard
2022-10-15 22:14 ` Fabrice Fontaine
2022-10-15 23:21   ` James Hilliard
2022-10-16  7:50     ` Yann E. MORIN
2022-10-16 15:44       ` James Hilliard
2022-10-16 16:05         ` Yann E. MORIN [this message]
2022-10-16 16:45           ` James Hilliard
2022-10-17  7:16             ` Thomas Petazzoni via buildroot
2022-10-17 11:29               ` James Hilliard
2022-10-17 12:21                 ` Thomas Petazzoni via buildroot
2022-10-17 19:02                   ` James Hilliard
2022-10-30 20:29 ` Thomas Petazzoni via buildroot
2022-10-31  2:54   ` James Hilliard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221016160505.GK2503@scaer \
    --to=yann.morin.1998@free.fr \
    --cc=buildroot@buildroot.org \
    --cc=fontaine.fabrice@gmail.com \
    --cc=james.hilliard1@gmail.com \
    --cc=thomas.petazzoni@bootlin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.