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-cache: init at 0.3.4 #77310

Merged
merged 1 commit into from Aug 23, 2020
Merged

cargo-cache: init at 0.3.4 #77310

merged 1 commit into from Aug 23, 2020

Conversation

@Br1ght0ne
Copy link
Contributor

@Br1ght0ne Br1ght0ne commented Jan 8, 2020

Motivation for this change

https://users.rust-lang.org/t/cargo-cache-0-3-4-faster-cargo-home-caching-on-ci
The tag for 0.3.4 is missing.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

This change is Reviewable

Co-authored-by: Evan Stoll <evanjsx@gmail.com>
@Br1ght0ne
Copy link
Contributor Author

@Br1ght0ne Br1ght0ne commented Jan 8, 2020

@GrahamcOfBorg build cargo-cache

Copy link
Member

@yegortimoshenko yegortimoshenko left a comment

Unfortunately, tests are failing:

running 1 test
test run_tests ... FAILED

failures:

---- run_tests stdout ----
REGEX:
CARGO_HOME .*./xyxyxxxyyyxxyxyxqwertywasd. is not an existing directory!\n
OUTPUT:
CARGO_HOME "/build/source/./xyxyxxxyyyxxyxyxqwertywasd" is not an existing directory!
src = fetchFromGitHub {
owner = "matthiaskrgr";
repo = pname;
# TODO: use v${version} when a tag is available

This comment has been minimized.

@yegortimoshenko

yegortimoshenko Feb 21, 2020
Member

I requested this at matthiaskrgr/cargo-cache#79 and maintainer immediately tagged the release. It should now be available as 0.3.4, without the v.

--skip CargoCachePaths \
--skip alternative_registry_works \
--skip cargo_new_and_run_local
Comment on lines 21 to 23

This comment has been minimized.

@yegortimoshenko

yegortimoshenko Feb 21, 2020
Member

After cursory look at these tests, my impression is that alternative_registry_works requires internet access, and the other two could probably be made to work by setting HOME = "."?

This comment has been minimized.

@matthiaskrgr

matthiaskrgr Feb 21, 2020

Hi, yes, a couple of the cargo-cache tests require internet because I download crates/registry indices or initialize a new $CARGO_HOME at test-runtime.

@matthiaskrgr
Copy link

@matthiaskrgr matthiaskrgr commented Feb 21, 2020

So, I guess you are running cargo test in some kind of sandboxed environment which has no connection to the outside world?

I did a quick experiment and these should be the tests that require internet connectivity:

CARGO_HOME_subdirs_are_known
alternative_registry_works
remove_dirs
build_and_check_size_test
spurious_files_in_cache_test

Maybe I can add some kind of feature flag that only runs offline-tests and skips those.

@yegortimoshenko
Copy link
Member

@yegortimoshenko yegortimoshenko commented Feb 21, 2020

Yes, we are building in a sandboxed environment. Having a feature flag for that would be great! But just knowing the affected tests is very useful, thank you :)

@matthiaskrgr
Copy link

@matthiaskrgr matthiaskrgr commented Feb 21, 2020

So, I added the offline_tests feature now, so in git, or after next release, you should be able to run cargo test --features offline_tests in your sandboxed env!

@Br1ght0ne
Copy link
Contributor Author

@Br1ght0ne Br1ght0ne commented Feb 24, 2020

@matthiaskrgr I will update this PR after the release. Thank you!

@evanjs
Copy link
Member

@evanjs evanjs commented Jun 28, 2020

I did not see this before creating matthiaskrgr/cargo-cache#84
The good (for me :D) news is that the tests I mentioned in said issue do still fail.

The diff I tested
diff --git a/pkgs/development/tools/rust/cargo-cache/default.nix b/pkgs/development/tools/rust/cargo-cache/default.nix
index 8dee10fe2f3..5215f971cbe 100644
--- a/pkgs/development/tools/rust/cargo-cache/default.nix
+++ b/pkgs/development/tools/rust/cargo-cache/default.nix
@@ -7,26 +7,22 @@ rustPlatform.buildRustPackage rec {
   src = fetchFromGitHub {
     owner = "matthiaskrgr";
     repo = pname;
-    # TODO: use v${version} when a tag is available
-    rev = "f3d113cc74644c4581c127e16b03e319677d669f";
-    sha256 = "1bc9c1r964w0766k3yhf79d30ldv37vyiji8wnqspk5sl2gzn0iq";
+    rev = "v${version}";
+    sha256 = "0xhm7jlqq9nl1r8plx51x7aisza665f28d9h649b62904mx2ad7k";
   };
 
-  cargoSha256 = "1wzl8xvn3i5qgc4zmdqdh4kd8ww2ll2g14w4rf31i9lvq1ipa031";
+  cargoSha256 = "09vsalxh07bk1d48iyl0ncyzsscxlnf0dn96a2vbb1lin2qa2axm";
 
   buildInputs = lib.optional stdenv.isDarwin libiconv;
 
   checkPhase = ''
-    cargo test -- \
-      --skip CargoCachePaths \
-      --skip alternative_registry_works \
-      --skip cargo_new_and_run_local
+    cargo test --features offline_tests
   '';
 
   meta = with lib; {
-    description = "Manage cargo cache";
+    description = "manage cargo cache (\${CARGO_HOME}, ~/.cargo/), print sizes of dirs and remove dirs selectively";
     homepage = "https://github.com/matthiaskrgr/cargo-cache";
-    license = with licenses; [ asl20 mit ];
+    license = with licenses; [ asl20 /* or */ mit ];
     maintainers = with maintainers; [ filalex77 ];
   };
 }
