Bump JDK 8u242-b08 to 17.0.7+7

[rmartin16]

Changes

• Bumps JDK from 8u242-b08 to 17.0.7+7
  • Fixes Bump OpenJDK Version #1065
  • Fixes KeytoolException: Failed to read key AndroidDebugKey from store: Invalid keystore format #1112
  • Fixes Android SDK XML warnings when using a user-provided SDK #827
• Dependent on bumping Gradle and the Android Gradle plugin in the template
  • Bump Gradle to 8.0 and Gradle plugin to 8.0.1 briefcase-android-gradle-template#68
• Implements version compatibility checking for templates
  • Now, when breaking changes are introduced for a platform's template, the version of Briefcase they are being introduced in to can be used as an epoch version to later determine compatibility
  • The target Briefcase version epoch for the template is specified via the target_version key in the briefcase section of briefcase.toml
  • The currently supported Briefcase version epoch for a platform is specified via platform_target_version on the Command for the platform
  • If a Briefcase platform declares an epoch version, the rolled out template must declare support for the same epoch version.

PR Checklist:

• All new features have been tested
• All new features have been documented
• I have read the CONTRIBUTING.md file
• I will abide by the code of conduct

[rmartin16]

Adding --add-opens=java.base/java.io=ALL-UNNAMED to jvmargs in gradle-wrapper.properties let the build finish for me.

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':app:processDebugMainManifest'.
> Unable to make field private final java.lang.String java.io.File.path accessible: module java.base does not "opens java.io" to unnamed module @da0e3e0

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 24s
12 actionable tasks: 1 executed, 11 up-to-date
Building...

briefcase.2023_05_12-16_07_54.build.log

[freakboy3742]

One thought - to make the migration path cleaner, do we want to unpack into a tools/java17 directory? That should dramatically simplify any issues around disambiguating java8 and java17 installs, or trying to do in-place upgrades.

[freakboy3742]

This isn't true any more; we can get a native ARM Java.

[rmartin16]

That's what I was thinking....so, all the logic to handle rosetta can come out?

[freakboy3742]

I think so - this was the only thing we were using it for, AFAIR.

[rmartin16]

One thought - to make the migration path cleaner, do we want to unpack into a tools/java17 directory? That should dramatically simplify any issues around disambiguating java8 and java17 installs, or trying to do in-place upgrades.

Great idea; this'll definitely simplify handling changing the java version in user installs.

As far as how this upgrade should interact with Gradle, I'll wait to see if Malcolm (or anyone else) has feedback here; while I got things working, I have practically no prior knowledge for all these interactions.

[mhsmith]

Thanks, I'll have a look at this on Monday.

[mhsmith]

Adding --add-opens=java.base/java.io=ALL-UNNAMED to jvmargs in gradle-wrapper.properties let the build finish for me.

The Briefcase Android template (and most existing user build directories) is currently on Android Gradle plugin 4.2, which was the last version that worked with Java 8. I haven't tested that with Java 17, and probably neither has Google, because Android Studio was still using Java 11 at that time. But as long as this workaround allows it to pass the Toga testbed, it should be fine.

Upgrading to Java 17 would also have the following benefits:

• It would fix KeytoolException: Failed to read key AndroidDebugKey from store: Invalid keystore format #1112, so I've added that to the description above.
• It would allow us to upgrade the Android Gradle plugin, which would remove the warnings mentioned in this comment (though not the sdkmanager warnings in the original report).

One thought - to make the migration path cleaner, do we want to unpack into a tools/java17 directory? That should dramatically simplify any issues around disambiguating java8 and java17 installs, or trying to do in-place upgrades.

Great idea; this'll definitely simplify handling changing the java version in user installs.

So the idea is that Briefcase would still only support one version of Java at a time, but unpack into a java17 directory in case that changes in the future?

In which case when upgrading from an older version of Briefcase, I guess the existing java directory would just be ignored, and left there forever?

[freakboy3742]

So the idea is that Briefcase would still only support one version of Java at a time, but unpack into a java17 directory in case that changes in the future?

Correct.

In which case when upgrading from an older version of Briefcase, I guess the existing java directory would just be ignored, and left there forever?

That's my thinking, yes. There's a very small edge case (mostly affecting us as developers) where we might need to run/test both java8 and java17 in different configurations, so there is some benefit to keeping the old java; at some point in the future. It's not a huge amount of disk space (200MB), and it's easily removed manually if there's an issue. I guess we could add a cleanup pass to briefcase upgrade, but honestly, I don't think it's worth the effort.

[mhsmith]

Adding --add-opens=java.base/java.io=ALL-UNNAMED to jvmargs in gradle-wrapper.properties let the build finish for me.

@freakboy3742 pointed out that updating the template wouldn't have any effect on existing build directories, which would fail to build with Java 17. While build directories are theoretically ephemeral, we do often recommend people to edit them to work around missing features in Toga, so we should avoid forcing people to regenerate them.

Here are some possible solutions:

• See if there's another way to pass this argument, e.g. on the command line or an environment variable.
• Make Briefcase edit existing gradle-wrapper.properties files to add the argument
• Make Briefcase support both Java 8 and Java 17, and have the template specify which one it wants, e.g. in briefcase.toml. Existing build directories which don't specify a version would default to Java 8.
  • This would require the Rosetta code to be retained, unless we switch over to Amazon Corretto, which provides macOS aarch64 builds for both Java 8 and 17. Since all JDKs have roughly the same file layout, I wouldn't expect this to be difficult.

[rmartin16]

Hmm....so, I guess we need to decide which trade-offs are most important....and what should be upgraded at this time.

Gradle Updates

I pushed beeware/briefcase-android-gradle-template#68 to upgrade Gradle and its Android plugin to their latest versions. That obviates the need for the jvmargs update....but it requires a different tweak to specify the package namespace.

Are you interested in updating Gradle at this time?

Breaking Changes

As for the breaking changes in the build directory....we'd be introducing a lot of complexity to avoid them. And it feels like users should have a way to apply their customizations on demand....for CI or just creating their App on an arbitrary machine. And supporting multiple versions of JDK....would this be necessary beyond avoiding breaking changes?

I think I'm leaning towards making the breaking changes and requiring users to rebuild the build directory....but I will defer to you two for this decision.

[mhsmith]

Are you interested in updating Gradle at this time?

Yes: I'll probably have to drop support for Android Gradle plugin 4.2 in the next version of Chaquopy anyway, so it's good that you're looking at this now.

I think I'm leaning towards making the breaking changes and requiring users to rebuild the build directory.

I'm OK with that, as long as the requirement is communicated clearly. Most users won't read release notes, and as this PR stands, a user with an existing Android build directory who runs pip install --upgrade briefcase and then briefcase build android would get an incomprehensible Java error message, which is a pretty bad user experience.

What do you think of this idea for a mechanism to declare when Briefcase has broken backward compatibility with existing build directories? That should come in useful on other platforms as well.

[rmartin16]

I think I'm leaning towards making the breaking changes and requiring users to rebuild the build directory.

I'm OK with that, as long as the requirement is communicated clearly. Most users won't read release notes, and as this PR stands, a user with an existing Android build directory who runs pip install --upgrade briefcase and then briefcase build android would get an incomprehensible Java error message, which is a pretty bad user experience.

What do you think of this idea for a mechanism to declare when Briefcase has broken backward compatibility with existing build directories? That should come in useful on other platforms as well.

Yeah, I agree - we should definitely avoid surfacing such errors from underlying tools.

I think that idea to track the source version of Briefcase in the build directory makes sense. There's also been discussion around tracking other "metadata" about a Briefcase project (like in #807)....I wonder if these should use a shared tracking infrastructure...

I'll play around a bit with it. Additional ideas/thoughts certainly welcome....I always love a ballooning scope 🥲

[mhsmith]

My initial thought was that each template could simply have a placeholder for the Briefcase version in its briefcase.toml file, so that wouldn't require much new code.

[rmartin16]

My initial thought was that each template could simply have a placeholder for the Briefcase version in its briefcase.toml file, so that wouldn't require much new code.

Right....my hesitation with this approach has been that it would require all users to re-run the create command each time Briefcase is updated. I would rather only require this to occur if it is necessary....however, as I've been thinking about it, I've realized this would only be possible if the information about compatibility between Briefcase's default templates and Briefcase itself was maintained out-of-band; that is, that information could not be a part of the template itself....because a cloned template cannot know which future versions of Briefcase it will be compatible with.

Hypothetically, an independent source of this compatibility map could be maintained somewhere on the internet....but then the trade-off of maintaining this map and checking it for every command a user runs....vs just detecting that the current Briefcase is different than the Briefcase that cloned the template and prompting the user to have the create command re-ran for them....starts to become more competitive.

So, ok; I'll move forward with this approach. To make the data more structured than a comment in briefcase.toml, I'm leaning towards making a briefcase section with a version key. I could also imagine maybe a metadata section and a briefcase_version key... And there will probably need to be some special logic to avoid re-running the create command for each different SCM version of Briefcase. Thoughts/ideas welcome.

[mhsmith]

this would only be possible if the information about compatibility between Briefcase's default templates and Briefcase itself was maintained out-of-band; that is, that information could not be a part of the template itself

Like I said in the original comment, my idea is that this information would be in Briefcase itself, with each platform module having a minimum template version it supports. So if this PR is released in Briefcase version 0.3.15, then the Android module would have a minimum template version of 0.3.15, while the other modules probably wouldn't need any minimum at all yet. Each module would only raise its minimum template version when necessary, so that would avoid forcing the user to rerun create for every new version of Briefcase.

[rmartin16]

this would only be possible if the information about compatibility between Briefcase's default templates and Briefcase itself was maintained out-of-band; that is, that information could not be a part of the template itself

Like I said in the original comment, my idea is that this information would be in Briefcase itself, with each platform module having a minimum template version it supports.

Ahh...I didn't realize you meant literally embed the compatibility information within Briefcase. Nonetheless, I considered this and maybe eliminated it from my implementation too hastily.

My primary reason for not wanting to do this was because of arbitrary user-specified templates. On the one hand, we're (kinda) introducing a mechanism to control the compatibility between Briefcase and templates. On the other hand, hardcoding the "oldest supported version" in to Briefcase really limits the usability of the mechanism for arbitrary templates.

I will need to think about these interactions more (and the one below) to fully flesh out an understanding.

(This also made me realize that the version of a template can be specified in pyproject.toml.....so, even if you're using a supported version of Briefcase to run the create command, you would actually need to verify that the target branch for the template is suitable for the current Briefcase version. Otherwise, a subsequent check of the Briefcase version against briefcase.toml will succeed...while the template is actually too old for this Briefcase....)

[rmartin16]

In light of the edge cases I've found so far with tracking the version of Briefcase that cloned the repo inside the rolled out template to determine compatibility between Briefcase and a template, I'm taking a step back to make sure I understand the problem.

Problem

Report incompatibility between the running version of Briefcase and a template that's already been rolled out.....or even potentially a template that's being considered for rolling out.

History

The discussion thus far has centered around capturing the version of Briefcase that actually rolls out the template. The primary issue with this strategy is it uses the version of Briefcase as a proxy for the version of the template....and this proxying is what introduces the edge cases.

Assessment

So, now stepping back, what are we actually trying to detect is incompatible between the two? For this PR, anyway, it is that Briefcase and the template agree about the requirements for the underlying dependencies to operate on the rolled out template. That being the case, dependency management and compatibility is a pretty well-trodden area.

Design

My new approach, therefore, is simply to require that Briefcase and the template being used make compatible assertions about underlying dependencies.

• Briefcase will assert lower-bound (and potentially upper-bound) version requirements (if any exist).
• Templates will assert the exact version of the dependency used in operation.
• If a template doesn't assert a version for the dependency or the version is out of range of what Briefcase is asserting, then Briefcase will error.
  • In here, instead of erroring, we should also be able to prompt users to simply run the create Command again.

Implementation

• Introduce a new step in verification that confirms template dependency compatibility
  • Create new section in briefcase.toml
    • New section would probably be along the lines of briefcase.requires
    • Add requirements for gradle==8.0 and android_gradle_plugin==8.0.1
  • Create new Briefcase Command class variable to declare template requirements
• TODO: need to flesh out the details

Thoughts, opinions, omissions, etc. welcome.

[freakboy3742]

Thoughts, opinions, omissions, etc. welcome.

It's true that we've been using the Briefcase version as a proxy for version compatibility; however, I'm not sure I follow why this introduces edge cases - or, at least, why the edge case is any more than being overly conservative.

I agree that for many releases, a vN template will be entirely compatible with vN+1, and doing a hard check on briefcase version will result in projects that would otherwise be fine from raising an error.

However, we generally encourage people to treat the build folder as ephemeral. There are some exceptions to this (most notably, iOS and Android projects where users need to modify permissions), but I'd suggest those exceptions are all examples of missing features. Since the folder is intended to be ephemeral, I don't see the conservative approach as being especially problematic - a "delete and start again" error message is a mostly acceptable solution.

We could get into micromanaging specific tool versions (in this case Java, but I guess the same could be true of Xcode, Visual Studio, or anything else); but I'm not sure the complexity is worth it. Ultimately, a check of "generated requiring Java==8" won't give you any more fidelity than "generated with Briefcase 0.3.15".

The only reason per-tool dependencies would make sense to me is if we were going to maintain support for older toolchains (e.g., keeping support for Java 8 when we add support for Java 17) - but honestly, I'm not convinced that's worth the complexity. We have limited resources; we already have enough problems maintaining Python and OS versions without borrowing more trouble :-)

[rmartin16]

I'm not sure I understand your conclusions. Are you arguing for status quo? or for stamping the Briefcase version in to the rolled out template? or something else perhaps?

I think I will have an easier time enumerating the compromises if I understand your current position better.

[freakboy3742]

I'm not sure I understand your conclusions. Are you arguing for status quo? or for stamping the Briefcase version in to the rolled out template? or something else perhaps?

I think I will have an easier time enumerating the compromises if I understand your current position better.

I'm not arguing for status quo - I'm saying that per-tool version/dependency checks are overkill, and a Briefcase version in the generated app template should be sufficient.

We definitely need some sort of version check to catch the "this app was generated expecting Java 8 and Gradle 4.2" case - but I think that check can be at the granularity of "This app was generated with Briefcase 0.3.X". We can then introduce checks on a per-backend basis to confirm if there are any known incompatibilities between the generated app and the current Briefcase version.

[rmartin16]

We definitely need some sort of version check to catch the "this app was generated expecting Java 8 and Gradle 4.2" case - but I think that check can be at the granularity of "This app was generated with Briefcase 0.3.X".

If such a check is performed using the Briefcase version as a proxy, there are two primary compromises I'm aware of:

• This assumes the template being cloned is already compatible with the version of Briefcase being used.
  • The most obvious example where this would occur is if an older branch of the Briefcase template is pinned.
  • I don't think there's a good use-case for this type of pinning except if someone is maintaining a cloned repo to track their own augmentation to the released Briefcase template.
  • So, the outcome being that a not near-zero likelihood of users encountering the errors we're trying to avoid after a Briefcase upgrade
• This also doesn't allow third parties to leverage this Briefcase <-> template dependency mechanism
  • Since we'd be tracking this dependency at the granularity of a Briefcase version, even unaffected templates would need to be re-created.
  • But additionally, we'd be stopping short of allowing third party templates a way to better control this dependency themselves
  • (This compromise is noticeably weaker and could reasonably be considered irrelevant/out-of-scope)

We can then introduce checks on a per-backend basis to confirm if there are any known incompatibilities between the generated app and the current Briefcase version.

I'm not sure how this would be implemented practically....unless we'd want Briefcase to start manually inspecting specific files within the rolled out template. I haven't been too keen on this so far because it really starts to interlock Briefcase and the released templates....as well as potentially unobvious mandates for third party templates.

Moreover, though, this really just sounds like the dependency checking I outlined...given it basically just says Briefcase requires these versions and the template provides these versions, are they compatible?

In conclusion, I also feel a bit like all this dependency checking is somewhat overkill....however, I also want to make sure we're on the same page about why stamping the version of Briefcase in to the rolled out template and using that for compatibility checks later has holes. This is why my original design was to simply require re-running the create command for every Briefcase upgrade (....although, even this can't protect against the pinned template version).

If this is still considered much ado about nothing, no worries. I can implement one of these proxies with the accepted trade-offs. However, I'll probably need more insight in to how specific backend dependency checks could be otherwise implemented.

[freakboy3742]

We definitely need some sort of version check to catch the "this app was generated expecting Java 8 and Gradle 4.2" case - but I think that check can be at the granularity of "This app was generated with Briefcase 0.3.X".

If such a check is performed using the Briefcase version as a proxy, there are two primary compromises I'm aware of:

• This assumes the template being cloned is already compatible with the version of Briefcase being used.

It has to be, by definition. Whenever we use a template, we clone a version-specific branch of that template. If the version-specific branch isn't compatible with that version of Briefcase, then that's a bug.

I feel like we're missing each other in the conversation here, but I can't work out where.

The problem we're trying to avoid is that Briefcase 0.3.15 (assuming this change is in the next release) will verify Java 17. An Android project generated using Briefcase 0.3.14 will contain a template that requires Java 8. Building that project using Java 17 will raise errors that will be mostly impenetrable to the end user.

What I'm proposing (and, AFAIK, what @mhsmith has proposed) is:

• adding a briefcase_version = '0.3.15' tag to the briefcase.toml for the Android template
• At some point in the build process, the Android backend will perform a check that briefcase_version >= 0.3.15. If briefcase.toml template doesn't provide this key, we can assume that the project was generated on 0.3.14 or earlier. If this condition isn't met, raise a clean "we can't build this" error.

When 0.3.16 is released, projects generated with 0.3.15 should still be compatible (assuming there isn't some other big change), so the version >= 0.3.15 check will still pass.

In this context briefcase_version is really only serving as a proxy for "Java 17, Android SDK vWhatever, Xcode 13.0.0, ...". It's encoding "This template should work with the tools that were provided by Briefcase version X", or "what is the oldest version of Briefcase that can run this template?".

The only edge case I can see is the case of a user providing a custom template branch; if they're doing that, then they're also responsible for updating the template. If they've forked the 0.3.14 Android template, then it won't run on Briefcase 0.3.15, because it won't be compatible with Java 17 (or, at least, we can't guarantee that). If it is, then they can update their template, or add briefcase_version = 0.3.15 to their existing project's briefcase.toml.

I'm not suggesting that we add this version check to every project, or every backend - only to backends where we know there is a compatibility problem. For now, the Android backend's version of Java is the only compatibility issue that I'm aware of.

• This also doesn't allow third parties to leverage this Briefcase <-> template dependency mechanism
  • Since we'd be tracking this dependency at the granularity of a Briefcase version, even unaffected templates would need to be re-created.
  • But additionally, we'd be stopping short of allowing third party templates a way to better control this dependency themselves
  • (This compromise is noticeably weaker and could reasonably be considered irrelevant/out-of-scope)

I guess we could break this down to a finer grained level; I guess it doesn't really matter if we define the minimum briefcase version and then imply the version of Java, or specify the exact version of Java that is a literal dependency. I'm not sure there's really a use case for this, but I guess there's some appeal to defining "actual" requirements rather than implied ones.

[rmartin16]

this grammar needs to be tweaked...i'll do it later if we're good with the rest of this.

Suggested change

	If you are using BeeWare's default template, then running the create command
	If you are using BeeWare's default template, then run the create command

[freakboy3742]

A couple of comments inline - the only major issue is the version check, which I don't think is correct - the Briefcase version shouldn't factor into the check. Either there's an exact match with the platform's "epoch" version, or there's no match.

I think the specific location of the check might also need tweaking, for the reason flagged inline.

[freakboy3742]

Not sure a waitbar is needed here. Verification should be near immediate.

[rmartin16]

IMO, this wait bar here is mostly intended to provide better continuity with the rest of the output.

While I consider the Wait Bar to be mostly about communicating a task is ongoing, I think the consistency of [task description]... done helps users understand what exactly is taking place in a consistent format. There's several small steps like this we're already wrapping with the Wait Bar elsewhere.

I suppose the end state of the Wait bar output could be manually printed...but that feels a little awkward.

Still want to remove/change this?

Example output:

[helloworld] Generating application template...
Using app template: /home/russell/github/beeware/briefcase-android-gradle-template, branch v0.3.15

[helloworld] Installing support package...
No support package required.

[helloworld] Installing application code...
Installing src/helloworld... done

[helloworld] Installing requirements...
Writing requirements file... done

[helloworld] Installing application resources...
Unable to find src/helloworld/resources/helloworld-round-48.png for 48px round application icon; using default
Unable to find src/helloworld/resources/helloworld-round-72.png for 72px round application icon; using default
Unable to find src/helloworld/resources/helloworld-round-96.png for 96px round application icon; using default
Unable to find src/helloworld/resources/helloworld-round-144.png for 144px round application icon; using default
Unable to find src/helloworld/resources/helloworld-round-192.png for 192px round application icon; using default
Unable to find src/helloworld/resources/helloworld-square-48.png for 48px square application icon; using default
Unable to find src/helloworld/resources/helloworld-square-72.png for 72px square application icon; using default
Unable to find src/helloworld/resources/helloworld-square-96.png for 96px square application icon; using default
Unable to find src/helloworld/resources/helloworld-square-144.png for 144px square application icon; using default
Unable to find src/helloworld/resources/helloworld-square-192.png for 192px square application icon; using default

[helloworld] Removing unneeded app content...
Removing directory app/src/main/python/helloworld/__pycache__
Removing unneeded app bundle content... done

[helloworld] Created build/helloworld/android/gradle

[helloworld] Checking template compatibility...
Verifying Briefcase support... done

[helloworld] Updating app metadata...
Setting main module... done

[helloworld] Building Android APK...
Starting a Gradle Daemon, 1 incompatible Daemon could not be reused, use --status for details

[freakboy3742]

I see what you're saying; I guess for me, "is this a valid template" is a fast validation check that should almost always pass. It's not "doing" anything - it's just verifying that everything else will work. 99% of the time, it won't be communicating anything - and the 1% of the time that it does, there will be a big "it won't work" error message.

Some of the other "short lived" tasks (e.g., updating app metadata) are actually doing something - they're part of the build process, writing/modifiying files, etc. I can't think of another task that is (a) short lived and (b) doesn't write/add/delete anything to the disk, with the exception of no-op tasks that are flagged has doing something, but then the platform backend doesn't need to actually do anything.

The closest I can think of is some of the Docker checks - but those (a) can take time, and (b) can fail in some creative ways, so flagging that you're in the middle of a Docker check can be useful debugging.

[rmartin16]

Ok...so, do you want this to print anything then? I feel like just printing the section header [helloworld] Checking template compatibility... with nothing else is also awkward.

That makes sense if so.

[freakboy3742]

Oh sorry - I thought that part was implied. My intention was to say that there should be no message when verifying the template version, unless there's a problem. Apologies for the ambiguity on my part.

[freakboy3742]

Can't verify this renders 100% correctly, but suggested text:

Suggested change

	The app template must declare a target version of Briefcase to confirm it is
	compatible with this version of Briefcase.
	The template's target Briefcase version must be {'.'.join(map(str, platform_target))} or later.
	Once the template is updated, run the create command:
	$ briefcase create {self.platform} {self.output_format}
	Briefcase requires that the app template explicitly declare that it is compatible with
	Briefcase {'.'.join(map(str, platform_target))} or later. However, the generated app's
	`briefcase.toml` has no `target_version` declaration.
	If the app was generated with an earlier version of Briefcase using the default Briefcase
	template, you can run:
	$ briefcase create {self.platform} {self.output_format}
	to re-generate your app.
	If you are using a custom template, you'll need to update the template to correct any
	compatibility problems, and then add the compatibility declaration.

[freakboy3742]

This should be an elif platform_target != template_target. There's no need for the actual Briefcase version to be involved, and it's not a > comparison - we're looking for a "Briefcase version as epoch" marker. For this change to Java, the template_target will be 0.3.15, and it will stay that way until we make another backwards incompatible change to the Android template format.

[mhsmith]

I already discussed this here and here.

[rmartin16]

this has definitely been a sticking point...i guess i won't say too much more except these approaches are effectively the same...with slightly different implementations.

I like the idea of an epoch....but I think using the Briefcase version is confusing....because people will think you're literally talking about the current version of Briefcase.

I also like the idea of supporting a version inside the template that's independent of the platform version support declaration in Briefcase. This allows third party templates to specify an arbitrary level of support with Briefcase. If we limit this to Briefcase's internal platform version, then we're limiting the number of potential breaking changes third party can protect themselves from....which could be missed breaking changes or even just nuances among versions.

At this point, I'm starting to feel we may just need an executive decision....if only because I'm a bit exhausted going round and round. I'm prepared to implement the epoch versioning or even something else. (I'm still working on promoting the check to all Commands since that's pretty impactful to the tests.)

[freakboy3742]

In which case, I'm going to put on my BDFN hat and say "epoch-style numbering, using the Briefcase version as the epoch marker".

Yes, there's a source of confusion in that a 0.3.16 template will have marker saying it is for 0.3.15. However, as this discussion has revealed, any discussion of compatibility and version numbers is going to be confusing; picking an identifier that has a clear interpretation (vs a new "epoch" number that will need it's own explanation) is about as good as we're going to get, IMHO.

It's also not especially user-visible - it's a value in briefcase.toml, which isn't something that run-of-the-mill users will ever need to see. This is a "confusing detail" that will mostly affect Briefcase maintainers, not the outside world.

Using the 0.3.15 version is also implicit documentation of which release notes you need to look at for a migration guide.

