indexmap-rs/indexmap

`indexmap` is rebuilt on every `cargo` run

alin-at-dfinity opened this issue · 5 comments

alin-at-dfinity commented

From what I can tell, the changes in #106 caused one of our CI jobs' run times to shoot up from ~23 minutes (while depending on indexmap 1.1.0) to 1 1/2 - 2 hours (about 4x, while depending on indexmap 1.3.2).

Due to the shortcomings of cargo we need to run it 4 times in succession to cover all bases:

$ cargo build --release --all-targets      # Build production code
$ cargo test --no-run --release            # Build tests and doc tests
$ cargo test --no-run --release --benches  # Build benchmarks
$ cargo test --release                     # Run tests and doc tests

Normally this is not an issue, as the cargo build at the top usually builds everything, the next two are no-ops, and the last one again doesn't need to build anything, only run the tests. But after bumping indexmap to 1.3.2 in Cargo.lock, suddenly each of those cargo commands first starts rebuilding indexmap and then everything that depends on it, essentially doing the exact same thing 4 times.

AFAICT that is because of the use of autocfg in build.rs. Every time cargo is run, it thinks indexmap has been modified and rebuilds it. Which in turn causes everything that depends on it to be rebuilt. Although nothing at all has changed.

I don't have a solution and I've already spent way too much time tracking this down and finding a workaround (rolled back indexmap to 1.2.0), but it would be great if you could find a way to convince cargo that nothing has changed and it doesn't need to rebuild indexmap every single time.

(BTW, this is not only an issue for CI, any local build or test run will also start by rebuilding indexmap and go from there.)

cuviper commented

Are you perhaps using --remap-path-prefix? We realized in rust-num/num-traits#139 that this is reflected in file!(), which makes autocfg::rerun_path(file!()) behave poorly since that path never actually exists from cargo's perspective. We should just use "build.rs" there.

  • 👍1
alin-at-dfinity commented

We do use Nix for reproducible builds, so it may well be using --remap-path-prefix under the cover. But I'm not closely familiar with Nix.

pikajude commented

Yeah we do use --remap-path-prefix

bluss commented

Thanks for providing so much good information in the bug report.

alin-at-dfinity commented

FYI, bumping the dependency to indexmap 1.4.0 appears to have fixed the issue for us.

Thanks a lot for the quick fix (and point release).

  • 👍2