Skip to content

Use R_NoSave over R_Slave for R 4.0+#6017

Merged
jmcphers merged 1 commit intomasterfrom
feature/r-4.0-compilation
Jan 15, 2020
Merged

Use R_NoSave over R_Slave for R 4.0+#6017
jmcphers merged 1 commit intomasterfrom
feature/r-4.0-compilation

Conversation

@jmcphers
Copy link
Member

This is the minimal change required to get RStudio building against R 4.0+. In R 4.0, the field R_Slave was removed from structRstart and replaced with R_NoEcho.

wch/r-source@f1ff49e#diff-178cb445c6da1cdb9d55c898b6118256R70

Since RStudio references this field, and we would like to be able to build against both R 4.0 and R 3.x (largely for ease of development and testing), we need to refer to the field by the appropriate name.

The irony of using --slave to determine whether we need to use R_Slave or R_NoEcho is not lost. R 4.0 (currently) only soft-deprecates --slave, whereas R_Slave has been simply removed.

Related to #5923. We will need to make another (significantly more invasive) change when R fully removes --slave.


# find the version of R in play
find_package(LibR REQUIRED)
execute_process(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we check for / report an error here if one occurs? (Basically signal for future-us that --slave is really gone now)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will already happen elsewhere:

# verify we got the required R version
if(LIBR_FOUND AND RSTUDIO_VERIFY_R_VERSION)
set(CMAKE_REQUIRED_INCLUDES ${LIBR_INCLUDE_DIRS})
execute_process(
COMMAND "${LIBR_EXECUTABLE}" "--vanilla" "--slave" "-e" "cat(as.character(getRversion()))"
OUTPUT_VARIABLE LIBR_VERSION)
if (LIBR_VERSION VERSION_LESS "3.0.1")
message(FATAL_ERROR "Minimum R version (${RSTUDIO_R_MAJOR_VERSION_REQUIRED}.${RSTUDIO_R_MINOR_VERSION_REQUIRED}.${RSTUDIO_R_PATCH_VERSION_REQUIRED}) not found.")
endif()
endif()

If you're wondering why we don't piggyback on this check, it's because the r/ folder is a separate CMake project which has no visibility of the parent project's variables.

@jmcphers jmcphers merged commit 60dd6e3 into master Jan 15, 2020
@Enchufa2
Copy link
Contributor

Are you going to backport this to the v1.2 branch?

@yurivict
Copy link

yurivict commented May 9, 2020

Please make a release because the currently released 1.2.5042 isn't compatible with R-4.0.0.

@jmcphers
Copy link
Member Author

We're working on it (should be within a few weeks).

@Jylpah
Copy link

Jylpah commented Jul 8, 2020

I am receiving the same error with 1.3.959 so is this still open? WSL 2, Ubuntu 20.04LTS, 64bit

/usr/local/src/rstudio-1.3.959/src/cpp/r/session/REmbeddedPosix.cpp:107:8: error: ‘struct structRstart’ has no member named ‘R_Slave’
  107 |    Rp->R_Slave = FALSE ;
      |        ^~~~~~~
make[2]: *** [src/cpp/r/CMakeFiles/rstudio-r.dir/build.make:544: src/cpp/r/CMakeFiles/rstudio-r.dir/session/REmbeddedPosix.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:647: src/cpp/r/CMakeFiles/rstudio-r.dir/all] Error 2
make: *** [Makefile:152: all] Error 2

@kevinushey
Copy link
Contributor

This should be fixed in the v1.3 patch release:

#if R_VERSION < R_Version(4, 0, 0)
Rp->R_Slave = FALSE;
#else
Rp->R_NoEcho = FALSE;
#endif

@valerie-rstudio valerie-rstudio deleted the feature/r-4.0-compilation branch January 21, 2022 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants