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

--directory should resolve relative paths relative to current working directory #5613

Closed
charliermarsh opened this issue Jul 30, 2024 · 19 comments · Fixed by #7603
Closed

--directory should resolve relative paths relative to current working directory #5613

charliermarsh opened this issue Jul 30, 2024 · 19 comments · Fixed by #7603
Assignees
Labels
cli Related to the command line interface

Comments

@charliermarsh
Copy link
Member

No description provided.

@charliermarsh charliermarsh added the cli Related to the command line interface label Jul 30, 2024
@bluss
Copy link
Contributor

bluss commented Jul 30, 2024

Uv run's external command is opaque (in meaning), so it's not possible to translate those arguments. One part of the solution for uv run could be either to never change current directory, or set cwd back to the original for the child process.

@charliermarsh
Copy link
Member Author

I was thinking that we’d remove the current set_directory call, and then pass around a project directory as dictated by the directory flag.

@abdok96
Copy link

abdok96 commented Sep 12, 2024

Do you have any timeline for this change by any chance? I'm currently using the hidden --directory option but it would be great if it can be made public (and ideally add an environment variable for it). Also resolving the paths relative to the passed directory is causing very weird issues in different places. I think this change is the correct choice.

One example where paths relative to the current directory are necessary is when running uv commands from pre-commit local hooks, paths of the files staged for commit are automatically passed to the hook entry (uv command in this case) and the user cannot control this behavior.

Thanks for uv it is looking great!

@zanieb
Copy link
Member

zanieb commented Sep 13, 2024

per #6733 (comment) our existing behavior matches the Git and Cargo CLIs so we're very unlikely to change it.

The pre-commit example is great though. How do people work around this in other tools? Why do you need to use --directory in this situation? Maybe we need an environment variable to toggle.

@abdok96
Copy link

abdok96 commented Sep 13, 2024

Thanks for the quick reply.
In our case, the simplified project structure was briefly something like:

my-project/
|__ app/
| |__ main.py
| |__ pyproject.toml
|__ docker/
|__ docs/
|__ bin/
|__ docker-compose.yml
|__ .pre-commit.config.yaml

We also have a local hook for mypy something like:

  - repo: local
    hooks:
      - id: mypy
        name: run mypy check
        language: system
        entry: uv run --directory app -- mypy
        types_or: [python, pyi]
        require_serial: true
        args:
          - --config-file=pyproject.toml

We want to run the mypy command from the virtual environment that has all the python dependencies so that mypy can resolve and follow all the imports when running the type checks.

Because of the behavior of --directory we are passing --config-file as pyproject.toml instead of app/pyproject.toml but that's okay since we can control it.

However, when the commit hook is triggered by pre-commit, we have less control.
Let's say app/main.py was changed, the command executed by pre-commit will be something like:
uv run --directory app -- mypy --config-file=pyproject.toml app/main.py

And since the paths are resolved relative to the --directory param, mypy won't find a module matching app/app/main.py.

In our case, we managed to get around it by moving the pyproject.toml to the root directory which is probably a more suitable place. However, there might be similar cases that are a bit harder to avoid.

@abdok96
Copy link

abdok96 commented Sep 13, 2024

Btw, I'm not sure if it helps, but we are in the process of migrating from poetry to uv. I can confirm that in the case of poetry, the behavior is not similar to cargo or git. It resolves paths based on the working directory.

For example, this is the same hook I mentioned in my previous comment, before the migration to uv:

  - repo: local
    hooks:
      - id: mypy
        name: run mypy check
        language: system
        entry: poetry --directory app run mypy
        types_or: [python, pyi]
        require_serial: true
        args:
          - --config-file=app/pyproject.toml

Notice how despite the --directory param, we used to pass the --config-file relative to the working directory not /app.
I guess both options have good arguments. A toggle like you suggested might help.

@bluss
Copy link
Contributor

bluss commented Sep 13, 2024

@zanieb Unlikely to implement this issue? What do you think about adding a separate flag to uv run with the behaviour intended by this issue?

@bluss
Copy link
Contributor

bluss commented Sep 13, 2024

If we again look at git it has git -C x --git-dir y and maybe this could be a good model.

Uv can have one flag that changes directory, like -C and one flag that points out which project or pyproject.toml to discover and use as starting point. For convenience, this second project flag should also take a directory value.

