R local hooks should not prefix hook path

[lorenzwalthert]

The entry for r hooks is very similar to language: script (in the case a path is supplied):

-   repo: local
    hooks:
    -   id: consistent-release-tag
        name: consistent-release-tag
        entry: Rscript inst/consistent-release-tag
        language: r

For local hooks, we don't need to prefix the path (which currently happens). This PR fixes this if scr of the hook is local.

[asottile]

hook.src is only meant for error messages so I don't really want to add a dependency on it

is this an actual problem? the prefix should be the working directory for local hooks so it should be joining to the right place anyway (just making an absolute path)? -- this is consistent with how script hooks work

[lorenzwalthert]

Starting point was one of my repos where I got the error: Here is an example repo. With entry: Rscript inst/hook2, I get

git commit -m 'more reasonable location for hook'              
Require bullet in NEWS.md................................................Failed
- hook id: require-news-on-push
- exit code: 2

Fatal error: cannot open file '/Users/lorenz/.cache/pre-commit/repoz2lpgpug/inst/hook2': No such file or directory

So I thought there is a path problem. I tried to understand how the prefix was set and arrived here:

def _prefix(language_name: str, deps: Sequence[str]) -> Prefix:
        language = languages[language_name]
        # pygrep / script / system / docker_image do not have
        # environments so they work out of the current directory
        if language.ENVIRONMENT_DIR is None:
            return Prefix(os.getcwd())
        else:
            return Prefix(store.make_local(deps))

This made me believe understanding why it works for script hooks but not for r hooks. Because one has an env and the other one does not.

[asottile]

oh hmmmm -- right for language'd hooks it does go through a different path. that's trickier then I'll have to think about this some more

[asottile]

hmmm ok so how about this idea -- what if there were a separate rscript and r language? or does PATH lookup not make a lot of sense for R in general ?

[lorenzwalthert]

or does PATH lookup not make a lot of sense for R in general ?

If you mean in the sense as venv and conda add their own python executable to PATH: No, this is not an option here because {renv} does not boostrap base R itself, only (addon) packages. I.e. it only modifies the path on which packages are loaded from inside R.

I am not sure we could get rid of that problem with a new language that way without still relying on hook.scr.¹ In run_hook, we must amend the script path based on whether or not the hook is local and I believe we can't solve it over hook.prefix (as it is solved for language: script in _prefix), because when running the hook, we must still know where the virtual environment is and that information is only transported via hook.prefix I think. So the only way I see to amend the path is via hook.src.

---

PS: To better understand pre-commit, I also tried to compare to the python language and realised there is also one combination of local/remote and path/expression that does not work (but one can circumvent it in python):

python hooks (prefix is not prepended to path in run_hook):

• local repos work for script paths (prefix is cloned repo).
• remote repos don't work for script paths (prefix is cloned repo). -> Hook authors can expose CLI, problem circumvented.

script hooks (prefix is prepended to path in run_hook):

• local repos work for script paths (prefix is cwd).
• remote repos work for script paths (prefix is cloned repo).

R cannot expose CLI tools like python: -> we can only use scripts and hence not use that work the around.

So it feels like we want the best of language: script and the other languages like python -.-.

---

¹ I am assuming this new language rscript would only allows paths to scripts, no R expressions, for local and remote hooks.

[asottile]

ah ok, when I mentioned the thing about PATH I meant more that r things don't really have a concept of a thing that installs a CLI tool (venv/bin doesn't have an equivalent renv/bin)

in that case, it's probably ok to make R a little special here and do similar to what this PR is doing 👍 (feel free to use .src even though it wasn't intended for this)

[lorenzwalthert]

Thanks Anthony. Then let's progress with hook.src. I am also sorry this whole R support topic needs so much attention from your side and I appreciate every minute you put in to make things happen. R is not a very conventional language and that is exemplified here (neither am I a very experienced Python coder unfortunately).

[asottile]

happy to spend time on this -- I think R is a valuable addition to the ecosystem (despite it being weird!)

[asottile]

oops, this conflicts with the other changes

[lorenzwalthert]

... which I caused 🙃. Fixed.

[asottile]