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

Andy Polyakov appro at openssl.org
Mon Sep 28 18:49:12 UTC 2015


>> 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.

Yes, I've figured out that and a patch is submitted to internal system.
So in sense this becomes bad example to underline the point I was trying
to make. And the point is that we don't want to become hostages of bugs
elsewhere :-)

>> - 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).

Cool! Thanks for the hint!

> 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.

Technical solution/workaround is already submitted, but my comment was
more about *formal* impossibility to solve *all* possible problems
(GetProcAddress being *an* example) according to the letter of the
standard in true neutral way. Because there *is*
system-/implementation-specific code and we can't/shouldn't pretend that
there is none.

>> 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.

--strict-warnings imply -Werror and then it's rendered all-or-nothing. I
mean you can't ignore -Werror failure, because binary code is not
generated. On the other hand generating warnings without -Werror is
hardly meaningful in CI scenario, because you don't look at log unless
there is problem. So it makes sense to bet on -Werror only if one is
confident about it being representative, and the problem is that there
is no guarantee that it's universally representative. Hence it can be
exercised only is specific cases.

>> 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.

Can you test and figure out? As implied, currently new 'make test'
doesn't work with 'wine' yet, but you can replace 'make test' with e.g.
test/sha1test.exe alone. Just to figure out if it works. It might happen
that it's not sufficient to simply install package, one might have to
perform per-user config prior Win32 .exes can be executed.

Thanks!



More information about the openssl-dev mailing list