On (26/09/16 12:14), Fabiano Fidêncio wrote: >Jakub, > >On Mon, Sep 26, 2016 at 11:35 AM, Jakub Hrozek <jhro...@redhat.com> wrote: >> On Mon, Sep 05, 2016 at 03:39:19PM +0200, Fabiano Fidêncio wrote: >>> On Thu, Aug 11, 2016 at 2:33 PM, Fabiano Fidêncio <fiden...@redhat.com> >>> wrote: >>> > Howdy! >>> > >>> > I've suggested, a long time ago, that we could start making use of >>> > GNULIB's compiler warnings from 'manywarnings' module. This is >>> > basically what we have been doing in a few projects that I (used to >>> > and still) maintain (like spice-gtk and libosinfo, for instance). >>> > >>> > For now I didn't try to fix any of the warnings that we cannot cope >>> > with, mainly because I'm not sure whether you guys will agree on using >>> > it or not. >>> > >>> > Here is an experimental patch that works properly on Fedora 24. I >>> > still have to make some tests on RHEL-6, RHEL-7 and a few other >>> > systems (Debian, at least) in order to make sure that we won't break >>> > the build because of the patch. >>> > >>> > If you are okay with the change, I'll start going through the warnings >>> > that we cannot cope with and slowly start fixing them. Although, I >>> > have the feeling that fixing some of them would cause a lot of >>> > undesired changes, which will just bring troubles for ourselves when >>> > backporting fixes downstream (and here I'm talking about >>> > -Wformat-signedess, -Wsign-compare, -Wunused-parameter, ... for >>> > instance). >>> > >>> > I'm looking forward to hear some feedback! >>> > >>> > Best Regards, >>> > -- >>> > Fabiano Fidêncio >>> >>> ping? >> >> I'm sorry this patch totally stalled. >> >> In general I'm all for adding more checks and let a machine help us >> write safer code. I'm not sure if adding all warnings on all platforms >> will ever be possible, though. For example, there was a warning in >> krb5_child because an old libkrb5 release used a "char *" parameter >> where a "const char *" was more appropriate and we said we'd never fix >> this because a newer version fixed the API (or used a different >> function? Not sure..) >> >> But I wonder how to move this patch forward the best. Are there any >> warnings that you think are more important to enable than others? >> >> What about enabling a single warning, then running SSSD build and >> creating a ticket with the warnings (running make 1</dev/null might be a >> good way to start..). Then we can see what needs fixing in SSSD for each >> of the additional warnings or if we decide this is not worth our time, >> we can either close or defer that ticket. >> >> These tickets might also be a good way for a newcomer to contribute some >> code to SSSD! > >So, general comments here. > >We have a list of the current warnings that we cannot deal with in >src/sssd-compile-warnings.m4. From this list we can, already, have a >good idea about what is worth to fix or not. > >About the idea to try to build SSSD ... well, it can be done. >I'll take some time later on and prepare a bug for each of the >warnings that we can't cope with and them we can discuss there whether >would make sense to fix those or not. > Here are few of my observation. The patch added 255 new configure time checks.
The configure tooks 5 extra seconds in my case (everything in ram-disk). But in our CI machines it added +20 seconds in average. The configure scriot is execute 5 times in our CI-script (debug build, distcheck, intgcheck, mock, coverage build) In Summary, the patch would add unnecessary 100 seconds. I would prefer if all compile time warnings were part of AM_CFLAGS and we shoul add compile time checks only for warnings which are supported by new compilers or are supported only by gcc or only by clang. LS _______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
