anki: 24.06.2 -> 24.06.3, rust 1.80 fix #333602
Conversation
This doesn't have anything too interesting that I can see but might as well update to latest release if we're going to rebuild
rust 1.80 requires updating the 'time' crate to at least 0.3.35. That update is already in anki's master branch but it isn't anywhere close to time for a new release, so just bump time and only time manually for our tree. (This also fixes anki-sync-server which usese the same sources/Cargo deps, when removing the patch during the next update it will need to be removed from both files as written in comment) Link: NixOS#332957
martinetd
force-pushed
the
anki_rust_1.80
branch
from
846e31b
to
b3fe728
Compare
|
It seems the anki-sync-server tests aren’t happy. (It’s funny to me that this is under |
hmm... For the tests while building the package... But it passes locally (I could build the package when submitting this); so the problem doesn't look like related to the Looking at the test it's checking answering the same card twice yields the same result but it looks like on ofborg one test ran in under 1s and not the other... I'll have a closer look tomorrow. |
|
It might be an ofborg‐specific problem, if Hydra has managed to build this package successfully in the past. I’ll try on the community builders. |
|
Result of 9 packages built:
Two things that stick out to me:
|
Yes, there's only a handful of patches between 24.06.2 and 24.06.3
Hmm, good point, but I prefer the patch for two reasons:
If you think it's better to re-use our Cargo.lock as you suggest I'll update the patch.
I've tried locally and that particular test passes whatever I do :/ it only runs in 0.02s on a fairly old laptop so I doubt my theory that something took over a second, unless there was some hiccup somewhere that froze everything for a while... |
There was a problem hiding this comment.
I think we can ignore the failing test unless it breaks on Hydra; it’s probably just flakiness that ofborg happened to stumble upon. The patch doesn’t meaningfully increase repository size, and it’s temporary, so I’m fine with keeping it. Thanks for working on this!
|
Thanks for the review! (and thanks for clearing up my misunderstanding with staging and staging-next in the other thread; I was thinking of the two the other way around...) |
|
I’m just waiting on the x86-64 community builder to get through the build to make sure it’ll go through okay, and I’ll merge. ( |
|
Alas, |
|
Eh, and that's another test... So, I managed to reproduce the first one: This fails in anywhere from 10s to a bit over 1min I thought I couldn't get it to run without adding the unicode_normalization test but I apparently can, so it's not something we can fix by just slapping a So for this one, we could just patch the test out (or fuzz the time), and I wouldn't loose sleep over it. For the unicode normalization test however I couldn't reproduce it and I have honestly no idea at this point; I couldn't get a similar loop to reproduce it, even running all the media checks in case tests interfere with each other (they each get their own tempdir but you never know...) Long term, I've opened ankitects/anki#3353 to report both problems Short term I don't know, since ofborg didn't hit it I assume the unicode error also is a race, just one I can't hit on my weak laptop (or perhaps something with the rust version? I've left the project's rust-toolchain.toml forcing 1.75 here); but I don't even know how to properly skip specific tests in rust so I'll give it a few days for upstream to react first... |
|
@emilazy would you happen to you know or be able to check what filesystem the community builder uses for /tmp (or whatever directory the build happens in)? In particular I'm curious about the file timestamp granularity, some filesystems have a one second granularity and that'd lead to these kinds of failures. |
|
ignore me, I tried with such a second-level filesystem and that didn't help. |
|
Thanks for the review! I've had a second look at the second failed test and it looks like they're using millisecond timestamps in the database, so if the test creation and update all happened within the same millisecond on the server that'd fail the test. I think that's what the community builder has hit here, and that'd explain why my slower machine cannot reproduce as the test takes ~10-20ms to runs here that laptop is an order of magnitude slower than recent hardware (since it's just part of the test probably a couple times faster might be enough, I don't know) Anyway, I feel no responsibility to actually try to figure out how to best fix these tests; especially since I'm afraid the later "test runs in less than 1ms" might affect other tests (all tests in media check?), so I'll leave that to upstream... And for now, I've just disabled these two tests through By the way, this made me notice both anki and anki-sync-server run the same (lib) tests as most of the tests are there; anki-sync-server itself has no (cargo) test: That's probably not a problem (more chance to notice flaky tests, yay?), and if anki-sync-server ever does adds tests it's probably overkill to disable them here as I definitely won't notice, so leaving it at that. And now, back to work; enough getting side-tracked for today. Will reply tomorrow if there is anything else to fix, that'll leave ofborg time to run the test. |
|
Yeah, the timing test seems eminently skippable. The Unicode normalization thing worries me because of the different results, but if it’s not because of this update I guess it shouldn’t matter. |
|
Well, let’s just merge and worry about the tests some other day; it couldn’t get more broken than it would after staging-next anyway. Thanks for your work on this! |
Description of changes
timedependency to fix rust 1.80 build (not easily testable, but build with 1.79 passes) ; see Rust 1.80.0 breaks some packages #332957Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.