public inbox for cygwin-apps@cygwin.com
 help / color / mirror / Atom feed
* [PATCH setup 2/2] Whitespace fixes
  2017-11-06 21:49 [PATCH setup 0/2] Improve behavior after download error Ken Brown
  2017-11-06 21:49 ` [PATCH setup 1/2] " Ken Brown
@ 2017-11-06 21:49 ` Ken Brown
  2017-11-07  4:28 ` [PATCH setup 0/2] Improve behavior after download error Brian Inglis
  2 siblings, 0 replies; 11+ messages in thread
From: Ken Brown @ 2017-11-06 21:49 UTC (permalink / raw)
  To: cygwin-apps

---
 download.cc | 146 ++++++++++++++++++++++++++++++------------------------------
 1 file changed, 73 insertions(+), 73 deletions(-)

diff --git a/download.cc b/download.cc
index 6ee64c1..89e2b7d 100644
--- a/download.cc
+++ b/download.cc
@@ -199,95 +199,95 @@ do_download_thread (HINSTANCE h, HWND owner)
 
   do
     {
-  errors = 0;
-  total_download_bytes = 0;
-  total_download_bytes_sofar = 0;
-
-  Progress.SetText1 ("Checking for packages to download...");
-  Progress.SetText2 ("");
-  Progress.SetText3 ("");
-
-  packagedb db;
-  /* calculate the amount needed */
-  for (packagedb::packagecollection::iterator i = db.packages.begin ();
-       i != db.packages.end (); ++i)
-    {
-      packagemeta & pkg = *(i->second);
-      if (pkg.desired.picked () || pkg.desired.sourcePackage ().picked ())
+      errors = 0;
+      total_download_bytes = 0;
+      total_download_bytes_sofar = 0;
+
+      Progress.SetText1 ("Checking for packages to download...");
+      Progress.SetText2 ("");
+      Progress.SetText3 ("");
+
+      packagedb db;
+      /* calculate the amount needed */
+      for (packagedb::packagecollection::iterator i = db.packages.begin ();
+	   i != db.packages.end (); ++i)
 	{
-	  packageversion version = pkg.desired;
-	  packageversion sourceversion = version.sourcePackage();
-	  try 
+	  packagemeta & pkg = *(i->second);
+	  if (pkg.desired.picked () || pkg.desired.sourcePackage ().picked ())
 	    {
-    	      if (version.picked())
+	      packageversion version = pkg.desired;
+	      packageversion sourceversion = version.sourcePackage();
+	      try
 		{
-		    if (!check_for_cached (*version.source()))
-		      total_download_bytes += version.source()->size;
+		  if (version.picked())
+		    {
+		      if (!check_for_cached (*version.source()))
+			total_download_bytes += version.source()->size;
+		    }
+		  if (sourceversion.picked () || IncludeSource)
+		    {
+		      if (!check_for_cached (*sourceversion.source()))
+			total_download_bytes += sourceversion.source()->size;
+		    }
 		}
-    	      if (sourceversion.picked () || IncludeSource)
+	      catch (Exception * e)
 		{
-		    if (!check_for_cached (*sourceversion.source()))
-		      total_download_bytes += sourceversion.source()->size;
+		  // We know what to do with these..
+		  if (e->errNo() == APPERR_CORRUPT_PACKAGE)
+		    fatal (owner, IDS_CORRUPT_PACKAGE, pkg.name.c_str());
+		  // Unexpected exception.
+		  throw e;
 		}
 	    }
-	  catch (Exception * e)
-	    {
-	      // We know what to do with these..
-	      if (e->errNo() == APPERR_CORRUPT_PACKAGE)
-		fatal (owner, IDS_CORRUPT_PACKAGE, pkg.name.c_str());
-	      // Unexpected exception.
-	      throw e;
-	    }
-	}
     }
 
-  /* and do the download. FIXME: This here we assign a new name for the cached version
-   * and check that above.
-   */
-  for (packagedb::packagecollection::iterator i = db.packages.begin ();
-       i != db.packages.end (); ++i)
-    {
-      packagemeta & pkg = *(i->second);
-      if (pkg.desired.picked () || pkg.desired.sourcePackage ().picked ())
+      /* and do the download. FIXME: This here we assign a new name for the cached version
+       * and check that above.
+       */
+      for (packagedb::packagecollection::iterator i = db.packages.begin ();
+	   i != db.packages.end (); ++i)
 	{
-	  int e = 0;
-	  packageversion version = pkg.desired;
-	  packageversion sourceversion = version.sourcePackage();
-	  if (version.picked())
-	    {
-		e += download_one (*version.source(), owner);
-	    }
-	  if (sourceversion && (sourceversion.picked() || IncludeSource))
+	  packagemeta & pkg = *(i->second);
+	  if (pkg.desired.picked () || pkg.desired.sourcePackage ().picked ())
 	    {
-		e += download_one (*sourceversion.source (), owner);
-	    }
-	  errors += e;
+	      int e = 0;
+	      packageversion version = pkg.desired;
+	      packageversion sourceversion = version.sourcePackage();
+	      if (version.picked())
+		{
+		  e += download_one (*version.source(), owner);
+		}
+	      if (sourceversion && (sourceversion.picked() || IncludeSource))
+		{
+		  e += download_one (*sourceversion.source (), owner);
+		}
+	      errors += e;
 #if 0
-	  if (e)
-	    pkg->action = ACTION_ERROR;
+	      if (e)
+		pkg->action = ACTION_ERROR;
 #endif
+	    }
 	}
-    }
 
-  if (errors)
-    {
-      /* In unattended mode, all dialog boxes automatically get
-         answered with a Yes/OK/other positive response.  This
-	 means that if there's a download problem, setup will
-	 potentially retry forever if we don't take care to give
-	 up at some finite point.  */
-      if (unattended_mode && --retries <= 0)
-        {
-	  Log (LOG_PLAIN) << "download error in unattended_mode: out of retries" << endLog;
-	  Logger ().setExitMsg (IDS_INSTALL_INCOMPLETE);
-	  Logger ().exit (1);
+      if (errors)
+	{
+	  /* In unattended mode, all dialog boxes automatically get
+	     answered with a Yes/OK/other positive response.  This
+	     means that if there's a download problem, setup will
+	     potentially retry forever if we don't take care to give
+	     up at some finite point.  */
+	  if (unattended_mode && --retries <= 0)
+	    {
+	      Log (LOG_PLAIN) << "download error in unattended_mode: out of retries" << endLog;
+	      Logger ().setExitMsg (IDS_INSTALL_INCOMPLETE);
+	      Logger ().exit (1);
+	    }
+	  else if (unattended_mode)
+	    Log (LOG_PLAIN) << "download error in unattended_mode: " << retries
+			    << (retries > 1 ? " retries" : " retry") << " remaining." << endLog;
+	  else if (yesno (owner, IDS_DOWNLOAD_INCOMPLETE) == IDNO)
+	    break;
 	}
-      else if (unattended_mode)
-	  Log (LOG_PLAIN) << "download error in unattended_mode: " << retries
-	    << (retries > 1 ? " retries" : " retry") << " remaining." << endLog;
-      else if (yesno (owner, IDS_DOWNLOAD_INCOMPLETE) == IDNO)
-	break;
-    }
     }
   while (errors);
 
-- 
2.15.0

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH setup 1/2] Improve behavior after download error
  2017-11-06 21:49 [PATCH setup 0/2] Improve behavior after download error Ken Brown
@ 2017-11-06 21:49 ` Ken Brown
  2017-11-06 21:49 ` [PATCH setup 2/2] Whitespace fixes Ken Brown
  2017-11-07  4:28 ` [PATCH setup 0/2] Improve behavior after download error Brian Inglis
  2 siblings, 0 replies; 11+ messages in thread
From: Ken Brown @ 2017-11-06 21:49 UTC (permalink / raw)
  To: cygwin-apps

If the user answers "Yes" to "Download incomplete.  Try again?",
simply try the download again, as the message suggests, instead of
going back to the site page.  If the user answers no, don't proceed
with the install unless the user confirms that this is what they want.

The rationale for the previous behavior was presumably to let the user
choose a different mirror after a download error.  But this is not
clear from the "Try again?" prompt, and it probably didn't work as
expected if the user did choose a different mirror.  To make this work
right, packagedb would have to have been cleared before going to the
chooser page for the second time.
---
 download.cc | 29 ++++++++++++++++++++---------
 res.rc      |  1 +
 resource.h  |  1 +
 3 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/download.cc b/download.cc
index 80615f3..6ee64c1 100644
--- a/download.cc
+++ b/download.cc
@@ -195,6 +195,11 @@ static int
 do_download_thread (HINSTANCE h, HWND owner)
 {
   int errors = 0;
+  int retries = 5;
+
+  do
+    {
+  errors = 0;
   total_download_bytes = 0;
   total_download_bytes_sofar = 0;
 
@@ -271,22 +276,20 @@ do_download_thread (HINSTANCE h, HWND owner)
 	 means that if there's a download problem, setup will
 	 potentially retry forever if we don't take care to give
 	 up at some finite point.  */
-      static int retries = 4;
-      if (unattended_mode && retries-- <= 0)
+      if (unattended_mode && --retries <= 0)
         {
 	  Log (LOG_PLAIN) << "download error in unattended_mode: out of retries" << endLog;
 	  Logger ().setExitMsg (IDS_INSTALL_INCOMPLETE);
 	  Logger ().exit (1);
 	}
       else if (unattended_mode)
-        {
 	  Log (LOG_PLAIN) << "download error in unattended_mode: " << retries
 	    << (retries > 1 ? " retries" : " retry") << " remaining." << endLog;
-	  return IDD_SITE;
-	}
-      else if (yesno (owner, IDS_DOWNLOAD_INCOMPLETE) == IDYES)
-	return IDD_SITE;
+      else if (yesno (owner, IDS_DOWNLOAD_INCOMPLETE) == IDNO)
+	break;
     }
+    }
+  while (errors);
 
   if (source == IDC_SOURCE_DOWNLOAD)
     {
@@ -296,8 +299,16 @@ do_download_thread (HINSTANCE h, HWND owner)
 	Logger ().setExitMsg (IDS_DOWNLOAD_COMPLETE);
       return IDD_DESKTOP;
     }
-  else
-    return IDD_S_INSTALL;
+
+  if (errors)
+    {
+      if (yesno (owner, IDS_CONTINUE_INSTALL) == IDNO)
+	{
+	  Logger ().setExitMsg (IDS_INSTALL_INCOMPLETE);
+	  Logger ().exit (1);
+	}
+    }
+  return IDD_S_INSTALL;
 }
 
 static DWORD WINAPI
diff --git a/res.rc b/res.rc
index 96dcf00..40e464b 100644
--- a/res.rc
+++ b/res.rc
@@ -523,6 +523,7 @@ BEGIN
     IDS_ERR_CHDIR           "Could not change dir to %s: %s [%.8x]"
     IDS_OLD_SETUP_VERSION   "This setup is version %s, but setup.ini claims version %s is available.\nYou might want to upgrade to get the latest features and bug fixes."
     IDS_DOWNLOAD_INCOMPLETE "Download Incomplete.  Try again?"
+    IDS_CONTINUE_INSTALL    "Download incomplete.  Continue with install anyway?"
     IDS_INSTALL_ERROR	    "Installation error (%s), Continue with other packages?"
     IDS_INSTALL_INCOMPLETE  "Installation incomplete.  Check %s for details"
     IDS_CORRUPT_PACKAGE     "Package file %s has a corrupt local copy, please remove and retry."
diff --git a/resource.h b/resource.h
index a2add84..b6685e5 100644
--- a/resource.h
+++ b/resource.h
@@ -39,6 +39,7 @@
 #define IDS_NO_LOCALDIR			  138
 #define IDS_ELEVATED			  139
 #define IDS_INSTALLEDB_VERSION            140
+#define IDS_CONTINUE_INSTALL              141
 
 // Dialogs
 
-- 
2.15.0

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH setup 0/2] Improve behavior after download error
@ 2017-11-06 21:49 Ken Brown
  2017-11-06 21:49 ` [PATCH setup 1/2] " Ken Brown
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Ken Brown @ 2017-11-06 21:49 UTC (permalink / raw)
  To: cygwin-apps

This is a followup to
https://sourceware.org/ml/cygwin-apps/2017-11/msg00003.html.  The
focus of that thread was a crash that occurs on the topic/libsolv
branch.  Here I'm more interested in a UI issue.  Namely, I don't
think it's reasonable that setup goes back to the site page if the
user clicks Yes in response to "Download Incomplete. Try again?".
This is not what the message says will happen, and I'm not convinced
that it even works right if the user changes mirrors after being sent
to the site page.

The first patch in this series instead puts the relevant code in a
retry loop.  This should require indentation of a large amount of
code.  To keep the patch readable, I've saved the indentation for a
separate patch.

Ken Brown (2):
  Improve behavior after download error
  Whitespace fixes

 download.cc | 161 ++++++++++++++++++++++++++++++++----------------------------
 res.rc      |   1 +
 resource.h  |   1 +
 3 files changed, 88 insertions(+), 75 deletions(-)

-- 
2.15.0

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH setup 0/2] Improve behavior after download error
  2017-11-06 21:49 [PATCH setup 0/2] Improve behavior after download error Ken Brown
  2017-11-06 21:49 ` [PATCH setup 1/2] " Ken Brown
  2017-11-06 21:49 ` [PATCH setup 2/2] Whitespace fixes Ken Brown
@ 2017-11-07  4:28 ` Brian Inglis
  2017-11-07 18:56   ` Jon Turney
  2 siblings, 1 reply; 11+ messages in thread
From: Brian Inglis @ 2017-11-07  4:28 UTC (permalink / raw)
  To: cygwin-apps

On 2017-11-06 14:49, Ken Brown wrote:
> This is a followup to
> https://sourceware.org/ml/cygwin-apps/2017-11/msg00003.html.  The
> focus of that thread was a crash that occurs on the topic/libsolv
> branch.  Here I'm more interested in a UI issue.  Namely, I don't
> think it's reasonable that setup goes back to the site page if the
> user clicks Yes in response to "Download Incomplete. Try again?".
> This is not what the message says will happen, and I'm not convinced
> that it even works right if the user changes mirrors after being sent
> to the site page.

Would it make more sense to drop to the package chooser page, after issuing the
error message and advising the user to: select Back to go to the mirror chooser
page, select Next to retry the downloads, or select Cancel to exit?

In the current released setup, selecting Back on the package chooser page just
unconditionally redownloads setup.ini..., with all download dialogue box buttons
disabled, and drops back into the package chooser page, rather than allowing the
choice of: select Back to go to the mirror chooser page, select Next to download
setup.ini..., or Cancel to exit.

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH setup 0/2] Improve behavior after download error
  2017-11-07  4:28 ` [PATCH setup 0/2] Improve behavior after download error Brian Inglis
@ 2017-11-07 18:56   ` Jon Turney
  2017-11-08 14:35     ` Ken Brown
  2017-11-08 18:46     ` Brian Inglis
  0 siblings, 2 replies; 11+ messages in thread
From: Jon Turney @ 2017-11-07 18:56 UTC (permalink / raw)
  To: cygwin-apps

On 07/11/2017 04:28, Brian Inglis wrote:
> On 2017-11-06 14:49, Ken Brown wrote:
>> This is a followup to
>> https://sourceware.org/ml/cygwin-apps/2017-11/msg00003.html.  The
>> focus of that thread was a crash that occurs on the topic/libsolv
>> branch.  Here I'm more interested in a UI issue.  Namely, I don't
>> think it's reasonable that setup goes back to the site page if the
>> user clicks Yes in response to "Download Incomplete. Try again?".
>> This is not what the message says will happen, and I'm not convinced
>> that it even works right if the user changes mirrors after being sent
>> to the site page.
> 
> Would it make more sense to drop to the package chooser page, after issuing the
> error message and advising the user to: select Back to go to the package chooser
> page, select Next to retry the downloads, or select Cancel to exit?

Do we actually report the package name for the failed download so that 
the user could make an informed change in the package chooser?

In any case, this is an incremental improvement, especially over the 
utterly mad thing we are currently doing in unattended mode.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH setup 0/2] Improve behavior after download error
  2017-11-07 18:56   ` Jon Turney
