Skip to content

Put Searching Parent Directories for Cargo.toml Behind Logical Gate #7871

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

Open
deg4uss3r opened this issue Feb 6, 2020 · 25 comments
Open

Put Searching Parent Directories for Cargo.toml Behind Logical Gate #7871

deg4uss3r opened this issue Feb 6, 2020 · 25 comments
Labels
A-cargo-api Area: cargo-the-library API and internal code issues A-filesystem Area: issues with filesystems A-manifest Area: Cargo.toml issues A-workspaces Area: workspaces C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-needs-team-input Status: Needs input from team on whether/how to proceed.

Comments

@deg4uss3r
Copy link

deg4uss3r commented Feb 6, 2020

See also rust-lang/rfcs#3279


Describe the problem you are trying to solve
In cargo-outdated we are seeing an issue (we build the project under /tmp) where if there's a cargo.toml that was moved to /tmp/ cargo build will fail.

Describe the solution you'd like
Either a seperate find_root() (link)that we can trigger from workspace::new(), or a config option to stop the function from crawling upwards (link) in the directory ancestors.

Notes
This was mentioned in #4992 (comment)

I understand this is necessary for cargo and it's the intended function; however, using cargo as an API there should be a way to avoid this :)

@deg4uss3r deg4uss3r added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Feb 6, 2020
@deg4uss3r deg4uss3r changed the title Put Searching Parent Directories Behind logical ate Put Searching Parent Directories Behind Logical Gate Feb 6, 2020
@deg4uss3r deg4uss3r changed the title Put Searching Parent Directories Behind Logical Gate Put Searching Parent Directories for Cargo.toml Behind Logical Gate Feb 6, 2020
@deg4uss3r
Copy link
Author

Just curious if we could chat about this issue? I’m happy to put in a PR :)

@TrondKjeldas
Copy link

Just want to mention another situation where this “feature” can cause problems.

When running cargo in a directory which resides on an NFS mount which is auto mounted, where e.g. /net is the automounter top level.

Cargo searches parent directories until it reaches /net/Cargo.toml, which causes automounter to try to find an NFS export for this name. Depending on the config this can take quite some time before it times out.

@goertzenator
Copy link

This also caused me problems when bringing up rust-analyzer in my dev environment: I was guided to create a Cargo.toml at the root of my large polyglot project so that rust-analyzer could find my crates. This of course moved where the expected Cargo.locks were being generated.

It is surprising to me that something in the Rust ecosystem is so vulnerable to environment side-effects when Rust the language is so very tight about that sort of thing.

@epage
Copy link
Contributor

epage commented Aug 15, 2023

rust-lang/rfcs#3279 is also related to this

@weihanglo weihanglo added A-cargo-api Area: cargo-the-library API and internal code issues S-needs-team-input Status: Needs input from team on whether/how to proceed. labels Aug 15, 2023
@samfux84
Copy link

samfux84 commented Mar 7, 2025

Is there any solution for this?

On our universities HPC cluster (with 4000+ users), the home directories are mounted via an automounter script in the path /cluster/home/$USER, which basically makes rust unusable:

sfux@eu-login-11:~$ module load stack/2024-06 rust/1.75.0-bsyu2z5
sfux@eu-login-11:~$ pwd
/cluster/home/sfux
sfux@eu-login-11:~$ cargo init hello
error: Failed to create package `hello` at `/cluster/home/sfux/hello`

Caused by:
  failed to read `/cluster/home/Cargo.toml`

Caused by:
  Permission denied (os error 13)
sfux@eu-login-11:~$

When cargo tries to access /cluster/home/Cargo.toml, then the automounter tries to mount a home directory for the inexistent user "Cargo.toml", which causes cargo to fail creating any new project.

Is there any workaround to make rust usable on systems that use an automounter?

@samfux84
Copy link

samfux84 commented Mar 7, 2025

Could it be a workaround to create an empty Cargo.toml file in $HOME?

sfux@eu-login-13:~$ module load stack/2024-06 rust/1.75.0-bsyu2z5
sfux@eu-login-13:~$ touch Cargo.toml
sfux@eu-login-13:~$ cargo init hello
warning: virtual workspace defaulting to `resolver = "1"` despite one or more workspace members being on edition 2021 which implies `resolver = "2"`
note: to keep the current resolver, specify `workspace.resolver = "1"` in the workspace root's manifest
note: to use the edition 2021 resolver, specify `workspace.resolver = "2"` in the workspace root's manifest
note: for more details see https://doc.rust-lang.org/cargo/reference/resolver.html#resolver-versions
     Created binary (application) package
sfux@eu-login-13:~$ cat Cargo.toml
workspace = { members = ["hello"] }
sfux@eu-login-13:~$

@samfux84
Copy link

The most recent stable release (1.85.0) still has this problem that makes it unusable on our HPC cluster:

sfux@eu-login-40:~$ module load stack/2024-06 rust/1.85.0
sfux@eu-login-40:~$ cargo new hello
    Creating binary (application) `hello` package
