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

Cargo geiger deletes the entire target directory without warning #168

Open
martin-t opened this issue Dec 29, 2020 · 4 comments
Open

Cargo geiger deletes the entire target directory without warning #168

martin-t opened this issue Dec 29, 2020 · 4 comments
Labels
enhancement New feature or request help wanted Extra attention is needed important If you want to contribute, please consider this issue before others.
Milestone

Comments

@martin-t
Copy link

This is very undesirable for several reasons:

  • Recompiling some projects can be very time consuming.
  • target/ can sometimes store things people want to keep long-term like results of benchmarks (criterion stores them in target/criterion/) - geiger deletes the entire target/ directory even though it seems to only need target/debug/ and target/.rustc_info.json.
  • When sharing target/ among multiple projects as described here, it deletes everything so i have to rebuild every rust project on my computer from scratch.
    • I have not tested what happens when i run geiger on several projects at the same time.
    • I have not looked at how geiger interacts with sccache.

I don't know why it needs to delete anything at all - my understanding is everything in target/ should be checksumed so if things get built with different config options, it'll result in different hashes - there should be no conflicts.

If geiger really needs to rebuild everything in some weird way incompatible with how target normally works, it should use a separate directory (perhaps target/geiger/).

@martin-t martin-t changed the title Cargo geiger delets the entire target directory without warning Cargo geiger deletes the entire target directory without warning Dec 29, 2020
@anderejd anderejd added enhancement New feature or request important If you want to contribute, please consider this issue before others. labels Jan 1, 2021
@anderejd
Copy link
Contributor

anderejd commented Jan 1, 2021

Avoiding deletion of the target directory or using a dedicated directory would be a great improvement. The reason it currently does that is to make sure everything under target is related to the cargo-geiger + cargo + rustc invocation.

@anderejd anderejd added the help wanted Extra attention is needed label Jan 1, 2021
@anderejd
Copy link
Contributor

anderejd commented Jan 5, 2021

I don't know why it needs to delete anything at all - my understanding is everything in target/ should be checksumed so if things get built with different config options, it'll result in different hashes - there should be no conflicts.

Also yes, if the checksums are possible to rely on by programs other than cargo (and rustc?) then that would be even better. When I decided to delete the target directory, as a quick and dirty solution, I did briefly look into the checksums as well but did not spend enough time to be sure if they could be relied upon and how to do it properly.

@martin-t
Copy link
Author

martin-t commented Jan 5, 2021

I think as a quickfix, it should at least ask before deleting and then it should only delete the debug/ subdirectory - that would fix the IMO 2 most serious issues (deleting more than needed and doing it without warning so people can lose data).

@Nemo157
Copy link

Nemo157 commented Jan 15, 2021

This also fails when the directory is undeletable (because in my case it's a mount-point):

> RUST_BACKTRACE=1 cargo geiger
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Cargo("could not remove build directory")', crates.io/cargo-geiger-0.10.2/src/cli.rs:277:58
stack backtrace:
   0: rust_begin_unwind
             at /rustc/da305a2b00530aa34dea4e48389204c26fa35dbb/library/std/src/panicking.rs:493:5
   1: core::panicking::panic_fmt
             at /rustc/da305a2b00530aa34dea4e48389204c26fa35dbb/library/core/src/panicking.rs:92:14
   2: core::option::expect_none_failed
             at /rustc/da305a2b00530aa34dea4e48389204c26fa35dbb/library/core/src/option.rs:1266:5
   3: cargo_geiger::cli::run_scan_mode_default
   4: cargo_geiger::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Though it did go through and blow away the entire contents before failing.

@pinkforest pinkforest added this to the M-0.12.0 milestone Jan 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed important If you want to contribute, please consider this issue before others.
Projects
None yet
Development

No branches or pull requests

4 participants