On Sun, Sep 27, 2015 at 10:32:11am +0200, Andy Polyakov wrote: > >>> - mingw debug and shared builds in master. > >> > >> While I can confirm problem with shared (fixable with attached > >> patch, please double-check), I can't confirm problem with debug > >> (please elaborate). > > > > I just opened a pull request on GitHub [0] to add your patch for > > mingw shared builds and another one to fix clang debug builds for > > master. Let's see what Travis thinks of it. > > The other modification was submitted by Emilia to internal system 4 days > ago. Or in other words fixes will show up. > > But I'd like to use this opportunity to challenge the choice of using > --strict-warnings in [all] CI scenarios. Two points. > > - Just like everything else warning system is subject to bugs and is a > changing field. For example -Wshadow complains about local variables > shadowing functions. It appeared in specific gcc version and then was > removed. Then mingw compiler complains about missing prototypes in a > number of *_asn1 modules. I fail to understand why only mingw compiler.
Regarding the missing prototypes, I think it's because (some?) Windows builds have OPENSSL_EXPORT_VAR_AS_FUNCTION defined, while normal builds don't. I haven't tried to build the code with normal GCC with that defined though so I don't know if that would fail on with non-mingw. > - While strict adherence to every letter of the standard is a good > thing, some code is (and always will be) system-specific and it might > be impractical to treat it otherwise. For example mingw compiler > complains about %I64i format string being non-standard. There is > nothing that can be done about it (except for implementing own > subroutine, but that would be definition of impractical). We could use -Wno-pedantic-ms-format for that (disabling the warning completely). As you said, there's nothing we can do about it. > Another > example is complain about assignment of getprocaddress to void *. > Well, it is meaningful warning and we do address it in other non-Win32 > places. But how do you handle it in truly standard manner on 32-bit > Win32? When you can use GetProcAddress to pull references to either > cdecl or stdcall, per-definition implementation-specific thing? The return value type of GetProcAddress is documented as being FARPROC, so using that may work. I actually fixed this warning in my famous patch using that, but I can't really say if the change was correct or not. > Note that I'm not saying that warnings should not be fixed. I'm saying > that it's impractical to treat all of them equally (not to mention that > there are legitimate false positives after all), and CI might be not the > right place to catch them. I would even say that on CI it is more > valuable to aim for running tests than to stop on warnings. Or maybe > it's not mutually exclusive? FWIW, Travis CI allows you to define specific builds to be "non-fatal". The failures would still be listed but they wouldn't affect the general state. See for example: https://travis-ci.org/openssl/openssl/builds/82401235 (the last two builds fail but are in the "Allowed Failures" section and the general build state is green). So we could add another debug-only configuration that is not allowed to fail, and mark the current debug+strict-warnings builds on mingw as non-fatal. This way the non-fatal builds are still run and someone can have a look at them and try to fix them if they want. > As for running tests in the context of query, i.e. mingw cross-compilation on > Linux. It actually was possible to perform under 'wine' before and surely can > be fixed now. Is 'wine' an option in this case? I don't know if it actually works, but we can configure Travis CI to install wine before starting the builds. > > Mingw's debug builds are still broken, but now the following error > > message is shown: > > > >> cc1: error: command line option ‘-foutput-class-dir=-DL_ENDIAN’ > >> is valid for Java but not for C [-Werror] > > > > The problem is that `-d` is a `./config` option, but in the case of > > mingw it is passed directly to `./Configure` which thinks it's a > > compiler flag so then gcc gets confused. A solution would be to > > somehow detect the mingw cross compilation from `./config` so that > > we would use it for mingw builds too, but I'm not sure what the > > best way to do that would be (and on top of this we still have the > > mingw warnings problem). > > I don't understand. Last modifications to .travis.yml switch from > ./config to ./Configure on mingw, so that where does this ./config > passing -d to ./Configure come from? I'd say that the switch is > appropriate, because ./config was never meant and is hardly proper > choice for cross-compiler cases. Yeah, but now we pass -d to ./Configure for mingw builds. -d is a ./config-only option, while --debug is the ./Configure one (which only works on master). I think we should restore '--debug' for builds on master (where it worked fine), and just disable mingw debug builds on older branches. Cheers
signature.asc
Description: PGP signature
_______________________________________________ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
