YehudaKremer / msix

Create Msix installer for flutter windows-build files.
https://pub.dev/packages/msix
MIT License
276 stars 68 forks source link

[FEATURE REQUEST] How would you release builds and versions?? #185

Closed gslender closed 1 year ago

gslender commented 1 year ago

With Android and iOS/macOS, you can release version 1.2.3 build 456, test and confirm that all is working, and increment to the next build, submit, and test again, submit a final build and release version 1.2.3 build 499

How do we do that with msix ?? As the version number is tied to the version number 1.2.3.0 there is no way to specify that this is build 456 or 477 etc. So how do you recommend we submit beta/test versions to the Package Flight that corresponds with some similar version / build numbering?

BentEngbers commented 1 year ago

Maybe we could change how the msix package determines the version based on the version keyword in the pubspec.yaml? So for example, if the the pubspec.yaml contains the following:

version: 1.2.3+45

The version in the msix package would be 1.2.3.45? Perhaps this could be a configuration flag that is disabled by default? If the maintainer (@YehudaKremer i think) finds this a good feature, i could take a look to create a PR?

Levi-Lesches commented 1 year ago

There is the option msix_version, where you can set that final number. I have

version: 2023.5.2+2

msix_config: 
  msix_version: 2023.5.2.2
gslender commented 1 year ago

It would be really cool if you could make it something like this...

version: 2023.5.2+2

msix_config: 
  msix_version: {$version}.{$build}

or even alterantively, if the msix_version field is missing, that it defaults to this particular arrangement.

Such that with either, the version in the msix package would be 2023.5.2.2

BentEngbers commented 1 year ago

Ive created a PR that combines patch and build number in #192. It is fully tested and documented. Please review/upvote it if you would like to have it merged. Documentation about the new flag can be found here: https://github.com/BentEngbers/msix/blob/main/doc/msix_version.md You can use it right now by changing your msix dependency in pubspec.yaml to.

  msix:
    git:
      url: https://github.com/BentEngbers/msix
      ref: main
YehudaKremer commented 1 year ago

I will look at it as soon as possible Thanks!

skyeskie commented 1 year ago

Comments on the (closed) PR:

Between the number limit and Microsoft reserving the fourth field, there can be some rather complex edge cases. Furthermore, with silent fallback, it could be rather difficult to debug. If you do want to add this in, clearly define the requirements and fail the build if they aren't met.

Alternatively, any sort of version mapping like this is inherently subjective to how someone uses the unstructured part of semver. So that logic should be in the package that's using msix, rather than trying to guess (or have an increasing number of transformations added). Since there isn't a way to just override like libraries, it'd be in the form of manually setting msix_version


I see that you discarded the PR, but probably should add a note in the documentation explaining why only using Major.Minor.Patch (to reduce repeat issues)

gslender commented 1 year ago

Any chance we can still have an option here or shall we just move on and not use this package if that's what you're looking for??

skyeskie commented 1 year ago

Two options to use the package as-is:

  1. Increment the patch version when you need to publish a new installer.
  2. Set the msix_version manually or through your own helper script.

This auto feature was based on Option 2: https://github.com/skyeskie/msix_config_version_match You'd need to adjust it to pack the 4 fields into 3. Or use it as basis for another script or tool.

YehudaKremer commented 1 year ago

@skyeskie Thank you for the detailed answer and for providing alternative solution for that problem (same as @Levi-Lesches 👍).

@BentEngbers Thanks again for your good PR.

Let's stay with the option of using the msix_version config field.

Levi-Lesches commented 1 year ago

I do think there is value in having just one version in your pubspec, if not just for the fact that CLI tools won't look at msix_version but only at the pubspec version. Making a second version field means you can't use tools like package:cider or other popular GitHub Actions for CI/CD on Windows.

In terms of intuitive behavior vs opinionated, I think having the pubspec version be w.x.y+z and the msix version be w.x.y.z is pretty simple, and if you want to change it manually, you could use msix_version to do so. In other words, msix_version being blank should mean "use the pubspec".

The real problem for me isn't the semantic meaning of the build number, it's the fact that windows will reject an update if it has exactly the same version as your local installation, so having something change will make it possible for a newer build to update an old one.

gslender commented 1 year ago

[quote] In terms of intuitive behavior vs opinionated, I think having the pubspec version be w.x.y+z and the msix version be w.x.y.z is pretty simple, and if you want to change it manually, you could use msix_version to do so. In other words, msix_version being blank should mean "use the pubspec". [/quote]

This !!!

skyeskie commented 1 year ago

I think having the pubspec version be w.x.y+z and the msix version be w.x.y.z is pretty simple

From Microsoft:

For Windows 10 or Windows 11 (UWP) packages, the last (fourth) section of the version number is reserved for Store use and must be left as 0 when you build your package (although the Store may change the value in this section). The other sections must be set to an integer between 0 and 65535 (except for the first section, which cannot be 0).

