Check for multiple executables

NEWS.md

@@ -1,12 +1,20 @@
 # precommit v0.2.0-rc (Development)
 
+**Installation**
+
+Please follow the instructions in the [README](https://lorenzwalthert.github.io/precommit/dev/)
+if you are a new user of pre-commit or if you want to update a current  
+installation.
+
 **API changes**
 
 - `version_precommit()` and `update_precommit()` are new functions to check the
   version of the installed pre-commit executable and to update it (#197).
-- `style-files` hook now supports the full [`style_file()`](https://styler.r-lib.org/dev/reference/style_file.html) API,
+- `style-files` hook now supports the full
+  [`style_file()`](https://styler.r-lib.org/dev/reference/style_file.html) API,
   e.g. you can supply `--scope=spaces` and similar via `args:` in your
-  `.pre-commit-config.yaml`. See the [docs](https://lorenzwalthert.github.io/precommit/articles/available-hooks.html#style-files-1)
+  `.pre-commit-config.yaml`. See the
+  [docs](https://lorenzwalthert.github.io/precommit/articles/available-hooks.html#style-files-1)
   for details.
 - `style-files` and `roxygenize` hooks now warn if there is no permanent 
   `{R.cache}` cache set up. You can silence the warning with the hook argument 
@@ -16,15 +24,16 @@
 
 - {precommit} now uses [`language: r`](https://pre-commit.com/#r) instead of 
   `language: script` from the [pre-commit framework](https://pre-commit.com). 
-  This requires `pre-commit >= 2.11.1`. All hooks and dependencies are now 
-  contained in a virtual environment with [`{renv}`](https://rstudio.github.io/renv/)
-  (#233, #250, #260, #264). Thanks to {renv}'s excellent 
+  This requires `pre-commit >= 2.11.1` (ideally even `>= 2.13.0`). All hooks and 
+  dependencies are now contained in a virtual environment with
+  [`{renv}`](https://rstudio.github.io/renv/). Thanks to {renv}'s excellent 
   [caching](https://rstudio.github.io/renv/articles/renv.html#cache-1), this 
-  does not consume much space and is fast. This makes output
+  hardly consumes any space and is fast. This makes output
   of hooks more consistent across different local setups, make manual dependency
-  management redundant and will facilitate running R hooks as part of CI/CD, 
-  e.g. https://pre-commit.ci or [GitHub Actions](https://github.com/pre-commit/action)
-  along with arbitrary other hooks. 
+  management redundant and will facilitate running R hooks as part of CI/CD in
+  the future, e.g. via https://pre-commit.ci or 
+  [GitHub Actions](https://github.com/pre-commit/action) along with hook 
+  implemented in other languages (#233, #250, #260, #264).
 - Because hooks run in a virtual environment and the `roxygenize` hook runs
   `pkgload::load_all()`, you need to list all dependencies of your package in
   `additional_dependencies` field in `.pre-commit-config.yaml`. You will be 
@@ -35,17 +44,20 @@
 
 **Minor changes**
 
+- In order to avoid multiple installations of the pre-commit framework, a 
+  warning is issued if multiple are found so the user can remove them (#266).
 - The cache for the roxygen2 hook is now also invalidated for changes in formals 
   if there are no changes in roxygen comments (#214).
-- `{renv}` infra files are not checked anymore by default (#237).
+- `{renv}` infra files are not checked anymore by default in the template config
+  files (#237).
 - `deps-in-desc` now checks `.Rprofile`, `.Rmd` and `.Rnw` files in addition to 
   `.R` files (#216).  
-- The location of the pre-commit executable is now also recognized on Apple Silicon
-  when installed with Homebrew (#240).
+- The location of the pre-commit executable is now also recognized on Apple 
+  Silicon when installed with Homebrew (#240).
 - The `deps-in-desc` hook now points to the hook argument 
   `--allow_private_imports` when the hook fails due to private imports (#254).
 - Hook dependency updates are proposed by an automatic monthly pull request 
-  (#430).
+  to `lorenzwalthert/precommit`. This does not affect users directly (#430).
 
 # precommit v0.1.3
 

R/exec.R

@@ -45,7 +45,7 @@ path_pre_commit_exec <- function(check_if_exists = TRUE) {
 
 #' Derive the path to the pre-commit executable
 #'
-#' All these functions return "" if search was not successful.
+#' Returns "" if search was not successful, the path otherwise.
 #' @section Heuristic:
 #' - First check if there is an executable on the `$PATH` using
 #'   [path_derive_precommit_exec_path()]
@@ -56,20 +56,23 @@ path_pre_commit_exec <- function(check_if_exists = TRUE) {
 #' @keywords internal
 path_derive_precommit_exec <- function() {
   path <- path_derive_precommit_exec_path()
-  if (path == "") {
-    os <- tolower(Sys.info()[["sysname"]])
-    if (os == "darwin") {
-      path <- path_derive_precommit_exec_macOS()
-    } else if (os == "windows") {
-      path <- path_derive_precommit_exec_win()
-    } else if (os == "linux") {
-      path <- path_derive_precommit_exec_linux()
-    }
+  os <- tolower(Sys.info()[["sysname"]])
+  if (os == "darwin") {
+    path <- c(path, path_derive_precommit_exec_macOS())
+  } else if (os == "windows") {
+    path <- c(path, path_derive_precommit_exec_win())
+  } else if (os == "linux") {
+    path <- c(path, path_derive_precommit_exec_linux())
   }
-  if (path == "") {
-    path <- path_derive_precommit_exec_conda()
+  path <- unique(path[path != ""])
+  if (length(path) == 0) {
+    path_derive_precommit_exec_conda()
+  } else if (length(path) == 1) {
+    path
+  } else {
+    path_warn_multiple_execs(path)
+    path[1]
   }
-  path
 }
 
 #' Find an executable
@@ -80,15 +83,32 @@ path_derive_precommit_exec <- function() {
 #'   directory may also not exist.
 #' @keywords internal
 path_derive_precommit_exec_impl <- function(candidate) {
-  assumed <- fs::path(candidate, precommit_executable_file())
-  existant <- assumed[fs::file_exists(assumed)]
-  if (length(existant) > 0) {
+  existant <- path_candidate_to_actual(candidate)
+  if (length(existant) == 1) {
     existant[1]
+  } else if (length(existant) > 1) {
+    path_warn_multiple_execs(existant)
   } else {
     ""
   }
 }
 
+path_warn_multiple_execs <- function(paths) {
+  rlang::warn(paste0(
+    "We detected multiple pre-commit executables. This is likely ",
+    "going to get you into trouble in the future, e.g. when you want to ",
+    "upgrade, as you easily loose track of different versions. We strongly ",
+    "suggest to only keep one pre-commit executable and delete the other ",
+    "ones. Here are the locations where we detected executables:\n\n",
+    "- ", paste0(paths, collapse = "\n- ")
+  ))
+}
+
+path_candidate_to_actual <- function(candidate) {
+  assumed <- fs::path(candidate, precommit_executable_file())
+  assumed[fs::file_exists(assumed)]
+}
+
 path_derive_precommit_exec_linux <- function() {
   path_derive_precommit_exec_impl(
     path_if_exist(fs::path_home(".local/bin")) # 18.04 and 16.04 with pip3.

R/install.R

@@ -14,9 +14,10 @@ install_system <- function(force) {
     options(precommit.executable = path_exec)
   } else {
     path_exec <- path_precommit_exec(check_if_exists = FALSE)
-    usethis::ui_info(c(
-      "pre-commit already installed at the following locations:",
-      paste0("- ", path_exec)
+    usethis::ui_info(paste0(
+      "pre-commit already installed at the following locations:\n\n",
+      paste0("- ", path_exec), "\n\nUse `precommit::update_precommit()` to ",
+      "update the executable."
     ))
   }
   invisible(path_exec)

R/update.R

@@ -21,8 +21,9 @@ update_precommit <- function() {
       "does not seem you installed via conda, because the path to the ",
       "executable in use is ", path_precommit_exec(), ". Please use the ",
       "update utilities of the installation method you chose. Alternatively, ",
-      "you can uninstall with the utility of your installation method and ",
-      " run `precommit::install_precommit()` to switch to the conda ",
+      "you can uninstall with the utility of your installation method / ",
+      "delete the executable and ",
+      "run `precommit::install_precommit()` to switch to the conda ",
       "installation method."
     ))
   }

README.Rmd

@@ -47,7 +47,7 @@ The following online docs are available:
 These only cover the functionality added on top of the pre-commit framework by
 this package. Everything else is covered in the extensive [online
 documentation](https://pre-commit.com) of the pre-commit framework itself,
-including how to create hooks for actions like `git push` or `git checkout`, 
+including how to create hooks for actions like `git push` or `git checkout`,
 create local hooks etc.
 
 ## Installation
@@ -143,12 +143,24 @@ which is usually under `$HOME/.cache/pre-commit/`.
 
 ## Update
 
-To update the pre-commit executable, use the update utilities provided by your
-installation method. If you chose conda, you can use
-`precommit::update_precommit()`.
+If you used {precommit} before, upgrade these three components for maximal
+compatibility:
 
-You can check the version of you executable with
-`precommit::version_precommit()`.
+* the R package {precommit} from CRAN with `install.packages("precommit")`.
+
+* the hook revisions in your `.pre-commit-config.yaml` with
+  `precommit::autoupdate()`. Hook revision updates are released in sync with R
+  package updates (exception: Patch releases for hooks don't have a
+  corresponding CRAN release).
+
+* the upstream pre-commit framework. Use the update utilities provided by your
+  installation method (i.e. `pip3` or `brew``). If you chose conda, you can use
+  `precommit::update_precommit()`. If you don't remember the installation method
+  you chose, just choose any and then upgrade. We'll warn you if you have
+  multiple executables installed and point you to their location so you can get
+  rid of all but one. You can check the version of you executable with
+  `precommit::version_precommit()`. Updates to the pre-commit framework are not
+  released in sync with the R or hook revision updates.
 
 ## Uninstallation
 

README.md

@@ -135,12 +135,22 @@ patch under your pre-commit cache, which is usually under
 
 ## Update
 
-To update the pre-commit executable, use the update utilities provided
-by your installation method. If you chose conda, you can use
-`precommit::update_precommit()`.
-
-You can check the version of you executable with
-`precommit::version_precommit()`.
+If you used {precommit} before, upgrade these three components for
+maximal compatibility:
+
+-   the R package {precommit} from CRAN with
+    `install.packages("precommit")`.
+
+-   the hook revisions in your `.pre-commit-config.yaml` with
+    `precommit::autoupdate()`. Hook revision updates are released in
+    sync with R package updates (exception: Patch releases for hooks
+    don’t have a corresponding CRAN release).
+
+-   the upstream pre-commit framework. Use the update utilities provided
+    by your installation method (i.e. `pip3` or
+    ``` brew``). If you chose conda, you can use ```precommit::update\_precommit()`. If you don't remember the installation method   you chose, just choose any and then upgrade. We'll warn you if you have   multiple executables installed and point you to their location so you can get   rid of all but one. You can check the version of you executable with`precommit::version\_precommit()\`.
+    Updates to the pre-commit framework are not released in sync with
+    the R or hook revision updates.
 
 ## Uninstallation
 

tests/testthat/test-exec.R

@@ -18,9 +18,46 @@ test_that("Path can be derived for windows Python >= 3.0", {
   )
   skip_if(!is_windows())
   skip_if(!not_conda())
-  expect_match(path_derive_precommit_exec_win_python3plus_base(), 'AppData/Roaming')
+  expect_match(path_derive_precommit_exec_win_python3plus_base(), "AppData/Roaming")
   expect_equal(
-    fs::path_file(path_derive_precommit_exec_win()), 
+    fs::path_file(path_derive_precommit_exec_win()),
     precommit_executable_file()
   )
 })
+
+
+test_that("Warns when there are multiple installations found", {
+  expect_warning(
+    with_mock(
+      "precommit::path_candidate_to_actual" = function(candidate) {
+        candidate
+      },
+      path_derive_precommit_exec_impl(
+        c(
+          fs::path_home("AppData/Roaming/Python/Python35"),
+          fs::path_home("AppData/Roaming/Python/Python37")
+        )
+      )
+    ),
+    "We detected multiple pre-commit executables"
+  )
+})
+
+
+test_that("Warns when there are multiple installations found", {
+  expect_warning(
+    with_mock(
+      "precommit::path_derive_precommit_exec_path" = function(candidate) {
+        fs::path_home("AppData/Roaming/Python/Python35")
+      },
+      "Sys.info" = function(...) {
+        c(sysname = "windows")
+      },
+      "precommit:::path_derive_precommit_exec_win" = function() {
+        fs::path_home("AppData/Roaming/Python/Python34")
+      },
+      path_derive_precommit_exec()
+    ),
+    "We detected multiple pre-commit executables"
+  )
+})