The still-failing tests
cargo-cache> failures:
cargo-cache> ---- library::libtests::test_CargoCachePaths_paths stdout ----
cargo-cache> thread 'library::libtests::test_CargoCachePaths_paths' panicked at 'assertion failed: `(left == right)`
cargo-cache> left: `["source", "target", "cargo_home_cargo_cache_paths"]`,
cargo-cache> right: `["cargo-cache", "target", "cargo_home_cargo_cache_paths"]`', src/test_helpers.rs:51:5
cargo-cache> note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
cargo-cache> ---- library::libtests::test_CargoCachePaths_print stdout ----
cargo-cache> thread 'library::libtests::test_CargoCachePaths_print' panicked at 'assertion failed: `(left == right)`
cargo-cache> left: `["source", "target", "cargo_home_cargo_cache_paths_print"]`,
cargo-cache> right: `["cargo-cache", "target", "cargo_home_cargo_cache_paths_print"]`', src/test_helpers.rs:51:5
cargo-cache> failures:
cargo-cache> library::libtests::test_CargoCachePaths_paths
cargo-cache> library::libtests::test_CargoCachePaths_print
cargo-cache> test result: FAILED. 80 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out
cargo-cache> error: test failed, to rerun pass '--bin cargo-cache'
builder for '/nix/store/c5ifzay45wcr0pqzx2pqmjksq3p0l68y-cargo-cache-0.4.3.drv' failed with exit code 101; last 10 log lines:
   right: `["cargo-cache", "target", "cargo_home_cargo_cache_paths_print"]`', src/test_helpers.rs:51:5
  
  
  failures:
      library::libtests::test_CargoCachePaths_paths
      library::libtests::test_CargoCachePaths_print
  
  test result: FAILED. 80 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out
@evanjs
Copy link
Member

@evanjs evanjs commented Jul 28, 2020

Huge derp. checkFlagsArray works for cargo, too....
In short, with checkFlagsArray = [ "offline_tests"];, everything seems to work fine 😅

Nix Expression for 0.4.3
{ stdenv, lib, fetchFromGitHub, rustPlatform, libiconv }:

rustPlatform.buildRustPackage rec {
  pname = "cargo-cache";
  version = "0.4.3";

  src = fetchFromGitHub {
    owner = "matthiaskrgr";
    repo = pname;
    rev = version;
    sha256 = "0xhm7jlqq9nl1r8plx51x7aisza665f28d9h649b62904mx2ad7k";
  };

  cargoSha256 = "0gqhyav3dk8rbkcnq9m66z8gv60ipvc556zri6m1hps5jrffsfik";

  buildInputs = lib.optional stdenv.isDarwin libiconv;

  checkFlagsArray = [ "offline_tests"];

  meta = with lib; {
    description = "manage cargo cache (\${CARGO_HOME}, ~/.cargo/), print sizes of dirs and remove dirs selectively";
    homepage = "https://github.com/matthiaskrgr/cargo-cache";
    license = with licenses; [ asl20 /* or */ mit ];
    maintainers = with maintainers; [ filalex77 evanjs ];
  };
}
Diff of my expression for 0.4.3 against this PR
diff --git a/pkgs/development/tools/rust/cargo-cache/default.nix b/pkgs/development/tools/rust/cargo-cache/default.nix
index 8dee10fe2f3..537419a6d6b 100644
--- a/pkgs/development/tools/rust/cargo-cache/default.nix
+++ b/pkgs/development/tools/rust/cargo-cache/default.nix
@@ -2,31 +2,25 @@

 rustPlatform.buildRustPackage rec {
   pname = "cargo-cache";
-  version = "0.3.4";
+  version = "0.4.3";

   src = fetchFromGitHub {
     owner = "matthiaskrgr";
     repo = pname;
-    # TODO: use v${version} when a tag is available
-    rev = "f3d113cc74644c4581c127e16b03e319677d669f";
-    sha256 = "1bc9c1r964w0766k3yhf79d30ldv37vyiji8wnqspk5sl2gzn0iq";
+    rev = version;
+    sha256 = "0xhm7jlqq9nl1r8plx51x7aisza665f28d9h649b62904mx2ad7k";
   };

-  cargoSha256 = "1wzl8xvn3i5qgc4zmdqdh4kd8ww2ll2g14w4rf31i9lvq1ipa031";
+  cargoSha256 = "0gqhyav3dk8rbkcnq9m66z8gv60ipvc556zri6m1hps5jrffsfik";

   buildInputs = lib.optional stdenv.isDarwin libiconv;

-  checkPhase = ''
-    cargo test -- \
-      --skip CargoCachePaths \
-      --skip alternative_registry_works \
-      --skip cargo_new_and_run_local
-  '';
-
+  checkFlagsArray = [ "offline_tests"];
+
   meta = with lib; {
-    description = "Manage cargo cache";
+    description = "manage cargo cache (\${CARGO_HOME}, ~/.cargo/), print sizes of dirs and remove dirs selectively";
     homepage = "https://github.com/matthiaskrgr/cargo-cache";
-    license = with licenses; [ asl20 mit ];
+    license = with licenses; [ asl20 /* or */ mit ];
     maintainers = with maintainers; [ filalex77 ];
   };
 }

