r-lib / styler

Non-invasive pretty printing of R code
https://styler.r-lib.org
Other
713 stars 70 forks source link

temp_dir in delete_if_cache_directory() does not work as intended, throws cryptic warnings #1125

Closed 0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q closed 1 year ago

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented 1 year ago

Recently when loading the styler package (usually through some other package importing it), I get this warning:

> library(styler)
Warning messages:
1: In normalizePath(Sys.getenv("TMPDIR", Sys.getenv("TMP"))) :
  path[1]="": No such file or directory
2: In normalizePath(Sys.getenv("TMPDIR", Sys.getenv("TMP"))) :
  path[1]="": No such file or directory
3: In normalizePath(Sys.getenv("TMPDIR", Sys.getenv("TMP"))) :
  path[1]="": No such file or directory

which comes down to the TMPDIR environment variable being undefined in my setup. That is annoying. But this code https://github.com/r-lib/styler/blob/aeaa1faef2e5ea11be26817d8cfa3774ad9d66e7/R/zzz.R#L41 has some problems:

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented 1 year ago

Possible solution:

base::tempdir() does the following

This will be a subdirectory of the per-session temporary directory found by the following rule when the R session is started. The environment variables TMPDIR, TMP and TEMP are checked in turn and the first found which points to a writable directory is used: if none succeeds ‘/tmp’ is used.

So the straight-forward solution would be to take whatever base::tempdir() is using:

-  temp_dir <- normalizePath(Sys.getenv("TMPDIR", Sys.getenv("TMP")))
+  temp_dir <- normalizePath(dirname(tempdir()))
lorenzwalthert commented 1 year ago

@0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q thanks for the issue and the outlined solution. I did also note that warnings are produced and of course that's not intended. A solution is already on its way to CRAN with #1124. Also, I fully agree that tempdir() should be used here because that's innately what we want to use anyways. Having used the function many times, I don't know why I did nto think of tempdir() here -.- I am happy for you to make a PR with the suggested fix.