r-lib / pkgbuild

Find tools needed to build R packages
https://pkgbuild.r-lib.org
Other
65 stars 33 forks source link

`needs_compile` produces false positives #130

Closed david-cortes closed 1 year ago

david-cortes commented 2 years ago

From https://github.com/rstudio/rstudio/issues/9264

pkgbuild::needs_compile() oftentimes incorrectly decides that a package needs recompilation even though neither the compiled source code nor the configure file have changed, which is especially problematic when using the RStudio IDE to develop R packages.

A couple examples to try: https://cran.r-project.org/web/packages/xgboost/index.html https://cran.r-project.org/web/packages/MatrixExtra/index.html https://cran.r-project.org/web/packages/rsparse/index.html

I am not 100% sure but I think:

Can be checked for example by using the RStudio IDE button Install and Restart on some R package project after modifying some .R files without touching anything under src/.

gaborcsardi commented 1 year ago

I can't reproduce this, I tried rsparse and MatrixExtra and get:

> pkgbuild::needs_compile()
[1] FALSE
> system("touch R/*")
> pkgbuild::needs_compile()
[1] FALSE

I also tried modifying a .R source file instead of touch, and pkgbuild::needs_compile() still returns FALSE.

It works correctly for /src as well:

> pkgbuild::needs_compile()
[1] FALSE
> system("touch src/*.c*")
> pkgbuild::needs_compile()
[1] TRUE
david-cortes commented 1 year ago

@gaborcsardi Does anything change if you set up a custom Makevars for your R user?

gaborcsardi commented 1 year ago

A custom Makevars does not change anything, waiting a couple of minutes after the compilation does not change anything, either.

Do you use ccache or some other similar tool? What do you have in your Makevars file?

david-cortes commented 1 year ago

I'm using the default gcc as it comes in debian. I don't think it includes ccache as the package doesn't show as installed in my system.

My Makevars file (~/.R/Makevars) is as follows:

CXXFLAGS += -O3 -march=native -fno-math-errno -fno-trapping-math
CXX11FLAGS += -O3 -march=native -fno-math-errno -fno-trapping-math
CXX14FLAGS += -O3 -march=native -fno-math-errno -fno-trapping-math
CXX17FLAGS += -O3 -march=native -fno-math-errno -fno-trapping-math
CXX20FLAGS += -O3 -march=native -fno-math-errno -fno-trapping-math
CFLAGS += -O3 -march=native -fno-math-errno -fno-trapping-math
FCFLAGS += -O3 -march=native -fno-math-errno -fno-trapping-math
FFLAGS += -O3 -march=native -fno-math-errno -fno-trapping-math
gaborcsardi commented 1 year ago

Do you also see this from the command line or only from RStudio?

david-cortes commented 1 year ago

I've never tried this from the command line. Do you mean doing something like this?

R -e "pkgbuild::needs_compile()"
gaborcsardi commented 1 year ago

Yeah, something like this in R:

pkgload::load_all()
pkgbuild::need_compile()
pkgbuild::need_compile()
system("touch R/*.R")
pkgbuild::need_compile()

Btw. in RStudio Build -> Install does not use pkgbuild, and it runs

R CMD INSTALL --preclean --no-multiarch --with-keep.source <pkg>

for me (whether I select the devtools tools or not), where --preclean will probably remove the object files, so yes, that will rebuild the package all the time, but this is independent of pkgbuild.

david-cortes commented 1 year ago

I btw also experience the issue with the Run Tests RStudio button in files put under tests/, which as I understood from the linked issue at the top should call pkgbuild::needs_compile() internally, and the same with the button to build roxygen docs.

gaborcsardi commented 1 year ago

I cannot reproduce it with Run Tests, either, on macOS, with a recent RStudio daily: 2022.11.0-daily+185.

If you cannot reproduce it in a terminal, then it is potentially an RStudio issue. If you are able to update RStudio, then you could try a build from https://dailies.rstudio.com/

gaborcsardi commented 1 year ago

I am going to close this because I cannot reproduce it, still.

Please reopen it if you can reproduce it without RStudio or with a recent version of RStudio.