On Tue, Jul 23, 2013 at 11:41:26PM +0100, Jon TURNEY wrote: >On 07/02/2013 05:06, Christopher Faylor wrote: >> On Wed, Feb 06, 2013 at 08:03:03PM +0000, Jon TURNEY wrote: >>> - Enumerate processes preventing a file from being written >>> - Replace the MessageBox reporting an in-use file with a DialogBox >>> reporting the >>> in-use file and the processes which are using that file >>> - Offer to kill processes which have files open, trying /usr/bin/kill with >>> SIGTERM, >>> then SIGKILL, then TerminateProcess >>> >>> v2: >>> - Use TerminateProcess directly, rather than using kill -f, so we will work >>> even >>> if kill doesn't >>> - Fix formatting: spaces before parentheses for functions and macros >> >> Thanks for doing this. I know that the setup source code doesn't follow this >> rule consistently. >> >> Please check in. This looks like a really nice change. > >Unfortunately, this change seems to have a few problems (See [1]): If we >can't enumerate the processes which are preventing a file from being written, >or some other error occurs which archive::extract_file() doesn't know how to >report (e.g. disk full), the user is presented with the dialog with an empty >process list, and it's unclear how they should proceed (Ideally finding the >processes and stopping them themselves, as before...) > >(In testing, on my W7 x86_64 system, it seems that service processes using a >file we want to replace could not be listed, even by an Adminstrator account. > I'm not sure if it's possible to do anything about that or not...) > >I couldn't really come up with a good way to show this in the new >IDD_FILE_INUSE dialog, so attached is patch which restores the custom message >box which was used previously to ask 'retry or continue' , and uses that when >the process list is empty. > >If in future archive::extract_file() learns how to report other conditions, >there's perhaps some re-factoring which can go on here so IDD_FILE_INUSE is >used when we know the error was a sharing violation, and this 'retry or >continue' custom message box can be used to handle generic errors. > >[1] http://cygwin.com/ml/cygwin/2013-07/msg00483.html > >2013-07-23 Jon TURNEY <jon.tur...@dronecode.org.uk> > > * install.cc (_custom_MessageBox): Restore custom message box. > (installOne): If processList is empty, use the custom message box > to ask if we should retry or continue. > * res.rc (IDD_FILE_INUSE): Use IDCONTINUE for continue button, to be > the same custom message box.
This is exactly what I was thinking we should do when I discussed this with you on IRC. Thank you. Please check this in and I'll generate a new setup.exe ASAP. Btw, it's nice to be able to kill processes that have opened files. I was delighted to see this dialog crop up during an installation. It saved me the effort of having to find and kill the process. cgf