I think this should be good to merge given the above diff.

@Br1ght0ne
Copy link
Contributor Author

@Br1ght0ne Br1ght0ne commented Jul 31, 2020

Thank you @evanjs for your help! I applied your diff in 382d612 and added you as co-author. Thanks again!

@evanjs
Copy link
Member

@evanjs evanjs commented Aug 18, 2020

0.5.1 seems to work fine without any further changes.

diff --git a/pkgs/development/tools/rust/cargo-cache/default.nix b/pkgs/development/tools/rust/cargo-cache/default.nix
index 8c9bc4bffe5..f3756fe9468 100644
--- a/pkgs/development/tools/rust/cargo-cache/default.nix
+++ b/pkgs/development/tools/rust/cargo-cache/default.nix
@@ -2,16 +2,16 @@
 
 rustPlatform.buildRustPackage rec {
   pname = "cargo-cache";
-  version = "0.4.3";
+  version = "0.5.1";
 
   src = fetchFromGitHub {
     owner = "matthiaskrgr";
     repo = pname;
     rev = version;
-    sha256 = "0xhm7jlqq9nl1r8plx51x7aisza665f28d9h649b62904mx2ad7k";
+    sha256 = "02d593w1x8160p4m3jwm1dyvv383cy7njijlcaw49jczxv5isqbi";
   };
 
-  cargoSha256 = "0gqhyav3dk8rbkcnq9m66z8gv60ipvc556zri6m1hps5jrffsfik";
+  cargoSha256 = "0wpg0phfavd8fxl36nvanldiysy5xpk99qpnsc1jd6dw4ml01mbi";
 
   buildInputs = lib.optional stdenv.isDarwin libiconv;
Cargo cache '/home/evanjs/.cargo':

Total:                           724.46 MB
  22 installed binaries:          95.66 MB
  Registry:                      591.76 MB
    2 registry indices:          275.47 MB
    2058 crate archives:         194.77 MB
    45 crate source checkouts:   121.52 MB
  Git db:                         37.03 MB
    13 bare git repos:            36.27 MB
    3 git repo checkouts:        765.72 KB

Clearing cache...

Size changed from 724.46 MB to 602.18 MB (-122.28 MB, -16.87%)

@jonringer @marsam think we could get this merged after it's bumped to 0.5.1?

@marsam marsam force-pushed the Br1ght0ne:cargo-cache-0.3.4 branch from 382d612 to afdb826 Aug 23, 2020
@marsam
Copy link
Contributor

@marsam marsam commented Aug 23, 2020

@GrahamcOfBorg eval

@marsam marsam merged commit 8e3035e into NixOS:master Aug 23, 2020
16 of 17 checks passed
16 of 17 checks passed
cargo-cache, cargo-cache.passthru.tests on x86_64-darwin
Details
Evaluation Performance Report Evaluator Performance Report
Details
cargo-cache, cargo-cache.passthru.tests on aarch64-linux Success
Details
cargo-cache, cargo-cache.passthru.tests on x86_64-linux Success
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="afdb826"; rev="afdb826d000e4b27cbecb41413a01f7c32eeb685"; } ./pkgs/t
Details
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="afdb826"; rev="afdb826d000e4b27cbecb41413a01f7c32eeb685"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="afdb826"; rev="afdb826d000e4b27cbecb41413a01f7c32eeb685"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="afdb826"; rev="afdb826d000e4b27cbecb41413a01f7c32eeb685"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="afdb826"; rev="afdb826d000e4b27cbecb41413a01f7c32eeb685"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="afdb826"; rev="afdb826d000e4b27cbecb41413a01f7c32eeb685"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="afdb826"; rev="afdb826d000e4b27cbecb41413a01f7c32eeb685"; } ./pkgs/t
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.