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

Invoke scripts from the project root #932

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

j178
Copy link
Contributor

@j178 j178 commented Mar 23, 2024

Fixes #930

@j178 j178 changed the title Invoke scripts from project root Invoke scripts from the project root Mar 23, 2024
@bluss
Copy link
Contributor

bluss commented Mar 23, 2024

I have some use cases that require rye run to preserve the current working directory, maybe this could be configurable in that case?

@j178
Copy link
Contributor Author

j178 commented Mar 23, 2024

@bluss I see three approaches:

  1. Add a cwd option proposed in ✨ Allow passing cwd in tool.rye.scripts #930, but it should support some special variabes to support your use cases, for example pwd = { cmd = "pwd", cwd: "${CWD}" }
  2. Defaults invoking scripts from current working directory (current behavior, not recommended). If the cwd option is set, invoking from that instead.
  3. We take npm approach, which sets INIT_CWD env to the current working directory when invoking scripts. Then in your script pwd = { cmd = "bash -c 'echo $INIT_CWD'"}

@bluss
Copy link
Contributor

bluss commented Mar 23, 2024

I'm not sure what's wrong with the current behaviour. Here's an argument in favour of keeping it.

These two should be equivalent:
cmd (with virtualenv active - it's a binary in .venv/bin) and rye run cmd (virtualenv doesn't matter)

They will only be the same if they both behave the same w.r.t cwd.

@j178
Copy link
Contributor Author

j178 commented Mar 23, 2024

Current behavior makes relative file path in the cmd does not work, see #930
Since scripts are defined in pyproject.toml, it is natural to think that relative path are resloved by the directory containing pyproject.toml. This is the default behavior of pipenv/pdm and npm.

@bluss
Copy link
Contributor

bluss commented Mar 23, 2024

Note that this function also handles rye run python, rye run cmd where cmd is an executable in the venv too (take for example cmd = pytest). Maybe it makes sense for tool.rye.scripts by itself, yes!

@j178 j178 marked this pull request as draft March 23, 2024 19:44
@mitsuhiko
Copy link
Collaborator

I feel like making it run from root is a reasonable default as many scripts will be taking relative file arguments. However I do think that if we change that, then an option should be added to override it like PDM does now. Also I do like what npm does with INIT_CWD, maybe we can add support for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

✨ Allow passing cwd in tool.rye.scripts
3 participants