shssoichiro / oxipng

Multithreaded PNG optimizer written in Rust
MIT License
2.89k stars 125 forks source link

Add Windows builds to rust everywhere #49

Closed shssoichiro closed 7 years ago

shssoichiro commented 8 years ago

This is a followup to #34. @japaric, @TPS suggested that you could help. Do you have experience with deploying rust-everywhere for Windows on Travis CI? The Travis docs seem to indicate that Windows isn't a supported platform. Is the workaround then to use Appveyor for Windows builds?

japaric commented 8 years ago

Do you have experience with deploying rust-everywhere for Windows on Travis CI?

Not with rust-everywhere. rust-everywhere specifically uses appveyor because it's easier to build and test rust programs for Windows via native compilation.

However, it is possible to cross compile for the x86_64-pc-windows-gnu on Linux using Travis. See 1 and 2 for an example.

Still, I would advise using rust-everywhere AppVeyor template instead of trying to do the Windows builds on Travis CI as the former approach is known to work.

shssoichiro commented 8 years ago

Thanks! I'll give Appveyor a try, then.

TPS commented 8 years ago

So, not seeing anything on https://github.com/shssoichiro/oxipng/releases for x86/x64 since adding https://github.com/shssoichiro/oxipng/commit/ce971cc665451a59863f95289992fe938e3d5843. Does it take a lot of time? I'd like to play with the binaries some before specifically pointing @javiergutierrezchamorro toward them.…

shssoichiro commented 8 years ago

It requires that I push a new tag first. I will go ahead and push a test tag to see if the build succeeds, and you can play with the binaries from that.

shssoichiro commented 8 years ago

The windows-gnu builds failed, but the windows-msvc builds appear to have passed and are on the releases page. Is there a reason you need the windows-gnu binaries? I may just remove the windows-gnu builds rather than try to fix them, as msvc generally produces faster binaries on Windows anyway.

TPS commented 8 years ago

Mostly for the sake of compatibility & options, since FileOptimizer is run across a large breadth of platforms. I'll try the current binaries for a bit & report back w/in a few days, (I'll want to stress-test & compare, & that'll take a few k files to do) unless a major problem crops up 1st; then, sooner.

TPS commented 8 years ago

Well, here's a problem, but I'll let you judge how major: vcruntime140 dll This (& all similar) dependencies really should be removed, or, failing that, compiled in statically, as it's not readily part of Window, & makes the tool less attractive, as OptiPNG has no such requirement.

shssoichiro commented 8 years ago

I'm not sure how easy this is to resolve in Appveyor, but I would also prefer to have Windows binaries compiled statically. I'm much less familiar with compiling Rust binaries on Windows and don't have a Windows machine readily available, so I'll have to fire up a virtual machine and experiment.

Worst case scenario, I'll need to add an instruction for Windows users to install the Visual C++ 2015 runtime. I'd prefer to avoid that though. This also likely won't be a problem with the GCC-compiled version, but this runtime is a dependency for all programs compiled by MSVC.

shssoichiro commented 7 years ago

I'm going to close this along with adding instructions to the readme for Windows users to ensure the Visual C++ runtime is installed. I've decided not to go forward with fixing the Windows-GCC builds, because it seems like an uncommon use case to need both the GCC and MSVC builds.

Connicpu commented 7 years ago

If you add the environment variable RUSTFLAGS="-C target-feature=+crt-static" when compiling, it will completely drop the dependency on vcruntime.dll and statically link. Rust is basically only pulling in memcpy/memset/etc. Not really things you'd be worried about statically linking.

TPS commented 7 years ago

@shssoichiro Do let me know if you get the static binary built via RUSTFLAGS="-C target-feature=+crt-static". (Thanks very much, @Connicpu! 🙇) I'll happily test it! 😃

shssoichiro commented 7 years ago

@TPS I have an appveyor-built i686 build with crt-static available here for testing: https://github.com/shssoichiro/oxipng/releases/download/1.0.86/oxipng--i686-pc-windows-msvc.zip (edit: x64 here: https://github.com/shssoichiro/oxipng/releases/download/1.0.86/oxipng--x86_64-pc-windows-msvc.zip)

TPS commented 7 years ago

I'm running x64 now overnight w/ -o 6 --zw 32k --zm 1-9 -Z -v -a -b --fix -p -s (I know some of these are mutually exclusive, but why not shake every bug I can attempt @ once? 😜), & will report back when done.

Thanks!

Connicpu commented 7 years ago

Can confirm with Dependency Walker, it no longer depends on any CRT DLLs ^_^ Dependency Walker

TPS commented 7 years ago

The previous run completed perfectly (no crashes, &c), but now I'll re-run w/o -Z, so w/ -o 6 --zw 32k --zm 1-9 -v -a -b --fix -p -s, & report in again.

@javiergutierrezchamorro OxiPNG looks like it's ready for inclusion (instead of OptiPNG) in FileOptimizer (especially since it includes Zopfli! 😄) if it passes my current run.

@shssoichiro Does OxiPNG use @Google's official Zopfli or something like @MrKrzYch00's optimized version?

TPS commented 7 years ago

This run completed perfectly, also, so this issue looks completely resolved! Thanks, again, @shssoichiro & @Connicpu! 🙇