gslender commented 1 year ago

I wonder if that is indeed correct, because that detail isn't reflected in the official SDK library for identity. https://learn.microsoft.com/en-us/uwp/schemas/appxpackage/uapmanifestschema/element-identity

The only official requirement that is documented is that it is Major.Minor.Build.Revision where Major cannot be 0.

That is THE only official requirement. Have you actually confirmed that the store won't accept a submistion where the version string is 1.2.3.4 - I've submitted many where the Revision isn't zero and the store hasn't had any issue with that.

BentEngbers commented 1 year ago

I do think it is important to add some kind of custom versioning to this package to reduce the complexity for the developers. Especially for developers who are using flutter to develop multiplatform apps. My reasons are as follows:

I am happy to make any changes to my original PR (#192) to facilitate this custom versioning. I agree that silent fallback is not the best idea. I can definitely change behaviour. @YehudaKremer & @skyeskie are there additional modifications that i would need to make to the PR to make it acceptable? Thanks for all your hard work in creating and maintaining this package!

skyeskie commented 1 year ago

If we are working from the assumption of x.y.z+b, where b is a always increasing build number, the main thing (besides silent fallback) is how to pack the version number.

  1. Which two(?) fields to combine?
    • Build number is likely to be the largest, so I don't think it should be combined at all
    • Either (Major & Minor) or (Minor and Patch). I'm kinda split on which one, but leaning towards (Minor and Patch)
    • If we combine (Major and Minor) and the major version gets too large, there really isn't a fix
    • For (Minor or Patch), it can be rolled over to the next number up
  2. How to combine the fields?
    • Bitwise - Largest allowed values, not as quickly translated back
    • Zero-padded concatenation - Readable, and should be large enough
    • Equal split (8bit or 2 digits each) or unequal split

Alternatively: If you really don't care about matching numbers for human readability, just treat it as having 48-bits to split between the four fields (10, 10, 10, 18) allowing for rather generous constraints (2^10 and 2^18)


My inclination would be combine minor and patch using zero-padded roughly equal split. Limits:

So then you have easy constraints to check. And if minor or patch is outside constraints, it can also include an easy instruction on how to fix

Levi-Lesches commented 1 year ago

From Microsoft:

For Windows 10 or Windows 11 (UWP) packages, the last (fourth) section of the version number is reserved for Store use and must be left as 0 when you build your package (although the Store may change the value in this section). The other sections must be set to an integer between 0 and 65535 (except for the first section, which cannot be 0).

Oh, interesting, I didn't realize. I haven't published to the store, I'm actually just using this package for private distribution among my team. That's unfortunate.

I wonder if that is indeed correct, because that detail isn't reflected in the official SDK library for identity. https://learn.microsoft.com/en-us/uwp/schemas/appxpackage/uapmanifestschema/element-identity

The only official requirement that is documented is that it is Major.Minor.Build.Revision where Major cannot be 0.

That is THE only official requirement.

It's not the "only" official requirement if it's documented in the store policies though. In other words, you're right that 1.2.3.4 is a valid version number, but the store only accepts versions that end in .0, and probably increments the revision number at its own discretion.

Have you actually confirmed that the store won't accept a submistion where the version string is 1.2.3.4

A quick Google search shows that this is in fact enforced often enough to be a problem.


If we are working from the assumption of x.y.z+b, where b is a always increasing build number, the main thing (besides silent fallback) is how to pack the version number -- Which two(?) fields to combine?

I would then say we shouldn't combine any of them. The msix_version field makes it unambiguous what the final version string will be, so any automatic fallback should be at least as obvious. It seems the only reasonable thing to do would be to drop the build number entirely and just use x.y.z. That's not just coming from my personal preference: Microsoft knows about build/revision numbers and chose to disallow them in the Store, so this package should probably do the same thing. Incrementing the patch number instead of the build number isn't too bad of a tradeoff.

BentEngbers commented 1 year ago

Ive implemented the feature in PR #199 according to @skyeskie's comment so please review it if you would like to see any changes.


I would then say we shouldn't combine any of them. The msix_version field makes it unambiguous what the final version string will be, so any automatic fallback should be at least as obvious. It seems the only reasonable thing to do would be to drop the build number entirely and just use x.y.z. That's not just coming from my personal preference: Microsoft knows about build/revision numbers and chose to disallow them in the Store, so this package should probably do the same thing. Incrementing the patch number instead of the build number isn't too bad of a tradeoff.

I think an optional flag that changes the way version numbers are handled by the msix package to get it more in line with the other stores is the best solution, because:

  1. It does not require a change existing configurations, because the flag is opt-in
  2. The default behaviour stays clear.
  3. It helps developers who are trying to synchronise their version of the app on multiple platforms based on the standard version string in the pubspec.yaml.