kbknapp / cargo-outdated

A cargo subcommand for displaying when Rust dependencies are out of date
MIT License
1.22k stars 96 forks source link

error if /tmp/Cargo.toml exists #194

Open warner opened 4 years ago

warner commented 4 years ago

cargo-outdated is great.. thanks for writing it!

I noticed recently that if my /tmp/ directory contains a Cargo.toml, running cargo outdated fails with a strange error:

warner:~$ cargo new foo
     Created binary (application) `foo` package
warner:~$ cd foo
warner:~/foo$ cargo run
   Compiling foo v0.1.0 (/home/warner/foo)
    Finished dev [unoptimized + debuginfo] target(s) in 0.91s
     Running `target/debug/foo`
Hello, world!

warner:~/foo$ cargo outdated
All dependencies are up to date, yay!

warner:~/foo$ cp Cargo.toml /tmp/
warner:~/foo$ cargo outdated
error: failed to parse manifest at `/tmp/Cargo.toml`

Caused by:
  no targets specified in the manifest
  either src/lib.rs, src/main.rs, a [lib] section, or [[bin]] section must be present
warner:~/foo$ 

(I don't remember why I wound up with a Cargo.toml in /tmp, maybe I moved something there to stash it momentarily).

Is the program maybe using /tmp/ as a working directory, rather than creating a new (empty) tempdir to work from? What other files is it modifying or reading from /tmp/ ?

deg4uss3r commented 4 years ago

Hi @warner I am not able to recreate this on MacOS. I copied both the same project's Cargo.toml and, for fun/trying to really confuse cargo-outdated, a different and more complicated project's Cargo.toml.

Which OS was this on, and what version of cargo-outdated are you running? I tried the one cargo installsv0.9.1, the one in masterv0.9.2, and the one in the PRv0.9.3 all of these passed.

warner commented 4 years ago

Hm. It was Linux (Debian "buster"), and cargo-outdated v0.9.1. I got the same result on an Ubuntu 18.04 "bionic" platform (and which was a server, rather than my development machine, so perhaps less likely to have a weird forgotten configuration change lurking about). My MacOS machine is in the shop, but in a week or so I can try to reproduce on there.

I know MacOS usually has a more interesting variety of tempdirs than linux.. what does python -c "import tempfile; print(tempfile.gettempdir())" show you?

I'll see if strace tells me anything about what it's doing in /tmp/.

warner commented 4 years ago

strace shows it doing a lot of work in a subdirectory of /tmp, but also looking in /tmp itself for some files. Here's a stripped down copy of the strace output:

mkdir("/tmp/cargo-outdated7qRl8L", 0777) = 0
openat(AT_FDCWD, "/home/warner/foo/Cargo.toml", O_RDONLY|O_CLOEXEC) = 3
openat(AT_FDCWD, "/tmp/cargo-outdated7qRl8L/Cargo.toml", O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0100644) = 4
copy_file_range(3, NULL, 4, NULL, 219, 0) = -1 EXDEV (Invalid cross-device link)
read(3, "[package]\nname = \"foo\"\nversion ="..., 8192) = 219
write(4, "[package]\nname = \"foo\"\nversion ="..., 219) = 219
read(3, "", 8192)                       = 0
close(4)                                = 0
close(3)                                = 0
openat(AT_FDCWD, "/tmp/cargo-outdated7qRl8L/Cargo.lock", O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0100644) = 4
...
statx(AT_FDCWD, "/tmp/cargo-outdated7qRl8L/.cargo/config", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffe35b788a0) = -1 ENOENT (No such file or directory)
statx(AT_FDCWD, "/tmp/cargo-outdated7qRl8L/.cargo/config.toml", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffe35b788a0) = -1 ENOENT (No such file or directory)
statx(AT_FDCWD, "/tmp/.cargo/config", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffe35b788a0) = -1 ENOENT (No such file or directory)
statx(AT_FDCWD, "/tmp/.cargo/config.toml", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffe35b788a0) = -1 ENOENT (No such file or directory)
statx(AT_FDCWD, "/.cargo/config", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffe35b788a0) = -1 ENOENT (No such file or directory)
statx(AT_FDCWD, "/.cargo/config.toml", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffe35b788a0) = -1 ENOENT (No such file or directory)
statx(AT_FDCWD, "/home/warner/.cargo/config", AT_STATX_SYNC_AS_STAT, STATX_ALL, {stx_mask=STATX_BASIC_STATS, stx_attributes=0, stx_mode=S_IFREG|0644, stx_size=411, ...}) = 0
statx(AT_FDCWD, "/home/warner/.cargo/config.toml", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffe35b788a0) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/home/warner/.cargo/config", O_RDONLY|O_CLOEXEC) = 3
openat(AT_FDCWD, "/tmp/cargo-outdated7qRl8L/Cargo.toml", O_RDONLY|O_CLOEXEC) = 3
openat(AT_FDCWD, "/tmp/cargo-outdated7qRl8L/Cargo.toml", O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0666) = 3
openat(AT_FDCWD, "/tmp/cargo-outdated7qRl8L/Cargo.toml", O_RDONLY|O_CLOEXEC) = 3
statx(AT_FDCWD, "/tmp/cargo-outdated7qRl8L/src/lib.rs", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffe35b711b0) = -1 ENOENT (No such file or directory)
...
statx(AT_FDCWD, "/tmp/cargo-outdated7qRl8L/build.rs", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffe35b711b0) = -1 ENOENT (No such file or directory)
statx(AT_FDCWD, "/tmp/Cargo.toml", AT_STATX_SYNC_AS_STAT, STATX_ALL, {stx_mask=STATX_ALL, stx_attributes=0, stx_mode=S_IFREG|0644, stx_size=219, ...}) = 0
openat(AT_FDCWD, "/tmp/Cargo.toml", O_RDONLY|O_CLOEXEC) = 3
statx(AT_FDCWD, "/tmp/src/lib.rs", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffe35b711b0) = -1 ENOENT (No such file or directory)
statx(AT_FDCWD, "/tmp/src/main.rs", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffe35b711b0) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/tmp/src/bin", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/tmp/examples", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/tmp/tests", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/tmp/benches", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
statx(AT_FDCWD, "/tmp/build.rs", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffe35b711b0) = -1 ENOENT (No such file or directory)
statx(AT_FDCWD, "/tmp/cargo-outdated7qRl8L", AT_STATX_SYNC_AS_STAT|AT_SYMLINK_NOFOLLOW, STATX_ALL, {stx_mask=STATX_ALL, stx_attributes=0, stx_mode=S_IFDIR|0755, stx_size=4096, ...}) = 0

after that it just deletes the contents of the tempdir and prints the error message.

It smells like it copies the project (including Cargo.toml) into a new empty working directory (/tmp/cargo-outdated7qRl8L), then walks upwards from there to find the project root, and for some reason it fails to stop in the working directory and just charges all the way up to /tmp.

deg4uss3r commented 4 years ago

I agree that’s probably what’s happening, I’ll boot up my Ubuntu VM and try there soon, thanks for all the wonderful info it will definitely help.

deg4uss3r commented 4 years ago

After diving into this I found it is an intended affect of the Cargo API. Looks like https://github.com/rust-lang/cargo/issues/1423 is an (older) issue tracking the affect of this on some projects.

And is specifically traced to this line here: https://github.com/rust-lang/cargo/blob/master/src/cargo/core/workspace.rs#L415

I took a look around and nothing in the API will prevent the line from getting called when we build the workspace in /tmp/, so having a Cargo.toml in /tmp/ or even / will trigger the cargo outdated command to fail.

I don't see a good way to fix this (only the bad way of removing any upper level Cargo.toml files under /tmp) unless cargo provides a config option to lock the workspace root as top level.

warner commented 4 years ago

Huh, weird, thanks for investigating!

I read through the ticket, but it wasn't clear to me whether their intended behavior was just to search downwards for Cargo.toml files, or whether it should also be walking upwards. I don't yet see how a project rooted in /tmp/FOO (using /tmp/FOO/Cargo.toml as the main configuration file) would benefit (according to the not-relying-on-a-top-level-manifest criteria in that ticket) from looking at /tmp/Cargo.toml. Sounds to me like an unintended behavior from a maybe-not-ideal feature.

If I can look into this some more, I'll add a note to that ticket mentioning the crawling-upwards behavior and see if that also feels to them like a regrettable-but-somewhat-intended.

Also I'll experiment with doing a normal build (cargo build, omitting cargo-oudated entirely) in a subdirectory, and see if strace shows it looking for Cargo.toml above that subdirectory.

deg4uss3r commented 4 years ago

The bug linked in that issue (https://github.com/rust-lang/cargo/issues/4992) is exactly what we’re seeing here as well, it was just closed in favor of the original issue.

I did do:

mkdir test
cd test
touch Cargo.toml
cargo new foo —bin 
cd foo
cargo build

And that fails as well, actually on the cargo new command will warn you about the upper level Cargo.toml file and assume you mean to use virtual workspaces, which makes sense.

I can also put in a comment to see if this could be gated with an option, i can see why this is normal behavior for Cargo, but the API it should have a way around it.

nashley commented 2 years ago

It looks like this was fixed upstream. Is this still an issue?