Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid unquoting weirdness of Windows for language: r #2870

Closed
lorenzwalthert opened this issue May 3, 2023 · 1 comment · Fixed by #2885
Closed

Avoid unquoting weirdness of Windows for language: r #2870

lorenzwalthert opened this issue May 3, 2023 · 1 comment · Fixed by #2885

Comments

@lorenzwalthert
Copy link
Contributor

search you tried in the issue tracker

never, r, found

describe your issue

Multiple reports in https://github.com/lorenzwalthert/precommit (lorenzwalthert/precommit#441, lorenzwalthert/precommit#473) were raised and describe a problem with (un)quoting the long string that runs when language: r is setup in Rscript -e 'xxx' where 'xxx' contains multiple levels of quotes. For the readers convenience, the output looks like:

[INFO] Installing environment for https://github.com/lorenzwalthert/precommit.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
[INFO] Restored changes from C:\Users\USER\.cache\pre-commit\patch1678401203-36472.
An unexpected error has occurred: CalledProcessError: command: ('C:/PROGRA~1/R/R-41~1.0\\bin\\Rscript.exe', '--vanilla', '-e', '    options(install.packages.compile.from.source = "never", pkgType = "binary")\n            prefix_dir <- \'C:\\\\Users\\\\USER\\\\.cache\\\\pre-commit\\\\repovawmpj_r\'\n        options(\n            repos = c(CRAN = "https://cran.rstudio.com"),\n            renv.consent = TRUE\n        )\n        source("renv/activate.R")\n        renv::restore()\n        activate_statement <- paste0(\n          \'suppressWarnings({\',\n          \'old <- setwd("\', getwd(), \'"); \',\n          \'source("renv/activate.R"); \',\n          \'setwd(old); \',\n          \'renv::load("\', getwd(), \'");})\'\n        )\n        writeLines(activate_statement, \'activate.R\')\n        is_package <- tryCatch(\n          {\n              path_desc <- file.path(prefix_dir, \'DESCRIPTION\')\n              suppressWarnings(desc <- read.dcf(path_desc))\n              "Package" %in% colnames(desc)\n          },\n          error = function(...) FALSE\n        )\n        if (is_package) {\n            renv::install(prefix_dir)\n        }\n        \n    ')
return code: 1
stdout: (none)
stderr:
    During startup - Warning messages:
    1: Setting LC_COLLATE=en_US.UTF-8 failed 
    2: Setting LC_CTYPE=en_US.UTF-8 failed 
    3: Setting LC_MONETARY=en_US.UTF-8 failed 
    4: Setting LC_TIME=en_US.UTF-8 failed 
    Error in options(install.packages.compile.from.source = never, pkgType = binary) : 
      object 'never' not found
    Execution halted
Check the log at C:\Users\USER\.cache\pre-commit\pre-commit.log

The solution described by @asottile in lorenzwalthert/precommit#473 (comment) is to probably write the contents to a temporary file and avoid unquoting within the expression (i.e. the term after -e). This should be quite straight forward.

Question is if we can create a good test first to reproduce the offending behavior and whether or not there are tools already in pre-commit to deal with temp files etc. that we could use.

pre-commit --version

precommit 3.1.1

.pre-commit-config.yaml

repos:
-   repo: https://github.com/lorenzwalthert/precommit
    rev: v0.3.2.9007
    hooks:
    -   id: style-files

~/.cache/pre-commit/pre-commit.log (if present)

No response

@asottile
Copy link
Member

asottile commented May 3, 2023

tempfile.NamedTemporaryFile from the stdlib should be fine. arguably this is a bug in R itself since the commandline is correct

jaypikay pushed a commit to jaypikay/doxy that referenced this issue May 24, 2023
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [pre-commit](https://github.com/pre-commit/pre-commit) | dev-dependencies | patch | `3.3.1` -> `3.3.2` |

---

### Release Notes

<details>
<summary>pre-commit/pre-commit</summary>

### [`v3.3.2`](https://github.com/pre-commit/pre-commit/blob/HEAD/CHANGELOG.md#&#8203;332---2023-05-17)

[Compare Source](pre-commit/pre-commit@v3.3.1...v3.3.2)

\==================

##### Fixes

-   Work around `r` on windows sometimes double-un-quoting arguments.
    -   [#&#8203;2885](pre-commit/pre-commit#2885) PR by [@&#8203;lorenzwalthert](https://github.com/lorenzwalthert).
    -   [#&#8203;2870](pre-commit/pre-commit#2870) issue by [@&#8203;lorenzwalthert](https://github.com/lorenzwalthert).

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS42MS4wIiwidXBkYXRlZEluVmVyIjoiMzUuNjEuMCJ9-->

Co-authored-by: Renovate Bot <renovate@localhost.localdomain>
Co-authored-by: JayPiKay <jpk@noreply.localhost>
Reviewed-on: https://git.goatpr0n.de/public/doxy/pulls/11
Co-authored-by: renovate <renovate@noreply.localhost>
Co-committed-by: renovate <renovate@noreply.localhost>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants