[wix-devs] Burn Variable breaking changes

Wed Jul 8 05:53:20 PDT 2020

I created a WIP for this at
https://wixtoolset.org/development/wips/4863-thmutil-literal-variables/. We
need to decide on the names and the default.

On Thu, Jul 2, 2020 at 9:04 AM Sean Hall <r.sean.hall at gmail.com> wrote:

> If we're going with the value being literal instead of the variable being
> literal, and the engine is always formatting variables when using them,
> then there's actually no difference between a literal string and formatted
> string that has been escaped. So behaviors 1 and 2 can be combined -
> thmutil always sets values from an editbox as string literals and always
> formats the variable before populating an editbox. I don't think we need to
> support anything else. Bob gets his wish of no new configuration in the
> theme. And no need to expose what the underlying type of the variant is.
>
> I don't know whether "literal" or "formatted" should be the default type
> for a string. I don't think there's a compelling enough reason to change
> the default, so I think we should add a new type of "literal" (although I'm
> not sure about dropping the "string" part from "literalString").
>
> On Thu, Jul 2, 2020 at 3:56 AM Rob Mensching <rob at firegiant.com> wrote:
>
>> (Focusing only on 4863)
>>
>> Using Variable/@Type="literal" is interesting. I wonder if we should
>> change the default for "string" to actually mean "literal string" and
>> introduce Type="formatted" instead? I don't know what kind of can of worms
>> that opens (but it would be wix convertible so an interesting idea).
>>
>> I wonder if the answer is that thmutil always *sets* values as string
>> literals but when it reads the value as formatted or not based on the
>> Variable/@Type. This would remove the ability for the end user to enter
>> "[ProgramFilesFolder]IKnowBurnWillProcessThis" via thmutil UI to have it
>> expanded. But I'm not sure that's a real scenario. Of course, custom BA
>> code would still be able to set variables literal and formatted.
>>
>> Thoughts?
>>
>>
>> -----Original Message-----
>> From: wix-devs <wix-devs-bounces at lists.wixtoolset.org> On Behalf Of Sean
>> Hall via wix-devs
>> Sent: Saturday, June 27, 2020 9:53 PM
>> 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] Burn Variable breaking changes
>>
>> There's a few scenarios for 4863.
>>
>> 1. Path variable. This one's straightforward, the editbox should be
>> populated with the literal value of the target variable. When saving, the
>> variable needs to have the contents of the editbox as a literal value.
>>
>> 2. Random string - user can't use Variables. The editbox should be
>> populated with the formatted value of the target variable. When saving, the
>> variable needs to have the escaped contents of the editbox as the value
>> ("abc[def]" needs to be saved as "abc[\[]def[\]]").
>>
>> 3. Random string - user can use Variables. The editbox should be
>> populated with... I don't know. Should it be the formatted or unformatted
>> value of the target variable? When saving, the variable needs to have the
>> unescaped contents of the editbox as the value.
>>
>> 4. Current v3 behavior. The editbox is populated with the unformatted
>> value of the target variable. The contents of the editbox are saved
>> unescaped.
>>
>> Even if we don't want to support some of these, I don't know how we get
>> around thmutil needing to know whether to alter the value before saving it.
>> In that case, I would rather not have Variable/@Literal and instead add a
>> new variant type of BURN_VARIANT_TYPE_LITERAL_STRING. I feel like it's more
>> correct in saying that a string value is literal, instead of declaring a
>> Variable to be literal.
>>
>> On Sat, Jun 27, 2020 at 4:22 AM Bob Arnson <bob at firegiant.com> wrote:
>>
>> > I can't speak to managed BAs but in general, it seems like the BA
>> > would know about the variables it's querying or, if not, wouldn't care
>> > about the underlying type.
>> >
>> > -----Original Message-----
>> > From: wix-devs <wix-devs-bounces at lists.wixtoolset.org> On Behalf Of
>> > Sean Hall via wix-devs
>> > Sent: Thursday, 25 June, 2020 19:18
>> > 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] Burn Variable breaking changes
>> >
>> > Does Variable/@Literal just work? I'd have to think about the
>> implications.
>> > There'd be a bit on the Variable saying it's Literal, and there's also
>> > the existing bit on the Variant saying the value's Literal. I'm not
>> > sure if there needs to be support for asking whether the variable is a
>> > literal string. Which is why I brought 5319 into this discussion. If
>> > we don't need or want to add the ability for the BA to query the
>> > current type of a variable, then I'll probably get rid of the separate
>> collections.
>> >
>> > On Fri, Jun 26, 2020 at 2:16 AM Bob Arnson <bob at firegiant.com> wrote:
>> >
>> > > On 4863, I'm a little leery of trying to make thmutil too smart; if
>> > > the bundle authoring can express the intent
>> > > (Variable/@Literal="yes"?), then I'd say that's probably the better
>> > option.
>> > >
>> > > On 5319, I don't know enough about managed BAs to say much, but I've
>> > > always found it weird that the different variable types are exposed
>> > > in different collections, even if it makes sense when using them.
>> > >
>> > > -----Original Message-----
>> > > From: wix-devs <wix-devs-bounces at lists.wixtoolset.org> On Behalf Of
>> > > Sean Hall via wix-devs
>> > > Sent: Thursday, 25 June, 2020 07:47
>> > > To: WiX Toolset Developer Mailing List
>> > > <wix-devs at lists.wixtoolset.org>
>> > > Cc: Sean Hall <r.sean.hall at gmail.com>
>> > > Subject: [wix-devs] Burn Variable breaking changes
>> > >
>> > > I'd like to design the following issues so I know which ones require
>> > > a breaking change:
>> > >
>> > > https://github.com/wixtoolset/issues/issues/4763
>> > > https://github.com/wixtoolset/issues/issues/4863
>> > > https://github.com/wixtoolset/issues/issues/5319
>> > >
>> > > 4763 added the ability for Burn to set a variable as a literal string.
>> > > I think it's pretty straightforward to expose
>> > > VariableSetLiteralString to the BA. Doesn't seem to be a breaking
>> change.
>> > >
>> > > 4863 is about wixstdba (through thmutil) allowing an editbox to be
>> > > either a literal string or a formatted string. If it's a literal
>> > > string, then the value should not be expanded when populating the
>> > > editbox. If it's a formatted string, then the value should be
>> > > expanded when populating the editbox. Should thmutil ask for a
>> > > SetLiteralStringVariable callback and the editbox is configured as
>> > > literal/formatted in the theme? Or can a variable be authored as a
>> > > literal string (so thmutil and wixstdba won't be aware that they're
>> > > setting a literal string, the Burn engine will just handle it)?
>> > > Making thmutil format the variable when populating the editbox is a
>> > > breaking
>> > change, but adding the literal support doesn't seem to be breaking.
>> > >
>> > > 5319 is about the managed BootstrapperEngine implementation where it
>> > > exposes StringVariables, NumericVariables, and VersionVariables. The
>> > > complaint is that calling
>> > > NumericVariables.Contains("StringVariableA")
>> > > returns true even though the variable is actually a string and not a
>> > > number. Do we remove the Contains method from all 3 and provide a
>> > > single Contains method so it doesn't imply that the variable is of
>> > > the requested type? Or does Burn provide a way to expose what the
>> > > current type of a variable is? Either way sounds like a breaking
>> change.
>> > > ____________________________________________________________________
>> > > 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/
>>
>