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

Doesn't seem to cache crates installed via cargo install #59

Closed
mightyiam opened this issue May 8, 2022 · 13 comments
Closed

Doesn't seem to cache crates installed via cargo install #59

mightyiam opened this issue May 8, 2022 · 13 comments

Comments

@mightyiam
Copy link

No description provided.

@mightyiam
Copy link
Author

Perhaps because the automatic key does not include ~/.cargo/.crates.toml (or ~/.cargo/.crates2.json — I don't know the story behind this duplicity)?

@mightyiam
Copy link
Author

I'm trying to read the code and understand how saving and restoring bins works. First question: why is there special handling of globally installed binary crates? Why aren't they simply an equal part of the cache?

What I expect is that if in my job I cargo install foo then on the next invocation of that in the next 'run' I will see the same as what I would locally. For example:

    Updating crates.io index
     Ignored package `itree v0.3.1` is already installed, use --force to override

But I don't see that. What I see in the 'run' logs is that the crate is compiled (and perhaps also downloaded, don't remember) again.

Is this by design, please?

Also, I don't see a test suite, so that doesn't make me confident in contributing.

@Swatinem
Copy link
Owner

why is there special handling of globally installed binary crates? Why aren't they simply an equal part of the cache?

Because the globally installed binary crates can be huge, and saving/loading them from cache adds considerable overhead in time and size. As they are part of the runner environment, the assumption is that they will be repopulated on every run, though that does not necessarily work for self hosted runners.

@mightyiam
Copy link
Author

I'd like to better understand this assumption, please. How about we start with "they are part of the runner environment". I'm not sure I understand what you mean by that. Do GitHub Actions hosted runners have prepopulated ~/.cargo/bin?

@Swatinem
Copy link
Owner

Do GitHub Actions hosted runners have prepopulated ~/.cargo/bin?

Yes, or maybe its the toolchain action that pre-populates .cargo/bin, not entirely sure. Either way, there can be hundreds of MiB worth of binaries in there that should not end up in the cache.a

Just removing all the pre-populated files however can cause issues like #16

@mightyiam
Copy link
Author

Yeah... The lack of an npx-like feature in cargo means that global executable crates are used instead. For what it's worth, we're using cargo-run-bin to have npx-like behavior and that means that our executables end up in .bin (not ~/.cargo/bin).

mgeisler added a commit to mgeisler/comprehensive-rust that referenced this issue Jan 18, 2023
@mgeisler
Copy link

Hey there, I've noticed the same problem, but in the context of a crate which I install with cargo install --path. I don't see any problem with crates installed from crates.io. As an example, this build took 2 seconds to restore the cache and then 0 seconds to "install" mdbook.

First, I can confirm that there are binaries in ~/.cargo/bin from the get-go:

$ ls ~/.cargo/bin
bindgen
cargo
cargo-audit
cargo-clippy
cargo-fmt
cargo-miri
cargo-outdated
cbindgen
clippy-driver
rls
rust-gdb
rust-gdbgui
rust-lldb
rustc
rustdoc
rustfmt
rustup

This is from a test run where ls ~/.cargo/bin was the very first build step. I don't do any calls to rustup or to any toolchain action.

As for my situation with cargo install --path not being cached, I think I'll simply change my

$ cargo install --path i18n-helpers --locked

command to something like

$ cd i18n-helpers && cargo build

and then I will execute the binaries from the target/ folder. Or perhaps copy them to a directory on my $PATH — such as ~/.cargo/bin! 😄

Doing this will also nicely work around rust-lang/cargo#7169 and actually give me a reproducible build ✨

mgeisler added a commit to google/comprehensive-rust that referenced this issue Jan 18, 2023
There are two problems with `cargo install --locked --path`:

- `cargo install --path` does not actually honor the lock file, see
  rust-lang/cargo#9289.

- `cargo install --path` is not cached, see
  Swatinem/rust-cache#59.
mgeisler added a commit to google/comprehensive-rust that referenced this issue Jan 18, 2023
There are two problems with using `cargo install --locked --path`:

- `cargo install --path` does not actually honor the lock file, see
  rust-lang/cargo#9289.

- `cargo install --path` is not cached, see
  Swatinem/rust-cache#59.

We should be able to work around them by using `cargo build` instead.
We don’t actually need a release build for the i18n-helpers, so this
will also be a little bit faster on the initial build.
mgeisler added a commit to google/comprehensive-rust that referenced this issue Jan 18, 2023
There are two problems with using `cargo install --locked --path`:

- `cargo install --path` does not actually honor the lock file, see
  rust-lang/cargo#9289.

- `cargo install --path` is not cached, see
  Swatinem/rust-cache#59.

We should be able to work around them by using `cargo build` instead.
We don’t actually need a release build for the i18n-helpers, so this
will also be a little bit faster on the initial build.
@mgeisler
Copy link

mgeisler commented Mar 7, 2023

Hi @Swatinem, do you have a suggestion for working around this?

I ran into a problem with the action saying Cache Configuration even though a cargo install --locked --version resulted in a long compilation.

That is, I'm installing mdbook using cargo install mdbook --locked --version 0.4.28 and when I bumped the version number just now, I notice that the new binary is not cached. This can be seen in this run.

From what I see, binaries in ~/.cargo/bin are cached fine and if I were to clear the caches, the new version would also be cached correctly. Could it be that the logic that checks if the cache is up to date uses the filenames only instead of looking at, say, the mtime or a checksum of the files? That would explain why updates binaries in ~/.cargo/bin aren't picked up.

@Swatinem
Copy link
Owner

Swatinem commented Mar 7, 2023

That is possible indeed, the .cargo/bin logic works only with filenames.

@mgeisler
Copy link

That is possible indeed, the .cargo/bin logic works only with filenames.

Okay, that explains it 😄 Hashing the files before/after would be great, but that might of course incur some unacceptable overhead.

@mightyiam
Copy link
Author

@stevenh
Copy link
Contributor

stevenh commented May 18, 2023

This should be fixed by #137

@stevenh
Copy link
Contributor

stevenh commented May 23, 2023

@Swatinem Ok to close this one?

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

No branches or pull requests

4 participants