[hxb] Add minor version handling, increase current version #12365
Conversation
| - all minor checks in hxb reader become obsolete and can be dropped too | ||
| *) | ||
| let hxb_major = 2 | ||
| let hxb_minor = 1 |
There was a problem hiding this comment.
The hack for retro compatibility with preview1 is to consider preview1 to be 2.0 (instead of 1.0) so that we're back to comparing minor versions
So here we'd start with 2.1 for the 12351 change
There was a problem hiding this comment.
But why do we check the minor version in the first place?
There was a problem hiding this comment.
To know when some data should (or not) be read, like https://github.com/HaxeFoundation/haxe/pull/12365/files#diff-e11692c26a1b5eb50b0ff3c69d1faa85d755afd3eb2e2daba537d844d8eddb21R1587-R1591 for 12351
There was a problem hiding this comment.
Yes but then why not check hxb_major >= 2 for this one?
There was a problem hiding this comment.
It would work (well it wouldn't with current impl) too, but for the next changes we'll always want to check minor versions, never major (if the major version didn't match we wouldn't be in hxb reader in the first place).
I figured I'd use that same behavior from the beginning
There was a problem hiding this comment.
I kind of get what you're saying, but this is completely internal and invisible. I'd find it more natural to go from version 1.0 (implied) to 2.0 instead of 2.1, and that's actually visible.
I am not using hxbVersion or the hxbWriterConfig + JSON for that matter. I appreciate bringing me into the discussion, it makes me less hesitant to continue building with HXB.
I'd like for HXB creation centered around HXMLs only, the JSON format although quite light, is still an extra format + file to think about, and the special config options could be brought into HXML via flags. For the HXB versions, hxbMajor and hxbMinor makes more sense, but are there any explored cases to use the HXB version? Right now my current understanding as an end user, is the best path is to generate an HXB per target I support, for a given library I want to HXBify. It must be built with Haxe 5, and after this PR in order to allow future Haxe version compiler usage. Would be interested if this is correct in your guys' view? If I update the Haxe compiler version to a major HXB version change after this PR, will the PR version still work with the newly generated HXB? |
I haven't really worked on the configuration part, but it seems too complex for flags with support for exclue/include arrays. It has probably not been used at all, but still.. I'm actually thinking of removing the version part of the config, as targeting a version different than current version would very likely just lead to issues.
I think so. Technically the zip files could even be merged I think, as iirc each target has its own directory inside the archive. Not sure if that would cause other issues, though.
An hxb file written with a major version won't be compatible with any other major version. But a major version bump shouldn't happen often, if at all. Maybe we could even have a directory per version inside the hxb file, so that a single hxb file could be compatible with multiple haxe versions 🤔 |
That's a fair point, at least for the exclude list, for include lists, I think it's pretty straightforward.
It's nice aesthetically but wouldn't change the file sizes or portability as long as
That would be ideal, the more HXB can feel like a first class citizen when it comes to compatibility like Haxe source code, the better on my end. Larger file sizes also wouldn't be that much of an issue, given that in my case HXB's are substantially smaller then the source code. 42.2 MB soure code vs 26.4 hxb.hl.zip MB |
Some changes had an impact on hxb writing/reading (latest being #12351) without changing hxb version number, meaning using hxb-lib with haxe versions from before and after that PR led to compiler failures or HxbFailure errors.
The
hxbWriterConfig.mlsituation is a bit weird.. and possibly not even needed. I have no idea what external tool might be usinghxbVersionfrom it in any way atm; maybe @PXshadow ? Two alternatives there:hxbVersionfield from there entirely, and only deal withhxbMajor/hxbMinorhxbVersion(for major) andhxbMinor, which is a weird combo but doesn't duplicate stuff