[openssl-dev] who wants to fix travis builds?

[Alessandro Ghedini]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://mta.openssl.org/pipermail/openssl-dev/attachments/20150928/72cd5b1b/attachment-0001.sig>