* [PATCH setup 2/5] Fix off-by-one error in download retry report
2017-11-10 14:43 [PATCH setup 0/5] Improve behavior after download error, v2 Ken Brown
2017-11-10 14:43 ` [PATCH setup 1/5] Just retry download after error in unattended mode Ken Brown
@ 2017-11-10 14:43 ` Ken Brown
2017-11-10 14:43 ` [PATCH setup 3/5] Remove "Try again?" from exit message Ken Brown
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Ken Brown @ 2017-11-10 14:43 UTC (permalink / raw)
To: cygwin-apps
'retries' was decremented after it was tested but before it was
reported in the log, so the reported number was always 1 too low.
---
download.cc | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/download.cc b/download.cc
index f6aa6fc..a430f7f 100644
--- a/download.cc
+++ b/download.cc
@@ -245,8 +245,8 @@ do_download_thread (HINSTANCE h, HWND owner)
if (errors)
{
// In unattended mode we retry the download, but not forever.
- static int retries = 4;
- if (unattended_mode && retries-- <= 0)
+ static int retries = 5;
+ if (unattended_mode && --retries <= 0)
{
Log (LOG_PLAIN) << "download error in unattended_mode: out of retries" << endLog;
Logger ().setExitMsg (IDS_INSTALL_INCOMPLETE);
--
2.15.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH setup 5/5] Limit the number of packages shown in the IDD_DOWNLOAD_ERROR listbox
2017-11-10 14:43 [PATCH setup 0/5] Improve behavior after download error, v2 Ken Brown
` (3 preceding siblings ...)
2017-11-10 14:43 ` [PATCH setup 4/5] Query user after download error in interactive mode Ken Brown
@ 2017-11-10 14:43 ` Ken Brown
2017-11-13 11:51 ` [PATCH setup 0/5] Improve behavior after download error, v2 Jon Turney
5 siblings, 0 replies; 7+ messages in thread
From: Ken Brown @ 2017-11-10 14:43 UTC (permalink / raw)
To: cygwin-apps
---
download.cc | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/download.cc b/download.cc
index b059bf5..6e6d6e8 100644
--- a/download.cc
+++ b/download.cc
@@ -188,6 +188,7 @@ download_one (packagesource & pkgsource, HWND owner)
static std::vector <packageversion> download_failures;
static std::string download_warn_pkgs;
+static const int max_pkgs = 20;
static INT_PTR CALLBACK
download_error_proc (HWND h, UINT message, WPARAM wParam, LPARAM lParam)
@@ -224,12 +225,19 @@ query_download_errors (HINSTANCE h, HWND owner)
{
download_warn_pkgs = "";
Log (LOG_PLAIN) << "The following package(s) had download errors:" << endLog;
+ int count = 0;
for (std::vector <packageversion>::const_iterator i = download_failures.begin (); i != download_failures.end (); i++)
{
packageversion pv = *i;
std::string pvs = pv.Name () + "-" + pv.Canonical_version ();
Log (LOG_PLAIN) << " " << pvs << endLog;
- download_warn_pkgs += pvs + "\r\n";
+ if (count < max_pkgs)
+ download_warn_pkgs += pvs + "\r\n";
+ else if (count == max_pkgs)
+ download_warn_pkgs += "...and "
+ + std::to_string (download_failures.size () - max_pkgs)
+ + " more.";
+ count++;
}
return DialogBox (h, MAKEINTRESOURCE (IDD_DOWNLOAD_ERROR), owner,
download_error_proc);
--
2.15.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH setup 3/5] Remove "Try again?" from exit message.
2017-11-10 14:43 [PATCH setup 0/5] Improve behavior after download error, v2 Ken Brown
2017-11-10 14:43 ` [PATCH setup 1/5] Just retry download after error in unattended mode Ken Brown
2017-11-10 14:43 ` [PATCH setup 2/5] Fix off-by-one error in download retry report Ken Brown
@ 2017-11-10 14:43 ` Ken Brown
2017-11-10 14:43 ` [PATCH setup 4/5] Query user after download error in interactive mode Ken Brown
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Ken Brown @ 2017-11-10 14:43 UTC (permalink / raw)
To: cygwin-apps
---
download.cc | 2 +-
res.rc | 1 +
resource.h | 1 +
3 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/download.cc b/download.cc
index a430f7f..841f680 100644
--- a/download.cc
+++ b/download.cc
@@ -266,7 +266,7 @@ do_download_thread (HINSTANCE h, HWND owner)
if (source == IDC_SOURCE_DOWNLOAD)
{
if (errors)
- Logger ().setExitMsg (IDS_DOWNLOAD_INCOMPLETE);
+ Logger ().setExitMsg (IDS_DOWNLOAD_INCOMPLETE_EXIT);
else if (!unattended_mode)
Logger ().setExitMsg (IDS_DOWNLOAD_COMPLETE);
return IDD_DESKTOP;
diff --git a/res.rc b/res.rc
index 76a871f..d1f0871 100644
--- a/res.rc
+++ b/res.rc
@@ -526,6 +526,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_DOWNLOAD_INCOMPLETE_EXIT "Download incomplete. Check %s for details"
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 172b2c8..98a4a0f 100644
--- a/resource.h
+++ b/resource.h
@@ -40,6 +40,7 @@
#define IDS_ELEVATED 139
#define IDS_INSTALLEDB_VERSION 140
#define IDS_TRUSTSYNC_TOOLTIP 141
+#define IDS_DOWNLOAD_INCOMPLETE_EXIT 142
// Dialogs
--
2.15.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH setup 4/5] Query user after download error in interactive mode
2017-11-10 14:43 [PATCH setup 0/5] Improve behavior after download error, v2 Ken Brown
` (2 preceding siblings ...)
2017-11-10 14:43 ` [PATCH setup 3/5] Remove "Try again?" from exit message Ken Brown
@ 2017-11-10 14:43 ` Ken Brown
2017-11-10 14:43 ` [PATCH setup 5/5] Limit the number of packages shown in the IDD_DOWNLOAD_ERROR listbox Ken Brown
2017-11-13 11:51 ` [PATCH setup 0/5] Improve behavior after download error, v2 Jon Turney
5 siblings, 0 replies; 7+ messages in thread
From: Ken Brown @ 2017-11-10 14:43 UTC (permalink / raw)
To: cygwin-apps
Instead of just giving the user a "Try again?" Yes/No choice that goes
to IDD_SITE on Yes, create a dialog IDD_DOWNLOAD_ERROR with the
following choices: 'Retry' (retry the download), 'Back' (return to
IDD_CHOOSE), 'Continue' (ignore the errors), or 'Cancel' (exit).
The dialog lists the packages that had download errors so that the
user can make an informed choice.
Users who liked the old behavior (IDD_SITE) can select Back twice.
---
download.cc | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
res.rc | 22 ++++++++++++++++++
resource.h | 2 ++
3 files changed, 95 insertions(+), 5 deletions(-)
diff --git a/download.cc b/download.cc
index 841f680..b059bf5 100644
--- a/download.cc
+++ b/download.cc
@@ -25,6 +25,7 @@
#include <stdio.h>
#include <unistd.h>
#include <process.h>
+#include <vector>
#include "resource.h"
#include "msg.h"
@@ -182,16 +183,65 @@ download_one (packagesource & pkgsource, HWND owner)
}
if (success)
return 0;
- /* FIXME: Do we want to note this? if so how? */
return 1;
}
+static std::vector <packageversion> download_failures;
+static std::string download_warn_pkgs;
+
+static INT_PTR CALLBACK
+download_error_proc (HWND h, UINT message, WPARAM wParam, LPARAM lParam)
+{
+ switch (message)
+ {
+ case WM_INITDIALOG:
+ eset (h, IDC_DOWNLOAD_EDIT, download_warn_pkgs);
+ SetFocus (GetDlgItem(h, IDRETRY));
+ return FALSE;
+
+ case WM_COMMAND:
+ switch (LOWORD (wParam))
+ {
+ case IDRETRY:
+ case IDC_BACK:
+ case IDIGNORE:
+ case IDABORT:
+ EndDialog (h, LOWORD (wParam));
+ default:
+ // Not reached.
+ return 0;
+ }
+
+ default:
+ // Not handled.
+ return FALSE;
+ }
+ return TRUE;
+}
+
+static int
+query_download_errors (HINSTANCE h, HWND owner)
+{
+ download_warn_pkgs = "";
+ Log (LOG_PLAIN) << "The following package(s) had download errors:" << endLog;
+ for (std::vector <packageversion>::const_iterator i = download_failures.begin (); i != download_failures.end (); i++)
+ {
+ packageversion pv = *i;
+ std::string pvs = pv.Name () + "-" + pv.Canonical_version ();
+ Log (LOG_PLAIN) << " " << pvs << endLog;
+ download_warn_pkgs += pvs + "\r\n";
+ }
+ return DialogBox (h, MAKEINTRESOURCE (IDD_DOWNLOAD_ERROR), owner,
+ download_error_proc);
+}
+
static int
do_download_thread (HINSTANCE h, HWND owner)
{
int errors = 0;
total_download_bytes = 0;
total_download_bytes_sofar = 0;
+ download_failures.clear ();
Progress.SetText1 ("Checking for packages to download...");
Progress.SetText2 ("");
@@ -235,6 +285,8 @@ do_download_thread (HINSTANCE h, HWND owner)
int e = 0;
e += download_one (*version.source(), owner);
errors += e;
+ if (e)
+ download_failures.push_back (version);
#if 0
if (e)
pkg->action = ACTION_ERROR;
@@ -246,21 +298,35 @@ do_download_thread (HINSTANCE h, HWND owner)
{
// In unattended mode we retry the download, but not forever.
static int retries = 5;
+ int rc;
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);
+ rc = IDABORT;
}
else if (unattended_mode)
{
Log (LOG_PLAIN) << "download error in unattended_mode: " << retries
<< (retries > 1 ? " retries" : " retry") << " remaining." << endLog;
+ rc = IDRETRY;
+ }
+ else
+ rc = query_download_errors (h, owner);
+ switch (rc)
+ {
+ case IDRETRY:
Progress.SetActivateTask (WM_APP_START_DOWNLOAD);
return IDD_INSTATUS;
+ case IDC_BACK:
+ return IDD_CHOOSE;
+ case IDABORT:
+ Logger ().setExitMsg (IDS_DOWNLOAD_INCOMPLETE_EXIT);
+ Logger ().exit (1);
+ case IDIGNORE:
+ break;
+ default:
+ break;
}
- else if (yesno (owner, IDS_DOWNLOAD_INCOMPLETE) == IDYES)
- return IDD_SITE;
}
if (source == IDC_SOURCE_DOWNLOAD)
diff --git a/res.rc b/res.rc
index d1f0871..62fbe40 100644
--- a/res.rc
+++ b/res.rc
@@ -416,6 +416,28 @@ BEGIN
END
+IDD_DOWNLOAD_ERROR DIALOG DISCARDABLE 0, 0, SETUP_STANDARD_DIALOG_DIMS
+STYLE DS_MODALFRAME | DS_CENTER | WS_POPUP | WS_CAPTION
+CAPTION "Download Incomplete"
+FONT 8, "MS Shell Dlg"
+BEGIN
+ ICON IDI_WARNING,IDC_HEADICON,10,10
+ LTEXT "The following package(s) had download errors:",
+ IDC_STATIC,7,8,320,16
+ EDITTEXT IDC_DOWNLOAD_EDIT,7,24,320,88,WS_VSCROLL |
+ ES_LEFT | ES_MULTILINE | ES_READONLY |
+ ES_AUTOVSCROLL
+ LTEXT "Select 'Retry' to retry the download, "
+ "'Back' to return to the package selection page, "
+ "'Continue' to go on anyway (NOT RECOMMENDED), or "
+ "'Cancel' to exit.",
+ IDC_STATIC,7,120,320,24
+ DEFPUSHBUTTON "&Retry",IDRETRY,45,150,50,15
+ PUSHBUTTON "&Back",IDC_BACK,110,150,50,15
+ PUSHBUTTON "&Continue",IDIGNORE,175,150,50,15
+ PUSHBUTTON "Cancel",IDABORT,240,150,50,15
+END
+
IDD_POSTINSTALL DIALOG DISCARDABLE 0, 0, SETUP_STANDARD_DIALOG_W, 142
STYLE DS_MODALFRAME | DS_3DLOOK | DS_CENTER | WS_CHILD | WS_VISIBLE |
WS_CAPTION | WS_SYSMENU
diff --git a/resource.h b/resource.h
index 98a4a0f..c04c13d 100644
--- a/resource.h
+++ b/resource.h
@@ -68,6 +68,7 @@
#define IDD_DROPPED 221
#define IDD_POSTINSTALL 222
#define IDD_FILE_INUSE 223
+#define IDD_DOWNLOAD_ERROR 224
// Bitmaps
@@ -178,3 +179,4 @@
#define IDC_FILE_INUSE_HELP 592
#define IDC_NET_DIRECT_LEGACY 593
#define IDC_CHOOSE_SYNC 594
+#define IDC_DOWNLOAD_EDIT 595
--
2.15.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH setup 0/5] Improve behavior after download error, v2
@ 2017-11-10 14:43 Ken Brown
2017-11-10 14:43 ` [PATCH setup 1/5] Just retry download after error in unattended mode Ken Brown
` (5 more replies)
0 siblings, 6 replies; 7+ messages in thread
From: Ken Brown @ 2017-11-10 14:43 UTC (permalink / raw)
To: cygwin-apps
Currently setup goes back to the mirror selection page after a
download error if the user answers "Yes" to "Download incomplete. Try
again?". The same happens in unattended mode until the retries have
been exhausted.
And if the user answers "No", then installation continues, even though
this can damage the user's installation. For example, if a package is
selected for reinstall but cannot be downloaded, it will be
uninstalled.
This series of patches changes the behavior as follows:
- In unattended mode, simply retry the download.
- In interactive mode, pop up a dialog showing which packages had
download errors and giving the user the following options:
- Retry (retries download)
- Back (return to the package selection page)
- Continue, with a warning
- Cancel (exit)
Ken Brown (5):
Just retry download after error in unattended mode
Fix off-by-one error in download retry report
Remove "Try again?" from exit message.
Query user after download error in interactive mode
Limit the number of packages shown in the IDD_DOWNLOAD_ERROR listbox
download.cc | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++---------
res.rc | 23 ++++++++++++++
resource.h | 3 ++
3 files changed, 111 insertions(+), 14 deletions(-)
--
2.15.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH setup 1/5] Just retry download after error in unattended mode
2017-11-10 14:43 [PATCH setup 0/5] Improve behavior after download error, v2 Ken Brown
@ 2017-11-10 14:43 ` Ken Brown
2017-11-10 14:43 ` [PATCH setup 2/5] Fix off-by-one error in download retry report Ken Brown
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Ken Brown @ 2017-11-10 14:43 UTC (permalink / raw)
To: cygwin-apps
After a download error, setup was going back to IDD_SITE. This is
pointless in unattended mode, since no changes in the mirrors or
packages can be made.
Change misleading comment about retries in unattended mode; the Yes/No
dialog is not used in that case.
---
download.cc | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/download.cc b/download.cc
index e561c24..f6aa6fc 100644
--- a/download.cc
+++ b/download.cc
@@ -244,11 +244,7 @@ do_download_thread (HINSTANCE h, HWND owner)
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. */
+ // In unattended mode we retry the download, but not forever.
static int retries = 4;
if (unattended_mode && retries-- <= 0)
{
@@ -260,7 +256,8 @@ do_download_thread (HINSTANCE h, HWND owner)
{
Log (LOG_PLAIN) << "download error in unattended_mode: " << retries
<< (retries > 1 ? " retries" : " retry") << " remaining." << endLog;
- return IDD_SITE;
+ Progress.SetActivateTask (WM_APP_START_DOWNLOAD);
+ return IDD_INSTATUS;
}
else if (yesno (owner, IDS_DOWNLOAD_INCOMPLETE) == IDYES)
return IDD_SITE;
--
2.15.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH setup 0/5] Improve behavior after download error, v2
2017-11-10 14:43 [PATCH setup 0/5] Improve behavior after download error, v2 Ken Brown
` (4 preceding siblings ...)
2017-11-10 14:43 ` [PATCH setup 5/5] Limit the number of packages shown in the IDD_DOWNLOAD_ERROR listbox Ken Brown
@ 2017-11-13 11:51 ` Jon Turney
5 siblings, 0 replies; 7+ messages in thread
From: Jon Turney @ 2017-11-13 11:51 UTC (permalink / raw)
To: cygwin-apps; +Cc: Ken Brown
On 10/11/2017 14:43, Ken Brown wrote:
> Currently setup goes back to the mirror selection page after a
> download error if the user answers "Yes" to "Download incomplete. Try
> again?". The same happens in unattended mode until the retries have
> been exhausted.
>
> And if the user answers "No", then installation continues, even though
> this can damage the user's installation. For example, if a package is
> selected for reinstall but cannot be downloaded, it will be
> uninstalled.
>
> This series of patches changes the behavior as follows:
>
> - In unattended mode, simply retry the download.
>
> - In interactive mode, pop up a dialog showing which packages had
> download errors and giving the user the following options:
> - Retry (retries download)
> - Back (return to the package selection page)
> - Continue, with a warning
> - Cancel (exit)
Thanks. This looks great. Please apply to master.
> Ken Brown (5):
> Just retry download after error in unattended mode
> Fix off-by-one error in download retry report
> Remove "Try again?" from exit message.
> Query user after download error in interactive mode
> Limit the number of packages shown in the IDD_DOWNLOAD_ERROR listbox
You can drop this last one. I don't think you need to limit the list if
we have a scrollable textbox to show it in.
I was just pointing out that the list could be large, because I was
assuming use of a MessageBox(), but you've sidestepped that since you
are using a custom dialog.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-11-13 11:51 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-10 14:43 [PATCH setup 0/5] Improve behavior after download error, v2 Ken Brown
2017-11-10 14:43 ` [PATCH setup 1/5] Just retry download after error in unattended mode Ken Brown
2017-11-10 14:43 ` [PATCH setup 2/5] Fix off-by-one error in download retry report Ken Brown
2017-11-10 14:43 ` [PATCH setup 3/5] Remove "Try again?" from exit message Ken Brown
2017-11-10 14:43 ` [PATCH setup 4/5] Query user after download error in interactive mode Ken Brown
2017-11-10 14:43 ` [PATCH setup 5/5] Limit the number of packages shown in the IDD_DOWNLOAD_ERROR listbox Ken Brown
2017-11-13 11:51 ` [PATCH setup 0/5] Improve behavior after download error, v2 Jon Turney
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).