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

Tue Jul 14 18:25:40 PDT 2020

Hmmm...OK, that's making me lean back toward strings, tbh...

-----Original Message-----
From: wix-devs <wix-devs-bounces at lists.wixtoolset.org> On Behalf Of Sean Hall via wix-devs
Sent: Tuesday, 14 July, 2020 20:58
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

Yes, the implementation needs to stay somewhere in dutil. But a BA has never been required to use dutil headers so I'm wondering where to define the BUNDLE_VERSION struct.

On Wed, Jul 15, 2020 at 10:52 AM Bob Arnson <bob at firegiant.com> wrote:

> 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> 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>
> Cc: Sean Hall <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> 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> 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>
> > Cc: Rob Mensching <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> 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>
> > Cc: Sean Hall <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.
> >
> > 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> 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-ve
> > rs
> > ioning/
> > .
> > > I would like feedback on this.
> > >
> > ____________________________________________________________________
> > WiX Toolset Developer Mailing List provided by FireGiant 
> > http://www.firegiant.com/ 
> > ____________________________________________________________________
> > WiX Toolset Developer Mailing List provided by FireGiant 
> > http://www.firegiant.com/ 
> > ____________________________________________________________________
> > WiX Toolset Developer Mailing List provided by FireGiant 
> > http://www.firegiant.com/
> >
> ____________________________________________________________________
> WiX Toolset Developer Mailing List provided by FireGiant 
> http://www.firegiant.com/
>
____________________________________________________________________
WiX Toolset Developer Mailing List provided by FireGiant http://www.firegiant.com/