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

Support for R-4.2.0 #4763

Closed
wants to merge 13 commits into from

Conversation

amirmasoudabdol
Copy link
Contributor

@amirmasoudabdol amirmasoudabdol commented Apr 25, 2022

For the most part, I expect things to work. but on Windows, I'm running to some issues compiling R-Interface. I need to investigate this a bit more, but I feel we need to add a few extra flags for the compiler to be more specific, and I've not found them so far.

In addition to upgrading R-4.2.0, this PR includes the following:

  • CMake now downloads the gfortran from statics.jasp-stats.org as well
  • MSYS2 will be replaced by Rtools42. This was not possible before, but the new Rtools is much nicer to work with, and it can be used to install new Boost, jsoncpp, etc.
    • All the references to the MINGW are (will be) replaced by RTOOLS
  • Fixed a few bugs that could have made CMake picking up the wrong library when building the R-Interface.
    • Specifically, I'm not linking to the libboost_nowide directly instead of relying on CMake to find it.
  • Replaced the JAGS pre-built with the newer version built using Rtools42

Know Issues,

  • On macOS, if any of the jaspBase requirements need building then the install-jaspBase.R will fail, and therefore the build fails. This is due to the strict signing requirement on macOS; and it is most likely going to be resolved by more packages having their build on ARM sort out.
    • Building fails because as we install packages, we patch them, therefore breaking their signature and since unlike jaspBase::installJaspModule where we sign them using the JASPEngine right away, here, install-jaspBase.R cannot do that, and therefore we have an unsigned library which R cannot load.
    • @vandenman proposed that we can take out the installJaspModules and make it a standalone script that we can just use in install-*.R files. I think this is a good idea, and will resolve the issue but it might need to wait for now.

Things to do,

  • Since, we have the UCRT build, we can probably remove bunch of legacy code; but I don't think I have time for that. I make sure that the build is smooth, and leave the rest to others.
    • For instance, I'm not sure if we still need the Boost::nowide or not.

For the most part, most of the CMake works, but R-Interface cannot be complied. I guess this is due to the changes to the UCRT64, and it can be resolved by setting one of the compilations flags somewhat stricter. I'm going to investigate that next.
@amirmasoudabdol amirmasoudabdol added the Layer: Build System Issues related to the the build system, or installer on different system label Apr 25, 2022
@amirmasoudabdol amirmasoudabdol self-assigned this Apr 25, 2022
Instead of asking CMake to find Boost when building R-Interface, I'm now only check if `libboost_nowide` exists, and only links that. This also removes a warning that we had before. I'm not sure, but this might have even solved a silent bug where there was a mixup linking to Boost::nowide. In addition, I made the R-Interface build more verbose.
@amirmasoudabdol amirmasoudabdol marked this pull request as ready for review April 27, 2022 07:23
@amirmasoudabdol
Copy link
Contributor Author

As I was expecting, some of the changes here are fixing the issue that Koen ran into. While I'm still not 100% sure, I suspect that part of the issue was that when passing the build to the R-Interface, CMake was getting confused a bit on what's what, and was picking up the wrong Boost, and therefore R-Interface was ended up having slightly different ABI.

In this situation, I basically don't trust CMake to pick up the right Boost, and only check if libboost_nowide is there, and if it's there, and since that's the only thing we need, I just pick that one up, and link to it. So, I can be sure that I'm looking into the right place. If you decided that you don't want to go with the R-4.2, you can carefully cherry pick these changes, and reflect it to the development.

@amirmasoudabdol amirmasoudabdol changed the title [WIP] Support for R-4.2.0 Support for R-4.2.0 Apr 28, 2022
- They are still broken, but I've removed some of the old configs, and set it up with Conan in mind.
- For some reason, Conan cannot build the `libiconv` on macOS! 🤷🏻‍♂️
- and on Windows, MERGE_MODULES cannot be found because of different PATHing for VS
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Layer: Build System Issues related to the the build system, or installer on different system
Projects
None yet
2 participants