OK it actually doesn't build... I'll have a look at it

I think you should probably stick to moving the cfg!(windows) to the runtime check. Moving everything will be a problem, as you're seeing.

That said, part of the reason I never bothered with this is cargo seems to set this properly in my experience. I did cross-compilation builds of rusqlite many times for https://github.com/mozilla/application-services, and its even part of one of the CI tasks there.

This version should now build.
I have also promoted cc to a mandatory dev-dependency, I hope that's something you're fine with (reading your comment, it looks like you preferred avoiding pulling cc as a dependency, but I'm not sure there is an alternative).

I'm interested about your last message. You're talking about CI tests that try to cross-compile rusqlite in application-services, but I couldn't find them there :-( Could you tell me where they are?
I'd like to see whether this current PR is mandatory, or if there are some other ways to tell cargo to properly cross-compile. (Yet, looking at the cargo book, it looks like we won't be able to properly cross-compile unless we get rid of these cfg! macros).

Codecov Report

Merging #961 (0b90002) into master (48e7561) will increase coverage by 0.21%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #961      +/-   ##
==========================================
+ Coverage   77.49%   77.71%   +0.21%     
==========================================
  Files          48       48              
  Lines        5719     5703      -16     
==========================================
  Hits         4432     4432              
+ Misses       1287     1271      -16     

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 48e7561...0b90002. Read the comment docs.

In here somewhere, https://github.com/mozilla/application-services/blob/main/taskcluster/ci/module-build/kind.yml. Note that it goes through https://github.com/mozilla/rust-android-gradle/.

I guess mozilla/application-services#3917 indicates that this no longer works in the CI, but that seems unrelated to the use of these macros in rusqlite, which have been there for quite a while. Also, supposedly the cross compile build still works manually, outside of CI (note that getting that repo setup to build manually is a hassle).

That said, I'm not strictly speaking opposed to these changes, as long as they're minimally disruptive. I don't know that I think requiring vcpkg and cc everywhere is minimally disruptive, though.

Anyway, I think the cfg!(windows) makes sense that it wouldn't work (my experience is that it does, although perhaps that branch of the build script never ran). So if you fix the clippy/rustfmt complaints I'll land this.

I've pushed a few review commits.

Thanks for the link, I would not have found it myself!

Note that (on the master branch, without these changes) running a cross compilation (from Linux to Windows) displays a warning, but does not fail the build:

~/src/rusqlite $ cargo build --target x86_64-pc-windows-msvc --features bundled
warning: sqlite3/sqlite3.c(22525,8): warning: implicit declaration of function 'localtime_r' is invalid in C99 [-Wimplicit-function-declaration]
warning:   rc = localtime_r(t, pTm)==0;
warning:        ^
warning: sqlite3/sqlite3.c(27292,3): warning: '_ReadWriteBarrier' is deprecated: use other intrinsics or C++11 atomics instead [-Wdeprecated-declarations]
warning:   _ReadWriteBarrier();
warning:   ^
warning: /usr/lib/llvm-10/lib/clang/10.0.0/include/intrin.h(176,16): note: '_ReadWriteBarrier' has been explicitly marked deprecated here
warning: __attribute__((__deprecated__("use other intrinsics or C++11 atomics instead")))
warning:                ^
warning: 2 warnings generated.
    Finished dev [unoptimized + debuginfo] target(s) in 0.21s

Maybe that's the reason your cross-compilation CI tests did not report an error?

But this turns into a hard error when I try to use rusqlite as a dependency from another rust crate (or from the tests themselves)

# On a Linux host, without the changes of this MR
~/src/rusqlite $ cargo test --target x86_64-pc-windows-msvc --features bundled
...
  = note: lld-link: error: undefined symbol: localtime_r
          >>> referenced by /home/daladim/src/rusqlite/libsqlite3-sys/sqlite3/sqlite3.c:22525
          >>>               liblibsqlite3_sys-b8a9034a2fdb3b0a.rlib(sqlite3.o):(osLocaltime)
          

error: aborting due to previous error

error: build failed

Any hesitation @gwenn?

Hm, I'm not sure why GHA CI didn't trigger here (the CI in appveyor isn't complete)...