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

[R-package] allow R to pick between -fPIC and -fpic for CRAN package #3261

Merged
merged 3 commits into from
Jul 29, 2020

Conversation

jameslamb
Copy link
Collaborator

I think we can remove setting CPICFLAGS in Makevars.in and Makevars.win.in in the R package. I noticed in the logs from R Hub (#629 (comment)) that my choioce was being ignored anyway...-fpic is used in many of the jobs.

Re-reading "Writing R Extensions", I see that I probably should have been using CXX11PICFLAGS.

In this PR, I propose just removing CPICFLAGS from these Makefiles and using whatever values R chooses based on the environment it is installed in. That should slightly simplify our configuration and reduce the risk of the CRAN package failing checks on the many obscure environments CRAN uses for testing.

@jameslamb jameslamb added the fix label Jul 29, 2020
@jameslamb jameslamb requested review from guolinke and StrikerRUS July 29, 2020 04:56
@jameslamb jameslamb requested a review from Laurae2 as a code owner July 29, 2020 04:56
@jameslamb jameslamb added maintenance and removed fix labels Jul 29, 2020
@StrikerRUS
Copy link
Collaborator

I'm glad to see configuration simplification! But I'm not happy with that we are getting more and more differences in code compilation for manual builds and CRAN ones. This means we will have to ask users how they installed LightGBM to debug reported issues.

@StrikerRUS
Copy link
Collaborator

According to the GitHub Actions logs from CRAN jobs, -fpic is set by default for Linux and macOS and isn't set for Windows.

image

image

image

@jameslamb
Copy link
Collaborator Author

I'm glad to see configuration simplification! But I'm not happy with that we are getting more and more differences in code compilation for manual builds and CRAN ones. This means we will have to ask users how they installed LightGBM to debug reported issues.

I think we will already have to do that anyway, and that the maintenance burden is well worth it. I think our current build process involving cmake can be quite intimidating for the typical R user who is used to just running install.packages()

@StrikerRUS StrikerRUS merged commit ff85e59 into microsoft:master Jul 29, 2020
@jameslamb jameslamb deleted the r/pi-flags branch October 11, 2020 04:37
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants