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

anki: 24.06.3 -> 24.11 #361951

Merged
merged 3 commits into from
Dec 9, 2024
Merged

anki: 24.06.3 -> 24.11 #361951

merged 3 commits into from
Dec 9, 2024

Conversation

cything
Copy link
Contributor

@cything cything commented Dec 5, 2024

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

Copy link
Member

@Defelo Defelo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested on x86_64-linux

@ofborg ofborg bot requested review from euank, oxij and martinetd December 5, 2024 22:40
@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Dec 5, 2024
@martinetd
Copy link
Member

passed on aarch64-linux but one of the rust tests failed on x86_64:

---- deckconfig::update::test::should_keep_at_least_one_remaining_relearning_step stdout ----
thread 'deckconfig::update::test::should_keep_at_least_one_remaining_relearning_step' panicked at rslib/src/scheduler/answering/mod.rs:557:61:
called `Result::unwrap()` on an `Err` value: InvalidInput { source: InvalidInputError { message: "bug: card modified without updating queue: id:1733439758805 card:1733439759 entry:1733439758", source: None, backtrace: None } }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
    deckconfig::update::test::should_keep_at_least_one_remaining_relearning_step

There have been other flaky tests that had been disabled in the past, would need to investigate these..
(iirc upstream fixed them so we could re-enable them)

@cything
Copy link
Contributor Author

cything commented Dec 7, 2024

That's weird cause it passes on my computer which is x86_64-linux too.

test deckconfig::update::test::should_increase_remaining_learning_steps_if_unpassed_learning_step_added ... ok
test dbcheck::test::tags ... ok
test deckconfig::update::test::should_increase_remaining_relearning_steps_if_unpassed_relearning_step_added ... ok
test deckconfig::update::test::should_keep_at_least_one_remaining_learning_step ... ok
test dbcheck::test::note_fields ... ok
test dbcheck::test::note_card_link ... ok

I also enabled the tests that were disabled for being flaky like you said and those pass too.

@martinetd
Copy link
Member

I could get deckconfig::update::test::should_keep_at_least_one_remaining_relearning_step to fail on my laptop by just running it in a loop (after ~1-2k tries; about 2 mins)

$ cargo test -vv should_keep_at_least_one_remaining_relearning_step
...
grab the very long line that actually ran something
$ i=0; while BUILDHASH='' CARGO=/home/asmadeus/.rustup/toolchains/1.82.0-x86_64-unknown-linux-gnu/bin/cargo CARGO_MANIFEST_DIR=/home/shared/anki/rslib CARGO_PKG_AUTHORS='Ankitects Pty Ltd and contributors <https://help.ankiweb.net>' CARGO_PKG_DESCRIPTION='Anki'\''s Rust library code' CARGO_PKG_HOMEPAGE='' CARGO_PKG_LICENSE=AGPL-3.0-or-later CARGO_PKG_LICENSE_FILE='' CARGO_PKG_NAME=anki CARGO_PKG_README=README.md CARGO_PKG_REPOSITORY='' CARGO_PKG_RUST_VERSION=1.80 CARGO_PKG_VERSION=0.0.0 CARGO_PKG_VERSION_MAJOR=0 CARGO_PKG_VERSION_MINOR=0 CARGO_PKG_VERSION_PATCH=0 CARGO_PKG_VERSION_PRE='' DESCRIPTORS_BIN=/home/shared/anki/out/rslib/proto/descriptors.bin LD_LIBRARY_PATH='/home/shared/anki/target/debug/build/blake3-9cf1804407af6b48/out:/home/shared/anki/target/debug/build/libsqlite3-sys-521a7856f28c3ca9/out:/home/shared/anki/target/debug/build/ring-1f5a96420d9a73dd/out:/home/shared/anki/target/debug/build/ring-de37fd4f38f3f6b1/out:/home/shared/anki/target/debug/build/zstd-sys-ceda26dcbb83d3fc/out:/home/shared/anki/target/debug/deps:/home/shared/anki/target/debug:/home/asmadeus/.rustup/toolchains/1.82.0-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib:/home/asmadeus/.rustup/toolchains/1.82.0-x86_64-unknown-linux-gnu/lib:/opt/wayland/lib64' MACOSX_DEPLOYMENT_TARGET=10.13.4 OUT_DIR=/home/shared/anki/target/debug/build/anki-913348601b716ed8/out PROTOC=/home/shared/anki/out/extracted/protoc/bin/protoc PYO3_NO_PYTHON=1 STRINGS_PY=/home/shared/anki/out/pylib/anki/_fluent.py STRINGS_TS=/home/shared/anki/out/ts/lib/generated/ftl.ts /home/shared/anki/target/debug/deps/anki-cb2bb8435ea2d543 should_keep_at_least_one_remaining_relearning_step; do i=$((i+1)); done; echo $i
....

---- deckconfig::update::test::should_keep_at_least_one_remaining_relearning_step stdout ----
thread 'deckconfig::update::test::should_keep_at_least_one_remaining_relearning_step' panicked at rslib/src/scheduler/answering/mod.rs:557:61:
called `Result::unwrap()` on an `Err` value: InvalidInput { source: InvalidInputError { message: "bug: card modified without updating queue: id:1733554619996 card:1733554620 entry:1733554619", source: None, backtrace: None } }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    deckconfig::update::test::should_keep_at_least_one_remaining_relearning_step

1759

If you have time reporting it like I did in ankitects/anki#3353 and just skipping it here is probably the way to go.

@cything
Copy link
Contributor Author

cything commented Dec 7, 2024

Thanks! I was able to reproduce it exactly like you did. First, it failed after 1.6k tries and during some high CPU usage, it failed after 400 tires. I also created an issue upstream: ankitects/anki#3619

Copy link
Member

@martinetd martinetd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@ofborg ofborg bot requested a review from martinetd December 8, 2024 15:42
@wegank wegank added 12.approvals: 2 This PR was reviewed and approved by two reputable people 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package and removed 12.approvals: 1 This PR was reviewed and approved by one reputable person labels Dec 8, 2024
Copy link
Member

@donovanglover donovanglover left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Grats on your first contribution to nixpkgs!

@donovanglover donovanglover merged commit 59d6168 into NixOS:master Dec 9, 2024
38 of 39 checks passed
@cything
Copy link
Contributor Author

cything commented Dec 9, 2024

Thank you so much!!

@cything cything deleted the bump-anki branch December 9, 2024 01:39
Copy link
Member

@dotlambda dotlambda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use fetchCargoVendor so we don't have to vendor the lock file.

@donovanglover
Copy link
Member

Forgot about that but agree. For anyone that has time, feel free to make a PR updating anki to use the new fetchCargoVendor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: games 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 12.approvals: 2 This PR was reviewed and approved by two reputable people 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants