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

[Sean Hall]

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-vers
> > 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/
>