Update to use Anki 2.1.54's code #202
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dae
added a commit
to ankitects/Anki-Android
that referenced
this pull request
Jun 16, 2022
Depends on ankidroid/Anki-Android-Backend#202 Due to the removal and change of a few backend methods, syncing, importing and the card templates screen will not work when the schema16 setting is active (actually schema18 now). To get them working again, those code paths will need to switch to the backend implementations. A few notes: - Downgrading happens automatically when loading the collection in schema11 mode, so the extra code dealing with downgrades & "can downgrade" reporting can be stripped. - Added the ability to run col.set_config("key", JSONObject.NULL), as unit tests were attempting to write the entire collection config, which is no longer supported. - All tests pass on both old and new backends, though the latter required disabling a few failed tests when running with the new schema (eg notetype updating). Integrates, and thus closes ankidroid#11579 and closes ankidroid#11581
|
This is a lot to review. I've had a skim and this looks good. Very happy to see Could we limit the translations to the following, as that should produce a well-needed reduction in apk size and serves as a stopgap before we think about combining CrowdIn and Pontoon 'yue' is 'Cantonese', 'heb' is 'Hebrew', 'ind' is Indonesian (gotta love Android). I think the rest are standardized. (For discussion) I'd prefer to stick to the 'standard' Java convention for package names in the generated code. I don't care what that is, if we want to move away from I'd prefer to keep SemVer and accept that once this is out of beta, it will lead to 'breaking changes' in the library once CodeGen causes breaks |
mikehardy
reviewed
Jun 16, 2022
should produce --> premature optimization. what's the size of the translations + size per translation + expected reduction? |
|
Translation size depends on how many strings are translated - almost full/almost empty examples: AnkiMobile has fewer active languages, and I don't bother to strip them there. The key names are shared across languages, and likely compress well.
My thinking here is was that if you see a class imported from 'anki..', you can tell it's a DTO from the backend, and easily look it up in the .proto files or desktop code. And |
- Reuses existing protobuf infrastructure to greatly cut down on the JNI boilerplate - Ensures all calls have errors handled, and happen behind a mutex - Removes separate downgrade + open11 calls in favour of a flag to the standard collection opening routine - Various code cleanups
Matches desktop behaviour
Accessed with backend.tr.someTranslationName()
- Extracted from pinned sha in upstream repo - Maintainer needs to commit change to submodule
|
The dependabot changes caused a conflict; I've rebased over them and updated the remaining deps in the last commit. |
|
Feel free to push changes into this branch if you need to, it's easy enough to rebase over. |
|
Got a warning when opening in Android Studio: One resolved error (to be peeled off into a docs update).Resolution: Needed a submodule update Unresolved Note: |
|
genfluent.sh uses /bin/bash; is .cargo/env on the PATH of the parent process? If you're loading AS from the macOS GUI, it may not have inherited path changes if they only apply to login shells |
|
Hmm - I opened in Android Studio on Ubuntu with no issue. Haven't tried in my x86-64 osx-kvms yet or on my wife's m1 machine which I have to ask for time slices on ;-). Maybe mac specific but that is unexpected. I'm out of time for the day though - this is my last out-the-door comment then I'll try again my Monday morning |
|
Note you'll need to do a ./gradlew clean after the latest commits, as the protobuf generation doesn't remove old files when generating new ones, and it appears to have broken the build in this case. (and will need to update subrepos) |
dae
changed the title
|
Thanks! Finder doesn't respect $PATH: https://apple.stackexchange.com/questions/51677/how-to-set-path-for-finder-launched-applications It would be useful if Alternately, this can be fixed by setting working directory is incorrect for |
david-allison
approved these changes
Jun 27, 2022
|
Verified the rslib/anki commit hash is 2f7a0202976b3a400542b0a9bf5312c0e633472a and that hash is both the actual hash on ankidroid/anki#ankidroid-2.1.54 and the one committed here. So the bump to that looks clean 🚀 - back to checking integration here then merge + release Something is rough on the whole submodule thing though. I basically just deleted my clone and made a new one in order for all the submodules to work well. Not a huge deal for me and probably a local problem but just noting it in case others get stuck. Killing a local clone and making a new one isn't that painful and it's a O(1) operation - always just a few minutes - vs troubleshooting |
mikehardy
approved these changes
Jun 27, 2022
|
It's a lot of commits but I like the commit history here, nearly all of them are important, I believe they all actually worked independently (so bisect will work) but we don't bisect a lot here and this change is so huge that bisect wouldn't be too useful before/after anyway. So I'm rebase merging |
mikehardy
pushed a commit
that referenced
this pull request
Jun 27, 2022
|
1- tagged it 0.1.14-anki2.1.54 to match version in properties file, pushed the tag |
dae
added a commit
to ankitects/Anki-Android
that referenced
this pull request
Jun 28, 2022
Simplify backend handling; rework collection instantiation When the Rust code was initially introduced, it was not clear whether it would be usable on all devices, or whether it would need to be removed for some reason. This no doubt influenced the design of the existing API, which tries to make it easy to swap the Rust code out with something else. Unfortunately this approach has some downsides: - It makes it somewhat harder to follow, as method calls jump through multiple interfaces before they're actually sent to the backend. - It makes utilizing new methods considerably more cumbersome. For example, take the extract_av_tags() call. It follows the following path: collection method or method in related helper class: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/TemplateManager.kt#L242 to generic interface: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/DroidBackend.kt#L83 to specific implementation: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustDroidV16Backend.kt#L57 and if it's unusable with the legacy schema (which I don't believe is actually true in this case), it also needs to be added to the other implementation: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustDroidBackend.kt#L87 and then finally, a method in the backend module is invoked. The backend module has code generation so that invoking a backend method is as simple as making a method call, but currently you have to weave the call through 3 or so levels of indirection before you can actually use it. With something like 170 available methods, that's a fair amount of extra work required. Rather than trying to insulate libanki from the backend code, this PR drops some of the indirection in favour of the approach the desktop takes: libanki is the insulation layer; it can call freely into the backend methods, but consumers (eg the GUI code) are expected to only call methods on the collection, and not access the backend directly. In addition to the above, collection initialization has been reworked to be more similar to the computer version. Instead of the collection being created from a database object, a backend is passed into the collection creation, and the collection takes care of creating a DB instance that wraps the backend. Remove always-on isUsingRustBackend Drop the legacy upgrade/initialization code Schema 11 was introduced in 2012, and decks that still are <11 are very rare. The desktop dropped support for schema 10 back in early 2020. This also removes the need to modify SCHEMA_VERSION when switching between TESTING_USE_V16_BACKEND. Remove DOWNGRADE_REQUIRED and slightly simplify startup error handling - The backend automatically downgrades when required, and possible. No backup is required, as the downgrade happens in a single transaction, and the downgrade code has proven itself over time. - Store the type of failure in getColSafe(), so it can be checked later. Update to work with desktop 2.1.53 code Depends on ankidroid/Anki-Android-Backend#202 Due to the removal and change of a few backend methods, syncing, importing and the card templates screen will not work when the schema16 setting is active (actually schema18 now). To get them working again, those code paths will need to switch to the backend implementations. A few notes: - Downgrading happens automatically when loading the collection in schema11 mode, so the extra code dealing with downgrades & "can downgrade" reporting can be stripped. - Added the ability to run col.set_config("key", JSONObject.NULL), as unit tests were attempting to write the entire collection config, which is no longer supported. - All tests pass on both old and new backends, though the latter required disabling a few failed tests when running with the new schema (eg notetype updating). Integrates, and thus closes ankidroid#11579 and closes ankidroid#11581
dae
added a commit
to ankitects/Anki-Android
that referenced
this pull request
Jun 28, 2022
Squashes a few commits: Move the legacy schema toggle into BackendFactory Simplify backend handling; rework collection instantiation When the Rust code was initially introduced, it was not clear whether it would be usable on all devices, or whether it would need to be removed for some reason. This no doubt influenced the design of the existing API, which tries to make it easy to swap the Rust code out with something else. Unfortunately this approach has some downsides: - It makes it somewhat harder to follow, as method calls jump through multiple interfaces before they're actually sent to the backend. - It makes utilizing new methods considerably more cumbersome. For example, take the extract_av_tags() call. It follows the following path: collection method or method in related helper class: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/TemplateManager.kt#L242 to generic interface: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/DroidBackend.kt#L83 to specific implementation: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustDroidV16Backend.kt#L57 and if it's unusable with the legacy schema (which I don't believe is actually true in this case), it also needs to be added to the other implementation: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustDroidBackend.kt#L87 and then finally, a method in the backend module is invoked. The backend module has code generation so that invoking a backend method is as simple as making a method call, but currently you have to weave the call through 3 or so levels of indirection before you can actually use it. With something like 170 available methods, that's a fair amount of extra work required. Rather than trying to insulate libanki from the backend code, this PR drops some of the indirection in favour of the approach the desktop takes: libanki is the insulation layer; it can call freely into the backend methods, but consumers (eg the GUI code) are expected to only call methods on the collection, and not access the backend directly. In addition to the above, collection initialization has been reworked to be more similar to the computer version. Instead of the collection being created from a database object, a backend is passed into the collection creation, and the collection takes care of creating a DB instance that wraps the backend. Remove always-on isUsingRustBackend Drop the legacy upgrade/initialization code Schema 11 was introduced in 2012, and decks that still are <11 are very rare. The desktop dropped support for schema 10 back in early 2020. This also removes the need to modify SCHEMA_VERSION when switching between TESTING_USE_V16_BACKEND. Remove DOWNGRADE_REQUIRED and slightly simplify startup error handling - The backend automatically downgrades when required, and possible. No backup is required, as the downgrade happens in a single transaction, and the downgrade code has proven itself over time. - Store the type of failure in getColSafe(), so it can be checked later. Update to work with desktop 2.1.53 code Depends on ankidroid/Anki-Android-Backend#202 Due to the removal and change of a few backend methods, syncing, importing and the card templates screen will not work when the schema16 setting is active (actually schema18 now). To get them working again, those code paths will need to switch to the backend implementations. A few notes: - Downgrading happens automatically when loading the collection in schema11 mode, so the extra code dealing with downgrades & "can downgrade" reporting can be stripped. - Added the ability to run col.set_config("key", JSONObject.NULL), as unit tests were attempting to write the entire collection config, which is no longer supported. - All tests pass on both old and new backends, though the latter required disabling a few failed tests when running with the new schema (eg notetype updating). Integrates, and thus closes ankidroid#11579 and closes ankidroid#11581
dae
added a commit
to ankitects/Anki-Android
that referenced
this pull request
Jun 28, 2022
Squashes a few commits: Move the legacy schema toggle into BackendFactory Simplify backend handling; rework collection instantiation When the Rust code was initially introduced, it was not clear whether it would be usable on all devices, or whether it would need to be removed for some reason. This no doubt influenced the design of the existing API, which tries to make it easy to swap the Rust code out with something else. Unfortunately this approach has some downsides: - It makes it somewhat harder to follow, as method calls jump through multiple interfaces before they're actually sent to the backend. - It makes utilizing new methods considerably more cumbersome. For example, take the extract_av_tags() call. It follows the following path: collection method or method in related helper class: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/TemplateManager.kt#L242 to generic interface: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/DroidBackend.kt#L83 to specific implementation: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustDroidV16Backend.kt#L57 and if it's unusable with the legacy schema (which I don't believe is actually true in this case), it also needs to be added to the other implementation: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustDroidBackend.kt#L87 and then finally, a method in the backend module is invoked. The backend module has code generation so that invoking a backend method is as simple as making a method call, but currently you have to weave the call through 3 or so levels of indirection before you can actually use it. With something like 170 available methods, that's a fair amount of extra work required. Rather than trying to insulate libanki from the backend code, this PR drops some of the indirection in favour of the approach the desktop takes: libanki is the insulation layer; it can call freely into the backend methods, but consumers (eg the GUI code) are expected to only call methods on the collection, and not access the backend directly. In addition to the above, collection initialization has been reworked to be more similar to the computer version. Instead of the collection being created from a database object, a backend is passed into the collection creation, and the collection takes care of creating a DB instance that wraps the backend. Remove always-on isUsingRustBackend Drop the legacy upgrade/initialization code Schema 11 was introduced in 2012, and decks that still are <11 are very rare. The desktop dropped support for schema 10 back in early 2020. This also removes the need to modify SCHEMA_VERSION when switching between TESTING_USE_V16_BACKEND. Remove DOWNGRADE_REQUIRED and slightly simplify startup error handling - The backend automatically downgrades when required, and possible. No backup is required, as the downgrade happens in a single transaction, and the downgrade code has proven itself over time. - Store the type of failure in getColSafe(), so it can be checked later. Update to work with desktop 2.1.53 code Depends on ankidroid/Anki-Android-Backend#202 Due to the removal and change of a few backend methods, syncing, importing and the card templates screen will not work when the schema16 setting is active (actually schema18 now). To get them working again, those code paths will need to switch to the backend implementations. A few notes: - Downgrading happens automatically when loading the collection in schema11 mode, so the extra code dealing with downgrades & "can downgrade" reporting can be stripped. - Added the ability to run col.set_config("key", JSONObject.NULL), as unit tests were attempting to write the entire collection config, which is no longer supported. - All tests pass on both old and new backends, though the latter required disabling a few failed tests when running with the new schema (eg notetype updating). Integrates, and thus closes ankidroid#11579 and closes ankidroid#11581
mikehardy
pushed a commit
to ankidroid/Anki-Android
that referenced
this pull request
Jun 29, 2022
Squashes a few commits: Move the legacy schema toggle into BackendFactory Simplify backend handling; rework collection instantiation When the Rust code was initially introduced, it was not clear whether it would be usable on all devices, or whether it would need to be removed for some reason. This no doubt influenced the design of the existing API, which tries to make it easy to swap the Rust code out with something else. Unfortunately this approach has some downsides: - It makes it somewhat harder to follow, as method calls jump through multiple interfaces before they're actually sent to the backend. - It makes utilizing new methods considerably more cumbersome. For example, take the extract_av_tags() call. It follows the following path: collection method or method in related helper class: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/TemplateManager.kt#L242 to generic interface: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/DroidBackend.kt#L83 to specific implementation: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustDroidV16Backend.kt#L57 and if it's unusable with the legacy schema (which I don't believe is actually true in this case), it also needs to be added to the other implementation: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustDroidBackend.kt#L87 and then finally, a method in the backend module is invoked. The backend module has code generation so that invoking a backend method is as simple as making a method call, but currently you have to weave the call through 3 or so levels of indirection before you can actually use it. With something like 170 available methods, that's a fair amount of extra work required. Rather than trying to insulate libanki from the backend code, this PR drops some of the indirection in favour of the approach the desktop takes: libanki is the insulation layer; it can call freely into the backend methods, but consumers (eg the GUI code) are expected to only call methods on the collection, and not access the backend directly. In addition to the above, collection initialization has been reworked to be more similar to the computer version. Instead of the collection being created from a database object, a backend is passed into the collection creation, and the collection takes care of creating a DB instance that wraps the backend. Remove always-on isUsingRustBackend Drop the legacy upgrade/initialization code Schema 11 was introduced in 2012, and decks that still are <11 are very rare. The desktop dropped support for schema 10 back in early 2020. This also removes the need to modify SCHEMA_VERSION when switching between TESTING_USE_V16_BACKEND. Remove DOWNGRADE_REQUIRED and slightly simplify startup error handling - The backend automatically downgrades when required, and possible. No backup is required, as the downgrade happens in a single transaction, and the downgrade code has proven itself over time. - Store the type of failure in getColSafe(), so it can be checked later. Update to work with desktop 2.1.53 code Depends on ankidroid/Anki-Android-Backend#202 Due to the removal and change of a few backend methods, syncing, importing and the card templates screen will not work when the schema16 setting is active (actually schema18 now). To get them working again, those code paths will need to switch to the backend implementations. A few notes: - Downgrading happens automatically when loading the collection in schema11 mode, so the extra code dealing with downgrades & "can downgrade" reporting can be stripped. - Added the ability to run col.set_config("key", JSONObject.NULL), as unit tests were attempting to write the entire collection config, which is no longer supported. - All tests pass on both old and new backends, though the latter required disabling a few failed tests when running with the new schema (eg notetype updating). Integrates, and thus closes #11579 and closes #11581
mikehardy
pushed a commit
to ankidroid/Anki-Android
that referenced
this pull request
Jun 29, 2022
Squashes a few commits: Move the legacy schema toggle into BackendFactory Simplify backend handling; rework collection instantiation When the Rust code was initially introduced, it was not clear whether it would be usable on all devices, or whether it would need to be removed for some reason. This no doubt influenced the design of the existing API, which tries to make it easy to swap the Rust code out with something else. Unfortunately this approach has some downsides: - It makes it somewhat harder to follow, as method calls jump through multiple interfaces before they're actually sent to the backend. - It makes utilizing new methods considerably more cumbersome. For example, take the extract_av_tags() call. It follows the following path: collection method or method in related helper class: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/TemplateManager.kt#L242 to generic interface: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/DroidBackend.kt#L83 to specific implementation: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustDroidV16Backend.kt#L57 and if it's unusable with the legacy schema (which I don't believe is actually true in this case), it also needs to be added to the other implementation: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustDroidBackend.kt#L87 and then finally, a method in the backend module is invoked. The backend module has code generation so that invoking a backend method is as simple as making a method call, but currently you have to weave the call through 3 or so levels of indirection before you can actually use it. With something like 170 available methods, that's a fair amount of extra work required. Rather than trying to insulate libanki from the backend code, this PR drops some of the indirection in favour of the approach the desktop takes: libanki is the insulation layer; it can call freely into the backend methods, but consumers (eg the GUI code) are expected to only call methods on the collection, and not access the backend directly. In addition to the above, collection initialization has been reworked to be more similar to the computer version. Instead of the collection being created from a database object, a backend is passed into the collection creation, and the collection takes care of creating a DB instance that wraps the backend. Remove always-on isUsingRustBackend Drop the legacy upgrade/initialization code Schema 11 was introduced in 2012, and decks that still are <11 are very rare. The desktop dropped support for schema 10 back in early 2020. This also removes the need to modify SCHEMA_VERSION when switching between TESTING_USE_V16_BACKEND. Remove DOWNGRADE_REQUIRED and slightly simplify startup error handling - The backend automatically downgrades when required, and possible. No backup is required, as the downgrade happens in a single transaction, and the downgrade code has proven itself over time. - Store the type of failure in getColSafe(), so it can be checked later. Update to work with desktop 2.1.53 code Depends on ankidroid/Anki-Android-Backend#202 Due to the removal and change of a few backend methods, syncing, importing and the card templates screen will not work when the schema16 setting is active (actually schema18 now). To get them working again, those code paths will need to switch to the backend implementations. A few notes: - Downgrading happens automatically when loading the collection in schema11 mode, so the extra code dealing with downgrades & "can downgrade" reporting can be stripped. - Added the ability to run col.set_config("key", JSONObject.NULL), as unit tests were attempting to write the entire collection config, which is no longer supported. - All tests pass on both old and new backends, though the latter required disabling a few failed tests when running with the new schema (eg notetype updating). Integrates, and thus closes #11579 and closes #11581 refactor: Rename Storage.java to .kt com.ichi2.libanki.Storage refactor: Convert Storage to Kotlin com.ichi2.libanki.Storage Remove the time argument to Storage.collection() Collection does not currently take a time argument, and relies on the global object instead, so this change brings Storage in line with it. Reuse the backend when closing+reopening a collection Avoids having to re-initialize the translations, and reduces leaks (the import + export code leaks backends still)
mikehardy
pushed a commit
to ankidroid/Anki-Android
that referenced
this pull request
Jun 29, 2022
Squashes a few commits: Move the legacy schema toggle into BackendFactory Simplify backend handling; rework collection instantiation When the Rust code was initially introduced, it was not clear whether it would be usable on all devices, or whether it would need to be removed for some reason. This no doubt influenced the design of the existing API, which tries to make it easy to swap the Rust code out with something else. Unfortunately this approach has some downsides: - It makes it somewhat harder to follow, as method calls jump through multiple interfaces before they're actually sent to the backend. - It makes utilizing new methods considerably more cumbersome. For example, take the extract_av_tags() call. It follows the following path: collection method or method in related helper class: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/TemplateManager.kt#L242 to generic interface: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/DroidBackend.kt#L83 to specific implementation: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustDroidV16Backend.kt#L57 and if it's unusable with the legacy schema (which I don't believe is actually true in this case), it also needs to be added to the other implementation: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustDroidBackend.kt#L87 and then finally, a method in the backend module is invoked. The backend module has code generation so that invoking a backend method is as simple as making a method call, but currently you have to weave the call through 3 or so levels of indirection before you can actually use it. With something like 170 available methods, that's a fair amount of extra work required. Rather than trying to insulate libanki from the backend code, this PR drops some of the indirection in favour of the approach the desktop takes: libanki is the insulation layer; it can call freely into the backend methods, but consumers (eg the GUI code) are expected to only call methods on the collection, and not access the backend directly. In addition to the above, collection initialization has been reworked to be more similar to the computer version. Instead of the collection being created from a database object, a backend is passed into the collection creation, and the collection takes care of creating a DB instance that wraps the backend. Remove always-on isUsingRustBackend Drop the legacy upgrade/initialization code Schema 11 was introduced in 2012, and decks that still are <11 are very rare. The desktop dropped support for schema 10 back in early 2020. This also removes the need to modify SCHEMA_VERSION when switching between TESTING_USE_V16_BACKEND. Remove DOWNGRADE_REQUIRED and slightly simplify startup error handling - The backend automatically downgrades when required, and possible. No backup is required, as the downgrade happens in a single transaction, and the downgrade code has proven itself over time. - Store the type of failure in getColSafe(), so it can be checked later. Update to work with desktop 2.1.53 code Depends on ankidroid/Anki-Android-Backend#202 Due to the removal and change of a few backend methods, syncing, importing and the card templates screen will not work when the schema16 setting is active (actually schema18 now). To get them working again, those code paths will need to switch to the backend implementations. A few notes: - Downgrading happens automatically when loading the collection in schema11 mode, so the extra code dealing with downgrades & "can downgrade" reporting can be stripped. - Added the ability to run col.set_config("key", JSONObject.NULL), as unit tests were attempting to write the entire collection config, which is no longer supported. - All tests pass on both old and new backends, though the latter required disabling a few failed tests when running with the new schema (eg notetype updating). Integrates, and thus closes #11579 and closes #11581
mikehardy
pushed a commit
to ankidroid/Anki-Android
that referenced
this pull request
Jun 29, 2022
Squashes a few commits: Move the legacy schema toggle into BackendFactory Simplify backend handling; rework collection instantiation When the Rust code was initially introduced, it was not clear whether it would be usable on all devices, or whether it would need to be removed for some reason. This no doubt influenced the design of the existing API, which tries to make it easy to swap the Rust code out with something else. Unfortunately this approach has some downsides: - It makes it somewhat harder to follow, as method calls jump through multiple interfaces before they're actually sent to the backend. - It makes utilizing new methods considerably more cumbersome. For example, take the extract_av_tags() call. It follows the following path: collection method or method in related helper class: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/TemplateManager.kt#L242 to generic interface: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/DroidBackend.kt#L83 to specific implementation: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustDroidV16Backend.kt#L57 and if it's unusable with the legacy schema (which I don't believe is actually true in this case), it also needs to be added to the other implementation: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustDroidBackend.kt#L87 and then finally, a method in the backend module is invoked. The backend module has code generation so that invoking a backend method is as simple as making a method call, but currently you have to weave the call through 3 or so levels of indirection before you can actually use it. With something like 170 available methods, that's a fair amount of extra work required. Rather than trying to insulate libanki from the backend code, this PR drops some of the indirection in favour of the approach the desktop takes: libanki is the insulation layer; it can call freely into the backend methods, but consumers (eg the GUI code) are expected to only call methods on the collection, and not access the backend directly. In addition to the above, collection initialization has been reworked to be more similar to the computer version. Instead of the collection being created from a database object, a backend is passed into the collection creation, and the collection takes care of creating a DB instance that wraps the backend. Remove always-on isUsingRustBackend Drop the legacy upgrade/initialization code Schema 11 was introduced in 2012, and decks that still are <11 are very rare. The desktop dropped support for schema 10 back in early 2020. This also removes the need to modify SCHEMA_VERSION when switching between TESTING_USE_V16_BACKEND. Remove DOWNGRADE_REQUIRED and slightly simplify startup error handling - The backend automatically downgrades when required, and possible. No backup is required, as the downgrade happens in a single transaction, and the downgrade code has proven itself over time. - Store the type of failure in getColSafe(), so it can be checked later. Update to work with desktop 2.1.53 code Depends on ankidroid/Anki-Android-Backend#202 Due to the removal and change of a few backend methods, syncing, importing and the card templates screen will not work when the schema16 setting is active (actually schema18 now). To get them working again, those code paths will need to switch to the backend implementations. A few notes: - Downgrading happens automatically when loading the collection in schema11 mode, so the extra code dealing with downgrades & "can downgrade" reporting can be stripped. - Added the ability to run col.set_config("key", JSONObject.NULL), as unit tests were attempting to write the entire collection config, which is no longer supported. - All tests pass on both old and new backends, though the latter required disabling a few failed tests when running with the new schema (eg notetype updating). Integrates, and thus closes #11579 and closes #11581 Remove the time argument to Storage.collection() Collection does not currently take a time argument, and relies on the global object instead, so this change brings Storage in line with it. Reuse the backend when closing+reopening a collection Avoids having to re-initialize the translations, and reduces leaks (the import + export code leaks backends still)
mikehardy
pushed a commit
to ankitects/Anki-Android
that referenced
this pull request
Jun 29, 2022
Squashes a few commits: Move the legacy schema toggle into BackendFactory Simplify backend handling; rework collection instantiation When the Rust code was initially introduced, it was not clear whether it would be usable on all devices, or whether it would need to be removed for some reason. This no doubt influenced the design of the existing API, which tries to make it easy to swap the Rust code out with something else. Unfortunately this approach has some downsides: - It makes it somewhat harder to follow, as method calls jump through multiple interfaces before they're actually sent to the backend. - It makes utilizing new methods considerably more cumbersome. For example, take the extract_av_tags() call. It follows the following path: collection method or method in related helper class: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/TemplateManager.kt#L242 to generic interface: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/DroidBackend.kt#L83 to specific implementation: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustDroidV16Backend.kt#L57 and if it's unusable with the legacy schema (which I don't believe is actually true in this case), it also needs to be added to the other implementation: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustDroidBackend.kt#L87 and then finally, a method in the backend module is invoked. The backend module has code generation so that invoking a backend method is as simple as making a method call, but currently you have to weave the call through 3 or so levels of indirection before you can actually use it. With something like 170 available methods, that's a fair amount of extra work required. Rather than trying to insulate libanki from the backend code, this PR drops some of the indirection in favour of the approach the desktop takes: libanki is the insulation layer; it can call freely into the backend methods, but consumers (eg the GUI code) are expected to only call methods on the collection, and not access the backend directly. In addition to the above, collection initialization has been reworked to be more similar to the computer version. Instead of the collection being created from a database object, a backend is passed into the collection creation, and the collection takes care of creating a DB instance that wraps the backend. Remove always-on isUsingRustBackend Drop the legacy upgrade/initialization code Schema 11 was introduced in 2012, and decks that still are <11 are very rare. The desktop dropped support for schema 10 back in early 2020. This also removes the need to modify SCHEMA_VERSION when switching between TESTING_USE_V16_BACKEND. Remove DOWNGRADE_REQUIRED and slightly simplify startup error handling - The backend automatically downgrades when required, and possible. No backup is required, as the downgrade happens in a single transaction, and the downgrade code has proven itself over time. - Store the type of failure in getColSafe(), so it can be checked later. Update to work with desktop 2.1.53 code Depends on ankidroid/Anki-Android-Backend#202 Due to the removal and change of a few backend methods, syncing, importing and the card templates screen will not work when the schema16 setting is active (actually schema18 now). To get them working again, those code paths will need to switch to the backend implementations. A few notes: - Downgrading happens automatically when loading the collection in schema11 mode, so the extra code dealing with downgrades & "can downgrade" reporting can be stripped. - Added the ability to run col.set_config("key", JSONObject.NULL), as unit tests were attempting to write the entire collection config, which is no longer supported. - All tests pass on both old and new backends, though the latter required disabling a few failed tests when running with the new schema (eg notetype updating). Integrates, and thus closes ankidroid#11579 and closes ankidroid#11581 Remove the time argument to Storage.collection() Collection does not currently take a time argument, and relies on the global object instead, so this change brings Storage in line with it. Reuse the backend when closing+reopening a collection Avoids having to re-initialize the translations, and reduces leaks (the import + export code leaks backends still)
mikehardy
pushed a commit
to ankidroid/Anki-Android
that referenced
this pull request
Jun 29, 2022
Squashes a few commits: Move the legacy schema toggle into BackendFactory Simplify backend handling; rework collection instantiation When the Rust code was initially introduced, it was not clear whether it would be usable on all devices, or whether it would need to be removed for some reason. This no doubt influenced the design of the existing API, which tries to make it easy to swap the Rust code out with something else. Unfortunately this approach has some downsides: - It makes it somewhat harder to follow, as method calls jump through multiple interfaces before they're actually sent to the backend. - It makes utilizing new methods considerably more cumbersome. For example, take the extract_av_tags() call. It follows the following path: collection method or method in related helper class: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/TemplateManager.kt#L242 to generic interface: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/DroidBackend.kt#L83 to specific implementation: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustDroidV16Backend.kt#L57 and if it's unusable with the legacy schema (which I don't believe is actually true in this case), it also needs to be added to the other implementation: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustDroidBackend.kt#L87 and then finally, a method in the backend module is invoked. The backend module has code generation so that invoking a backend method is as simple as making a method call, but currently you have to weave the call through 3 or so levels of indirection before you can actually use it. With something like 170 available methods, that's a fair amount of extra work required. Rather than trying to insulate libanki from the backend code, this PR drops some of the indirection in favour of the approach the desktop takes: libanki is the insulation layer; it can call freely into the backend methods, but consumers (eg the GUI code) are expected to only call methods on the collection, and not access the backend directly. In addition to the above, collection initialization has been reworked to be more similar to the computer version. Instead of the collection being created from a database object, a backend is passed into the collection creation, and the collection takes care of creating a DB instance that wraps the backend. Remove always-on isUsingRustBackend Drop the legacy upgrade/initialization code Schema 11 was introduced in 2012, and decks that still are <11 are very rare. The desktop dropped support for schema 10 back in early 2020. This also removes the need to modify SCHEMA_VERSION when switching between TESTING_USE_V16_BACKEND. Remove DOWNGRADE_REQUIRED and slightly simplify startup error handling - The backend automatically downgrades when required, and possible. No backup is required, as the downgrade happens in a single transaction, and the downgrade code has proven itself over time. - Store the type of failure in getColSafe(), so it can be checked later. Update to work with desktop 2.1.53 code Depends on ankidroid/Anki-Android-Backend#202 Due to the removal and change of a few backend methods, syncing, importing and the card templates screen will not work when the schema16 setting is active (actually schema18 now). To get them working again, those code paths will need to switch to the backend implementations. A few notes: - Downgrading happens automatically when loading the collection in schema11 mode, so the extra code dealing with downgrades & "can downgrade" reporting can be stripped. - Added the ability to run col.set_config("key", JSONObject.NULL), as unit tests were attempting to write the entire collection config, which is no longer supported. - All tests pass on both old and new backends, though the latter required disabling a few failed tests when running with the new schema (eg notetype updating). Integrates, and thus closes #11579 and closes #11581 Remove the time argument to Storage.collection() Collection does not currently take a time argument, and relies on the global object instead, so this change brings Storage in line with it. Reuse the backend when closing+reopening a collection Avoids having to re-initialize the translations, and reduces leaks (the import + export code leaks backends still)
dorrin-sot
pushed a commit
to dorrin-sot/Anki-Android
that referenced
this pull request
Jul 14, 2022
Squashes a few commits: Move the legacy schema toggle into BackendFactory Simplify backend handling; rework collection instantiation When the Rust code was initially introduced, it was not clear whether it would be usable on all devices, or whether it would need to be removed for some reason. This no doubt influenced the design of the existing API, which tries to make it easy to swap the Rust code out with something else. Unfortunately this approach has some downsides: - It makes it somewhat harder to follow, as method calls jump through multiple interfaces before they're actually sent to the backend. - It makes utilizing new methods considerably more cumbersome. For example, take the extract_av_tags() call. It follows the following path: collection method or method in related helper class: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/TemplateManager.kt#L242 to generic interface: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/DroidBackend.kt#L83 to specific implementation: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustDroidV16Backend.kt#L57 and if it's unusable with the legacy schema (which I don't believe is actually true in this case), it also needs to be added to the other implementation: https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustDroidBackend.kt#L87 and then finally, a method in the backend module is invoked. The backend module has code generation so that invoking a backend method is as simple as making a method call, but currently you have to weave the call through 3 or so levels of indirection before you can actually use it. With something like 170 available methods, that's a fair amount of extra work required. Rather than trying to insulate libanki from the backend code, this PR drops some of the indirection in favour of the approach the desktop takes: libanki is the insulation layer; it can call freely into the backend methods, but consumers (eg the GUI code) are expected to only call methods on the collection, and not access the backend directly. In addition to the above, collection initialization has been reworked to be more similar to the computer version. Instead of the collection being created from a database object, a backend is passed into the collection creation, and the collection takes care of creating a DB instance that wraps the backend. Remove always-on isUsingRustBackend Drop the legacy upgrade/initialization code Schema 11 was introduced in 2012, and decks that still are <11 are very rare. The desktop dropped support for schema 10 back in early 2020. This also removes the need to modify SCHEMA_VERSION when switching between TESTING_USE_V16_BACKEND. Remove DOWNGRADE_REQUIRED and slightly simplify startup error handling - The backend automatically downgrades when required, and possible. No backup is required, as the downgrade happens in a single transaction, and the downgrade code has proven itself over time. - Store the type of failure in getColSafe(), so it can be checked later. Update to work with desktop 2.1.53 code Depends on ankidroid/Anki-Android-Backend#202 Due to the removal and change of a few backend methods, syncing, importing and the card templates screen will not work when the schema16 setting is active (actually schema18 now). To get them working again, those code paths will need to switch to the backend implementations. A few notes: - Downgrading happens automatically when loading the collection in schema11 mode, so the extra code dealing with downgrades & "can downgrade" reporting can be stripped. - Added the ability to run col.set_config("key", JSONObject.NULL), as unit tests were attempting to write the entire collection config, which is no longer supported. - All tests pass on both old and new backends, though the latter required disabling a few failed tests when running with the new schema (eg notetype updating). Integrates, and thus closes ankidroid#11579 and closes ankidroid#11581 Remove the time argument to Storage.collection() Collection does not currently take a time argument, and relies on the global object instead, so this change brings Storage in line with it. Reuse the backend when closing+reopening a collection Avoids having to re-initialize the translations, and reduces leaks (the import + export code leaks backends still)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requires the following branch: ankidroid/anki#2
120 lines of Rust remain here, to implement the small JNI interface. The
AnkiDroid-specific routines flow through the same backend call interface
as the other backend code, with errors handled correctly.
figure out how to get the Kotlin plugin working on the tests. The conversion
introduced some warnings on compile that should probably be addressed at one
point.
accessors in Kotlin.
input arguments as non-optional unless they've been explicitly declared
as nullable in the proto files.
Some design notes:
anki.<module>as the package path forgenerated protobuf, and I've kept this for the Kotlin code as well (
eg
import anki.notes.Noteto import theNotemessage defined in proto/anki/notes.proto).The rsdroid module is still using net.ankiweb.rsdroid; not sure if you want
that changed or kept the same.
worth considering in the future is to encode the desktop version number in the release
instead, eg 2.1.53 -> 21.53.0 (last number for patch/revision).
the locking can be handled there instead. Only the transaction methods handle the locking
specially.
Rust object at the same time, instead of requiring a two stage initialization.
There are some warnings showing that were introduced as part of the Kotlin conversion,
that can be fixed in a future commit.
Closes #209
Closes #175
Related #168 (was 'closes' but it currently stops on 1.58.1, so does not close although moves us from rust 1.54)
Closes #133
Closes #36
Closes #24
Closes #22
Closes #21
Closes #18
Closes #17
Closes #2