[wix-devs] OnExecutePackageComplete without OnExecutePackageBegin

Sean Hall r.sean.hall at gmail.com
Thu Mar 17 10:27:51 PDT 2016


I believe there is already precedence in burn to have a boolean initially
set to false, then set to true once something happened.  In LExit, it then
performs additional work if it was set to true.

On Thu, Mar 17, 2016 at 12:19 PM, Hoover, Jacob <Jacob.Hoover at greenheck.com>
wrote:

> Putting that in is bad, but if we HAVE to do it...  I would envision an
> ExitEarlyWithSuccess macro. The real issue is going to be not jumping to
> LExit where all the cleanup is guaranteed to happen.
>
> -----Original Message-----
> From: wix-devs [mailto:wix-devs-bounces at lists.wixtoolset.org] On Behalf
> Of Heath Stewart
> Sent: Thursday, March 17, 2016 12:11 PM
> To: WiX Toolset Developer Mailing List <wix-devs at lists.wixtoolset.org>
> Subject: Re: [wix-devs] OnExecutePackageComplete without
> OnExecutePackageBegin
>
> So how do people - Rob and Bob in particular - feel about an early return
> (i.e. "return S_OK") in this case? Since this may, in fact, set a precedent
> I'd like to know what's acceptable and I can code up a fix.
>
> Thanks,
>
> Heath Stewart
> Visual Studio, Microsoft
> http://blogs.msdn.com/heaths
>
>
> -----Original Message-----
> From: wix-devs [mailto:wix-devs-bounces at lists.wixtoolset.org] On Behalf
> Of Heath Stewart
> Sent: Wednesday, March 16, 2016 1:45 PM
> To: WiX Toolset Developer Mailing List <wix-devs at lists.wixtoolset.org>
> Subject: Re: [wix-devs] OnExecutePackageComplete without
> OnExecutePackageBegin
>
> OnCache* is fine. I was using that as an example of what is, IMO, coded
> correctly. But when package execution begins it checks the cache result and
> bails before telling the BA package execution started (which is actually in
> each engine's execute functions), but the general LExit block calls
> OnExecutePackageComplete. That's all in the code.
>
> From Rob, I'll file a bug and get a PR out, but proposing a true early
> termination via return.
>
> Here's the gist of the code:
>
> Execute*Package(...) {
>   if (pPackage->hrCacheResult) {
>     Log();
>     ExitFunction1(hr = S_OK); // propose to just return S_OK;
>   }
>   ba->OnPackageExecuteBegin(...);
>   // other stuff, all which often uses Exit*() macros
> LExit:
>   hr = ExecutePackageComplete(...); // calls ba->OnPackageExecuteComplete
>   return hr;
> }
>
> It makes sense we bail from execute() if caching failed (no package on
> which to execute, in most cases anyway) but execute() was already
> scheduled. The runtime couldn't have known (practically) that caching would
> fail, so the extra check makes sense. But calling the ExitFunction macro
> ends up calling OnExecutePackageComplete, so we either return early or
> maybe skip to another label like LExit2 after LExit, in which case any
> future general cleanup could be done. I don't see any precedent so curious
> what others think.
>
> Heath Stewart
> Visual Studio, Microsoft
>
> https://na01.safelinks.protection.outlook.com/?url=http%3a%2f%2fblogs.msdn.com%2fheaths&data=01%7c01%7cHeath.Stewart%40microsoft.com%7cf1621d6cadb1448b3ad308d34ddbcd82%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=JBWCYu5g%2blw0tqTjz6qZDqQdDeTh3j21if%2bQ95AT8XQ%3d
>
> -----Original Message-----
> From: wix-devs [mailto:wix-devs-bounces at lists.wixtoolset.org] On Behalf
> Of Sean Hall
> Sent: Wednesday, March 16, 2016 10:10 AM
> To: WiX Toolset Developer Mailing List <wix-devs at lists.wixtoolset.org>
> Subject: Re: [wix-devs] OnExecutePackageComplete without
> OnExecutePackageBegin
>
> I think every Begin callback should always have a corresponding Complete
> callback, even if an error occurred.  A Complete callback should never be
> sent unless the corresponding Begin callback was sent, if Burn is doing
> this then I think that's a bug.
>
> I'd have to look at code for the OnCacheBegin/OnCacheComplete question, if
> the whole cache phase is skipped I don't think they should be sent.
>
> On Wed, Mar 16, 2016 at 12:03 PM, Heath Stewart <
> Heath.Stewart at microsoft.com
> > wrote:
>
> > We ran into a scenario where a caching error prevented
> > OnExecutePackageBegin from executing (basically, in apply.cpp, the
> > check for the caching HRESULT happens just before calling
> > OnExecutePackageBegin on the BA) but OnExecutePackageComplete was
> > called (it's the first line after the LExit label for each
> > package-specific execute function in apply.cpp. We can work around
> > this (just keep track of what packages were started and check that set
> > in the completion callback), but begs the question whether in burn we
> > should call a Complete callback without a corresponding Begin
> > callback. It's not common to see that sort of behavior (thinking about
> async APIs in various frameworks I've used).
> >
> > What is more consistent with typical async behavior - and allows a BA
> > to clean anything specific to the downloading/caching of a package
> > still - is that ApplyCache does call OnCacheBegin invariably (i.e. no
> > exits before said call) and OnCacheComplete in the LExit (again with
> > no early exits) so they are always a pair.
> >
> > So should this maybe changed for wix4?
> >
> > Heath Stewart
> > Visual Studio, Microsoft
> > https://na01.safelinks.protection.outlook.com/?url=http%3a%2f%2fblogs.
> > msdn.com%2fheaths&data=01%7c01%7cHeath.Stewart%40microsoft.com%7cebe5c
> > 84c7e9141ecfa8308d34dbdd44d%7c72f988bf86f141af91ab2d7cd011db47%7c1&sda
> > ta=E%2bPWtOXEyuBU3%2bx7%2fZbPH50OTH1hwEOMy1WfFptWFek%3d
> >
> > ____________________________________________________________________
> > WiX Toolset Developer Mailing List provided by FireGiant
> > https://na01.safelinks.protection.outlook.com/?url=http%3a%2f%2fwww.fi
> > regiant.com%2f&data=01%7c01%7cHeath.Stewart%40microsoft.com%7cebe5c84c
> > 7e9141ecfa8308d34dbdd44d%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=
> > AYTU4NnSFIqWFLIZvQqVPQPcma7lhgwNr6unMJBA0XI%3d
> >
> ____________________________________________________________________
> WiX Toolset Developer Mailing List provided by FireGiant
> https://na01.safelinks.protection.outlook.com/?url=http%3a%2f%2fwww.firegiant.com%2f&data=01%7c01%7cHeath.Stewart%40microsoft.com%7cebe5c84c7e9141ecfa8308d34dbdd44d%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=AYTU4NnSFIqWFLIZvQqVPQPcma7lhgwNr6unMJBA0XI%3d
> ____________________________________________________________________
> WiX Toolset Developer Mailing List provided by FireGiant
> https://na01.safelinks.protection.outlook.com/?url=http%3a%2f%2fwww.firegiant.com%2f&data=01%7c01%7cHeath.Stewart%40microsoft.com%7cf1621d6cadb1448b3ad308d34ddbcd82%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=znjVw8QcKUThgGCvUdXkqfvUAO4fL3RNk4iZ6DGzbV0%3d
> ____________________________________________________________________
> WiX Toolset Developer Mailing List provided by FireGiant
> http://www.firegiant.com/
> ____________________________________________________________________
> WiX Toolset Developer Mailing List provided by FireGiant
> http://www.firegiant.com/
>


More information about the wix-devs mailing list