On (26/09/16 21:26), Lukas Slebodnik wrote: >On (26/09/16 12:14), Fabiano Fidêncio wrote: >>Jakub, >> >>On Mon, Sep 26, 2016 at 11:35 AM, Jakub Hrozek <[email protected]> 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 <[email protected]> >>>> 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. > BTW Stew Walter wrote similar patch for sssd as well. It was much simpler.
https://lists.fedorahosted.org/archives/list/[email protected]/message/GI2XSYNUOCLNPJYLNVPDP4J2GSWX4MIW/attachment/2/0001-configure-Enable-usual-SSSD_WARNINGS-automatically.patch https://lists.fedorahosted.org/archives/list/[email protected]/message/GI2XSYNUOCLNPJYLNVPDP4J2GSWX4MIW/ LS _______________________________________________ sssd-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
