Fix MinGW cross

[Ericson2314]

The batch file was very unnecessary.

[facebook-github-bot]

Hi Ericson2314! Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[Ericson2314]

Should be all good now

[Ericson2314]

CC @vtorri @jdxAtGitHub

[vtorri]

it would be nice to fix import lib name : see issue #1877

[Ericson2314]

@vtorri it would, but I would do that separately as I was able to link fine with this name.

[vtorri]

the name of the import lib can have .lib extension, but the static lib can also have .lib extension. That's what visual studio does, but it is not a good idea. libtool, and the vast majority of open source project uses .dll.a as extension to enforce the fact that it's the import lib

ok to do that separately

[Cyan4973]

Looks good to me !

The only point I wonder is : how to test this capability ?

I believe it's an important point, otherwise it would be easy to break this capability in any future patch.

I guess this should be done on AppveyorCI, our current Windows based CI ?

[Ericson2314]

Do what https://github.com/facebook/rocksdb does, it is also a Facebook project so shouldn't be too dificult. MinGW cross will catch a few more things than MinGW native too.

https://github.com/facebook/rocksdb/blob/master/.travis.yml#L115-L116

I unfortunately don't have much more time to put into this, sorry, but I also think these fixes won't bit-rot too badly, which makes it less urgent.

[vtorri]

i'm using zstd in my package manager and i test each new release with MSYS2 + mingw-w64, so i also don't think it will bitrot

[Cyan4973]

In https://travis-ci.org/facebook/zstd/jobs/617904275, I've added a mingw cross-compilation test based on the suggested RocksDB's script to the travisTest branch, in preparation for this patch.

As expected, if fails, since it doesn't include this patch :

timefn.h:44:47: fatal error: Windows.h: No such file or directory
https://travis-ci.org/facebook/zstd/jobs/617904275#L343

I guess it's the issue of casing that this patch fixes.

Also notable, library compilation doesn't seem to like the -fPIC directive for mingw cross-compilation.
That's a problem if one wants to neutralize any kind of warning during CI tests, which is generally a good policy. In which case, this issue should be fixed too.
Another method could be to restrict the scope of the test, and by extension of the patch : is it just meant to compile only the CLI in /programs ? Or is library compilation also part of the scope ?

[vtorri]

always use lowercase header file name on windows for cross compilation

[Cyan4973]

The mingw cross-compilation test passes after this patch.
Still, it doesn't solve the -fPIC warning for the library,
so I think I will restrict the scenario to compilation of the CLI only.
This will make it possible to enforce the strictest zero-warning tolerance policy, associated with an extended list of correctness compilation flags.

[Cyan4973]

redirecting the PR towards travisTest, where the mingwcross-compilation test is present.

[Cyan4973]

Well, I was expected CI tests to be triggered after the change of destination branch, but apparently, they are not.

Merging. Tests to be continued onto travisTest branch.

[Ericson2314]

Can you make an issue for the testing the library? I was building this to build rocksdb in NixOS/nixpkgs#73316, so I can assure you the library is being cross compiled in practice.

[Cyan4973]

The CLI-limited mingw cross-compilation test has been added in #1910.

Compiling the library is likely to work fine too, it's just that it generates a ton of -fPIC warnings.
The issue would be to find a way to avoid these warnings, so that CI test results can remain clean.

[Ericson2314]

Right, the issue would mention the PIC warnings as the blocker.

[Cyan4973]

#1911

[Ericson2314]

Thanks! Subscribed