Skip to content
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

Move logic to determine support revision from templates #1943

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rmartin16
Copy link
Member

@rmartin16 rmartin16 commented Aug 7, 2024

Proposal

  • Instead of logic specified in a template to derive the default support revision, this moves that logic in to Briefcase.
  • We're currently using three different sources of support packages:
    • macOS: first-party BeeWare packages
    • Windows: third-party python.org packages
    • Linux: third-party Standalone Python packages
  • These defaults of where to source a support package from are specified directly within Briefcase
  • So, also specifying the default revision to use for these packages makes sense to me
  • Also, it'll make it much simpler to bump the revision if we don't have to create a PR for each output format 🙂
  • None of the semantics of overriding these defaults in pyproject.toml changes
  • What do you think?

WiP Changes

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
Copy link
Member Author

@freakboy3742, explicitly tagging since I normally open draft PRs that aren't ready for review; is this a change you support? I can probably finish rolling this out tomorrow if so.

@freakboy3742
Copy link
Member

@rmartin16 I see the direction your heading here, and I think I understand the motivation, but I'm not 100% on board. I'm not fundamentally against it - I'm trying to get the consequences straight in my head.

Some of the following will be me thinking out loud. What I'm trying to wrap my head around comes down understanding where Briefcase's responsibility ends, and the template responsibility starts.

While bumping a support revision does require multiple PRs, I don't fundamentally see that as a problem. Yes, it's more work, but it's fairly straightforward work, and I think there is something to be said for explicit per-template CI verifying each change is current.

I agree that at the moment, there's a weird split of responsibilities around the support package - the base URL for the support package is defined in Briefcase, but the revision is defined in the template, with both being overridable on a per-project basis in pyproject.toml. There's an argument to be made that the change should go the other way - that the template should be defining the base URL (admittedly, the logic for building the Linux support package name would be messy to define in template, and harder to test for the cases that we can't easily run in CI).

Moving the URL definition the other way would have the additional benefit that generating a template in support of an alternative support package would be much more plausible. For example, you recently made the suggestion about switching to the Standalone version of the Windows and macOS support packages. If the base URL was defined in the template, that would make it possible for an end-user to maintain a Standalone set of templates without requiring per-project configuration.

That said - I guess the change you've proposed here actually makes both possible. The default is computed in Briefcase, and it's encoded in the template when the template is generated - but a custom template could define its own logic for computing the desired support revision.

I guess, if anything, my argument would be that this PR doesn't take the suggested change to its logical conclusion. If we're going to do this for the support revision, we should also do it for the support URL, and probably also for the stub binary revision and URL. In addition, if there's any variable that we are using to compute the URL or revision (such as target architecture), that variable should also be passed to the template. That way, we keep testable defaults in a single source of truth (Briefcase itself); but everything that Briefcase is doing could be replicated by a third-party template, should there be a need to do so. Doing this likely means more changes to how the support package URL and stub binary URL are computed, but I think the end state will be more consistent.

As a broader topic; it's worth keeping in mind how this sort of change interacts with this sort of landing sequence. In the case of that PR, we're bumping the support revision on macOS templates, and bumping the template epoch, which has complicated the landing sequence. I don't think the changes you've suggested here would change anything - but if we're able to make changes that would simplify things, that's definitely something we should consider.

@rmartin16
Copy link
Member Author

nooo, not my beautiful scope 💥 💥 💥 🥹

😉 Makes sense, though; I'll see what this might take.

@Deepak6546kumar

This comment was marked as spam.

@rmartin16
Copy link
Member Author

High-level design

  • All parameters that Briefcase is currently using to drive output format template rollout will have key/value pairs in briefcase.toml
  • Briefcase will own default values for output format template
    • These defaults will be included in the template context passed to the template rollout process
    • Then, instead of being generated at app build-time, they will be read from briefcase.toml
    • Further, any contextual data (e.g. host architecture) will be included in the context as well
  • Briefcase will defer to the output format template's briefcase.toml for these configuration options after the template has been rolled out
  • All project-specific overrides in pyproject.toml will continue to take precedence

New output format template context fields

  • General fields to include in context
    • python_version_tag
    • platform
    • is_32bit_python
  • New briefcase.toml fields for support package
    • package_url
    • package_revision
  • New briefcase.toml fields for stub binary
    • binary_url
    • binary_revision

Currently assumed paths

  • A few paths are hardcoded in Briefcase
    • binary_path
    • binary_executable_path
    • unbuilt_executable_path
  • Should we add these to paths in briefcase.toml?

Structure of briefcase.toml

  • Currently there are tables for briefcase and paths
  • New tables for support and stub will be added

Current template context fields

Shared

  • project_name
  • app_name
  • version
  • bundle
  • description
  • sources
  • formal_name
  • url
  • author
  • author_email
  • requires
  • icon
  • document_types
  • test_sources
  • test_requires
  • supported
  • long_description
  • license
  • console_app
  • format
  • python_version
  • host_arch
  • class_name
  • module_name
  • package_name
  • bundle_identifier
  • year
  • month

Linux System

  • debian
  • rhel
  • suse
  • arch
  • target_vendor
  • target_codename
  • target_vendor_base
  • target_image
  • system_requires
  • system_runtime_requires
  • glibc_version
  • python_version_tag
  • docker_base_image
  • vendor_base
  • use_non_root_user

Linux Flatpak

  • flatpak_runtime
  • flatpak_runtime_version
  • flatpak_sdk
  • finish_args

Linux AppImage

  • manylinux
  • system_requires
  • linuxdeploy_plugins
  • manylinux_image
  • vendor_base
  • use_non_root_user

Windows App / Visual Studio

  • version_triple
  • guid
  • install_scope

Android Gradle

  • base_theme
  • build_gradle_dependencies
  • permissions
  • features
  • version_code
  • safe_formal_name
  • extract_packages

macOS App / Xcode

  • universal_build
  • info
  • entitlements

iOS Xcode

  • info

Web Static

  • style_framework

@rmartin16
Copy link
Member Author

Looking at how this might work for the support package URL, there's a lot of existing parameterization... However, most of the information is not expected to change; or, at least, if the information changes (such as the Python version), then the user is expected to re-rollout the output format template. Therefore, it should be safe to simply derive these values at the time the template is rolled out and hardcode them in briefcase.toml.

For instance, if we consider BeeWare's first-party support packages, the URL is:

f"https://briefcase-support.s3.amazonaws.com/python/{self.python_version_tag}/{self.platform}/{self.support_package_filename(support_revision)}"

Here, python_version_tag and platform would be derived and just hardcoded in to the URL.

The support revision is special, though; it can currently change without expecting the user to re-rollout the whole template. Because of this, I am thinking of templating the URL in briefcase.toml.

For instance, it could look like this:

[support]
package_url = "https://briefcase-support.s3.amazonaws.com/python/3.12/macOS/Python-3.10-macOS-support.b{package_revision}.tar.gz"
package_revision = "2"

Then, when Briefcase goes to install or update the support package, it would just call str.format() to expand the templated URL:

support_url = package_url.format(package_revision=package_revision)

This could even be extended to allow templating other values in the future.

For instance, it will probably be necessary to incorporate the concept of a support package "release" for Standalone Python:

[support]
package_url = "https://github.com/indygreg/python-build-standalone/releases/download/{package_release}/cpython-{package_revision}-x86-64-unknown-linux-gnu-install_only_stripped.tar.gz"
package_revision = "3.10.14+20240726"
package_release = "20240726"

This is kinda makes me want to just call str.format() on all of these fields with the key/value pairs of the current table; that way, the templating is a bit more robust....just don't want to open too many cans of worms....

Anyway, some initial thoughts....feedback welcome as I get in to the implementation.

@freakboy3742
Copy link
Member

High-level design

Agreed on these as general principles.

New output format template context fields

...

  • New briefcase.toml fields for stub binary

    • binary_url
    • binary_revision

Do we maybe want to call these stub_binary_* to make it clear what "binary" we're talking about?

Currently assumed paths

  • A few paths are hardcoded in Briefcase

    • binary_path
    • binary_executable_path
    • unbuilt_executable_path
  • Should we add these to paths in briefcase.toml?

So - a template overriding these is definitely an edge case. It would mean you are (for example) building macOS app bundle whose structure is fundamentally different to the default app template (say, pushing the .app folder down into a subfolder or similar).

If we're being completely consistent, then I guess that should be exposed to briefcase.toml... but the use case is very narrow, so if this fell out of scope (or was deferred), I wouldn't mind.

Structure of briefcase.toml

  • Currently there are tables for briefcase and paths
  • New tables for support and stub will be added

I can see how this might help with organisation and keeping keys shorter; my only question is how these new tables interact with the existing configuration. What's the plan for backwards compatibility? We have historical usage of support_path and support_revision; how are those keys interpreted with a new support table? Or do we just declare a new template epoch, and don't worry about the backwards compatibility?

The support revision is special, though; it can currently change without expecting the user to re-rollout the whole template. Because of this, I am thinking of templating the URL in briefcase.toml.

We need to be careful about how we choose the fields that are "meta-parameterised" in this way, and how we document the feature; but I think it potentially makes sense for some of these configuration items to be explicitly "parameterised", at least to some extent.

A (possibly unintended, possibly beneficial?) side effect of this is that pyproject.toml keys can be overridden at the command line with -C, so you could potentially set up a CI action that overrides the properties of the support package URL as part of the CI configuration. This is a niche thing that we see with the Python-support-testbed project. It's absolutely a niche edge case, but consistency sometimes leads to interesting solutions to otherwise complex problems.

@rmartin16
Copy link
Member Author

  • New briefcase.toml fields for stub binary

    • binary_url
    • binary_revision

Do we maybe want to call these stub_binary_* to make it clear what "binary" we're talking about?

Since I was planning to introduce a new [stub] table for these entries, I left off the prefix for the key; but I'm not married to any of these names; especially if we think it should require users to change existing entries in their pyproject.toml.

Structure of briefcase.toml

  • Currently there are tables for briefcase and paths
  • New tables for support and stub will be added

I can see how this might help with organisation and keeping keys shorter

Right; organization is my primary impetus for this since putting everything under [paths] doesn't feel right.

my only question is how these new tables interact with the existing configuration. What's the plan for backwards compatibility? We have historical usage of support_path and support_revision; how are those keys interpreted with a new support table? Or do we just declare a new template epoch, and don't worry about the backwards compatibility?

Yeah; I was basically planning to set a new template epoch for most of the output formats. As for uses of support_path/revision in pyproject.toml, they can continue to serve the same purpose without being renamed.

Also, on a related note, have you thought about how the template epoch will work once we transition to calendar versioning? Seems like it'll be a bit harder to know what the epoch should be...

@freakboy3742
Copy link
Member

Yeah; I was basically planning to set a new template epoch for most of the output formats. As for uses of support_path/revision in pyproject.toml, they can continue to serve the same purpose without being renamed.

👍 . In which case, naming and structural changes are essentially moot, as any template epoch can essentially adopt whatever format it wants.

Also, on a related note, have you thought about how the template epoch will work once we transition to calendar versioning? Seems like it'll be a bit harder to know what the epoch should be...

Is there any reason that calver wouldn't work? One of the nice things about effects of calver is that YY.MM.DD version numbers are both logically sortable and evaluate as "newer" than 0.3, 1.2, etc. And, for that matter, make more sense when read - you've essentially got a value in the template that says "This was updated in August of 2024". It's no harder than knowing that 0.3.15 is the current template epoch for Android, except you also have a date to work with.

@rmartin16
Copy link
Member Author

Is there any reason that calver wouldn't work? One of the nice things about effects of calver is that YY.MM.DD version numbers are both logically sortable and evaluate as "newer" than 0.3, 1.2, etc. And, for that matter, make more sense when read - you've essentially got a value in the template that says "This was updated in August of 2024". It's no harder than knowing that 0.3.15 is the current template epoch for Android, except you also have a date to work with.

Oh, not that it wouldn't work mechanically. Just, at dev time, it seems to present a procedural conundrum. Mostly, how would I know, at dev time, what the epoch should be? I feel like this would require either a set release schedule (like pip or pillow, for instance) or a process to adjust the epoch when cutting the release. I could be missing something....but I was thinking about and figured I'd bring it up.

@freakboy3742
Copy link
Member

Oh, not that it wouldn't work mechanically. Just, at dev time, it seems to present a procedural conundrum.

I mean... yes... but that's true today as well. How does a new template developer know that their Android template is in the 0.3.15 epoch - other than the template they forked was 0.3.15? And how do they know that their template needs to be updated, other than the error message when the epoch is bumped tells them that the template is no longer epoch-appropriate? Calver doesn't change anything - it just changes the number.

@rmartin16
Copy link
Member Author

I don't think I'm explaining this very well; let me elaborate. I'm actually talking about us, the developers of Briefcase and its first-party templates.

To play out a scenario, let the current version of Briefcase be v25.03.15 and the current date is 1 April 2025. I am making a change (perhaps not too dissimilar from this PR) that requires a new epoch for the Linux Flatpak template. The template currently has an epoch of 0.3.20 and I need to update it...what do I set it to?

Since the epoch is tied to the versions of Briefcase, in the pre-calver world, I would simply extrapolate the current version of Briefcase to the next one; so, v0.3.21 will almost certainly follow v0.3.20. However, on calver, this extrapolation is less prescriptive; which version of Briefcase will follow v25.03.15? Unless there is a similar process to a priori know/guess the next version of Briefcase....I'm uncertain how we will set these epochs prior to the actual release of Briefcase itself.

Hopefully my thought process makes more sense now....or I'm just missing something altogether.

@freakboy3742
Copy link
Member

Oh! Now I see what you're saying! 🤦

Good question. I don't know. 😀

Moving to a simple integer version would be one option; another would be to use the same version but interpret it as "version must be >", rather than >=. Neither of those are very appealing, though...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants