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

castor, gitAndTools.git-interactive-rebase-tool, the-way: disable checks #97347

Merged
merged 3 commits into from Sep 7, 2020

Conversation

@zowoq
Copy link
Contributor

@zowoq zowoq commented Sep 7, 2020

Motivation for this change

These three have been broken since #95622 as we can't pass test-threads twice.

Just disabling the checks for now as 20.09 branch off is close and a proper fix will probably cause a rust rebuild.

Running cargo test --release --target x86_64-unknown-linux-gnu --frozen --  --test-threads=1
   Compiling git-interactive-rebase-tool v1.2.1 (/build/source)
    Finished release [optimized] target(s) in 6.78s
     Running target/x86_64-unknown-linux-gnu/release/deps/interactive_rebase_tool-2ed5e41c7143da47
error: Option 'test-threads' given more than once
error: test failed, to rerun pass '--bin interactive-rebase-tool'
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.
@fgaz
fgaz approved these changes Sep 7, 2020
Copy link
Member

@fgaz fgaz left a comment

Looks fine. How do we remember to revert this when the issue is fixed?

@masaeedu
Copy link
Contributor

@masaeedu masaeedu commented Sep 7, 2020

@zowoq If I remember correctly, the tests for git-interactive-rebase-tool sporadically fail unless --test-threads is set to 1. See MitMaro/git-interactive-rebase-tool#172 (comment). I don't know if this has since been fixed, but if it hasn't, do the current settings mean that a user with NIX_BUILD_CORES set to e.g. 4 will experience failing tests while trying to build this package?

EDIT: Ah, nevermind, I didn't spot the doCheck = false. LGTM.

zowoq added 3 commits Sep 7, 2020
disable until buildRustPackage supports setting test-threads in packages
disable until buildRustPackage supports setting test-threads in packages
disable until buildRustPackage supports setting test-threads in packages
@zowoq zowoq force-pushed the zowoq:rust-test-threads branch from 62de0c1 to 28f3559 Sep 7, 2020
@ofborg ofborg bot requested review from fgaz and masaeedu Sep 7, 2020
@zowoq zowoq merged commit 8b1690a into NixOS:master Sep 7, 2020
19 of 20 checks passed
19 of 20 checks passed
tests tests
Details
action
Details
castor, castor.passthru.tests, gitAndTools.git-interactive-rebase-tool, gitAndTools.git-interactive-rebase-tool.passthru.tests, the-way, the-way.passthru.tests on x86_64-darwin
Details
Evaluation Performance Report Evaluator Performance Report
Details
Wait for ofborg
Details
castor, castor.passthru.tests, gitAndTools.git-interactive-rebase-tool, gitAndTools.git-interactive-rebase-tool.passthru.tests, the-way, the-way.passthru.tests on aarch64-linux Success
Details
castor, castor.passthru.tests, gitAndTools.git-interactive-rebase-tool, gitAndTools.git-interactive-rebase-tool.passthru.tests, the-way, the-way.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="28f3559"; rev="28f3559cc2d5d7ed7c1704ea1ef1ebe8a5f7aee5"; } ./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="28f3559"; rev="28f3559cc2d5d7ed7c1704ea1ef1ebe8a5f7aee5"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="28f3559"; rev="28f3559cc2d5d7ed7c1704ea1ef1ebe8a5f7aee5"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="28f3559"; rev="28f3559cc2d5d7ed7c1704ea1ef1ebe8a5f7aee5"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="28f3559"; rev="28f3559cc2d5d7ed7c1704ea1ef1ebe8a5f7aee5"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="28f3559"; rev="28f3559cc2d5d7ed7c1704ea1ef1ebe8a5f7aee5"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="28f3559"; rev="28f3559cc2d5d7ed7c1704ea1ef1ebe8a5f7aee5"; } ./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
@zowoq zowoq deleted the zowoq:rust-test-threads branch Sep 7, 2020
@wkral
Copy link
Contributor

@wkral wkral commented Sep 9, 2020

I noticed that jwt-cli is also failing because of the --test-threads argument: Hydra log.

I can disable checks and backport to 20.09 for ZHF, but I'm a little unclear are you not expecting the proper fix to land for 20.09?

@zowoq
Copy link
Contributor Author

@zowoq zowoq commented Sep 9, 2020

I've opened a draft with a fix, I'll test/finish it later today. #97603


I can disable checks

Please do. Feel free to ping me, I'll merge and cherrypick it to 20.09.

are you not expecting the proper fix to land for 20.09?

We can backport the fix after it lands in master.

@wkral
Copy link
Contributor

@wkral wkral commented Sep 9, 2020

Looking at your fix and at the jwt-cli failure it's actually a little different. It seams that maybe the jwt-cli test is trying to parse the --test-threads argument which should be consumed by cargo. I'm not sure why exactly, but I don't think the single threaded test fix you're proposing will fix this particular issue.

I'll look into why it's happening, but if you've seen this before then I'd appreciate the help.

@wkral
Copy link
Contributor

@wkral wkral commented Sep 10, 2020

Figured it out, propsoed an upstream fix and created #97615. Thanks for the help @zowoq.

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

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