[PATCH setup 0/4] Setup misc patches

[PATCH setup 0/4] Setup misc patches

[Jon TURNEY]

Jon TURNEY (4):
  Fix truncation of "Bin?" and "Src?" column headers
  Add a 'Retry' button to the IDD_FILE_INUSE dialog
  Don't write LOG_BABBLE output to stdout
  Remove msg() from LogFile::endEntry()

 LogFile.cc  |  9 +++++++--
 PickView.cc | 16 +++++++++-------
 install.cc  | 22 ++++++++++++++--------
 res.rc      | 25 +++++++++++++++----------
 4 files changed, 45 insertions(+), 27 deletions(-)

-- 
2.1.4

[PATCH setup 2/4] Add a 'Retry' button to the IDD_FILE_INUSE dialog

[Jon TURNEY]

I have been meaning to add this for a while, and was reminded of this by [1],
although that is really about a different problem.

[1] https://cygwin.com/ml/cygwin/2015-02/msg00679.html

Add a 'Retry' button to the IDD_FILE_INUSE dialog, to press when you want to
manually stop the processes which were preventing a file from being replaced,
after you have done so.

Unfortunately the IDD_FILE_INUSE DIALOG is already rather cramped, so define
SETUP_MEDIUM_DIALOG_DIMS for a medium-size dialog, and adjust the dialog layout
appropriately.

Also do a bit of polish on the text used: "you will need to reboot" -> "the file
will be updated after a reboot"

XXX: This text should be dynamic, and should not appear if the NoReplaceOnReboot
option has been used.

Also spread out the text in the fallback MessageBox used when we can't enumerate
the processes which have the file in-use.

XXX: Note that there is no user notification, only a log message, if scheduling
a file to be replaced on reboot failed.

2015-03-02  Jon TURNEY  <jon.turney@dronecode.org.uk>

	* res.rc (IDD_FILE_INUSE): Add 'Retry' button. Make larger.
	* install.cc (FileInuseDlgProc): Polish text.  Handle IDIGNORE.
	(installOne): Ditto.

Signed-off-by: Jon TURNEY <jon.turney@dronecode.org.uk>
---
 install.cc | 22 ++++++++++++++--------
 res.rc     | 25 +++++++++++++++----------
 2 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/install.cc b/install.cc
index 2162902..aa7c2e9 100644
--- a/install.cc
+++ b/install.cc
@@ -261,16 +261,18 @@ FileInuseDlgProc (HWND hwndDlg, UINT uMsg, WPARAM wParam, LPARAM lParam)
           case 1:
             SetDlgItemText (hwndDlg, IDRETRY, "&Kill Processes");
             SetDlgItemText (hwndDlg, IDC_FILE_INUSE_HELP,
-                            "Select 'Kill' to kill Cygwin processes and retry, or "
-                            "select 'Continue' to go on anyway (you will need to reboot).");
+                            "Select 'Retry' to retry, "
+                            "Select 'Kill' to kill processes and retry, or "
+                            "select 'Continue' to go on anyway (the file will be updated after a reboot).");
             break;
 
           default:
           case 2:
             SetDlgItemText (hwndDlg, IDRETRY, "&Kill Processes");
             SetDlgItemText (hwndDlg, IDC_FILE_INUSE_HELP,
-                            "Select 'Kill' to forcibly kill all processes and retry, or "
-                            "select 'Continue' to go on anyway (you will need to reboot).");
+                            "Select 'Retry' to retry, "
+                            "select 'Kill' to forcibly kill all processes and retry, or "
+                            "select 'Continue' to go on anyway (the file will be updated after a reboot).");
           }
       }
       return TRUE; // automatically set focus, please
@@ -280,6 +282,7 @@ FileInuseDlgProc (HWND hwndDlg, UINT uMsg, WPARAM wParam, LPARAM lParam)
         {
           switch (LOWORD (wParam))
             {
+            case IDIGNORE:
             case IDRETRY:
             case IDCONTINUE:
               EndDialog (hwndDlg, LOWORD (wParam));
@@ -543,10 +546,10 @@ Installer::installOne (packagemeta &pkgm, const packageversion &ver,
                             // to fix the problem themselves
                             char msg[fn.size() + 300];
                             sprintf (msg,
-                                     "Unable to extract /%s\r\n"
-                                     "The file is in use or some other error occurred.\r\n"
-                                     "Please stop all Cygwin processes and select \"Retry\", or\r\n"
-                                     "select \"Continue\" to go on anyway (you will need to reboot).\r\n",
+                                     "Unable to extract /%s\r\n\r\n"
+                                     "The file is in use or some other error occurred.\r\n\r\n"
+                                     "Please stop all Cygwin processes and select \"Retry\", or "
+                                     "select \"Continue\" to go on anyway (the file will be updated after a reboot).\r\n",
                                      fn.c_str());
 
                             rc = MessageBox (owner, msg, "Error writing file",
@@ -556,6 +559,9 @@ Installer::installOne (packagemeta &pkgm, const packageversion &ver,
 
                     switch (rc)
                       {
+                      case IDIGNORE:
+                        // manual intervention may have fixed the problem, retry
+                        continue;
                       case IDRETRY:
                         if (!processes.empty())
                           {
diff --git a/res.rc b/res.rc
index 9339b53..5013b31 100644
--- a/res.rc
+++ b/res.rc
@@ -3,10 +3,13 @@
 
 #define SETUP_STANDARD_DIALOG_W	339
 #define SETUP_STANDARD_DIALOG_H	179
+#define SETUP_MEDIUM_DIALOG_W		277
+#define SETUP_MEDIUM_DIALOG_H		137
 #define SETUP_SMALL_DIALOG_W		215
 #define SETUP_SMALL_DIALOG_H		95
 
 #define SETUP_STANDARD_DIALOG_DIMS	SETUP_STANDARD_DIALOG_W, SETUP_STANDARD_DIALOG_H
+#define SETUP_MEDIUM_DIALOG_DIMS	SETUP_MEDIUM_DIALOG_W, SETUP_MEDIUM_DIALOG_H
 #define SETUP_SMALL_DIALOG_DIMS	SETUP_SMALL_DIALOG_W, SETUP_SMALL_DIALOG_H
 
 #define SETUP_HEADICON_X		(SETUP_STANDARD_DIALOG_W - 27)
@@ -428,24 +431,26 @@ BEGIN
 
 END
 
-IDD_FILE_INUSE DIALOG DISCARDABLE  0, 0, SETUP_SMALL_DIALOG_DIMS
+IDD_FILE_INUSE DIALOG DISCARDABLE  0, 0, SETUP_MEDIUM_DIALOG_DIMS
 STYLE DS_MODALFRAME | DS_CENTER | WS_POPUP | WS_CAPTION
 CAPTION "In-use file detected"
 FONT 8, "MS Shell Dlg"
 BEGIN
-    ICON            IDI_WARNING,IDC_HEADICON,5,5
+    ICON            IDI_WARNING,IDC_HEADICON,10,10
     LTEXT           "Unable to extract %s",
-                    IDC_FILE_INUSE_MSG,27,5,183,8,SS_PATHELLIPSIS
+                    IDC_FILE_INUSE_MSG,33,10,234,8,SS_PATHELLIPSIS
     LTEXT           "The file is in use by the following processes:",
-                    IDC_STATIC,27,14,183,8
-    EDITTEXT        IDC_FILE_INUSE_EDIT,27,23,183,28,WS_VSCROLL |
+                    IDC_STATIC,33,28,234,8
+    EDITTEXT        IDC_FILE_INUSE_EDIT,33,40,234,32,WS_VSCROLL |
                     ES_LEFT | ES_MULTILINE | ES_READONLY |
                     ES_AUTOVSCROLL | NOT WS_TABSTOP
-    LTEXT           "Select 'Stop' to stop Cygwin processes and retry, or "
-                    "select 'Continue' to go on anyway (you will need to reboot).",
-                    IDC_FILE_INUSE_HELP,27,52,183,16,NOT WS_GROUP
-    DEFPUSHBUTTON   "&Stop Processes",IDRETRY,47,75,55,15
-    PUSHBUTTON      "&Continue",IDCONTINUE,113,75,55,15
+    LTEXT           "Select 'Retry' to retry, "
+                    "select 'Stop' to stop processes and retry, or "
+                    "select 'Continue' to go on anyway (the file will be updated after a reboot).",
+                    IDC_FILE_INUSE_HELP,33,80,234,24,NOT WS_GROUP
+    PUSHBUTTON      "&Retry",IDIGNORE,45,112,55,15
+    DEFPUSHBUTTON   "&Stop Processes",IDRETRY,111,112,55,15
+    PUSHBUTTON      "&Continue",IDCONTINUE,177,112,55,15
 END
 
 /////////////////////////////////////////////////////////////////////////////
-- 
2.1.4

Re: [PATCH setup 2/4] Add a 'Retry' button to the IDD_FILE_INUSE dialog

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 431 bytes --]

On Mar  2 13:55, Jon TURNEY wrote:
> 2015-03-02  Jon TURNEY  <...>
> 
> 	* res.rc (IDD_FILE_INUSE): Add 'Retry' button. Make larger.
> 	* install.cc (FileInuseDlgProc): Polish text.  Handle IDIGNORE.
> 	(installOne): Ditto.

Looks good, please apply.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

[PATCH setup 3/4] Don't write LOG_BABBLE output to stdout

[Jon TURNEY]

It somewhat hampers the use of setup as a command line tool that it writes
everything to stdout.  Don't write LOG_BABBLE output to stdout.  Add '--verbose'
flag to restore the previous behaviour.

ChangeLog:

2015-03-02  Jon TURNEY  <jon.turney@dronecode.org.uk>

	* LogFile.cc (VerboseOutput): Add option.
	(endEntry): Only write LOG_PLAIN to stdout, unless VerboseOutput.

Signed-off-by: Jon TURNEY <jon.turney@dronecode.org.uk>
---
 LogFile.cc | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/LogFile.cc b/LogFile.cc
index dfb9622..b45d38c 100644
--- a/LogFile.cc
+++ b/LogFile.cc
@@ -36,6 +36,10 @@ static const char *cvsid =
 #include "AntiVirus.h"
 #include "filemanip.h"
 #include "String++.h"
+#include "getopt++/BoolOption.h"
+
+static BoolOption VerboseOutput (false, 'v', "verbose",
+                                 "Verbose output");
 
 using namespace std;
 
@@ -219,7 +223,9 @@ LogFile::endEntry()
   string buf = theStream->str();
   delete theStream;
 
-  cout << buf << endl;
+  /* also write to stdout */
+  if ((currEnt->level >= LOG_PLAIN) || VerboseOutput)
+    cout << buf << endl;
 
   if (!currEnt)
     {
-- 
2.1.4

Re: [PATCH setup 3/4] Don't write LOG_BABBLE output to stdout

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 612 bytes --]

On Mar  2 13:55, Jon TURNEY wrote:
> It somewhat hampers the use of setup as a command line tool that it writes
> everything to stdout.  Don't write LOG_BABBLE output to stdout.  Add '--verbose'
> flag to restore the previous behaviour.
> 
> ChangeLog:
> 
> 2015-03-02  Jon TURNEY  <...>
> 
> 	* LogFile.cc (VerboseOutput): Add option.
> 	(endEntry): Only write LOG_PLAIN to stdout, unless VerboseOutput.

Good idea, go ahead.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

[PATCH setup 1/4] Fix truncation of "Bin?" and "Src?" column headers

[Jon TURNEY]

It seems that the the "Bin?" and "Src? columns in the PickView are usually not
quite wide enough, and display truncated with an ellipsis, e.g. 'B...' and
'S...', which is completely unintelligible.

Try to more correctly size these colums.  3*GetSystemMetrics(SM_CXEDGE) seems to
be documented as the default value for the margin of a header control, so text
length + twice that is the minimum header width needed.

ChangeLog:

2015-03-02  Jon TURNEY  <jon.turney@dronecode.org.uk>

	* PickView.cc (init_headers): More correctly size "Bin?" and
	"Src?" columns.

Signed-off-by: Jon TURNEY <jon.turney@dronecode.org.uk>
---
 PickView.cc | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/PickView.cc b/PickView.cc
index 00a3c3a..4630ee9 100644
--- a/PickView.cc
+++ b/PickView.cc
@@ -441,13 +441,15 @@ PickView::init_headers (HDC dc)
       headers[i].x = 0;
     }
 
-  // accomodate widths of the 'bin' and 'src' checkbox columns
-  // FIXME: What's up with the "0"? It's probably a mistake, and should be
-  // "". It used to be written as 0, and was subject to a bizarre implicit
-  // conversion by the unwise String(int) constructor.
-  note_width (headers, dc, "0", HMARGIN + 11, bintick_col);
-  note_width (headers, dc, "0", HMARGIN + 11, srctick_col);
-  
+  // A margin of 3*GetSystemMetrics(SM_CXEDGE) is used at each side of the
+  // header text.  (Probably should use that rather than hard-coding HMARGIN
+  // everywhere)
+  int addend = 2*3*GetSystemMetrics(SM_CXEDGE);
+
+  // accommodate widths of the 'bin' and 'src' checkbox columns
+  note_width (headers, dc, headers[bintick_col].text, addend, bintick_col);
+  note_width (headers, dc, headers[srctick_col].text, addend, srctick_col);
+
   // accomodate the width of each category name
   packagedb db;
   for (packagedb::categoriesType::iterator n = packagedb::categories.begin();
-- 
2.1.4

Re: [PATCH setup 1/4] Fix truncation of "Bin?" and "Src?" column headers

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 826 bytes --]

On Mar  2 13:55, Jon TURNEY wrote:
> It seems that the the "Bin?" and "Src? columns in the PickView are usually not
> quite wide enough, and display truncated with an ellipsis, e.g. 'B...' and
> 'S...', which is completely unintelligible.
> 
> Try to more correctly size these colums.  3*GetSystemMetrics(SM_CXEDGE) seems to
> be documented as the default value for the margin of a header control, so text
> length + twice that is the minimum header width needed.
> 
> ChangeLog:
> 
> 2015-03-02  Jon TURNEY  <...>
> 
> 	* PickView.cc (init_headers): More correctly size "Bin?" and
> 	"Src?" columns.

This patch is fine.  Please apply.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

[PATCH setup 4/4] Remove msg() from LogFile::endEntry()

[Jon TURNEY]

Every single line of log output is written to OutputDebugString() using msg().
This makes setup run quite slowly under a debugger.  If you have stdout
connected to the terminal where your debugger is running (e.g. 'gdb setup'),
this output is duplicated.

Future work: there are some other uses of msg() to report real errors.  At the
moment, these are completely invisible unless you are running setup under a
debugger.  These should be converted to use the logger.

2015-03-02  Jon TURNEY  <jon.turney@dronecode.org.uk>

	* LogFile.cc (endEntry): Remove msg().

Signed-off-by: Jon TURNEY <jon.turney@dronecode.org.uk>
---
 LogFile.cc | 1 -
 1 file changed, 1 deletion(-)

diff --git a/LogFile.cc b/LogFile.cc
index b45d38c..69bf70b 100644
--- a/LogFile.cc
+++ b/LogFile.cc
@@ -248,7 +248,6 @@ LogFile::endEntry()
    * non-0 memory on alloc
    */
   currEnt->msg += buf;
-  msg ("LOG: %d %s", currEnt->level, buf.c_str());
 
   /* reset for next use */
   theStream = new std::stringbuf;
-- 
2.1.4

Re: [PATCH setup 4/4] Remove msg() from LogFile::endEntry()

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 870 bytes --]

On Mar  2 13:55, Jon TURNEY wrote:
> Every single line of log output is written to OutputDebugString() using msg().
> This makes setup run quite slowly under a debugger.  If you have stdout
> connected to the terminal where your debugger is running (e.g. 'gdb setup'),
> this output is duplicated.
> 
> Future work: there are some other uses of msg() to report real errors.  At the
> moment, these are completely invisible unless you are running setup under a
> debugger.  These should be converted to use the logger.
> 
> 2015-03-02  Jon TURNEY  <...>
> 
> 	* LogFile.cc (endEntry): Remove msg().

Approved, under the assumption that you'll rework the other msg() calls
as well ;)


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]