-
Notifications
You must be signed in to change notification settings - Fork 258
[Feature] Dynamic version detection; Version display screen #858
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
Open
kdmukai
wants to merge
51
commits into
SeedSigner:dev
Choose a base branch
from
kdmukai:version_screen_with_git_info
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or 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
…OS builder output even in local dev
git info in local dev
4 tasks
030b492 to
797d036
Compare
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.
Description
NOTES:
Versioninfo screen; writesversion.jsonduring build seedsigner-os#102Brief summary
Eliminates the hard-coded
Controller.VERSIONstring in favor of using the currentgitstatus (either written to disk when a SeedSigner OS image is built or pulled live at run time in local dev).Dynamic version detection
Version info is collected from multiple possible sources, depending on the context:
version.jsonfile written into a SeedSigner OS image.gitshell calls (typical local dev).rsyncjust the minimal required .git/ files to retrieve current git state forVersion.New "Version" screen in the Settings menu
Output is dynamic to support a clean, simple display for release images as well as more detailed info for local dev or test images.
Motivation
Nice to have for official release images:
The timestamp at the bottom of the new version screen is the date of the last commit included in the release. May provide a nudge for users to update if they see that a lot of time has passed. The timestamp is also one more thing that a paranoid user could verify for themselves (though an evil image can always just lie about its timestamp).
Win for test images / release candidates:
As the project keeps growing we'll need more "black box testers" (hands-on testing w/out knowing any code details). Ideally we'd have test images automatically built for PRs, when new translations are submitted, etc.
We currently have to manually modify the
Controller.VERSIONstring to indicate a special build version on the opening splash screen.This PR instead automatically displays the current git branch / tag / commit hash as the version string. And as a test branch goes through further iterations, the commit hash and timestamp will help us keep track of who's feedback is still relevant ("Wait, can you confirm you saw this on
abcd123? That should have been fixed.").And of course it'll always be a bit of a security risk to have people use test images, but the extra bit of transparency provided by this info (which exact commit from what exact timestamp) helps a little. It's still no guarantee, though.
Extra info and debugging in local dev:
How it works
Copied from the new
Versionclass' docstring:Release images
Release images are built in SeedSigner OS with
--app-branchpointing to a git tag and--app-repopointing to our main repo (i.e. not someone's fork). If the specified git tag is a "clean" semantic version (e.g. "0.8.6" vs "0.8.7-rc1") and we're pulling from the main repo, we deduce that this is a release image and so the version info is displayed in its simplified form.Current branch name
In this local dev / test image scenario we get a more detailed dev/debugging version of the screen.
fork: PR author (when run in github CI) or owner of the current repo (local dev).commit: short commit hash for the current git state.Makes a minimal effort to add line breaks for long branch names. Identifying the current branch name is really only for local dev or possibly something like a test image. So we don't need to aim for perfect presentation here.
Tag name
If the current git state is pointing to a tag but we're running on a fork (repo owner is not "SeedSigner") OR the tag name is not a clean semantic version, then the version info stays in its dev output format:

Commit hash
Falls back to the short commit hash when git is in a detached HEAD state (and not on a tag). Also not meant for normal users to ever see.
Coding considerations
Coverage goal: 100%
This all ended up being much bigger than I originally anticipated due to the number of different ways to retrieve the version information. So to facilitate testing, you'll see that basically every tiny unit of functionality is broken out into its own
VersionUtilsmethod, with some meta-methods invoking various smaller ones.But so many little pieces, it felt crucial to reach 100% test coverage. So the accompanying tests are unavoidably numerous. And there are some trivial tests noted that only exist so we can get to that 100% coverage. It's both a worthy goal and a little ridiculous at the same time.
Note that our
pyproject.tomlsettings haveskip_covered = trueso whenversion.pyis at 100% you won't see it listed in the coverage report at all.Simplicity elsewhere
The
Versionclass is the only place where any other part of the main code should interact with the version data. It is just a simple placeholder that determines the version data when theVersionsingleton is first instantiated, and then provides basicget_*methods.Sandboxing riskier calls
I was initially very uncomfortable adding the
gitshell calls; any shell command introduces some new risks. But those calls are only made when running in local dev. So I added a newnot_allowed_in_seedsigner_osdecorator and its associatedNotAllowedInSeedSignerOS.This same restriction was applied to all
VersionUtilscalls that try to read directly from the .git/ subdir.Testing tips
The
VersionUtilstests are grouped into specific sub-collections that reflect the different ways it retrieves version info.e.g.
TestVersionUtils_VersionFilevsTestVersionUtils_GitShelletc.One useful way to review the test suite and its coverage is to run just one of these test classes at a time and then inspect the html coverage report:
You'll see hit or miss coverage in various places, but the specific area being tested (e.g. the
*_from_git_shell()calls) will show full coverage.Test various local git states
For quick testing against different local git states, you can run the
write_versionfile.pyscript and see the version info based on your current git state:# writes to src/seedsigner/version.json python tools/write_versionfile.pyThe screenshot generator also creates screenshot variants for both:
And of course you can run this branch on your local dev device. However, how this code retrieves version info will depend on if your dev device has the
gitshell command available and if you transfer the .git/ dir. I have set uprsyncin my local dev to sync:Try from:
git checkout <tag_name>git checkout <commit_hash>.You can run all of this as a clone of my repo on this
version_screen_with_git_infobranch.Or you can run it as a PR branch in your fork via:
Integration with SeedSigner OS
See the accompanying SeedSigner/seedsigner-os#102
A simple new script (
tools/write_versionfile.py) instantiates theVersionwhich, in this context, will retrieve the current git information viagitshell commands. It then writes the version info tosrc/seedsigner/version.jsonwhich is included in the final image.The
write_versionfile.pyscript doesn't do any new work on its own; all of the functionality was implemented inVersionUtilsso that the script would only invoke commands that are already covered by the test suite.It really only exists as a separate script because we have previously preferred to keep any
if __name__ == "__main__":command line executable bits out of the main source code.Misc Notes
Screenshot generator
The screnshot generator's
run_beforeandrun_afteroptional params have always been pretty primitive. The newVersion-related screenshots required mocking/patching that is more like individual test cases (e.g. mocking us into SeedSigner OS for one screenshot).So this PR has already merged in #860 as a dependency.
This pull request is categorized as a:
Checklist
pytestand made sure all unit tests pass before submitting the PRIf you modified or added functionality/workflow, did you add new unit tests?
I have tested this PR on the following platforms/os: