[PATCH setup 0/9] Setup patches

[PATCH setup 0/9] Setup patches

[Jon Turney]

Add the --user-agent option, and fix a couple of bugs, including those 
mentioned in the thread https://cygwin.com/ml/cygwin/2017-05/msg00513.html

Ake Rehnman (1):
  Avoid messagebox spam with file:// protocol URLs

Jon Turney (8):
  Allow options which only have long names
  Alphabetically sort options in usage help
  Access StringOption's value by reference
  Add Option::isPresent() method
  Allow user-agent string to be customized
  Allow click-to-activate in PickView list control
  Fix that clicking on any column changes "Keep" to "Uninstall"
  Fix useless error message

 PickPackageLine.cc                              | 12 +++++++++--
 PickView.cc                                     |  3 +++
 install.cc                                      |  9 ++++++++-
 libgetopt++/include/getopt++/DefaultFormatter.h |  7 +++++--
 libgetopt++/include/getopt++/Option.h           |  4 ++++
 libgetopt++/include/getopt++/StringOption.h     |  2 +-
 libgetopt++/src/Option.cc                       |  2 +-
 libgetopt++/src/OptionSet.cc                    | 10 ++++++++-
 libgetopt++/src/StringOption.cc                 |  2 +-
 main.cc                                         |  2 +-
 nio-file.cc                                     |  5 +++--
 nio-ie5.cc                                      | 27 ++++++++++++++++++++++++-
 12 files changed, 72 insertions(+), 13 deletions(-)

-- 
2.12.3

[PATCH setup 2/9] Alphabetically sort options in usage help

[Jon Turney]

---
 libgetopt++/src/OptionSet.cc | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/libgetopt++/src/OptionSet.cc b/libgetopt++/src/OptionSet.cc
index bc3f018..82e1253 100644
--- a/libgetopt++/src/OptionSet.cc
+++ b/libgetopt++/src/OptionSet.cc
@@ -314,9 +314,16 @@ OptionSet::Register (Option * anOption)
     options.push_back(anOption);
 }
 
+static bool
+comp_long_option(const Option *a, const Option *b)
+{
+  return (a->longOption().compare(b->longOption()) < 0);
+}
+
 void
 OptionSet::ParameterUsage (ostream &aStream)
 {
+    std::sort(options.begin(), options.end(), comp_long_option);
     for_each (options.begin(), options.end(), DefaultFormatter (aStream));
 }
 
-- 
2.12.3

[PATCH setup 4/9] Add Option::isPresent() method

[Jon Turney]

Add Option::isPresent() method, so we can distinguish between the cases of
an option which is present with the default value, and an option which is
absent.
---
 libgetopt++/include/getopt++/Option.h | 4 ++++
 libgetopt++/src/Option.cc             | 2 +-
 libgetopt++/src/OptionSet.cc          | 3 ++-
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/libgetopt++/include/getopt++/Option.h b/libgetopt++/include/getopt++/Option.h
index 7ea0786..a32f949 100644
--- a/libgetopt++/include/getopt++/Option.h
+++ b/libgetopt++/include/getopt++/Option.h
@@ -46,8 +46,12 @@ public:
   };
   virtual Argument argument () const = 0;
 
+  void setPresent(bool _present) { present = _present; }
+  bool isPresent() { return present; }
+
 protected:
     Option ();
+    bool present;
 };
 
 #endif // _OPTION_H_
diff --git a/libgetopt++/src/Option.cc b/libgetopt++/src/Option.cc
index ac13ab9..7c61eba 100644
--- a/libgetopt++/src/Option.cc
+++ b/libgetopt++/src/Option.cc
@@ -15,7 +15,7 @@
 
 #include "getopt++/Option.h"
 
-Option::Option ()
+Option::Option () : present(false)
 {
 }
 
diff --git a/libgetopt++/src/OptionSet.cc b/libgetopt++/src/OptionSet.cc
index 82e1253..f57b89a 100644
--- a/libgetopt++/src/OptionSet.cc
+++ b/libgetopt++/src/OptionSet.cc
@@ -229,7 +229,8 @@ OptionSet::doOption(string &option, string::size_type const &pos)
 	    optionValue = value.c_str();
         }
 	break;
-    } 
+    }
+    theOption->setPresent(true);
     lastResult = theOption->Process(optionValue);
 }
 
-- 
2.12.3

[PATCH setup 9/9] Avoid messagebox spam with file:// protocol URLs

[Jon Turney]

From: Ake Rehnman <ake.rehnman@gmail.com>

When using a file:// protocol URL for package repo, don't spam messageboxes
warning about absence of compressed setup files.  We don't do that for
ftp:// or http:// protocol URLs.

A warning is still given we couldn't find a useable setup.ini from the URL
provided
---
 nio-file.cc | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/nio-file.cc b/nio-file.cc
index e69f1ff..fce1b2c 100644
--- a/nio-file.cc
+++ b/nio-file.cc
@@ -26,6 +26,7 @@
 #include "resource.h"
 #include "msg.h"
 #include "filemanip.h"
+#include "LogSingleton.h"
 
 NetIO_File::NetIO_File (char const *Purl):
 NetIO (Purl)
@@ -39,8 +40,8 @@ NetIO (Purl)
     {
       const char *err = strerror (errno);
       if (!err)
-	err = "(unknown error)";
-      note (NULL, IDS_ERR_OPEN_READ, path, err);
+        err = "(unknown error)";
+      Log (LOG_BABBLE) << "Can't open " << path << " for reading: " << err << endLog;
     }
 }
 
-- 
2.12.3

[PATCH setup 8/9] Fix useless error message

[Jon Turney]

If source.Cached() is empty, we produce the amazingly helpful error message
"Can't open (null) for reading: No such file".

Improve the error message so it reports that we can't open the archive since
we don't know a filename for the locally cached archive file.

This can occur if download failed for an archive, but we chose to continue.
---
 install.cc | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/install.cc b/install.cc
index 79ddd20..f8f0b59 100644
--- a/install.cc
+++ b/install.cc
@@ -366,7 +366,14 @@ Installer::installOne (packagemeta &pkgm, const packageversion &ver,
 
   io_stream *pkgfile = NULL;
 
-  if (!source.Cached() || !io_stream::exists (source.Cached ())
+  if (!source.Cached())
+    {
+      note (NULL, IDS_ERR_OPEN_READ, source.Canonical (), "Unknown filename");
+      ++errors;
+      return;
+    }
+
+  if (!io_stream::exists (source.Cached ())
       || !(pkgfile = io_stream::open (source.Cached (), "rb", 0)))
     {
       note (NULL, IDS_ERR_OPEN_READ, source.Cached (), "No such file");
-- 
2.12.3

[PATCH setup 5/9] Allow user-agent string to be customized

[Jon Turney]

If the option is present without a string, this means that no user-agent
header should be added.

Also include version in default user agent string

v2:
Add logging of User-Agent: header override

v3:
Be more careful about scope User-Agent std::string object
---
 nio-ie5.cc | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/nio-ie5.cc b/nio-ie5.cc
index 7708d4c..24d2c13 100644
--- a/nio-ie5.cc
+++ b/nio-ie5.cc
@@ -27,6 +27,11 @@
 #include "netio.h"
 #include "nio-ie5.h"
 #include "LogSingleton.h"
+#include "setup_version.h"
+#include "getopt++/StringOption.h"
+
+static StringOption UserAgent ("", '\0', "user-agent", "User agent string for HTTP requests");
+static std::string default_useragent = std::string("Cygwin-Setup/") + setup_version;
 
 static HINTERNET internet_direct = 0;
 static HINTERNET internet_preconfig = 0;
@@ -45,7 +50,27 @@ NetIO (_url)
   if (*internet == 0)
     {
       InternetAttemptConnect (0);
-      *internet = InternetOpen ("Cygwin Setup",
+
+      const char *lpszAgent = default_useragent.c_str();
+      if (UserAgent.isPresent())
+        {
+          const std::string &user_agent = UserAgent;
+          if (user_agent.length())
+            {
+              // override the default user agent string
+              lpszAgent = user_agent.c_str();
+              Log (LOG_PLAIN) << "User-Agent: header overridden to \"" << lpszAgent << "\"" << endLog;
+            }
+          else
+            {
+              // user-agent option is present, but no string is specified means
+              // don't add a user-agent header
+              lpszAgent = NULL;
+              Log (LOG_PLAIN) << "User-Agent: header suppressed " << lpszAgent << endLog;
+            }
+        }
+
+      *internet = InternetOpen (lpszAgent,
 				direct ? INTERNET_OPEN_TYPE_DIRECT : INTERNET_OPEN_TYPE_PRECONFIG,
 				NULL, NULL, 0);
     }
-- 
2.12.3

[PATCH setup 6/9] Allow click-to-activate in PickView list control

[Jon Turney]

This helps somewhat with the problem reported in
https://cygwin.com/ml/cygwin/2017-05/msg00513.html
---
 PickView.cc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/PickView.cc b/PickView.cc
index 222bcb8..0d7af7a 100644
--- a/PickView.cc
+++ b/PickView.cc
@@ -808,6 +808,9 @@ PickView::WindowProc (UINT message, WPARAM wParam, LPARAM lParam)
         lastWindowRect = windowRect;
         return 0;     
       }
+    case WM_MOUSEACTIVATE:
+      SetFocus(GetHWND());
+      return MA_ACTIVATE;
     }
   
   // default: can't handle this message
-- 
2.12.3

[PATCH setup 1/9] Allow options which only have long names

[Jon Turney]

We're kind of running out of letters for short options :)
---
 libgetopt++/include/getopt++/DefaultFormatter.h | 7 +++++--
 main.cc                                         | 2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/libgetopt++/include/getopt++/DefaultFormatter.h b/libgetopt++/include/getopt++/DefaultFormatter.h
index 38287f1..440eb54 100644
--- a/libgetopt++/include/getopt++/DefaultFormatter.h
+++ b/libgetopt++/include/getopt++/DefaultFormatter.h
@@ -45,8 +45,11 @@ class DefaultFormatter {
         theStream(aStream)
     {}
     void operator () (Option *anOption) {
-      theStream << s_lead << anOption->shortOption ()[0]
-		<< l_lead << anOption->longOption ()
+      if (anOption->shortOption ()[0] == '\0')
+        theStream << "   ";
+      else
+        theStream << s_lead << anOption->shortOption ()[0];
+      theStream << l_lead << anOption->longOption ()
 		<< std::string (o_len
 				- s_lead.size () - 1 - l_lead.size ()
 				- anOption->longOption ().size (), ' ');
diff --git a/main.cc b/main.cc
index fe1d6c1..b44f9b6 100644
--- a/main.cc
+++ b/main.cc
@@ -94,7 +94,7 @@ static BoolOption NoAdminOption (false, 'B', "no-admin", "Do not check for and e
 static BoolOption WaitOption (false, 'W', "wait", "When elevating, wait for elevated child process");
 static BoolOption HelpOption (false, 'h', "help", "print help");
 static StringOption SetupBaseNameOpt ("setup", 'i', "ini-basename", "Use a different basename, e.g. \"foo\", instead of \"setup\"", false);
-BoolOption UnsupportedOption (false, '0', "allow-unsupported-windows", "Allow old, unsupported Windows versions");
+BoolOption UnsupportedOption (false, '\0', "allow-unsupported-windows", "Allow old, unsupported Windows versions");
 std::string SetupBaseName;
 
 static void inline
-- 
2.12.3

[PATCH setup 7/9] Fix that clicking on any column changes "Keep" to "Uninstall"

[Jon Turney]

A package's status is only changed by clicking on the "New" column, with one
exception: If the status is "Keep", it is changed to "Uninstall" after a
click (even accidental) on any other column.

"Keep" means desired == installed, picked = false.  Only run the code which
is supposed to detect both "src?" and "bin?" unchecked when clicking on
those columns.

Addresses: https://cygwin.com/ml/cygwin/2017-05/msg00525.html
---
 PickPackageLine.cc | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/PickPackageLine.cc b/PickPackageLine.cc
index 60ece7f..a158966 100644
--- a/PickPackageLine.cc
+++ b/PickPackageLine.cc
@@ -142,12 +142,20 @@ PickPackageLine::click (int const myrow, int const ClickedRow, int const x)
 	pkg.desired.sourcePackage ().pick (
 			!pkg.desired.sourcePackage ().picked (), NULL);
     }
+
   /* Unchecking binary while source is unchecked or vice versa is equivalent
      to uninstalling.  It's essential to set desired correctly, otherwise the
      package gets uninstalled without visual feedback to the user.  The package
      will not even show up in the "Pending" view! */
-  if (!pkg.desired.picked () && !pkg.desired.sourcePackage ().picked ())
-    pkg.desired = packageversion ();
+  if ((x >= theView.headers[theView.bintick_col].x - HMARGIN / 2
+      && x <= theView.headers[theView.bintick_col + 1].x - HMARGIN / 2) ||
+      (x >= theView.headers[theView.srctick_col].x - HMARGIN / 2
+       && x <= theView.headers[theView.srctick_col + 1].x - HMARGIN / 2))
+    {
+      if (!pkg.desired.picked () && !pkg.desired.sourcePackage ().picked ())
+        pkg.desired = packageversion ();
+    }
+
   return 0;
 }
 
-- 
2.12.3

[PATCH setup 3/9] Access StringOption's value by reference

[Jon Turney]

Access StringOption's value by reference, to avoid unnecessary temporaries.
---
 libgetopt++/include/getopt++/StringOption.h | 2 +-
 libgetopt++/src/StringOption.cc             | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libgetopt++/include/getopt++/StringOption.h b/libgetopt++/include/getopt++/StringOption.h
index f13bc22..f13be8c 100644
--- a/libgetopt++/include/getopt++/StringOption.h
+++ b/libgetopt++/include/getopt++/StringOption.h
@@ -32,7 +32,7 @@ public:
   virtual std::string const shortHelp () const;
   virtual Result Process (char const *);
   virtual Argument argument () const;
-  operator std::string () const;
+  operator const std::string& () const;
 
 private:
   Argument _optional;
diff --git a/libgetopt++/src/StringOption.cc b/libgetopt++/src/StringOption.cc
index 462cf0d..210b00a 100644
--- a/libgetopt++/src/StringOption.cc
+++ b/libgetopt++/src/StringOption.cc
@@ -60,7 +60,7 @@ StringOption::Process (char const *optarg)
   return Failed;
 }
 
-StringOption::operator string () const
+StringOption::operator const string& () const
 {
   return _value;
 }
-- 
2.12.3