native_activity: Demote spammy info!/eprintln! debug logs to trace!
#49
Conversation
…ce!` Ideally information - especially when spamming per `Looper` poll - used for debugging `android-activity` doesn't show to the user unless they use the `Trace` level (eventually for this specific crate/module). This is already adhered to in most places of the code but there were a few high-volume cases still remaining.
|
@rib unrelated side-question: shall we enable branch deletion after merging, and is it safe to delete everything that was merged? https://github.com/rust-mobile/android-activity/branches 26 branches is a bit excessive :) |
|
https://github.com/rust-mobile/android-activity/actions/runs/3697954719/attempts/1 @rib should we just remove |
Now the repo is under the rust-mobile org I'll make a personal fork of the repo for keeping any branches I want and then we can delete all the extra branches from this repo yep. |
okey, I've created a personal fork now and taken a pass at deleting stale / merged branches in this repo |
The more general plan was to actually split the examples out into this filtered repo here: https://github.com/rust-mobile/rust-android-examples (I just haven't had a chance to follow up on that while we were splitting out the other android-ndk-rs repos.) The general idea there was that we could host Android examples that may not always be solely focused on the android-activity API itself here but they are Android specific. (I had been wondering about moving ndk-examples here before realizing it would also make sense to keep some of them e.g. in cargo-apk for the sake of the CI) It had been intentional to have a This is generally in keeping with the idea that library crates shouldn't have lock files in a repo whereas binaries should. At the very least I do follow some private notes that remind me to update the deps for all examples as part of the android-activity release process. This will probably need a bit of re-consideration if the examples are all split out. |
Yeah those for library crates are not even taken into account (when included externally), only for binaries/examples/tests etc. If anyone wants to guard against anything they should set up CI on a schedule that tests both against the latest crates and
Not entirely sure how to parse that, technically semver shouldn't allow any breakage here? |
|
At various times examples were dependent on specific git commits for egui or for the Winit backend branch while there weren't releases available. These would have been pinned in the lock files - as opposed to specifying a commit in the Cargo.toml which I could have done too. It was beneficial to have these be strictly pinned otherwise it would have been likely that someone wanting to try out an example would have have had a bad experience with pulling in slightly different versions and hitting more build failures. Even with semver it's still not uncommon for crates to inadvertently introduce incompatibilities, esp crates like egui that have a pretty fast development velocity. I've tried to make sure I test examples on a device when making releases which I trust more than just the greenlight from CI at the moment which doesn't currently test things on an emulator (and even if we tested on an emulator then it's still very challenging to do good CI for interactive UIs / examples). Pinning dependencies in a lock file at the time that I test the examples on a device helps reduce the number of moving parts and increases the chance that other people trying the examples will see them working too. (E.g. egui, winit, wgpu and openxr can all quite easily introduce bugs inadvertently in a semver "compatible" release) |
I should remove the lock files for the examples I've kept in this repo for CI (na-mainloop + agdk-mainloop) but for the examples that have been split out then I think the lock files are beneficial for being able to control the updating of dependencies in sync with actually verifying the examples are running as intended on a device. The purpose of those examples isn't to test the latest versions of things - it's much more important that they work as intended when someone comes to reference them, which is more likely when you minimize how many things are changing regularly. Unfortunately the CI is not currently adequate for this. CI is much more dependable for libraries with test suites etc than it is for interactive applications - which is essentially one of the reasons lock files exist, so you can keep a human in the loop for updating dependencies. |
|
I think I prefer pinning commits in |
|
Yeah, there are two separate things here and we're agreed about at least one of them. I agree that pinning git commits for direct dependencies in Cargo.toml is definitely better than using a Cargo.lock for that purpose (I was simply lazy about for a while in this repo). So we're in agreement about using The more general purpose for the lock file though (which is why I still think it's useful) is that it reduces the number of moving parts for executables that are more likely to have larger numbers of dependencies than a single library and they are also harder to test via CI than libraries. Most dependencies aren't going to be direct dependencies so they aren't in the Cargo.toml. Lock files are common to many package managers and they are supported for a good reason. Referring to release management + CI I think you're talking more about library crates here. I generally trust Rust library crates to follow semver and most of the time that's fine but plenty of mistakes happen too. It's not really a question of blaming anyone for being a bad maintainer, it just happens that people make mistakes or things slip through the cracks unnoticed. A typical issue would be with a change in semantics for an API which a maintainer believes is backwards compatible but in some overlooked case that's not true. They are often rectified fairly swiftly if noticed but the more deps you have the more likely it becomes that you'll be exposed to one of those mistakes. This problem is generally more relevant for applications than it is for libraries. This isn't likely to be such a big issue for these examples but the more dependencies you have then statistically the chance that one of those dependencies introduces an unintended regression that maybe just happens to show up for your application / use case increases until it's basically inevitable. I figure that the lock files just help increase the chance that if someone clones the examples and tries to build and run them then they are running the exact same thing that I have hopefully tested on a device too. Without the lock file there's just a bit of extra risk that they hit an unrelated issue caused by some subtle change in semantics for an obscure API being used in a way that had been overlooked. If we want to also utilize the standalone examples as way to integration test libraries we could conceivably update the CI to build the examples with and without the lock files so we'd also get feedback for obvious API breakages in dependencies. |
No. About the crates we depend on, that themselves have transitive dependencies. Which you follow up on in the next paragraph, that mistakes are bound to happen and accidental incompatibilities or API/ABI breakage likely to be published every once in a while as the number of (transitive) dependencies stays large. That's a more structural problem to solve though, way out of our scope (tools exist to compare public crate ABI, for example). I wouldn't have kept the lock files just for that reason, but since you seem to favour them heavily, let's leave it. We could of course help the ecosystem out a bit by embedding a build step that builds+tests on the latest (semver-compatible) crate versions on a weekly schedule for example, but I doubt any of us has time to enact on those CI failures. |
Ideally information - especially when spamming per
Looperpoll - used for debuggingandroid-activitydoesn't show to the user unless they use theTracelevel (eventually for this specific crate/module). This is already adhered to in most places of the code but there were a few high-volume cases still remaining.