[wix-devs] 5950 - Rollback ExePackage on cancel

[Rob Mensching]

> You are not recalling correctly. Burn doesn't use UninstallArguments for anything other than building the uninstall command line.

Well, that is simpler and it does align with the original Burn design of doing as little calculation during execution as possible. I should have guessed that first.


> Which means requiring a DetectCondition when UninstallArguments are provided doesn't help here.

Indeed. I think it also says a DetectCondition is necessary unless the ExePackage is Permanent. I should probably update the compiler logic to that instead of keying off UninstallArguments. I'll do that.


> The bug for 5950 is specifically that the rollback action isn't executed for an ExePackage when it fails.

Yes, understood. Specifically, my concern is uninstalling an ExePackage during the rollback (of an install execution) after it was not successfully uninstalled. Doing so means that ExePackage uninstalls are a no-op when they are not installed. We know that uninstall of MsiPackage's that are not installed is a no-op (explicit error code, ERROR_UNKNOWN_PRODUCT, IIRC). 

But maybe it just isn't a real issue. Maybe we just expect non-installed ExePackage's uninstall to be a no-op and fix 5950 to make everything behave like MsiPackage.


-----Original Message-----
From: wix-devs <wix-devs-bounces at lists.wixtoolset.org> On Behalf Of Sean Hall via wix-devs
Sent: Sunday, January 17, 2021 1:10 PM
To: WiX Toolset Developer Mailing List <wix-devs at lists.wixtoolset.org>
Cc: Sean Hall <r.sean.hall at gmail.com>
Subject: [wix-devs] 5950 - Rollback ExePackage on cancel

I changed the title since I originally only mentioned 5950 to provide context for my questions about 6297.

>  IIRC, no uninstall arguments means ExePackage is not uninstallable

You are not recalling correctly. Burn doesn't use UninstallArguments for anything other than building the uninstall command line. The documentation even calls out: "If this attribute is absent the executable will be launched with no command-line arguments".

Which means requiring a DetectCondition when UninstallArguments are provided doesn't help here. This is actually a bug today, Burn never should have tried to schedule a rollback action of uninstall if there was no DetectCondition.

> I don't have a good solution to this problem and I'm still wary of
running uninstall on a ExePackage if it wasn't detected.

The bug for 5950 is specifically that the rollback action isn't executed for an ExePackage when it fails. The rollback action would be executed if a later package failed (assuming no RollbackBoundary between them). Are you suggesting that we never schedule rollback actions for ExePackages? Or just in the case where the execute action is Install and the rollback action is Uninstall? I'll mention it again, MsiPackage always schedules the rollback action such that it will be executed during rollback even if the package had failed. It does this for execute=Uninstall and rollback=Install, and all other combinations of actions.

On Sun, Jan 17, 2021 at 2:40 PM Rob Mensching <rob at firegiant.com> wrote:

> > good idea to re-detect during Apply because we would have to do a
> complete Detect cycle to run all the searches and the BA can do its 
> thing in response to the various detect BA messages.
>
> That's a good point. I overlooked how complex a detection chain could get.
> I don't have a good solution to this problem and I'm still wary of 
> running uninstall on a ExePackage if it wasn't detected. Maybe that 
> means ExePackage shouldn't behave like MsiPackage during rollback?
>
>
> > ExePackage should require a detect condition (and be non-permanent) 
> > in
> order to schedule a rollback action of uninstall
>
> Very recent change (like last week), compiler now requires a 
> DetectCondition when UninstallArguments are provided (fixed an open bug).
> So, this should always be satisfied.
>
>
> > It's possible that the ExePackage knows to uninstall with no arguments.
>
> I don't believe this is supported today. IIRC, no uninstall arguments 
> means ExePackage is not uninstallable (arguably the same thing as 
> ExePackage/@Permanent='yes').
>
>
> -----Original Message-----
> From: wix-devs <wix-devs-bounces at lists.wixtoolset.org> On Behalf Of 
> Sean Hall via wix-devs
> Sent: Thursday, January 14, 2021 7:27 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] Rollback quirks
>
> > It is probably a bit more complicated with ExePackages since they 
> > would
> need to re-detect to see if they were successfully installed before 
> trying to launch their uninstall command. An ExePackage's uninstall 
> behavior is too undefined to simply launch uninstall without knowing 
> that the exe was installed.
>
> Actually, I'm not sure it's a good idea to re-detect during Apply 
> because we would have to do a complete Detect cycle to run all the 
> searches and the BA can do its thing in response to the various detect BA messages.
>
> I think I understand what you're saying about ExePackages, if the 
> ExePackage didn't even have a detect condition then it wouldn't 
> normally uninstall. I think that means that an ExePackage should 
> require a detect condition (and be non-permanent) in order to schedule 
> a rollback action of uninstall. I'm not sure about basing it off of 
> UninstallArguments, can Burn even tell the difference today between 
> UninstallArguments that were authored as empty vs not specified? It's 
> possible that the ExePackage knows to uninstall with no arguments.
>
> On Wed, Jan 13, 2021 at 2:23 PM Sean Hall <r.sean.hall at gmail.com> wrote:
>
> > > If it is helpful, I agree in 5950 it would be ideal if all 
> > > packages
> > rolled back the way MSI packages do now--if the package can be 
> > uninstalled
> >
> > I do think the fix for 5950 is to make it consistent for all packages.
> > Are you aware that it's done this way for all operations, not just 
> > execute=install and rollback=uninstall?
> >
> > > It is probably a bit more complicated with ExePackages since they 
> > > would
> > need to re-detect
> >
> > If we do make the behavior consistent, I'm starting to wonder if we 
> > should do that for all package types to see whether or not to 
> > actually do the planned rollback action. In which case, why not just 
> > re-detect as the way to track whether it's installed?
> >
> > >> But what happens if the rollback also fails?
> >
> > You skipped the question in step 3, but your response makes it sound 
> > like you think the tracking should assume that the execute action 
> > was successful even if the exit code indicated failure. If we're 
> > making that assumption during execute, then yes we should make the 
> > same
> assumption during rollback.
> >
> > > I'd have to defer to the current behavior of Burn with
> > DisableRollback="yes". Does failure during install of a latter 
> > package in the chain with disabled rollback keep the Bundle 
> > registration or
> remove it?
> >
> > The current ARP behavior is useless to me. But if you want to know, 
> > in
> > v3 with the following scenario the registration is kept:
> >
> > 0. Bundle was not already installed, executing Install action.
> > 1. non-permanent MsiPackageB is executed for install.
> > 2. MsiPackageB returns success.
> > 3. vital MsiPackageC is executed for install.
> > 4. MsiPackageC returns failure.
> > 5. Rollback is disabled so execution ends immediately.
> >
> > That's not the scenario I was interested in, though. I was wondering 
> > about what happens if MsiPackageB failed.
> >
> > > Aside: IIRC, Burn in rollback mode ignores cancel requests
> >
> > Maybe my reason for rollback failing is a bad example. Just imagine 
> > something else causing the same outcome, which could be much easier 
> > with ExePackages and brittle DetectConditions. (Also, v4 allows 
> > showing UI for MSI during uninstall).
>