search

eddelbuettel / rcppsimdjson

Rcpp Bindings for the 'simdjson' Header Library
116 stars 13 forks source link

Windows? #14

Closed anderic1 closed 4 years ago

anderic1 commented 4 years ago

Copying and adding a .win extension to configure and cleanup, and removing the line OS_type: unix from DESCRIPTION makes it compile on windows with the new toolchain. Would you be interested in a PR ?

Thanks

eddelbuettel commented 4 years ago

I actually (very briefly) tried that yesterday and failed. Maybe I skipped a step.

Did you build locally or did you ship to win-builder or rhub?

eddelbuettel commented 4 years ago

(Apparently I do have three cleanup.win here in various git repos, including some where I am author or co-author. The things one relearns ;-) )

knapply commented 4 years ago

This should do the trick: https://github.com/knapply/rcppsimdjson/tree/b35379054de99ad9a04a503aca6e05dbadf2d9b9

Specifically just these files (no cleanup.win)...

eddelbuettel commented 4 years ago

@knapply Not sure I follow:

Communication "in prose" is hard about code. Maybe we can in diffs like normal coders? ;-)

I tossed a first build pending at win-builder, it died on "cannot do 32bit..." For ~ 72 hours at https://win-builder.r-project.org/1bm4Q74hJG8N and https://win-builder.r-project.org/x69AUDDrOGD2

knapply commented 4 years ago

Apologies. It was an attempt to point anyone working on it in a direction that seems to work (stop-gap, as I don't have a proper diff handy). The 3rd point was only that it does build and pass.

The solution to the 32 bit issue is passing --no-multiarch to R CMD check. Otherwise it will try to run on both the 32 bit (which simdjson doesn't support) and 64 bit versions that ship for Windows (instead of only the "main version"). The bottom of Checking packages has more details under "Multiple sub-architectures".

I'm not certain at the moment how that's done for https://win-builder.r-project.org (or whether we can expect the 64-bit version of R to always be the default... meaning -no-multiarch is sufficient ).

eddelbuettel commented 4 years ago

No worries, also had three balls in the air.

Thanks for the reminder about --no-multiarch. I keep seeing it (e.g. as a default arg tossed on by RStudio) and still forgetting about it.

I think we are NOT allowed to skip 32 bit builds so that would be a roadblock for CRAN, no? I have the feeling that came up recently on r-package-devel.

knapply commented 4 years ago

Adding Biarch: FALSE to the DESCRIPTION seems relevant, but I can't tell if it's actually similar to --no-multiarch. check doesn't seem to respect it as such, but it's hard to say locally.

I think we are NOT allowed to skip 32 bit builds so that would be a roadblock for CRAN, no? I have the feeling that came up recently on r-package-devel.

That would be disappointing. A quick search of related terms didn't find the relevant thread, but that doesn't mean much.

eddelbuettel commented 4 years ago

Was worth a try, but it failed the same way at win-builder.

Re-checking WRE it actually says

The 'Biarch' logical field is used on Windows to select the 'INSTALL' option '--force-biarch' for this package.

And was you said

That would be disappointing.

Indeed. We could get more serious about it and (ahem, cough, cough) simply make 32-bit "empty". Look closely at what I did for the top of src/simdjson_example.cpp. It is basically already only real code when a C++17 compiler is met. We could simply OR that test with a test for 32bit windoze...

anderic1 commented 4 years ago

I am building locally, with the --no-multiarch flag set (rstudio).

eddelbuettel commented 4 years ago

Yes but

  1. We cannot upload to CRAN without 32 bit builds; no-multiarch is not supported there
  2. Your point is subtly wrong. The fixed filed for Windows has to be called src/Makevars.win as the other platforms still derive src/Makevars from src/Makevars.in by means of configure
  3. Thanks for the reminder on Gabor's post. Declaring it is fine; overall it is still a band-aid because what we really want it to have binaries for Windows on CRAN. Which we can't have :-/

But I agree with you: the repo should make it easier for users like to build a binary, and maybe we can even look into creating a Windows binary (for 64 bit only) using win-builder and serving it via drat.

anderic1 commented 4 years ago

Ok I see. Too bad hopefully they will evolve. Meanwhile, this is more of a (very-)nice-to-have than a must-have, there are other solutions on windows. Feel free to close

lemire commented 4 years ago

Though we do not advertise it, simdjson works under Win32 (in master). And we test it.

It puzzles me, however, that people feel the need to use Windows 32-bit.

eddelbuettel commented 4 years ago

Interesting.

CRAN's position is that some vendor-supplied software -- think odbc drivers -- still ships 32 bit only so they don't want to turn it off.

lemire commented 4 years ago

I think that 32-bit support should be at least deprecated. Microsoft is phasing out 32-bit windows: New Windows 10 PCs, starting with the May 2020 Update/2004, will be able to run 64-bit Windows 10 only.

But anyhow, yes, we support 32-bit Windows... We have that in CI.

lemire commented 4 years ago

As of simdjson 0.4.1, we can compile and pass tests under x86 (32-bits) using both GCC 7 and clang 6. So you should be able to build simdjson under mingw32.

eddelbuettel commented 4 years ago

Very nice, and confirmed! Tossed a first build at builder.r-hub.io and it passed, see
https://builder.r-hub.io/status/RcppSimdJson_0.0.6.1.tar.gz-b273f84ba5a7470b939ac22fbfbc7124

Or by email report back:

RcppSimdJson 0.0.6.1: NOTE

Build ID: RcppSimdJson_0.0.6.1.tar.gz-b273f84ba5a7470b939ac22fbfbc7124 Platform: Windows Server 2008 R2 SP1, R-release, 32/64 bit Submitted: 5 minutes 30.2 seconds ago Build time: 5 minutes 12.5 seconds

NOTES:

See the full build log: HTML: https://builder.r-hub.io/status/RcppSimdJson_0.0.6.1.tar.gz-b273f84ba5a7470b939ac22fbfbc7124 Text: https://builder.r-hub.io/status/original/RcppSimdJson_0.0.6.1.tar.gz-b273f84ba5a7470b939ac22fbfbc7124 Artifacts: https://artifacts.r-hub.io/RcppSimdJson_0.0.6.1.tar.gz-b273f84ba5a7470b939ac22fbfbc7124

Have questions, suggestions or want to report a bug? Please file an issue ticket at GitHub at https://github.com/r-hub/rhub/issues

Thank You for using the R-hub builder.

(c) 2016 The R Consortium

eddelbuettel commented 4 years ago

Coming in version 0.1.0 in a few days, see #28 for more.

In the meantime just build from the main branch in the repo and/or fetch a binary from the rhub log above (though those only stay around for a day or two...)