Of course in the normal case these are linked and can be used for similar tasks.

@aivarannamaa
Copy link

aivarannamaa commented Sep 13, 2024

Expanding on the comment from bluss -- What about adding --project, which doesn't change the current directory, but simply indicates where to pick up the environment (or its specification)?

@aivarannamaa
Copy link

aivarannamaa commented Sep 13, 2024

One more idea: if --project references a script with inline metadata, this script would be used as the spec for the enironment to use.

For example:

  • uv python --project my_script_with_metadata.py find would create and report the ephemeral interpreter which would be used for running this script
  • uv tree --project my_script_with_metadata.py would list the packages in this ephemeral env. This would a good alternative for uv tree --script my_script.py, which I proposed under Support uv tree --script my_script.py #7328

I would use both of these forms for adding uv mode to Thonny IDE.

@charliermarsh
Copy link
Member Author

I am open to a dedicated --project flag on the uv run, uv sync, uv lock, etc. to inform workspace discovery. \cc @zanieb

@dragly
Copy link

dragly commented Sep 18, 2024

We are also very interested in a --project argument where the paths can remain relative to the current working directory. Our use-case is a monorepo where we are unable to use the workspace feature of uv due to conflicting dependencies between projects. This is the exact use of workspaces that is discouraged in the uv docs today.

(Another solution for us would be if uv had a feature where one could define a workspace without a shared uv.lock, but I think the --project argument sounds like a general solution that could solve more issues overall).

@bluss
Copy link
Contributor

bluss commented Sep 18, 2024

When I take a first look at this, I think --project in uv run --project maybe needs to be a global argument as well? (Or run-specific but handled early.) Because this logic is global (happens before any subcommand) and it is about project/workspace discovery. By the simplest logic: currently it's assumed project root is CWD (or found from CWD), the updated code needs to start with the user provided project directory instead of CWD here.

uv/crates/uv/src/lib.rs

Lines 103 to 129 in e36cc99

// Load configuration from the filesystem, prioritizing (in order):
// 1. The configuration file specified on the command-line.
// 2. The nearest configuration file (`uv.toml` or `pyproject.toml`) above the workspace root.
// If found, this file is combined with the user configuration file.
// 3. The nearest configuration file (`uv.toml` or `pyproject.toml`) in the directory tree,
// starting from the current directory.
let filesystem = if let Some(config_file) = cli.top_level.config_file.as_ref() {
if config_file
.file_name()
.is_some_and(|file_name| file_name == "pyproject.toml")
{
warn_user!("The `--config-file` argument expects to receive a `uv.toml` file, not a `pyproject.toml`. If you're trying to run a command from another project, use the `--directory` argument instead.");
}
Some(FilesystemOptions::from_file(config_file)?)
} else if deprecated_isolated || cli.top_level.no_config {
None
} else if matches!(&*cli.command, Commands::Tool(_)) {
// For commands that operate at the user-level, ignore local configuration.
FilesystemOptions::user()?
} else if let Ok(workspace) = Workspace::discover(&CWD, &DiscoveryOptions::default()).await {
let project = FilesystemOptions::find(workspace.install_path())?;
let user = FilesystemOptions::user()?;
project.combine(user)
} else {
let project = FilesystemOptions::find(&*CWD)?;
let user = FilesystemOptions::user()?;
project.combine(user)

@zanieb
Copy link
Member

zanieb commented Sep 18, 2024

I'm also willing to have --project — though Charlie said it'd be a pain to add support :)

@charliermarsh
Copy link
Member Author

I think project is pretty straightforward since it’s limited in scope as to what it affects. I assume it affects project discovery and maybe python-version?

@charliermarsh charliermarsh self-assigned this Sep 20, 2024
@charliermarsh
Copy link
Member Author

I'll do it.

@zanieb
Copy link
Member

zanieb commented Sep 20, 2024

Epic. Sounds good to me. The python-version file stuff might be easier with #6370

charliermarsh added a commit that referenced this issue Sep 21, 2024
## Summary

`uv run --project ./path/to/project` now uses the provided directory as
the starting point for any file discovery. However, relative paths are
still resolved relative to the current working directory.

Closes #5613.
@dragly
Copy link

dragly commented Oct 9, 2024

Thanks for implementing this! It works perfectly for our needs :)

@charliermarsh
Copy link
Member Author

That's great to hear :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Related to the command line interface
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants