-
-
Notifications
You must be signed in to change notification settings - Fork 6
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
Use Shared CI Workflows #5
Conversation
db734f9
to
879b80f
Compare
6639272
to
8abd9c7
Compare
Leaving at draft for now. The caching may not be working....and thinking about it, app packaging should probably have failed with the Windows Store Python and Briefcase from PyPI....but it's not... Needs more investigation. |
.github/workflows/ci.yml
Outdated
strategy: | ||
fail-fast: false | ||
matrix: | ||
python_version: ["3.8", "3.9", "3.10", "3.11"] | ||
python-version: ["3.8", "3.9", "3.10", "3.11"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While we're here, we should probably add 3.12-dev
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this'll work unless a binary stub is also created for 3.12.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah - good point; and we can't do that until there's a NuGET release of Python 3.12.
ed6ca9b
to
22a08f6
Compare
22a08f6
to
ddf4298
Compare
I tracked it down. The GitHub runner comes with CI now fails for the Windows Store Python and this PR needs to wait on Briefcase v0.3.12. CI Run using Briefcase |
5ce0b92
to
1dd7599
Compare
.github/workflows/ci.yml
Outdated
- name: Cache Briefcase tools | ||
uses: actions/cache@v3.0.11 | ||
with: | ||
key: briefcase-${{ matrix.platform }}-${{ matrix.python_version }} | ||
key: briefcase-${{ runner.os }}-${{ matrix.python-version }} | ||
path: | | ||
~/.cookiecutters | ||
${{ matrix.briefcase-data-dir }} | ||
${{ matrix.pip-cache-dir }} | ||
${{ matrix.docker-cache-dir }} | ||
~/AppData/Local/BeeWare/briefcase/Cache | ||
~/AppData/Local/pip/Cache | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While caching the Briefcase cache directory nullifies a lot of the rationale for testing against Windows Store Python, it still seemed like a good idea to keep in place.
Github caches are evicted after 7 days. Given this repo doesn't get many PRs, it is likely the action will run without a cache available. I've also included the python source as part of the cache key; so, a cache will only be available for subsequent runs if a previous run succeeded. This helps avoid papering over a failure because the next run would pull the successfully created cache from the github python job.
1dd7599
to
bc06f51
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes look good; it's currently failing CI due to a timeout running WiX - but I'm not sure if that's just Github having a moment. I've kicked off a re-run just in case.
Good catch on the fact that this is dependent on Briefcase 0.3.12 at present; while "waiting" will fix the problem, I actually think the bigger problem is that the template should be running with the dev version of Briefcase, not a formal release. The main
branch of the template will be used by the main
branch of Briefcase; so that should probably be reflected in our CI as well.
The only reason we should be using an officially tagged Briefcase version in CI is when we're merging into a specific branch (i.e., a fix to the 0.3.11 template branch), and that version number has been tagged in the Briefcase repo (e.g., when we cut the 0.3.12 branch, there will be a short window where the 0.3.12 branch exists, but Briefcase 0.3.12 hasn't been released yet). In all other cases (merging to main, merging to a pre-release Briefcase), we should be installing the current dev version of Briefcase.
It's timing out because WiX is failing to launch heat.exe because it's looking for it in the wrong place. With a GUI, you get that dialog error message....in GH CI, it just locks up. I'll see about getting Briefcase dev installed for testing. |
It occurs to me that the Github actions logic to determine "install the version of Briefcase that matches the branch of this PR" is going to be moderately complex logic that we will probably want to use in every template; so it's a good candidate for factoring out into the |
57bc64c
to
9e556b5
Compare
4bb69ba
to
8a67cc7
Compare
8a67cc7
to
b80630b
Compare
b80630b
to
ee559f8
Compare
Changes
pre-commit
checks.main
or using appropriate git tag.pre-commit-run
app-build-verify
PR Checklist: