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

[Sean Hall]

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.
>