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

Fix recursively chained script that were causing stack overflow #983

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ that were not yet released.

_Unreleased_

- Fixed a stack overflow issue caused by recursively chained Rye scripts. #983

<!-- released start -->

## 0.32.0
Expand Down
4 changes: 3 additions & 1 deletion docs/guide/commands/run.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
# `run`

Runs a command installed into this package. This either runs a script or application
made available in the virtualenv or a Rye specific script.
made available in the virtualenv or a Rye specific script. If no command is provided, it will list all available commands.

If there is a script in the virtualenv having the same name as a Rye script, the virtualenv script will take precedence over the Rye script.

For more information see [`rye.tool.scripts`](../pyproject.md#toolryescripts).

Expand Down
20 changes: 18 additions & 2 deletions docs/guide/pyproject.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,28 @@ devserver-alt = ["flask", "run", "--app", "./hello.py", "--debug"]
devserver-explicit = { cmd = "flask run --app ./hello.py --debug" }
```

In addition to the shell's pre-existing PATH, Rye run adds `.venv/bin` to the PATH provided to scripts. Any scripts provided by locally-installed dependencies can be used without the `.venv/bin` prefix. For example, if there is a
`dev-dependencies` on pytest in your package, you should write:

```toml
[tool.rye.scripts]
test = "pytest --lf"
```

instead of

```toml
[tool.rye.scripts]
test = ".venv/bin/pytest --lf"
```

Scripts are not run in a shell, so shell specific interpolation is unavailable.

The following keys are possible for a script:

### `cmd`

The command to execute. This is either a `string` or an `array` of arguments. In either case
shell specific interpolation is unavailable. The command will invoke one of the tools in the
The command to execute. This is either a `string` or an `array` of arguments. The command will invoke one of the tools in the
virtualenv if it's available there.

```toml
Expand Down
35 changes: 28 additions & 7 deletions rye/src/cli/run.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};
use std::env::{self, join_paths, split_paths};
use std::ffi::OsString;
use std::path::PathBuf;
Expand All @@ -11,7 +11,7 @@ use console::style;
use crate::pyproject::{PyProject, Script};
use crate::sync::{sync, SyncOptions};
use crate::tui::redirect_to_stderr;
use crate::utils::{exec_spawn, get_venv_python_bin, success_status, IoPathContext};
use crate::utils::{exec_spawn, get_venv_python_bin, success_status, CommandOutput, IoPathContext};

/// Runs a command installed into this package.
#[derive(Parser, Debug)]
Expand All @@ -26,6 +26,12 @@ pub struct Args {
/// Use this pyproject.toml file
#[arg(long, value_name = "PYPROJECT_TOML")]
pyproject: Option<PathBuf>,
/// Enables verbose diagnostics.
#[arg(short, long)]
verbose: bool,
/// Turns off all output.
#[arg(short, long, conflicts_with = "verbose")]
quiet: bool,
}

#[derive(Parser, Debug)]
Expand All @@ -36,28 +42,35 @@ enum Cmd {

pub fn execute(cmd: Args) -> Result<(), Error> {
let _guard = redirect_to_stderr(true);
let output = CommandOutput::from_quiet_and_verbose(cmd.quiet, cmd.verbose);
let pyproject = PyProject::load_or_discover(cmd.pyproject.as_deref())?;

// make sure we have the minimal virtualenv.
sync(SyncOptions::python_only().pyproject(cmd.pyproject))
.context("failed to sync ahead of run")?;
sync(
SyncOptions::python_only()
.pyproject(cmd.pyproject)
.output(output),
)
.context("failed to sync ahead of run")?;

if cmd.list || cmd.cmd.is_none() {
drop(_guard);
return list_scripts(&pyproject);
}
let args = match cmd.cmd {
Some(Cmd::External(args)) => args,
None => unreachable!(),
};

invoke_script(&pyproject, args, true)?;
invoke_script(&pyproject, args, true, &mut HashSet::new())?;
unreachable!();
}

fn invoke_script(
pyproject: &PyProject,
mut args: Vec<OsString>,
exec: bool,
seen_chain: &mut HashSet<OsString>,
) -> Result<ExitStatus, Error> {
let venv_bin = pyproject.venv_bin_path();
let mut env_overrides = None;
Expand Down Expand Up @@ -114,9 +127,17 @@ fn invoke_script(
if args.len() != 1 {
bail!("extra arguments to chained commands are not allowed");
}
if seen_chain.contains(&args[0]) {
bail!("found recursive chain script");
}
seen_chain.insert(args[0].clone());
for args in commands {
let status =
invoke_script(pyproject, args.into_iter().map(Into::into).collect(), false)?;
let status = invoke_script(
pyproject,
args.into_iter().map(Into::into).collect(),
false,
seen_chain,
)?;
if !status.success() {
if !exec {
return Ok(status);
Expand Down
5 changes: 5 additions & 0 deletions rye/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ impl SyncOptions {
self.pyproject = pyproject;
self
}

pub fn output(mut self, output: CommandOutput) -> Self {
self.output = output;
self
}
}

/// Config written into the virtualenv for sync purposes.
Expand Down
1 change: 1 addition & 0 deletions rye/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ pub const INSTA_FILTERS: &[(&str, &str)] = &[
(r" in (\d+\.)?\d+(ms|s)\b", " in [EXECUTION_TIME]"),
(r"\\\\?([\w\d.])", "/$1"),
(r"rye.exe", "rye"),
(r"exit (code|status)", "exit code"),
];

fn marked_tempdir() -> TempDir {
Expand Down
Loading
Loading