I acknowledge that there could be a need for a third-party template provider to lock against a different Briefcase version. However, we don't have any particular evidence that this requirement exists - it's mostly speculation and trying to foretell the future. On that basis, I'm comfortable living with the limitation until such time as we can clearly identify a specific use-case. If such a requirement were to develop, I can imagine adding an additional template option (or command line option) to ignore template version checks, or impose a different version check; however, I think we should cross that bridge when (and if) we come to it.

[freakboy3742]

Suggested change

	The app template is not compatible with this version of Briefcase since it is
	targeting version {'.'.join(map(str, template_target))}.
	If you are using BeeWare's default template, then running the create command
	to update the app using a compatible version of the app template.
	If a custom template is being used, it must be updated to be compatible with
	Briefcase version {minimum_version} before re-running the create command.
	To run the create command:
	$ briefcase create {self.platform} {self.output_format}
	"""
	The app templated used to generate this app is not compatible with this version of Briefcase.
	Briefcase requires a template that is compatible with version {platform_target}; the template
	used to generate this app is compatible with version {template_target}.
	If the app was generated with an earlier version of Briefcase using the default Briefcase
	template, you can run:
	$ briefcase create {self.platform} {self.output_format}
	to re-generate your app.
	If you are using a custom template, you'll need to update the template to correct any
	compatibility problems, and then add the compatibility declaration.

[freakboy3742]

This should probably be in the base command, and called as part of a more generic check (finalize?). If you've got an app that was generated for 0.3.14, and you try to run it without building, you're going to have the same class of problem - it's not exclusively a build problem.

[rmartin16]

Yeah...I initially tried to put it there but ran in to issue. I'll try to move it up again.

[rmartin16]

I re-implemented the template verification using a target version epoch.

• Verifying app template for all commands
  • Moved app template verification in to base.py
  • This verification couldn't be done in finalize() since it runs before pre-requisite commands run....therefore, a build command would call finalize() before it called a pre-requisite create command and that isn't going to work for this
• Reoriented the configuration from the word version to the word epoch
  • Given the inherent confusion around all this...it seemed like using the word epoch at least avoids the easy confusion with just treating this config as a normal Briefcase version
• Simplified the error reporting in to a single error message
  • The two different error messages based on whether the app template contains a target_epoch or not weren't very different to begin with....so, I combined the error conditions
  • I removed references to Briefcase versions (and especially saying something like "Briefcase 0.3.15 or later") since the actual version of Briefcase isn't really relevant....just that they aren't the same
  • And plus....unless you already understand the epoch thing going on....showing a version seems more likely to just be confusing
• Added some typing
  • I did not go full bore on this like I did integrations....I just fixed the most problematic typing (like using BaseConfig since practically no attributes are defined for it) and added future annotations
  • That said, I can type everything if you want...

[rmartin16]

hmm....when verifying the test coverage is actually meaningful, I realized none of the Gradle platform tests were exercising the target epoch logic because I put the declaration in the Gradle mixin. It should be on the actual Commands.

So, question: should we add the epoch declaration to all Gradle commands? or just the Gradle Build command since I think that's the only one actually dependent on JDK 17?

[edit] think my brain stopped working for a little while....this isn't the reason Gradle platform tests seemingly aren't exercising this code.....or in the least, a different reason must be preventing the tests from failing as I'm expecting...

[mhsmith]

• Reoriented the configuration from the word version to the word epoch
  • Given the inherent confusion around all this...it seemed like using the word epoch at least avoids the easy confusion with just treating this config as a normal Briefcase version

I'd prefer to keep it as target_version, so we won't need to rename it if we ever give it the more complex interpretation discussed before.

From the Briefcase user's point of view, removing the version number from the error message should already be enough to prevent confusion.

As for custom template authors, they should be using the official templates as a guide anyway, so that should lead them to pick the correct value for this setting. And I don't think renaming the setting would make them any less likely to change it.

[rmartin16]

• Reoriented the configuration from the word version to the word epoch

  • Given the inherent confusion around all this...it seemed like using the word epoch at least avoids the easy confusion with just treating this config as a normal Briefcase version

I'd prefer to keep it as target_version, so we won't need to rename it if we ever give it the more complex interpretation discussed before.

That's a fair point; I'll rename it.

in the least, a different reason must be preventing the tests from failing as I'm expecting...

I also found the source of my missing expectations for testing. Currently, this app template compatibility check only runs if the template has been rolled out in the file system. (This is only required for the dev command since all other commands will have a rolled out template (except upgrade but it doesn't do app verification anyway).)

During testing, we don't always mock the app in the file system...and I don't think we should enforce that it is. Instead, we just populate members on the command that effectively mock the app.

So, I'll just re-architect the gate for template compatibility checking on whether template configuration/metadata exists for an app regardless of the source.

[freakboy3742]

I also found the source of my missing expectations for testing. Currently, this app template compatibility check only runs if the template has been rolled out in the file system. (This is only required for the dev command since all other commands will have a rolled out template (except upgrade but it doesn't do app verification anyway).)

Is it required for dev? What's the failure mode that requires a check on dev? As you say, you can run dev without having an app template at all - and this (AIUI) is entirely an issue with generated app templates.

• That said, I can type everything if you want...

I think I'd prefer to keep to "1 improvement per PR" as a general philosophy. Add types to new code, or code that is directly impacted by this change if it helps, but "I'm in base.py so I'll update all the types" only serves to make the PR harder to review, because the reviewer has to separate typing improvements from the core functionality that has changed.

[rmartin16]

I also found the source of my missing expectations for testing. Currently, this app template compatibility check only runs if the template has been rolled out in the file system. (This is only required for the dev command since all other commands will have a rolled out template (except upgrade but it doesn't do app verification anyway).)

Is it required for dev? What's the failure mode that requires a check on dev? As you say, you can run dev without having an app template at all - and this (AIUI) is entirely an issue with generated app templates.

To be clear, I was saying that supporting a way to skip this template version check is only required for the dev command because it can run without a rolled out template. I didn't want to skip the check on command = "dev" or similar....since it should be possible to just discover if a template is rolled out and check it. Although, now it's problematic because dev command doesn't have an output_format specified...

[freakboy3742]

To be clear, I was saying that supporting a way to skip this template version check is only required for the dev command because it can run without a rolled out template.

Ack - that makes more sense :-)

[rmartin16]

I added explicit tests for Gradle to verify a template's compatibility. This was a bit less straightforward than I expected because verifying the template happens after any prerequisite commands have run....but before you actually get in to the Gradle-specific code. So, I had to mock that specific steps already occurred and skip some steps entirely since mocking them can be a little arduous.

I also tweaked skipping the template compatibility check; it is now skipped if a new dedicated error is raised for a missing briefcase.toml or if a command raises NotImplementedError suggesting that something about inspecting a template is not applicable to this Command. I went round and round with this and may have gotten tunnel vision....so fresh eyes may see better alternatives that I'd be open to hearing.

[mhsmith]

Looks reasonable to me, but I'll let @freakboy3742 do the final review because he's more familiar with the code.

[rmartin16]

FYI...I realized something about this coding style while testing this.

When Rich prints the MissingAppMetadata exception to the log file, it doesn't actually roll back up stack of exceptions in to the true source of the exception (that is, whatever code is trying to read briefcase.toml). If, however, you raise the MissingAppMetadata from the KeyError exception, then Rich includes the full stack in the exception print out.

I'm not sure if I think that's a good way to mitigate this problem....but it's something to thing about for this coding style.

[freakboy3742]

To my eyes, this is Rich doing the Right Thing (tm) - "from e" is supposed to provide the context of the exception raise, and there's nothing in this code (as currently written) that connects to the KeyError. The KeyError is being used as flow control, so it's being lost as context.

I don't see any problem with raising MissingAppMetadata "from KeyError" in this case; it gives more direct access to the original cause, and we're not really losing any context from the OSError.

[freakboy3742]

Looks good, and works well in my testing.

I've made a couple of minor tweaks prior to landing:

1. Minor tweak to the language when downloading the new JDK
2. Removed the custom config classes - we weren't using them, and based on where we are now, I'm having difficulty imaging how we would use them
3. Testing was broken on ARM64 hardware, due to a couple of tests that were hard coding the Linux host OS, but not the host architecture. This wasn't picked up because we don't (and can't) do any CI testing on ARM.

[freakboy3742]

To my eyes, this is Rich doing the Right Thing (tm) - "from e" is supposed to provide the context of the exception raise, and there's nothing in this code (as currently written) that connects to the KeyError. The KeyError is being used as flow control, so it's being lost as context.

I don't see any problem with raising MissingAppMetadata "from KeyError" in this case; it gives more direct access to the original cause, and we're not really losing any context from the OSError.

[freakboy3742]

I think we can probably clear these two out - I don't know what I was originally anticipating, but based on where we are now, I can't see any use case for per-command customised AppConfig classes.