error: Failed to create package `hello` at `/cluster/home/sfux/hello`

Caused by:
  failed to read `/cluster/home/Cargo.toml`

Caused by:
  Permission denied (os error 13)
sfux@eu-login-40:~$

Any idea for a workaround? Help on this matter would be appreciated a lot.

@LvAprograms
Copy link

I am trying to start moving to rust for my scientific research on a university cluster, and am running in the same issue as @samfux84 above. It would be great if you could do something about this! I'd be happy to assist in any way I can.

@g-braeunlich
Copy link

A very quick and dirty hack as a workaround:
Assume /home/ is managed by an auto mounter and returns "permission denied" for /home/Cargo.toml.
Create a /home/Cargo.toml symlink, pointing to a non-existing file.
This will turn "permission denied" to "not found" which cargo can handle.

@RalfJung
Copy link
Member

This will turn "permission denied" to "not found" which cargo can handle.

This seems like there is some place where cargo distinguishes different error codes, and if we could teach it to treat "permission denied" as equivalent to "not found" while doing the workspace root search, that would fix the problem? (Well, it could still be slow if the automounter takes a while to determine that this file cannot be mounted, but hopefully it would cache that result. And better slow than broken. ;)

I dug through the codebase a bit but couldn't find where "not found" is treated specially. @epage do you think this could be an option?

@ChrisDenton
Copy link
Member

This seems like there is some place where cargo distinguishes different error codes, and if we could teach it to treat "permission denied" as equivalent to "not found" while doing the workspace root search, that would fix the problem?

What about if it's a genuine permissions issue? The user may intend for the root to be read but some issue with permissions or some other access issue prevents it and the failure mode is to just silently ignore the error?

@RalfJung
Copy link
Member

RalfJung commented Mar 19, 2025

The failure mode is to assume that the current crate is the workspace root, rather than being part of an outer workspace. The user may also have typo'd the root file name and then we can't find the root -- there's tons of failures we'll not be able to diagnose.

And the current alternative here is to make cargo essentially impossible to use on a range of university and HPC clusters, so... that seems worse?^^

@ChrisDenton
Copy link
Member

Right but "ignore errors" and "the status quo" hopefully aren't the only two possible options.

@RalfJung
Copy link
Member

RalfJung commented Mar 19, 2025

"ignore more errors", we're alreading ignoring ENOENT, obviously.

I have a hard time imagining other options. It turns out the reality is that EPERM can be a completely normal error code for missing files near the root of the directory tree, so we really shouldn't treat that as fatal. Obviously if the toml file the user explicitly asked to open via --manifest-path yields EPERM then we shouldn't ignore that, but we also don't ignore ENOENT there so that seems already fine.

Arguably the automounter should return ENOENT rather than EPERM, but well, that might be hard to change...

@Skgland
Copy link

Skgland commented Mar 19, 2025

For existing packages that don't already define a workspace, based on https://doc.rust-lang.org/cargo/reference/workspaces.html#root-package it should be sufficient to add an otherwise empty [workspace] declaration so that cargo knows the package dir is also the workspace root and doesn't search further for it. I would expect then only cargo init to be a problem and would expect the workaround of placing a Cargo.toml defining a workspace in ~user/ to work.

Though fixing the problem to not require a workaround would still be good.

@RalfJung
Copy link
Member

RalfJung commented Mar 19, 2025

So that means everyone on the cluster has to patch every Rust package they download from the web before they can build it? I don't think that can count as an acceptable work-around.^^

the workaround of placing a Cargo.toml defining a workspace in ~user/ to work.

That seems likely to break a lot of things as it will make the entire user dir a single huge workspace.

@Skgland
Copy link

Skgland commented Mar 19, 2025

So that means everyone on the cluster has to patch every Rust package they download from the web before they can build it? I don't think that can count as an acceptable work-around.^^

It's not a fix and definitely not ideal, but better than nothing in my opinion.

the workaround of placing a Cargo.toml defining a workspace in ~user/ to work.

That seems likely to break a lot of things as it will make the entire user dir a single huge workspace.

I currently don't have access to a computer to test. But based on https://doc.rust-lang.org/cargo/reference/workspaces.html#the-members-and-exclude-fields
I would expect cargo to stop searching once it finds a Cargo.toml defining a workspace and as the package is neither implicitly nor explicitly added as a member, I would expect the package to form it's own workspace.

cargo init adds created packages automatically to the workspace by adding it to the member list, as demonstrated in #7871 (comment), so after initializing a new package that explicit entry would need to be removed and then I would expect it to work as described in the previous paragraph.

@LvAprograms
Copy link

Update: I installed rust 1.85.0 on my cluster/home/user. cargo build didn't work out of the box, but after reading all your comments above I tried explicitly specifying the manifest to use cargo build --manifest-path $PWD/Cargo.toml --release. The build works! I had already tried that when loading the rust module on the cluster (see @samfux84's comments), but then it didn't work.

