Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows? #14

Closed
anderic1 opened this issue May 23, 2020 · 17 comments
Closed

Windows? #14

anderic1 opened this issue May 23, 2020 · 17 comments

Comments

@anderic1
Copy link

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
Copy link
Owner

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

  • configure.win is a good point, I did not do that. OTOH we probably do not even need it as we can just write down what we know is fixed given the toolchaim
  • cleanup.win ? Seriously? Never tried. cleanup without an extension also runs
  • OS_type: unix is what I removed, but that may not have been enough

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

@eddelbuettel
Copy link
Owner

(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
Copy link
Collaborator

knapply commented May 23, 2020

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

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

@eddelbuettel
Copy link
Owner

eddelbuettel commented May 23, 2020

@knapply Not sure I follow:

  • no change needed for SystemRequirements (and in src/Makevars.win anyway)
  • src/Makevars.win.in is wrong (as foo.in gets converted to foo by configure but we do not run configure here)
  • no idea what you want to say with this third point (as I missed that it was meant to be a link to a GH Actions build, my bad)

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
Copy link
Collaborator

knapply commented May 23, 2020

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
Copy link
Owner

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
Copy link
Collaborator

knapply commented May 23, 2020

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
Copy link
Owner

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
Copy link
Author

anderic1 commented May 24, 2020

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

  • configure and cleanup (and the .win equivalents) can indeed be removed altogether if Makevars.in is renamed Makevars and the relevant line changed to CXX_STD = CXX17. I don't know if this is true on all platforms.
  • Regarding the 32-bit issue: as alluded to by Dirk, this was discussed in R-pkg-devel and adding a Archs: x64 to the DESCRIPTION file might help. It will not make the build pass but at least it will give an indication.

@eddelbuettel
Copy link
Owner

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
Copy link
Author

anderic1 commented May 24, 2020

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
Copy link
Collaborator

lemire commented May 24, 2020

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
Copy link
Owner

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
Copy link
Collaborator

lemire commented May 25, 2020

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
Copy link
Collaborator

lemire commented Jun 27, 2020

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
Copy link
Owner

eddelbuettel commented Jun 27, 2020

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:

  • checking installed package size ... NOTE
    installed size is 7.6Mb
    sub-directories of 1Mb or more:
    jsonexamples 3.8Mb
    libs 2.8Mb

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants