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

Wed Jul 15 11:34:50 PDT 2020

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> 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>
Cc: Hoover, Jacob <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>
Cc: Bob Arnson <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>> 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>>
Cc: Sean Hall <r.sean.hall at gmail.com<mailto: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.

On Wed, Jul 15, 2020 at 5:41 AM Rob Mensching via wix-devs < wix-devs at lists.wixtoolset.org<mailto:wix-devs at lists.wixtoolset.org>> wrote:

> Looking closer at the PR, the more it seems that what we have 
> discussed and Sean has coded, is a very good marriage of "4-part 
> versions" with "SemVer".
>
> Is there any reason we shouldn't promote this versioning to 
> "verutil.h" in dutil and make it the standard handling of versions going forward?
>
> -----Original Message-----
> From: wix-devs 
> <wix-devs-bounces at lists.wixtoolset.org<mailto:wix-devs-bounces at lists.w
> ixtoolset.org>> On Behalf Of Rob Mensching via wix-devs
> Sent: Tuesday, July 14, 2020 12:36 PM
> To: WiX Toolset Developer Mailing List 
> <wix-devs at lists.wixtoolset.org<mailto:wix-devs at lists.wixtoolset.org>>
> Cc: Rob Mensching <rob at firegiant.com<mailto:rob at firegiant.com>>
> Subject: Re: [wix-devs] 6210 - Burn engine should handle non-QWORD 
> versions
>
> Correct me if I'm wrong but using EvaluateCondition() would require 
> all version comparisons to be created using string concatenation in 
> the BA then call to evaluate condition? That's pretty ugly.
>
> Dutil isn't easily discoverable but there is precedent as wcautil 
> depends on it. Balutil could do similarly.
>
> I guess I'm not against sending versions around as strings but I think 
> we do need to make some methods available for doing standard version 
> comparisons (anything else) on those strings.
>
> Is the breaking changes concern related to shipping around a struct?
> If so, I get that (thus string fallback is better than an opaque 
> handle because at least someone can display a string). But the actual 
> version comparisons, we'll have to get right because changing those 
> will be breaking change whether a string or struct or whatever.
>
>
> -----Original Message-----
> From: wix-devs 
> <wix-devs-bounces at lists.wixtoolset.org<mailto:wix-devs-bounces at lists.w
> ixtoolset.org>> On Behalf Of Sean Hall via wix-devs
> Sent: Monday, July 13, 2020 3:08 AM
> To: WiX Toolset Developer Mailing List 
> <wix-devs at lists.wixtoolset.org<mailto:wix-devs at lists.wixtoolset.org>>
> Cc: Sean Hall <r.sean.hall at gmail.com<mailto:r.sean.hall at gmail.com>>
> Subject: Re: [wix-devs] 6210 - Burn engine should handle non-QWORD 
> versions
>
> I updated the WIP based on what we discussed in last week's meeting.
> I've got the implementation of parsing and comparing the versions in 
> butil in https://github.com/wixtoolset/dutil/pull/16<https://github.com/wixtoolset/dutil/pull/16>.
>
> I don't really want to expose an API from Burn to parse a version.
> Hiding the implementation details allows us to make breaking changes.
> Also, there's no relationship between Burn's headers and dutil's 
> headers so it would be tricky to do.
>
> Burn already exposes an API to compare versions - EvaluateCondition.
> So I think the only Burn API changes will be changing QWORD to LPWSTR.
>
> On Wed, Jul 8, 2020 at 10:59 PM Sean Hall <r.sean.hall at gmail.com<mailto:r.sean.hall at gmail.com>> wrote:
>
> > I went ahead and created a separate issue for Burn supporting 
> > non-QWORD versions, and 4666 can be about authoring them. I created 
> > a WIP for this at
> https://wixtoolset.org/development/wips/6210-burn-engine-semantic-vers
> <https://wixtoolset.org/development/wips/6210-burn-engine-semantic-ver
> s>
> ioning/
> .
> > I would like feedback on this.
> >