This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Add parameter to allow enabling/disabling selftests via configure
- From: Sergio Durigan Junior <sergiodj at redhat dot com>
- To: Simon Marchi <simark at simark dot ca>
- Cc: GDB Patches <gdb-patches at sourceware dot org>, Pedro Alves <palves at redhat dot com>
- Date: Wed, 10 Oct 2018 16:31:04 -0400
- Subject: Re: [PATCH] Add parameter to allow enabling/disabling selftests via configure
- References: <20180814054221.13061-1-sergiodj@redhat.com> <20180917202242.28583-1-sergiodj@redhat.com> <d84a5691-166f-e252-fcb3-a247720e492e@simark.ca>
On Friday, October 05 2018, Simon Marchi wrote:
> On 2018-09-17 4:22 p.m., Sergio Durigan Junior wrote:
>> This is a follow-up of:
>>
>> https://sourceware.org/ml/gdb-patches/2018-08/msg00347.html
>>
>> Instead of going throttle and always enabling our selftests (even in
>> non-development builds), this patch is a bit more conservative and
>> introduces a configure option ("--enable-unit-tests") that allows the
>> user to choose whether she wants unit tests in the build or not. Note
>> that the current behaviour is retained: if no option is provided, GDB
>> will have selftests included in a development build, and will *not*
>> have selftests included in a non-development build.
>>
>> The rationale for having this option is still the same: due to the
>> many racy testcases and random failures we see when running the GDB
>> testsuite, it is unfortunately not possible to perform a full test
>> when one is building a downstream package. As the Fedora GDB
>> maintainer and one of the Debian GDB uploaders, I feel like this
>> situation could be improved by, at least, executing our selftests
>> after the package has been built.
>>
>> This patch introduces no regressions to our build.
>
> Hi Sergio,
Hi Simon,
Thanks for the review.
> I think having an configure switch with that default value makes sense.
>
> I have two suggestions:
>
> - Mention the switch explicitly in the error message (--{enable,disable}-unit-tests)
> so the user knows exactly what you are talking about.
> - Share the code between gdb's and gdbserver's configure script
>
>> +`--enable-unit-tests[=yes|no]'
>> + Enable (i.e., include) support for unit tests when compiling GDB
>> + and GDBServer. Note that if this option is not passed, GDB will
>> + have selftests if it is a development build, and will *not* have
>> + selftests if it a non-development build.
>
> Missing a word: "if it is a".
>
> Here's a patch that goes on top of yours that does what I suggest (I had to try it
> before suggesting it to make sure I don't ask for the impossible, so I might as well
> share it). With this (especially the code-sharing part), the patch LGTM (the
> ChangeLog entry would need to be updated to account for the new changes).
Fair enough. I applied your patch, updated the ChangeLog, and pushed
it.
8ecfd7bd4acd69213c06fac6de9af38299123547
Thanks,
--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/