@ 2017-11-08 14:35     ` Ken Brown
  2017-11-08 18:52       ` Brian Inglis
  2017-11-08 18:46     ` Brian Inglis
  1 sibling, 1 reply; 11+ messages in thread
From: Ken Brown @ 2017-11-08 14:35 UTC (permalink / raw)
  To: cygwin-apps

On 11/7/2017 1:56 PM, Jon Turney wrote:
> On 07/11/2017 04:28, Brian Inglis wrote:
>> On 2017-11-06 14:49, Ken Brown wrote:
>>> This is a followup to
>>> https://sourceware.org/ml/cygwin-apps/2017-11/msg00003.html.  The
>>> focus of that thread was a crash that occurs on the topic/libsolv
>>> branch.  Here I'm more interested in a UI issue.  Namely, I don't
>>> think it's reasonable that setup goes back to the site page if the
>>> user clicks Yes in response to "Download Incomplete. Try again?".
>>> This is not what the message says will happen, and I'm not convinced
>>> that it even works right if the user changes mirrors after being sent
>>> to the site page.
>>
>> Would it make more sense to drop to the package chooser page, after 
>> issuing the
>> error message and advising the user to: select Back to go to the 
>> package chooser
>> page, select Next to retry the downloads, or select Cancel to exit?
> 
> Do we actually report the package name for the failed download so that 
> the user could make an informed change in the package chooser?

No.  Currently the only way for the user to find out is to finish the 
setup run and then look at the log.  There's been a FIXME about this at 
the end of download.cc:download_one() since 2001.  Maybe it's time to 
fix this.  We could simply keep a list of packages (or files?) for which 
the download failed, and then report this in the "Download incomplete" 
dialog.

Ken

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH setup 0/2] Improve behavior after download error
  2017-11-07 18:56   ` Jon Turney
  2017-11-08 14:35     ` Ken Brown
@ 2017-11-08 18:46     ` Brian Inglis
  1 sibling, 0 replies; 11+ messages in thread
From: Brian Inglis @ 2017-11-08 18:46 UTC (permalink / raw)
  To: cygwin-apps

On 2017-11-07 11:56, Jon Turney wrote:
> On 07/11/2017 04:28, Brian Inglis wrote:
>> On 2017-11-06 14:49, Ken Brown wrote:
>>> This is a followup to
>>> https://sourceware.org/ml/cygwin-apps/2017-11/msg00003.html.  The
>>> focus of that thread was a crash that occurs on the topic/libsolv
>>> branch.  Here I'm more interested in a UI issue.  Namely, I don't
>>> think it's reasonable that setup goes back to the site page if the
>>> user clicks Yes in response to "Download Incomplete. Try again?".
>>> This is not what the message says will happen, and I'm not convinced
>>> that it even works right if the user changes mirrors after being sent
>>> to the site page.
>>
>> Would it make more sense to drop to the package chooser page, after issuing the
>> error message and advising the user to: select Back to go to the package chooser
>> page, select Next to retry the downloads, or select Cancel to exit?
> 
> Do we actually report the package name for the failed download so that the user
> could make an informed change in the package chooser?

Don't remember ever having a package download failure, normally mirror download
problems are at the setup.ini... stage, possibly accessing during an rsync, then
the packages download okay.

> In any case, this is an incremental improvement, especially over the utterly mad
> thing we are currently doing in unattended mode.

I like to retry the download on the current mirror, then either pick another
mirror, or quit and wait awhile.
In (semi-)unattended mode, you should either decide to quit as if unattended, or
possibly go into full interactive mode and allow Back, Next, Cancel without limits?

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH setup 0/2] Improve behavior after download error
  2017-11-08 14:35     ` Ken Brown
@ 2017-11-08 18:52       ` Brian Inglis
  2017-11-09 13:21         ` Jon Turney
  0 siblings, 1 reply; 11+ messages in thread
From: Brian Inglis @ 2017-11-08 18:52 UTC (permalink / raw)
  To: cygwin-apps

On 2017-11-08 07:35, Ken Brown wrote:
> On 11/7/2017 1:56 PM, Jon Turney wrote:
>> On 07/11/2017 04:28, Brian Inglis wrote:
>>> On 2017-11-06 14:49, Ken Brown wrote:
>>>> This is a followup to
>>>> https://sourceware.org/ml/cygwin-apps/2017-11/msg00003.html.  The
>>>> focus of that thread was a crash that occurs on the topic/libsolv
>>>> branch.  Here I'm more interested in a UI issue.  Namely, I don't
>>>> think it's reasonable that setup goes back to the site page if the
>>>> user clicks Yes in response to "Download Incomplete. Try again?".
>>>> This is not what the message says will happen, and I'm not convinced
>>>> that it even works right if the user changes mirrors after being sent
>>>> to the site page.
>>>
>>> Would it make more sense to drop to the package chooser page, after issuing the
>>> error message and advising the user to: select Back to go to the package chooser
>>> page, select Next to retry the downloads, or select Cancel to exit?
>>
>> Do we actually report the package name for the failed download so that the
>> user could make an informed change in the package chooser?
> 
> No.  Currently the only way for the user to find out is to finish the setup run
> and then look at the log.  There's been a FIXME about this at the end of
> download.cc:download_one() since 2001.  Maybe it's time to fix this.  We could
> simply keep a list of packages (or files?) for which the download failed, and
> then report this in the "Download incomplete" dialog.

An approach I've used in various apps which generate logs or other output files
which contain a status report on errors, is to launch the txt, html, xls, etc.
using the default user app, as with cygstart.
Whether you should do that in (semi-)unattended mode is your design decision.

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH setup 0/2] Improve behavior after download error
  2017-11-08 18:52       ` Brian Inglis
