fix: Allow non-semver versions in getVersionInWeirdWindowsForm

[faern]

Fixes #7173

This new code allows the user to not the lower version parts, and have them default to zero instead. With this change the resulting weird-windows-version will be:

# this.version -> windows-weird-version (buildNumber = 0)
'1.2.3' => '1.2.3.0'
'1.2' => '1.2.0.0'
'9.2-beta' => '9.2.0.0'
'45' => '45.0.0.0'
'2022.6-beta2-dev-afafaf' => '2022.6.0.0'

[changeset-bot]

🦋 Changeset detected

Latest commit: 073daca

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages

Name	Type
app-builder-lib	Patch
dmg-builder	Patch
electron-builder-squirrel-windows	Patch
electron-builder	Patch
electron-forge-maker-appimage	Patch
electron-forge-maker-nsis-web	Patch
electron-forge-maker-nsis	Patch
electron-forge-maker-snap	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[netlify]

✅ Deploy Preview for car-park-attendant-cleat-11576 ready!

Name	Link
🔨 Latest commit	073daca
🔍 Latest deploy log	https://app.netlify.com/sites/car-park-attendant-cleat-11576/deploys/634f8ed134cd7500086e2ae5
😎 Deploy Preview	https://deploy-preview-7174--car-park-attendant-cleat-11576.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[mmaietta]

Hi there! I'm not sure I understand. Why would something like 3.9.2-beta not want to include the -beta in the metadata version? Or is it already parsing out the -beta with the current SemVer logic?

Side note, this might be a breaking change, right? (not opposed, just would need to coordinate with other "alpha" release submissions)

[faern]

Hi there! I'm not sure I understand. Why would something like 3.9.2-beta not want to include the -beta in the metadata version? Or is it already parsing out the -beta with the current SemVer logic?

Yes, semver already removes this. Or well, of course semver understands it's a beta. But the old code here just used the major, minor and patch fields. So it was already stripping out any -beta, -dev, -pre etc tags. The "windows weird format" is just four integers. It does not allow for other data than those four integers anyway.

If you set extraMetadata: verion to 3.9.2-beta the result would be the same before and after this PR: Windows version metadata field set to 3.9.2.0. The difference is just that now it's also allowed to give the version as 3.9-beta and have that gracefully translate to 3.9.0.0. For apps that don't use patch releases at all.

And this will not affects the version in most parts of the app. Only what's filled into the "weird version field" on Windows, not other parts.

[faern]

Side note, this might be a breaking change, right?

I don't think so. I can't come up with any version that was previously allowed that is now forbidden, or that translates to something different than before. The version x.y.z[whatever] will still translate to x.y.z.buildNumber. The only difference is that you are now allowed to omit z, z+y or z+y+x and have those default to zero instead of throwing an exception.

If I managed to implement it as I intended it, this should just allow for more versions to be valid, not change behavior any currently allowed version.

Sidenote: We might want to still require the major and not have that default to zero. Making it a tiny bit stricter, but so we at least have some number to give to Windows... I'll look into that.

[mmaietta]

We might want to still require the major and not have that default to zero. Making it a tiny bit stricter, but so we at least have some number to give to Windows... I'll look into that.

I'm in favor of being stricter. That was how SemVer was being used, so I think it's worth enforcing a major version being present/provided

[mmaietta]

Please also create a changeset file for this change (pnpm generate-changeset) to have this autopicked up in the release notes :)

[faern]

I managed to generate a changeset, and I added it to the same commit and force-pushed. I hope this is OK now 👍

[mmaietta]

Can you please rebase/merge latest master?

Unrelated to your PR, unit tests broke on master due to an npm package changing file contents (we snapshot the unpacked node_modules dir in some tests). I just pushed the changes to fix it

[faern]

Speaking of tests. I could add tests to getVersionInWeirdWindowsForm or somewhere higher to ensure that the non-semver versions are now accepted on all platforms, if you want me to. But I'm not sure where such a test would go, so some directions would be nice.

[mmaietta]

Oooo I like your idea.

My best route would be to extract it to a function outside of the AppInfo class (just an exported function), we'd then have a utilsTest.ts that would explicitly test for various entries of semver.
Similar to this smarten being an exported function:

electron-builder/packages/app-builder-lib/src/appInfo.ts

Lines 10 to 20 in 8166267

	export function smarten(s: string): string {
	// opening singles
	s = s.replace(/(^|[-\u2014\s(["])'/g, "$1\u2018")
	// closing singles & apostrophes
	s = s.replace(/'/g, "\u2019")
	// opening doubles
	s = s.replace(/(^|[-\u2014/[(\u2018\s])"/g, "$1\u201c")
	// closing doubles
	s = s.replace(/"/g, "\u201d")
	return s
	}

We can rename urlUtilsTest.ts to just be utilsTest.ts and then you can write very similar tests for handling getVersionInWeirdWindowsForm

Thoughts?

I'd love to get this into the upcoming v24-alpha release

[faern]

To not miss the release, maybe we should merge as is and consider tests later? Because I will probably not have the time to do that right now 🙈

[mmaietta]

https://github.com/electron-userland/electron-builder/releases/tag/v24.0.0-alpha.1
🙂