diff mbox series

[v2] genmultilib: Add sanity check

Message ID 20221103095259.4095606-1-christophe.lyon@arm.com
State Accepted
Headers
Series [v2] genmultilib: Add sanity check |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Christophe Lyon Nov. 3, 2022, 9:52 a.m. UTC 
  When a list of dirnames is provided to genmultilib, its length is
expected to match the number of options.  If this is not the case, the
build fails later for reasons not obviously related to this mistake.
This patch adds a sanity check to help diagnose such cases.

Tested by adding an option to t-aarch64 and no corresponding dirname,
with both bash and dash.

v2: do not use arrays (bash feature).

OK for trunk?

gcc/ChangeLog:

	* genmultilib: Add sanity check.
---
 gcc/genmultilib | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Jeff Law Nov. 18, 2022, 7:42 p.m. UTC | #1 
On 11/3/22 03:52, Christophe Lyon via Gcc-patches wrote:
> When a list of dirnames is provided to genmultilib, its length is
> expected to match the number of options.  If this is not the case, the
> build fails later for reasons not obviously related to this mistake.
> This patch adds a sanity check to help diagnose such cases.
>
> Tested by adding an option to t-aarch64 and no corresponding dirname,
> with both bash and dash.
>
> v2: do not use arrays (bash feature).
>
> OK for trunk?
>
> gcc/ChangeLog:
>
> 	* genmultilib: Add sanity check.

OK.  It should be interesting to see if it trips.


jeff
Mark Wielaard Nov. 21, 2022, 10:16 a.m. UTC | #2 
Hi,

On Fri, Nov 18, 2022 at 12:42:10PM -0700, Jeff Law wrote:
> > 	* genmultilib: Add sanity check.
> 
> OK.  It should be interesting to see if it trips.

It trips up various buildbot setups:
https://builder.sourceware.org/buildbot/#/changes/13720

Error calling ../../gcc/gcc/genmultilib: Number of dirnames (3) does not match number of options (2)
options: m64/m31
dirnames: 64 32
make[2]: *** [Makefile:2240: s-mlib] Error 1

Mark
Christophe Lyon Nov. 21, 2022, 12:12 p.m. UTC | #3 
On 11/21/22 11:16, Mark Wielaard wrote:
> Hi,
> 
> On Fri, Nov 18, 2022 at 12:42:10PM -0700, Jeff Law wrote:
>>> 	* genmultilib: Add sanity check.
>>
>> OK.� It should be interesting to see if it trips.
> 
> It trips up various buildbot setups:
> https://builder.sourceware.org/buildbot/#/changes/13720
> 
> Error calling ../../gcc/gcc/genmultilib: Number of dirnames (3) does not match number of options (2)
> options: m64/m31
> dirnames: 64 32
> make[2]: *** [Makefile:2240: s-mlib] Error 1
> 
> Mark

Indeed, sorry for that.

I've just sent a fix:
https://gcc.gnu.org/pipermail/gcc-patches/2022-November/606887.html

Hopefully that one is right

Christophe
Mark Wielaard Nov. 21, 2022, 12:32 p.m. UTC | #4 
Hi Christophe,

On Mon, 2022-11-21 at 13:12 +0100, Christophe Lyon wrote:
> On 11/21/22 11:16, Mark Wielaard wrote:
> > On Fri, Nov 18, 2022 at 12:42:10PM -0700, Jeff Law wrote:
> > > > 	* genmultilib: Add sanity check.
> > > 
> > > OK. It should be interesting to see if it trips.
> > 
> > It trips up various buildbot setups:
> > https://builder.sourceware.org/buildbot/#/changes/13720
> > 
> > Error calling ../../gcc/gcc/genmultilib: Number of dirnames (3)
> > does not match number of options (2)
> > options: m64/m31
> > dirnames: 64 32
> > make[2]: *** [Makefile:2240: s-mlib] Error 1
> 
> Indeed, sorry for that.
> 
> I've just sent a fix:
> https://gcc.gnu.org/pipermail/gcc-patches/2022-November/606887.html
> 
> Hopefully that one is right

The buildbot gcc builds are turning green again!
https://builder.sourceware.org/buildbot/#/changes/13736

Thanks,

Mark
Christophe Lyon Nov. 21, 2022, 12:35 p.m. UTC | #5 
On 11/21/22 13:32, Mark Wielaard wrote:
> Hi Christophe,
> 
> On Mon, 2022-11-21 at 13:12 +0100, Christophe Lyon wrote:
>> On 11/21/22 11:16, Mark Wielaard wrote:
>>> On Fri, Nov 18, 2022 at 12:42:10PM -0700, Jeff Law wrote:
>>>>> 	* genmultilib: Add sanity check.
>>>>
>>>> OK. It should be interesting to see if it trips.
>>>
>>> It trips up various buildbot setups:
>>> https://builder.sourceware.org/buildbot/#/changes/13720
>>>
>>> Error calling ../../gcc/gcc/genmultilib: Number of dirnames (3)
>>> does not match number of options (2)
>>> options: m64/m31
>>> dirnames: 64 32
>>> make[2]: *** [Makefile:2240: s-mlib] Error 1
>>
>> Indeed, sorry for that.
>>
>> I've just sent a fix:
>> https://gcc.gnu.org/pipermail/gcc-patches/2022-November/606887.html
>>
>> Hopefully that one is right
> 
> The buildbot gcc builds are turning green again!
> https://builder.sourceware.org/buildbot/#/changes/13736
> 

Good! I didn't imagine the feedback would be so fast :-)

BTW, I don't remember receiving an email from the buildbot after the 
breakage, does it send such emails for binutils/gdb only?

> Thanks,
> 
> Mark
diff mbox series

Patch

diff --git a/gcc/genmultilib b/gcc/genmultilib
index 1e387fb1589..b5f372c8358 100644
--- a/gcc/genmultilib
+++ b/gcc/genmultilib
@@ -141,6 +141,20 @@  multiarch=$9
 multilib_reuse=${10}
 enable_multilib=${11}
 
+# Sanity check: make sure we have as many dirnames as options
+if [ -n "${dirnames}" ]; then
+    set x $options
+    nboptions=$#
+    set x $dirnames
+    nbdirnames=$#
+    if [ $nbdirnames -ne $nboptions ]; then
+	echo 1>&2 "Error calling $0: Number of dirnames ($nbdirnames) does not match number of options ($nboptions)"
+	echo 1>&2 "options: ${options}"
+	echo 1>&2 "dirnames: ${dirnames}"
+	exit 1
+    fi
+fi
+
 echo "static const char *const multilib_raw[] = {"
 
 mkdir tmpmultilib.$$ || exit 1