@ 2017-11-09 13:21         ` Jon Turney
  2017-11-09 16:42           ` Ken Brown
  0 siblings, 1 reply; 11+ messages in thread
From: Jon Turney @ 2017-11-09 13:21 UTC (permalink / raw)
  To: cygwin-apps

On 08/11/2017 18:52, Brian Inglis wrote:
> On 2017-11-08 07:35, Ken Brown wrote:
>> On 11/7/2017 1:56 PM, Jon Turney wrote:
>>> On 07/11/2017 04:28, Brian Inglis wrote:
>>>> On 2017-11-06 14:49, Ken Brown wrote:
>>>>> This is a followup to
>>>>> https://sourceware.org/ml/cygwin-apps/2017-11/msg00003.html.  The
>>>>> focus of that thread was a crash that occurs on the topic/libsolv
>>>>> branch.  Here I'm more interested in a UI issue.  Namely, I don't
>>>>> think it's reasonable that setup goes back to the site page if the
>>>>> user clicks Yes in response to "Download Incomplete. Try again?".
>>>>> This is not what the message says will happen, and I'm not convinced
>>>>> that it even works right if the user changes mirrors after being sent
>>>>> to the site page.
>>>>
>>>> Would it make more sense to drop to the package chooser page, after issuing the
>>>> error message and advising the user to: select Back to go to the package chooser
>>>> page, select Next to retry the downloads, or select Cancel to exit?
>>>
>>> Do we actually report the package name for the failed download so that the
>>> user could make an informed change in the package chooser?
>>
>> No.  Currently the only way for the user to find out is to finish the setup run
>> and then look at the log.  There's been a FIXME about this at the end of
>> download.cc:download_one() since 2001.  Maybe it's time to fix this.  We could
>> simply keep a list of packages (or files?) for which the download failed, and
>> then report this in the "Download incomplete" dialog.

Note that in the pathological case of a mirror which only has a 
setup.ini, the list of failed packages could be very large.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH setup 0/2] Improve behavior after download error
  2017-11-09 13:21         ` Jon Turney
@ 2017-11-09 16:42           ` Ken Brown
  2017-11-10 14:41             ` Ken Brown
  0 siblings, 1 reply; 11+ messages in thread
From: Ken Brown @ 2017-11-09 16:42 UTC (permalink / raw)
  To: cygwin-apps

On 11/9/2017 8:21 AM, Jon Turney wrote:
> On 08/11/2017 18:52, Brian Inglis wrote:
>> On 2017-11-08 07:35, Ken Brown wrote:
>>> On 11/7/2017 1:56 PM, Jon Turney wrote:
>>>> On 07/11/2017 04:28, Brian Inglis wrote:
>>>>> On 2017-11-06 14:49, Ken Brown wrote:
>>>>>> This is a followup to
>>>>>> https://sourceware.org/ml/cygwin-apps/2017-11/msg00003.html.  The
>>>>>> focus of that thread was a crash that occurs on the topic/libsolv
>>>>>> branch.  Here I'm more interested in a UI issue.  Namely, I don't
>>>>>> think it's reasonable that setup goes back to the site page if the
>>>>>> user clicks Yes in response to "Download Incomplete. Try again?".
>>>>>> This is not what the message says will happen, and I'm not convinced
>>>>>> that it even works right if the user changes mirrors after being sent
>>>>>> to the site page.
>>>>>
>>>>> Would it make more sense to drop to the package chooser page, after 
>>>>> issuing the
>>>>> error message and advising the user to: select Back to go to the 
>>>>> package chooser
>>>>> page, select Next to retry the downloads, or select Cancel to exit?
>>>>
>>>> Do we actually report the package name for the failed download so 
>>>> that the
>>>> user could make an informed change in the package chooser?
>>>
>>> No.  Currently the only way for the user to find out is to finish the 
>>> setup run
>>> and then look at the log.  There's been a FIXME about this at the end of
>>> download.cc:download_one() since 2001.  Maybe it's time to fix this.  
>>> We could
>>> simply keep a list of packages (or files?) for which the download 
>>> failed, and
>>> then report this in the "Download incomplete" dialog.
> 
> Note that in the pathological case of a mirror which only has a 
> setup.ini, the list of failed packages could be very large.

I guess we should limit the number of failed packages that we report. 
Alternatively, we could make it a fatal error if there are too many, and 
refer the user to the log for the full list.

I'm working on a patch.

Ken

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH setup 0/2] Improve behavior after download error
  2017-11-09 16:42           ` Ken Brown
@ 2017-11-10 14:41             ` Ken Brown
  0 siblings, 0 replies; 11+ messages in thread
From: Ken Brown @ 2017-11-10 14:41 UTC (permalink / raw)
  To: cygwin-apps

On 11/9/2017 11:42 AM, Ken Brown wrote:
> On 11/9/2017 8:21 AM, Jon Turney wrote:
>> On 08/11/2017 18:52, Brian Inglis wrote:
>>> On 2017-11-08 07:35, Ken Brown wrote:
>>>> On 11/7/2017 1:56 PM, Jon Turney wrote:
>>>>> On 07/11/2017 04:28, Brian Inglis wrote:
>>>>>> On 2017-11-06 14:49, Ken Brown wrote:
>>>>>>> This is a followup to
>>>>>>> https://sourceware.org/ml/cygwin-apps/2017-11/msg00003.html.  The
>>>>>>> focus of that thread was a crash that occurs on the topic/libsolv
>>>>>>> branch.  Here I'm more interested in a UI issue.  Namely, I don't
>>>>>>> think it's reasonable that setup goes back to the site page if the
>>>>>>> user clicks Yes in response to "Download Incomplete. Try again?".
>>>>>>> This is not what the message says will happen, and I'm not convinced
>>>>>>> that it even works right if the user changes mirrors after being 
>>>>>>> sent
>>>>>>> to the site page.
>>>>>>
>>>>>> Would it make more sense to drop to the package chooser page, 
>>>>>> after issuing the
>>>>>> error message and advising the user to: select Back to go to the 
>>>>>> package chooser
>>>>>> page, select Next to retry the downloads, or select Cancel to exit?
>>>>>
>>>>> Do we actually report the package name for the failed download so 
>>>>> that the
>>>>> user could make an informed change in the package chooser?
>>>>
>>>> No.  Currently the only way for the user to find out is to finish 
>>>> the setup run
>>>> and then look at the log.  There's been a FIXME about this at the 
>>>> end of
>>>> download.cc:download_one() since 2001.  Maybe it's time to fix this. 
>>>> We could
>>>> simply keep a list of packages (or files?) for which the download 
>>>> failed, and
>>>> then report this in the "Download incomplete" dialog.
>>
>> Note that in the pathological case of a mirror which only has a 
>> setup.ini, the list of failed packages could be very large.
> 
> I guess we should limit the number of failed packages that we report.

I'm about to send a patch series that implements Brian's suggestion and 
adds a (limited) list of failed packages.

Jon, these patches are to be applied to the libsolv branch, on top of 
your recent series of 5 patches.

Ken

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-11-10 14:41 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-06 21:49 [PATCH setup 0/2] Improve behavior after download error Ken Brown
2017-11-06 21:49 ` [PATCH setup 1/2] " Ken Brown
2017-11-06 21:49 ` [PATCH setup 2/2] Whitespace fixes Ken Brown
2017-11-07  4:28 ` [PATCH setup 0/2] Improve behavior after download error Brian Inglis
2017-11-07 18:56   ` Jon Turney
2017-11-08 14:35     ` Ken Brown
2017-11-08 18:52       ` Brian Inglis
2017-11-09 13:21         ` Jon Turney
2017-11-09 16:42           ` Ken Brown
2017-11-10 14:41             ` Ken Brown
2017-11-08 18:46     ` Brian Inglis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).