Re: call for testing of latest setup.exe snapshot

[Igor Peshansky <pechtcha at cs dot nyu dot edu>]

On Tue, 8 Aug 2006, Igor Peshansky wrote:

> On Sun, 6 Aug 2006, Brian Dessent wrote:
>
> [snip]
> > http://cygwin.com/ml/cygwin-apps/2006-02/msg00099.html
> >   So, we have a couple of issues here.  Firstly, the bug/unintended
> >   feature of -r causing the infinite retries until the file can be
> >   written (if I understand correctly.)  Second the patch by Igor that
> >   adds a dialog when trying to replace an in-use file.
>
> Right.
>
> >   Here is my opinion on the matter: I like the dialog idea, but I don't
> >   think having "Abort" as an option is appropriate, as it will
> >   potentially cause a really screwed up install, plus it was left
> >   unimplemented in the patch submitted.  So let's just have two
> >   options: "Retry" and "Replace on Reboot".  I know that this means we
> >   can't use the stock "Abort/Retry/Ignore" dialog but I think it's
> >   worth it for clarity.
>
> Agreed on all points.  However, there is a technical issue here.  Stock
> MessageBoxes come in many flavors -- there actually is a Retry/Cancel box.
> There is no Retry/Continue stock box, unfortunately.
>
> We can use the Retry/Cancel one, and perhaps play some games with the
> WNDPROC of the MessageBox class to make "Cancel" look like "Replace on
> reboot" (or "Continue", which I like better -- we can explain in the
> MessageBox text that pressing "Continue" will require a reboot later), but
> I'm not sure it'll work, and it'll be ugly.  Plus, I don't know that much
> about the WNDPROC, so it'll take me a bit of time to get something like
> this working.
>
> OTOH, I can change my patch to use the Retry/Cancel box today, and add the
> following to the text: "Pressing 'Cancel' will cause setup to use Windows
> mechanisms for replacing in-use files.  It will be necessary for you to
> reboot after setup completes."  I know, the label "Cancel" is evocative of
> aborting the whole installation, but this functionality is so useful, IMO,
> that I, for one, would put up with a little annoyance of a wrong label.
> Changing the label in a way I've described above could be a later
> enhancement.

Well, lo and behold, I overreacted.  It turned out to be much easier than
I anticipated, so attached is the patch with the Retry/Continue message
box.

> > http://cygwin.com/ml/cygwin-apps/2006-01/msg00204.html
> [snip]
> However, there was another issue in that thread (the inline patch).  It
> seems that applying that will cause the code to be simpler, but I'm afraid
> there's some little issue I'm missing.

I still have that one in my private sandbox, and had it in my running copy
of setup for a while with no observed problems.  Any opinions?

> > If we can finish off the "Retry/Replace" file-in-use thing and assuming
> > there are no reports of new issues with this snapshot then I think we
> > can push out a release.
>
> Sounds good.  If people are fine with the Retry/Cancel box, I can have a
> new patch by the end of this week.

So my weeks end on Wednesdays... :-)  Since this change involved indenting
an 80-line chunk of code, I'm also attaching a whitespace-indifferent
patch for ease of reviewing.  ChangeLog is below.
	Igor
==============================================================================
2006-08-16  Igor Peshansky  <pechtcha@cs.nyu.edu>

	* install.cc (Installer::installOne): If file is in use, ask the user
	to stop processes and retry.
	(MB_RETRYCONTINUE, IDCONTINUE): New macros.
	(hMsgBoxHook): New static field.
	(CBTProc): New window hook function.
	(_custom_MessageBox): New function.
	* CHANGES: Update with the above.

-- 
				http://cs.nyu.edu/~pechtcha/
      |\      _,,,---,,_	    pechtcha@cs.nyu.edu | igor@watson.ibm.com
ZZZzz /,`.-'`'    -.  ;-;;,_		Igor Peshansky, Ph.D. (name changed!)
     |,4-  ) )-,_. ,\ (  `'-'		old name: Igor Pechtchanski
    '---''(_/--'  `-'\_) fL	a.k.a JaguaR-R-R-r-r-r-.-.-.  Meow!

"Las! je suis sot... -Mais non, tu ne l'es pas, puisque tu t'en rends compte."
"But no -- you are no fool; you call yourself a fool, there's proof enough in
that!" -- Rostand, "Cyrano de Bergerac"

Attachment: setup-retry-replace.patch
Description: patch to apply

Attachment: setup-retry-replace.patch.-bB
Description: patch to review