Set lock_envir to TRUE by default

[wlandau]

cc @krlmlr, @pat-s, @rkrug

Work up to this point

If a user implements impure commands/functions as in #615 (comment), the act of building a target threatens to change upstream dependencies. To solve this problem, I implemented a lock_envir argument to make() and drake_config(). If set to TRUE, drake locks the user's environment and all its non-hidden bindings to prevent imported functions etc. from being modified.

Also, drake itself no longer inserts built targets into the user's environment. Instead, it uses a separate config$eval <- new.env(parent = config$envir).

This PR

I believe make(lock_envir = TRUE) is super important for reproducibility. However, it is a disruptive change, and existing impure projects may error out. That is why I think drake version 7.0.0 will be a great time to merge this PR to set the lock_envir default value to TRUE.

Related GitHub issues and pull requests

• Ref: Lock the user's enviornment during make() #619, Fix #267 #269

Checklist

• I have read drake's code of conduct, and I agree to follow its rules.
• I have listed any substantial changes in the development news.
• I have added testthat unit tests to tests/testthat to confirm that any new features or functionality work correctly.
• I have tested this pull request locally with devtools::check()
• This pull request is ready for review.
• I think this pull request is ready to merge.

[codecov-io]

Codecov Report

Merging #621 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff          @@
##           master   #621   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          78     78           
  Lines        6802   6802           
=====================================
  Hits         6802   6802

Impacted Files	Coverage Δ
R/config.R	100% <ø> (ø)	⬆️
R/make.R	100% <ø> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2cb272...fc294e1. Read the comment docs.

[wlandau]

How it works

lock_environment() locks an environment an all its non-hidden bindings:

lock_environment <- function(envir) {
  lockEnvironment(envir, bindings = FALSE)
  lock_these <- ls(envir, all.names = FALSE)
  skip_these <- ".Random.seed" # Probably unnecessary, but just in case we set all.names to TRUE above
  lock_these <- setdiff(lock_these, skip_these)
  lapply(X = lock_these, FUN = lockBinding, env = envir)
  invisible()
}

unlock_environment() uses a bit of C code I borrowed and modified from https://gist.github.com/wch/3280369#file-unlockenvironment-r and https://github.com/SurajGupta/r-source/blob/master/src/main/envir.c.

The executing process locks the environment just before the target's command is executed and unlocks the environment afterwards. In addition, it blocks the lock so no other process can come along and lock/unlock it (guarantees the environment stays locked while the command is running, and it does not seem to cause a bottleneck in parallel computing).

drake/R/run.R

Lines 65 to 74 in e2cb272

	if (config$lock_envir) {
	i <- 1
	# Lock the environment only while running the command.
	while (environmentIsLocked(config$envir)) {
	Sys.sleep(config$sleep(max(0L, i))) # nocov
	i <- i + 1 # nocov
	}
	lock_environment(config$envir)
	on.exit(unlock_environment(config$envir))
	}

[wlandau]

Merging because the next release will be 7.0.0.