| Message ID | 20200427234353.4246-1-pedrocga1@gmail.com |
|---|---|
| State | Rejected, archived |
| Headers | show |
| Series | [pacman-dev] Adds a "retry?" prompt for when transaction creation fails for user-friendlier fail support. | expand |
Hi, > From what I could see, the function trans_init that comes right before the prompt has already specific messages > for each operation, so I consider that this new implementation fits the purpose. > My motivation to submit this patch is that it's pretty common for AUR-helpers to automatically invoke pacman after a build, > but correct transaction error handling is quite rare. So I thought that to implement a handling mechanism in the user side > would solve the problem not only in that case, but also for other scripts that invoke pacman. This description makes me doubt as to whether this is something that pacman needs, but rather something that these AUR-helpers may want? I also wonder if it'd be friendlier to notify the user the *reason* why the transaction failed, so they can better decide whether to try again or not. Cheers! -Santiago > Signed-off-by: Pedro <pedrocga1@gmail.com> > --- > src/pacman/remove.c | 13 ++++++++++--- > src/pacman/sync.c | 13 ++++++++++--- > src/pacman/upgrade.c | 13 ++++++++++--- > 3 files changed, 30 insertions(+), 9 deletions(-) > > diff --git a/src/pacman/remove.c b/src/pacman/remove.c > index 9920f502..66bb100d 100644 > --- a/src/pacman/remove.c > +++ b/src/pacman/remove.c > @@ -87,9 +87,16 @@ int pacman_remove(alpm_list_t *targets) > } > > /* Step 0: create a new transaction */ > - if(trans_init(config->flags, 0) == -1) { > - return 1; > - } > + while(1) { > + if(trans_init(config->flags, 1) == -1) { > + if(config->noconfirm || yesno(_("Retry the operation with the same flags?")) == 0) { > + return 1; > + } > + /* Try again */ > + continue; > + } > + break; > + } > > /* Step 1: add targets to the created transaction */ > for(i = targets; i; i = alpm_list_next(i)) { > diff --git a/src/pacman/sync.c b/src/pacman/sync.c > index f7dcb958..e472d89a 100644 > --- a/src/pacman/sync.c > +++ b/src/pacman/sync.c > @@ -694,9 +694,16 @@ static int sync_trans(alpm_list_t *targets) > alpm_list_t *i; > > /* Step 1: create a new transaction... */ > - if(trans_init(config->flags, 1) == -1) { > - return 1; > - } > + while(1) { > + if(trans_init(config->flags, 1) == -1) { > + if(config->noconfirm == 1 || yesno(_("Retry the operation with the same flags?")) == 0) { > + return 1; > + } > + /* Try again */ > + continue; > + } > + break; > + } > > /* process targets */ > for(i = targets; i; i = alpm_list_next(i)) { > diff --git a/src/pacman/upgrade.c b/src/pacman/upgrade.c > index 5f984e44..2ea74618 100644 > --- a/src/pacman/upgrade.c > +++ b/src/pacman/upgrade.c > @@ -79,9 +79,16 @@ int pacman_upgrade(alpm_list_t *targets) > } > > /* Step 1: create a new transaction */ > - if(trans_init(config->flags, 1) == -1) { > - retval = 1; > - goto fail_free; > + while(1) { > + if(trans_init(config->flags, 1) == -1) { > + if(config-->noconfirm || yesno(_("Retry the operation with the same flags?")) == 0) { > + retval = 1; > + goto fail_free; > + } > + /* Try again */ > + continue; > + } > + break; > } > > printf(_("loading packages...\n")); > -- > 2.26.2
On 28/4/20 9:43 am, Pedro wrote: > EDIT: Rewrote the prompt message and added a check for the --noconfirm flag > >>From what I could see, the function trans_init that comes right before the prompt has already specific messages > for each operation, so I consider that this new implementation fits the purpose. > My motivation to submit this patch is that it's pretty common for AUR-helpers to automatically invoke pacman after a build, > but correct transaction error handling is quite rare. So I thought that to implement a handling mechanism in the user side > would solve the problem not only in that case, but also for other scripts that invoke pacman. > > Signed-off-by: Pedro <pedrocga1@gmail.com> > --- I don't see how retrying the same operation with the same input is going to help anyone. If pacman fails in these ways, it is not going to succeed the next attempt. Allan > src/pacman/remove.c | 13 ++++++++++--- > src/pacman/sync.c | 13 ++++++++++--- > src/pacman/upgrade.c | 13 ++++++++++--- > 3 files changed, 30 insertions(+), 9 deletions(-) > > diff --git a/src/pacman/remove.c b/src/pacman/remove.c > index 9920f502..66bb100d 100644 > --- a/src/pacman/remove.c > +++ b/src/pacman/remove.c > @@ -87,9 +87,16 @@ int pacman_remove(alpm_list_t *targets) > } > > /* Step 0: create a new transaction */ > - if(trans_init(config->flags, 0) == -1) { > - return 1; > - } > + while(1) { > + if(trans_init(config->flags, 1) == -1) { > + if(config->noconfirm || yesno(_("Retry the operation with the same flags?")) == 0) { > + return 1; > + } > + /* Try again */ > + continue; > + } > + break; > + } > > /* Step 1: add targets to the created transaction */ > for(i = targets; i; i = alpm_list_next(i)) { > diff --git a/src/pacman/sync.c b/src/pacman/sync.c > index f7dcb958..e472d89a 100644 > --- a/src/pacman/sync.c > +++ b/src/pacman/sync.c > @@ -694,9 +694,16 @@ static int sync_trans(alpm_list_t *targets) > alpm_list_t *i; > > /* Step 1: create a new transaction... */ > - if(trans_init(config->flags, 1) == -1) { > - return 1; > - } > + while(1) { > + if(trans_init(config->flags, 1) == -1) { > + if(config->noconfirm == 1 || yesno(_("Retry the operation with the same flags?")) == 0) { > + return 1; > + } > + /* Try again */ > + continue; > + } > + break; > + } > > /* process targets */ > for(i = targets; i; i = alpm_list_next(i)) { > diff --git a/src/pacman/upgrade.c b/src/pacman/upgrade.c > index 5f984e44..2ea74618 100644 > --- a/src/pacman/upgrade.c > +++ b/src/pacman/upgrade.c > @@ -79,9 +79,16 @@ int pacman_upgrade(alpm_list_t *targets) > } > > /* Step 1: create a new transaction */ > - if(trans_init(config->flags, 1) == -1) { > - retval = 1; > - goto fail_free; > + while(1) { > + if(trans_init(config->flags, 1) == -1) { > + if(config-->noconfirm || yesno(_("Retry the operation with the same flags?")) == 0) { > + retval = 1; > + goto fail_free; > + } > + /* Try again */ > + continue; > + } > + break; > } > > printf(_("loading packages...\n")); >
On 4/28/20 8:40 PM, Allan McRae wrote: > On 28/4/20 9:43 am, Pedro wrote: >> EDIT: Rewrote the prompt message and added a check for the --noconfirm flag >> >> >From what I could see, the function trans_init that comes right before the prompt has already specific messages >> for each operation, so I consider that this new implementation fits the purpose. >> My motivation to submit this patch is that it's pretty common for AUR-helpers to automatically invoke pacman after a build, >> but correct transaction error handling is quite rare. So I thought that to implement a handling mechanism in the user side >> would solve the problem not only in that case, but also for other scripts that invoke pacman. >> >> Signed-off-by: Pedro <pedrocga1@gmail.com> >> --- > > I don't see how retrying the same operation with the same input is going > to help anyone. If pacman fails in these ways, it is not going to > succeed the next attempt. > > Allan Just like I said the first time this patch was submitted -- and now it just has a slightly longer "retry" message, but it still doesn't answer my question "what is the user supposed to do about it"? Pedro, a AUR helper is most likely going to fail because of dependency checks, which happen later. Do you have examples of a situation where pacman cannot even create the transaction, let alone populate it with packages? If so, why is it happening? These are questions that should be answered before adding a prompt with no clear purpose.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 On Wed, 29 Apr 2020, Allan McRae wrote: > On 28/4/20 9:43 am, Pedro wrote: >> EDIT: Rewrote the prompt message and added a check for the --noconfirm flag >> >>> From what I could see, the function trans_init that comes right before the prompt has already specific messages >> for each operation, so I consider that this new implementation fits the purpose. >> My motivation to submit this patch is that it's pretty common for AUR-helpers to automatically invoke pacman after a build, >> but correct transaction error handling is quite rare. So I thought that to implement a handling mechanism in the user side >> would solve the problem not only in that case, but also for other scripts that invoke pacman. >> >> Signed-off-by: Pedro <pedrocga1@gmail.com> >> --- > > I don't see how retrying the same operation with the same input is going > to help anyone. If pacman fails in these ways, it is not going to > succeed the next attempt. I repeatedly get download errors due to slow initial speed or whatnot (updating 20+ clients simultanously seems to overload my local mirrors somehow) - a "retry" option would indeed make a difference there. However, I think, such a functionality should be provided by whoever calls pacman. regards, Erich -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEE3p92iMrPBP64GmxZCu7JB1Xae1oFAl6pLYEACgkQCu7JB1Xa e1rMyBAAsyaighGgMenA3CMaJQftB2SQ6BYb1r4OgDZCAx97mpyAD/6JwWrskp5k d5i2RwGV0d98DS+7khFOchpnOE4gYjW3uaIXUtSoL6MOU9OReehVcn9Fy/O2rZjc hD/UUnH28HLx2ao7pg5MOuY2mxqW0X0REYScXHwcr7yVQM6DzkNEoRwqD5B9OPeY xPZAFPcIOiJ9kSDQtGoTbm16RLydpJrf8eS67hThHeav+SUs8pWIkFdqWKkYkg6b TZbrIvnUVFFFnov4rEe62tJ72xRAtkP4yp8dcIEbetguqc2i4liIlJCJv8yWllIo RWgHyHUBll4WMLowCBzfFBl6fQ+k5Frd4TdHP6e3h6nFvVjrlAlgZfzgo0S7XQVF KPFtHchcvvsFbaZt4/B2vCPZ/caGYfximXL/212oobAWyDwIdXmWLIeSE0sVQ/a6 HwHUs4WEC5MdPyoHAJCeW8zo8dg4S97NXOycJ2IlRw5fww1X/L7LbLGoiz9H99jk PGUWa7NEfg5v2tWkjSPiUAFCAf9mSKtG2p8F3WKMguSUWDT8Kv8pPyeKjNRRjWN+ cUAI8H/tWw9n2REWlwbIDlPSvrCepWvVhYONHSdkq3rjdkvqwzvwviRgRn0fB0nc yYub93IPGLIcwdBQlIXQz9G6paZuqJAzNgDH68H2draQvgCcTQY= =2IVH -----END PGP SIGNATURE-----
diff --git a/src/pacman/remove.c b/src/pacman/remove.c index 9920f502..66bb100d 100644 --- a/src/pacman/remove.c +++ b/src/pacman/remove.c @@ -87,9 +87,16 @@ int pacman_remove(alpm_list_t *targets) } /* Step 0: create a new transaction */ - if(trans_init(config->flags, 0) == -1) { - return 1; - } + while(1) { + if(trans_init(config->flags, 1) == -1) { + if(config->noconfirm || yesno(_("Retry the operation with the same flags?")) == 0) { + return 1; + } + /* Try again */ + continue; + } + break; + } /* Step 1: add targets to the created transaction */ for(i = targets; i; i = alpm_list_next(i)) { diff --git a/src/pacman/sync.c b/src/pacman/sync.c index f7dcb958..e472d89a 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -694,9 +694,16 @@ static int sync_trans(alpm_list_t *targets) alpm_list_t *i; /* Step 1: create a new transaction... */ - if(trans_init(config->flags, 1) == -1) { - return 1; - } + while(1) { + if(trans_init(config->flags, 1) == -1) { + if(config->noconfirm == 1 || yesno(_("Retry the operation with the same flags?")) == 0) { + return 1; + } + /* Try again */ + continue; + } + break; + } /* process targets */ for(i = targets; i; i = alpm_list_next(i)) { diff --git a/src/pacman/upgrade.c b/src/pacman/upgrade.c index 5f984e44..2ea74618 100644 --- a/src/pacman/upgrade.c +++ b/src/pacman/upgrade.c @@ -79,9 +79,16 @@ int pacman_upgrade(alpm_list_t *targets) } /* Step 1: create a new transaction */ - if(trans_init(config->flags, 1) == -1) { - retval = 1; - goto fail_free; + while(1) { + if(trans_init(config->flags, 1) == -1) { + if(config-->noconfirm || yesno(_("Retry the operation with the same flags?")) == 0) { + retval = 1; + goto fail_free; + } + /* Try again */ + continue; + } + break; } printf(_("loading packages...\n"));
EDIT: Rewrote the prompt message and added a check for the --noconfirm flag From what I could see, the function trans_init that comes right before the prompt has already specific messages for each operation, so I consider that this new implementation fits the purpose. My motivation to submit this patch is that it's pretty common for AUR-helpers to automatically invoke pacman after a build, but correct transaction error handling is quite rare. So I thought that to implement a handling mechanism in the user side would solve the problem not only in that case, but also for other scripts that invoke pacman. Signed-off-by: Pedro <pedrocga1@gmail.com> --- src/pacman/remove.c | 13 ++++++++++--- src/pacman/sync.c | 13 ++++++++++--- src/pacman/upgrade.c | 13 ++++++++++--- 3 files changed, 30 insertions(+), 9 deletions(-)