I will try to run it with slurm tomorrow morning with my data. That's on a different machine.

@epage
Copy link
Contributor

epage commented Mar 19, 2025

Create a /home/Cargo.toml symlink, pointing to a non-existing file.
This will turn "permission denied" to "not found" which cargo can handle.

I'd caution relying on this too much. This seems odd enough for this to be supported by design and may be in a gray area where behavior may change in the future.

I dug through the codebase a bit but couldn't find where "not found" is treated specially

The place where the logic lives is in find_workspace_root_with_loader.

This seems like there is some place where cargo distinguishes different error codes, and if we could teach it to treat "permission denied" as equivalent to "not found" while doing the workspace root search, that would fix the problem? (

I also am hesitant to read too much into specific errors. The line between "we should ignore an error" and "the error is preventing loading important information" is subtle.

I have a hard time imagining other options.

rust-lang/rfcs#3279 is one.

We have some built-in stop-paths

  • CARGO_HOME (which we should extend to CARGO_CACHE_HOME when we make that change)
  • target/package

I wonder if we should also have

  • HOME
  • TMPDIR

@mkj
Copy link

mkj commented Mar 20, 2025

We have some built-in stop-paths

* `CARGO_HOME` (which we should extend to `CARGO_CACHE_HOME` when we make that change)

At present I don't think CARGO_HOME stops the traversal? eg in https://bugzilla.yoctoproject.org/show_bug.cgi?id=15637 it is set but the traversal continues up to $HOME. (A bit unrelated to the EPERM issue here).

@epage
Copy link
Contributor

epage commented Mar 20, 2025

At present I don't think CARGO_HOME stops the traversal?

See

.take_while(|path| !path.curr.ends_with("target/package"))
// Don't walk across `CARGO_HOME` when we're looking for the
// workspace root. Sometimes a package will be organized with
// `CARGO_HOME` pointing inside of the workspace root or in the
// current package, but we don't want to mistakenly try to put
// crates.io crates into the workspace by accident.
.take_while(|path| {
if let Some(last) = path.last {
gctx.home() != last
} else {
true
}
})

eg in https://bugzilla.yoctoproject.org/show_bug.cgi?id=15637 it is set but the traversal continues up to $HOME. (A bit unrelated to the EPERM issue here).

That is for .cargo/config.toml and not Cargo.toml which is what we've mostly been talking about.

@RalfJung
Copy link
Member

RalfJung commented Mar 20, 2025

The place where the logic lives is in find_workspace_root_with_loader.

Yeah I found that, but nowhere there does not treat NotFound different from PermissionDenied. I couldn't find an exists check either. Am I blind?

rust-lang/rfcs#3279 is one.

I don't see anything in that RFC that would change behavior on EPERM, or that would stop the search when leaving the user's home dir. All I could find is that it introduces new errors when encountering a file owned by a different user.

We have some built-in stop-paths

Right, adding HOME there would help for these cases. It still feels a bit fragile to me, one could be working outside one's home directory and then cargo will fail just because I don't have permission to check whether /Cargo.toml exists?

@epage
Copy link
Contributor

epage commented Mar 20, 2025

Yeah I found that, but nowhere there does not treat NotFound different from PermissionDenied. I couldn't find an exists check either. Am I blind?

I think at this point, I'd have to debug it to understand it.

We have an exists check at

.filter(|ances_manifest_path| ances_manifest_path.exists())
so a symlink that points to nothing should be considered non-existent but then we should keep walking up, rather than stop.

I don't see anything in that RFC that would change behavior on EPERM, or that would stop the search when leaving the user's home dir. All I could find is that it introduces new errors when encountering a file owned by a different user.

I forgot it only errored and included an option to keep going rather than error, instead of stop.

Right, adding HOME there would help for these cases. It still feels a bit fragile to me, one could be working outside one's home directory and then cargo will fail just because I don't have permission to check whether /Cargo.toml exists?

Sorry I wasn't clear. That wasn't intended as a complete solution but one that would at least cover this case and likely lead to fewer problems generally so that if/when we have a more general solution, fewer people will need it.

@RalfJung
Copy link
Member

We have an exists check at

Ah! That's probably it, thanks. So there's a TOCTOU issue here, if a file gets deleted between that check and when cargo tries to open the file. But that's orthogonal to this issue.

Sorry I wasn't clear. That wasn't intended as a complete solution but one that would at least cover this case and likely lead to fewer problems generally so that if/when we have a more general solution, fewer people will need it.

👍

@samfux84
Copy link

Hi,

a quick status update from our side. Our system administrator has now implemented an exception in the automounter script for Cargo.toml, such that it will return a "file not found" instead of "permission denied".

Now cargo works on the cluster.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cargo-api Area: cargo-the-library API and internal code issues A-filesystem Area: issues with filesystems A-manifest Area: Cargo.toml issues A-workspaces Area: workspaces C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-needs-team-input Status: Needs input from team on whether/how to proceed.
Projects
None yet
Development

No branches or pull requests