pik-piam / lucode2

A collection of tools which allow to manipulate and analyze code.
Other
0 stars 16 forks source link

styler issues let runs and tests fail #172

Closed orichters closed 1 year ago

orichters commented 1 year ago

One of the AMT REMIND runs failed very early, just stating this in log.txt:

`Global .Rprofile loaded! (R version 4.1.2 (2021-11-01))
Error:
! package or namespace load failed for 'lucode2':
 .onLoad failed in loadNamespace() for 'styler', details:
  call: mkdirs.default(path, mustWork = TRUE)
  error: Failed to create directory (tried 5 times), most likely because of lack of file permissions (directory '/home/lavinia/.cache/R/R.cache/styler' exists but nothing beyond): /home/lavinia/.cache/R/R.cache/styler/1.10.0
Backtrace:
    ▆
 1. └─base::library(lucode2, quietly = TRUE, warn.conflicts = FALSE)
 2.   └─base::tryCatch(...)
 3.     └─base (local) tryCatchList(expr, classes, parentenv, handlers)
 4.       └─base (local) tryCatchOne(expr, names, parentenv, handlers[[1L]])
 5.         └─value[[3L]](cond)
Execution halted

see /p/projects/remind/modeltests/remind/output/SSP5-NDC-AMT_2023-06-03_07.50.43/log_beforeRestart.txt

This check job in pik-piam/modelstats failed with:

aborting due to test error:
Error : .onLoad failed in loadNamespace() for 'styler', details:
  call: mkdirs.default(path, mustWork = TRUE)
  error: Failed to create directory (tried 5 times), most likely because of lack of file permissions (directory '/home/runner/.cache/R/R.cache/styler' exists but nothing beyond): /home/runner/.cache/R/R.cache/styler/1.10.1
Error in lucode2::check(runLinter = FALSE) : lucode2::check failed
Execution halted
Error: Process completed with exit code 1.

That is a bit annoying, can we somehow make sure this does not happen anymore? Maybe @pfuehrlich-pik, do you have an idea?

pfuehrlich-pik commented 1 year ago

Seems like /home/lavinia/.cache/R/R.cache/styler has too restrictive permissions, but that's in Lavinia's home folder (because cronjobs now run using her user account) so I cannot check or change that. Can you have a look @LaviniaBaumstark ?

orichters commented 1 year ago

The strange thing is that this happens not every time. The other AMTs went well, same in the past. Same with the second attempt of the test. No idea what is going on there.

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented 1 year ago

Just deactivate the styler caching. It's not used anyway.

orichters commented 1 year ago

@0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q: How can this be done?

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented 1 year ago

echo 'options(R.cache.enable = FALSE)' >> .Rprofile

pfuehrlich-pik commented 1 year ago

options("styler.cache_name" = NULL) I'll just put this into our global .Rprofile. We're using styler so infrequently that I don't think small performance improvements justify dealing with weird errors like this. I checked with Lavinia and her file permissions on the folder seem fine.

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented 1 year ago

Seems like /home/lavinia/.cache/R/R.cache/styler has too restrictive permissions, but that's in Lavinia's home folder (because cronjobs now run using her user account) […]

So Lavinia's home directory has too restrictive permissions for the cron job running on her account?

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented 1 year ago

options("styler.cache_name" = NULL) I'll just put this into our global .Rprofile.

You can't set NULL options. That won't work.

pfuehrlich-pik commented 1 year ago

Setting an option to NULL removes it altogether (as if it was not set at all). In this case it indeed doesn't work because styler checks onLoad whether the option is set. I don't see a way to disable the styler cache via options/envvars so I'll ask Lavinia to create the missing folder manually and hope for the best.

pfuehrlich-pik commented 1 year ago

echo 'options(R.cache.enable = FALSE)' >> .Rprofile

This does disable R.cache in general, not only for styler, so it seems overkill to put this into the global .Rprofile

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented 1 year ago

echo 'options(R.cache.enable = FALSE)' >> .Rprofile

This does disable R.cache in general, not only for styler, so it seems overkill to put this into the global .Rprofile

We don't use any other package that uses R.cache.

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented 1 year ago

Alternatively, since styler is only used via style_file() in lucode2::autoFormat(), and since that function is called nowhere and only going to be invoked by the user, one can move styler to Suggests: and call styler::style_file(), while guarding with rlang::check_installed('styler'). That way styler is not uselessly attached every time lucode2 is attached, and doesn't fail fiddling with its cache for whatever reason.

pfuehrlich-pik commented 1 year ago

That's a good idea, will do that :+1:

orichters commented 1 year ago

Thank you both!