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

Rscript not found in Windows #2599

Closed
SInginc opened this issue Nov 16, 2022 · 3 comments · Fixed by #2605
Closed

Rscript not found in Windows #2599

SInginc opened this issue Nov 16, 2022 · 3 comments · Fixed by #2605

Comments

@SInginc
Copy link

SInginc commented Nov 16, 2022

search you tried in the issue tracker

Rscript not found

describe your issue

Hi,

I was adding hook for R language in pre-commit for the first time. And pre-commit throw me an error as

Executable `C:\path\to\R\R-4.0.5\bin\Rscript` not found

Because I was using hooks from lorenzwalthert/precommit, I found a similar issue was raised(lorenzwalthert/precommit#441). But it looks like this issue is not related to the hooks and it is related to the R language support of pre-commit.

The error was raised in cmd_output_b of pre_commit\util.py, which was called by install_environment of pre_commit/languages/r.py like.

cmd_output_b(
    _rscript_exec(), *RSCRIPT_OPTS, '-e',
    _inline_r_setup(r_code_inst_add),
    *additional_dependencies,
    cwd=env_dir,
)

The _rscrip_exec() will return C:\path\to\R\R-4.0.5\bin\Rscript in my case because $R_HOME is well set.

def _rscript_exec() -> str:
    r_home = os.environ.get('R_HOME')
    if r_home is None:
        return 'Rscript'
    else:
        return os.path.join(r_home, 'bin', 'Rscript')

When C:\path\to\R\R-4.0.5\bin\Rscript was passed to parse_shebang.normalize_cmd, and this function uses normexe to examine whether exe is executable.

def cmd_output_b(
        *cmd: str,
        retcode: int | None = 0,
        **kwargs: Any,
) -> tuple[int, bytes, bytes | None]:
    _setdefault_kwargs(kwargs)

    try:
        cmd = parse_shebang.normalize_cmd(cmd)
    except parse_shebang.ExecutableNotFoundError as e:
        returncode, stdout_b, stderr_b = e.to_output()
    else:
        try:
            proc = subprocess.Popen(cmd, **kwargs)
        except OSError as e:
            returncode, stdout_b, stderr_b = _oserror_to_output(e)
        else:
            stdout_b, stderr_b = proc.communicate()
            returncode = proc.returncode

    if retcode is not None and retcode != returncode:
        raise CalledProcessError(returncode, cmd, retcode, stdout_b, stderr_b)

    return returncode, stdout_b, stderr_b

def normalize_cmd(cmd: tuple[str, ...]) -> tuple[str, ...]:
    """Fixes for the following issues on windows
    - https://bugs.python.org/issue8557
    - windows does not parse shebangs

    This function also makes deep-path shebangs work just fine
    """
    # Use PATH to determine the executable
    exe = normexe(cmd[0])

    # Figure out the shebang from the resulting command
    cmd = parse_filename(exe) + (exe,) + cmd[1:]

    # This could have given us back another bare executable
    exe = normexe(cmd[0])

    return (exe,) + cmd[1:]

And it seems that C:\path\to\R\R-4.0.5\bin\Rscript didn't pass normexe's test and the code flowed into:

except parse_shebang.ExecutableNotFoundError as e:
    returncode, stdout_b, stderr_b = e.to_output()

normexe checks exe's executability basing on two conditions:

  1. os.sep not in orig and (not os.altsep or os.altsep not in orig)
  2. not os.path.isfile(orig)
def normexe(orig: str) -> str:
    def _error(msg: str) -> NoReturn:
        raise ExecutableNotFoundError(f'Executable `{orig}` {msg}')

    if os.sep not in orig and (not os.altsep or os.altsep not in orig):
        exe = find_executable(orig)
        if exe is None:
            _error('not found')
        return exe
    elif os.path.isdir(orig):
        _error('is a directory')
    elif not os.path.isfile(orig):
        _error('not found')
    elif not os.access(orig, os.X_OK):  # pragma: win32 no cover
        _error('is not executable')
    else:
        return orig

Since os.sep is in C:\path\to\R\R-4.0.5\bin\Rscript, orig will flow into not os.path.isfile(orig). And os.path.isfile(orig) is False, because in Windows, the actual file is like C:\path\to\R\R-4.0.5\bin\Rscript.exe.

I am wondering, to fix this issue on Windows, can we do an OS check in _rscrip_exec() and return /path/to/bin/Rscript.exe when the User is using Windows?

Thank you!

pre-commit --version

pre-commit 2.20.0

.pre-commit-config.yaml

repos:
-   repo: https://github.com/lorenzwalthert/precommit
    rev: v0.3.2.9003
    hooks:
    -   id: parsable-R
    -   id: lintr
        args: [--warn-only, --key=value]
    -   id: style-files
        args: 
            - --scope=token
    -   id: roxygenize
    -   id: use-tidy-description
-   repo: https://github.com/asottile/seed-isort-config
    rev: v2.2.0
    hooks:
    -   id: seed-isort-config
-   repo: https://github.com/pre-commit/mirrors-isort
    rev: v5.10.1
    hooks:
    -   id: isort
-   repo: https://github.com/psf/black
    rev: 22.10.0
    hooks:
    -   id: black
        args:
            - --line-length=79
        language_version: python3.9
-   repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v2.3.0
    hooks:
    -   id: check-yaml
    -   id: end-of-file-fixer
    -   id: trailing-whitespace
    -   id: flake8
        args:
            - --extend-ignore=F401, E501, W503

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

version information

pre-commit version: 2.20.0
git --version: git version 2.37.3.windows.1
sys.version:
    3.9.0 (tags/v3.9.0:9cf6752, Oct  5 2020, 15:34:40) [MSC v.1927 64 bit (AMD64)]
sys.executable: C:\path\to\scripts\python.exe
os.name: nt
sys.platform: win32

error information

An unexpected error has occurred: CalledProcessError: command: ('C:\\path\\to\\R\\R-4.0.5\\bin\\Rscript', '--vanilla', '-e', '    options(install.packages.compile.from.source = "never")\n                prefix_dir <- \'C:\\\\path\\\\to\\\\.cache\\\\pre-commit\\\\repoo8yj9t7d\'\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
expected return code: 0
stdout:
    Executable `C:\path\to\R\R-4.0.5\bin\Rscript` not found
stderr: (none)
Traceback (most recent call last):
  File "C:\path\to\lib\site-packages\pre_commit\error_handler.py", line 73, in error_handler
    yield
  File "C:\path\to\lib\site-packages\pre_commit\main.py", line 358, in main
    return hook_impl(
  File "C:\path\to\lib\site-packages\pre_commit\commands\hook_impl.py", line 254, in hook_impl
    return retv | run(config, store, ns)
  File "C:\path\to\lib\site-packages\pre_commit\commands\run.py", line 424, in run
    install_hook_envs(to_install, store)
  File "C:\path\to\lib\site-packages\pre_commit\repository.py", line 223, in install_hook_envs
    _hook_install(hook)
  File "C:\path\to\lib\site-packages\pre_commit\repository.py", line 79, in _hook_install
    lang.install_environment(
  File "C:\path\to\lib\site-packages\pre_commit\languages\r.py", line 139, in install_environment
    cmd_output_b(
  File "C:\path\to\lib\site-packages\pre_commit\util.py", line 146, in cmd_output_b
    raise CalledProcessError(returncode, cmd, retcode, stdout_b, stderr_b)
pre_commit.util.CalledProcessError: command: ('C:\\path\\to\\R\\R-4.0.5\\bin\\Rscript', '--vanilla', '-e', '    options(install.packages.compile.from.source = "never")\n                prefix_dir <- \'C:\\\\path\\\\to\\\\.cache\\\\pre-commit\\\\repoo8yj9t7d\'\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
expected return code: 0
stdout:
    Executable `C:\path\to\R\R-4.0.5\bin\Rscript` not found
stderr: (none)
@asottile
Copy link
Member

probably just needs a call to pre_commit.util.win_exe here:

return os.path.join(r_home, 'bin', 'Rscript')

cc @lorenzwalthert

@lorenzwalthert
Copy link
Contributor

Great issue description and investigation @SInginc. Indeed this might be the same as lorenzwalthert/precommit#441, except that in addition to not finding the executable because of a lacking suffix, there might have been another issue with ~ usage:

Executable `C:/PROGRA~1/R/R-41~1.0\bin\Rscript` not found

Me and the OP did not take the time to create such a good reprex and description, also because I was not sure it was even reproducible on other people's machines. Now it seems this is the case. Also, how is this not caught with tests? I am trying to expand integration tests in lorenzwalthert/precommit where I run end to end with pre-commit run.

@SInginc Do you want to make a PR or should I?

@SInginc
Copy link
Author

SInginc commented Nov 17, 2022

Great issue description and investigation @SInginc. Indeed this might be the same as lorenzwalthert/precommit#441, except that in addition to not finding the executable because of a lacking suffix, there might have been another issue with ~ usage:

Executable `C:/PROGRA~1/R/R-41~1.0\bin\Rscript` not found

Me and the OP did not take the time to create such a good reprex and description, also because I was not sure it was even reproducible on other people's machines. Now it seems this is the case. Also, how is this not caught with tests? I am trying to expand integration tests in lorenzwalthert/precommit where I run end to end with pre-commit run.

@SInginc Do you want to make a PR or should I?

Thank you! @lorenzwalthert

Yes, please make a PR because I am still getting familiar with how to use Github...

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.

3 participants