[wix-devs] 6210 - Burn engine should handle non-QWORD versions

[Rob Mensching]

> I'll probably leave FileVersionFromStringEx alone

Sure.


> I think I'll leave the internal Burn engine version as a QWORD though.

Bummer. If Burn used the new string/version struct that would allow people to use both dotted versions and semver for their new bundles. It'd also guarantee one user of the code. ;)


> a standalone application would want "@#$%^&*" to be "successfully" parsed into a version.

I didn't think "@#$%^&*" would be successfully parsed. The version string would have to be something like "1", "1.2", "1.2.3", "1.2.3.4", "1.2.3-preview.5" etc. I'm probably missing something.


> . Maybe this will help - what would we call this version struct if it was verutil.h? Would it still be BUNDLE_VERSION?

No, not BUNDLE_VERSION. What about "VERSION_PARSED"? Or "VER_VERSION"? Or "VERUTIL_VERSION"? Or "VER_PARSED_VERSION"?


-----Original Message-----
From: wix-devs <wix-devs-bounces at lists.wixtoolset.org> On Behalf Of Sean Hall via wix-devs
Sent: Thursday, July 16, 2020 1:55 AM
To: WiX Toolset Developer Mailing List <wix-devs at lists.wixtoolset.org>
Cc: Sean Hall <r.sean.hall at gmail.com>
Subject: Re: [wix-devs] 6210 - Burn engine should handle non-QWORD versions

Ok, strings it is. Though I would like to point out that the struct has a copy of the original string (well, with the leading 'v' removed if it was there).

Jacob, I will change the data type for all of the version variables (C++ not Burn) in Burn to LPWSTR. That should include atomutil and apuputil.
I'll probably leave FileVersionFromStringEx alone, unless someone wants that to be changed too. I think I'll leave the internal Burn engine version as a QWORD though.

I don't know where we're storing versions as QWORDs in 3.x? I'm pretty sure all of the registry keys we write are strings. I'll probably add methods in dutil to get a BUNDLE_VERSION from a QWORD version.

I can expose a CompareVersions method from the engine that takes 2 strings.

I can understand why Burn-adjacent applications would want to be able to parse versions exactly like the Burn engine does, but I don't understand why a standalone application would want "@#$%^&*" to be "successfully"
parsed into a version. Maybe this will help - what would we call this version struct if it was verutil.h? Would it still be BUNDLE_VERSION?

On Thu, Jul 16, 2020 at 5:26 AM Hoover, Jacob via wix-devs < wix-devs at lists.wixtoolset.org> wrote:

> If the version is exposed as a string, then I think we need to expose 
> a compare at a minimum.  Ex: I have a baf that scans for other 
> semi-related bundles, and wants to import settings from the latest 
> version of the related bundle.
>
> I also have my update feed parsing that was done in my custom WixStdBA.
> If I am no longer getting a QWORD, then I need to be able to have the 
> engine compare these black magic strings.
>
> I would also assume the atom feed parsing should then also be updated 
> to handle the new string...  Once Sean gets his changes merged in I 
> can do that part, unless he's close enough to it, that he can tweak 
> it.  Next question is, if it was a QWORD in 3.x and a string in 4, we 
> need to account for handling both old and new values.  (My guess is 
> the engine will have to read the old QWORD and format it as a string?)
>
> From: Rob Mensching [mailto:rob at firegiant.com]
> Sent: Wednesday, July 15, 2020 1:35 PM
> To: WiX Toolset Developer Mailing List <wix-devs at lists.wixtoolset.org>
> Cc: Hoover, Jacob <Jacob.Hoover at greenheck.com>
> Subject: RE: [wix-devs] 6210 - Burn engine should handle non-QWORD 
> versions
>
> No, I was convinced strings are the best thing for Burn to pass around.
> Strings avoid all these struct definition problems *and* provide the 
> BA something it could reasonably display to the user. It's especially 
> useful if we preserve the original string version value from the .wxs code.
>
> > At this point, I'd like to archive the BootstrapperCore repo and 
> > move
> the Burn headers into the Burn repo
>
> This sounds good. As per above, I think we use strings to avoid all 
> the remaining header problems.
>
> > If we're going to be exposing the struct from Burn, then we might as
> well add a ParseBundleVersion method/message and CompareBundleVersions 
> method. Would we add a CompareBundleVersionStrings method?
>
> I'm not 100% up to speed whether a "CompareVersion" method needs to be 
> exposed by Burn but if you find it is needed, I think it should take 
> strings (as per above). I also wouldn't add the word "Bundle" in there 
> because... see next point.
>
> > With regards to putting this in "verutil.h", I've been treating this
> proposal as teaching the Burn engine how to handle the crazy strings 
> that can be thrown at it. I would think a more general purpose library 
> would have its own spec on what a valid version is and reject strings 
> that aren't valid. So I think the data structure and comparison logic 
> make sense outside of Burn, but not the parser.
>
> So far, the list of " crazy strings that can be thrown at [Burn]" 
> match the crazy strings Id' like to be able to handle globally. 
> Additionally, the rules described match the rules I imagined globally. 
> So, if all processing (parsing/comparision/conversion from struct to string) lived in a "verutil"
> I'm pretty confident it would be globally useful (especially based on 
> what I saw in your butil.h PR). I understand if you only started with 
> the verutil functions required by Burn (but I'm guessing Burn will 
> need the same list global support needs based on what I saw in your 
> butil.h PR), but I hope you'd consider putting them in verutil first. 
> I do believe you've got the global solution already (delta bugs or 
> small tweaks when v4 finally interacts with reality).
>
>
> -----Original Message-----
> From: wix-devs <wix-devs-bounces at lists.wixtoolset.org<mailto:
> wix-devs-bounces at lists.wixtoolset.org>> On Behalf Of Hoover, Jacob via 
> wix-devs
> Sent: Wednesday, July 15, 2020 7:47 AM
> To: WiX Toolset Developer Mailing List <wix-devs at lists.wixtoolset.org 
> <mailto:wix-devs at lists.wixtoolset.org>>
> Cc: Hoover, Jacob <Jacob.Hoover at greenheck.com<mailto:
> Jacob.Hoover at greenheck.com>>
> Subject: Re: [wix-devs] 6210 - Burn engine should handle non-QWORD 
> versions
>
> Pointers to the struct, unless the struct is a copy, is bad.
>
> Are we trying too hard to do crazy things? What if we exposed version 
> data as a void* and cb DWORD, and allowed BA's to define their own 
> VerParse and VerCompare functions?
>
> From: wix-devs [mailto:wix-devs-bounces at lists.wixtoolset.org] On 
> Behalf Of Bob Arnson via wix-devs
> Sent: Tuesday, July 14, 2020 7:52 PM
> To: WiX Toolset Developer Mailing List <wix-devs at lists.wixtoolset.org 
> <mailto:wix-devs at lists.wixtoolset.org>>
> Cc: Bob Arnson <bob at firegiant.com<mailto:bob at firegiant.com>>
> Subject: Re: [wix-devs] 6210 - Burn engine should handle non-QWORD 
> versions
>
> I'd say there's value in being able to parse versions exactly like 
> Burn without being a BA. Maybe butil instead?
>
> -----Original Message-----
> From: wix-devs <wix-devs-bounces at lists.wixtoolset.org<mailto:
> wix-devs-bounces at lists.wixtoolset.org<mailto:
> wix-devs-bounces at lists.wixtoolset.org%
> 3cmailto:wix-devs-bounces at lists.wixtoolset.org>>> On Behalf Of Sean 
> Hall via wix-devs
> Sent: Tuesday, 14 July, 2020 20:24
> To: WiX Toolset Developer Mailing List <wix-devs at lists.wixtoolset.org 
> <mailto:wix-devs at lists.wixtoolset.org<mailto:wix-devs at lists.wixtoolset
> .org %3cmailto:wix-devs at lists.wixtoolset.org>>>
> Cc: Sean Hall <r.sean.hall at gmail.com<mailto:r.sean.hall at gmail.com<mailto:
> r.sean.hall at gmail.com%3cmailto:r.sean.hall at gmail.com>>>
> Subject: Re: [wix-devs] 6210 - Burn engine should handle non-QWORD 
> versions
>
> Your comment about any change to the actual version comparisons being 
> a breaking change is making me wonder why we would ever make a 
> breaking change to the underlying BUNDLE_VERSION struct. I can't think 
> of any such changes, so maybe we should just pass around pointers to 
> the struct instead of a string.
>
> In order to do that, we'll need to reference the BUNDLE_VERSION struct 
> in the Burn headers (e.g. BootstrapperApplication.h) which live in the 
> BootstrapperCore repo today. Those Burn headers are separate from the 
> balutil headers (e.g. IBootstrapperApplication.h) in the balutil repo. 
> How do we want to do this? Define them in both dutil and Burn (but 
> conditionally so they don't conflict)? Define them in one and create a 
> dependency to it in the other?
>
> Originally BootstrapperCore had the Burn headers and what was 
> BootstrapperCore.dll in v3. I eventually split up 
> BootstrapperCore.dll, putting most of it in the balutil repo and the 
> host part of it into Bal.wixext. At this point, I'd like to archive 
> the BootstrapperCore repo and move the Burn headers into the Burn 
> repo. This would mean we couldn't define BUNDLE_VERSION solely in the 
> Burn headers without creating a cyclic dependency between Burn and dutil.
>
> If we're going to be exposing the struct from Burn, then we might as 
> well add a ParseBundleVersion method/message and CompareBundleVersions method.
> Would we add a CompareBundleVersionStrings method?
>
> With regards to putting this in "verutil.h", I've been treating this 
> proposal as teaching the Burn engine how to handle the crazy strings 
> that can be thrown at it. I would think a more general purpose library 
> would have its own spec on what a valid version is and reject strings 
> that aren't valid. So I think the data structure and comparison logic 
> make sense outside of Burn, but not the parser.
>
____________________________________________________________________
WiX Toolset Developer Mailing List provided by FireGiant http://www.firegiant.com/