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

Wed Jul 15 07:46:46 PDT 2020

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.wixtoolset.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.wixtoolset.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-vers>
> ioning/
> .
> > I would like feedback on this.
> >
> ____________________________________________________________________
> WiX Toolset Developer Mailing List provided by FireGiant
> http://www.firegiant.com/<http://www.firegiant.com>
> ____________________________________________________________________
> WiX Toolset Developer Mailing List provided by FireGiant
> http://www.firegiant.com/<http://www.firegiant.com/>
> ____________________________________________________________________
> WiX Toolset Developer Mailing List provided by FireGiant
> http://www.firegiant.com/<http://www.firegiant.com/>
>
____________________________________________________________________
WiX Toolset Developer Mailing List provided by FireGiant http://www.firegiant.com/<http://www.firegiant.com/>
____________________________________________________________________
WiX Toolset Developer Mailing List provided by FireGiant http://www.firegiant.com/<http://www.firegiant.com/>
NOTE: This email was received from an external source. Please use caution when opening links or attachments in the message.