darcs

Patch 1333 resolve issue 2327

Title resolve issue 2327
Superseder Nosy List alain91
Related Issues
Status accepted Assigned To
Milestone

Created on 2015-05-07.21:38:27 by alain91, last changed 2015-06-23.22:57:25 by bfrk.

Files
File name Status Uploaded Type Edit Remove
Lock.diff alain91, 2015-05-07.21:38:26 application/octet-stream
patch-preview.txt bfrk, 2015-06-13.21:36:34 text/x-darcs-patch
resolve-issue2327_-ask-user-to-keep-trying-if-locking-fails.dpatch bfrk, 2015-06-13.21:36:34 application/x-darcs-patch
unnamed bfrk, 2015-06-13.21:36:34
unnamed alain91, 2015-06-23.20:21:32 text/html
See mailing list archives for discussion on individual patches.
Messages
msg18445 (view) Author: bfrk Date: 2015-06-11.16:59:48
Changed the title so that roundup (hopefully) recognizes the issue number.
msg18475 (view) Author: bfrk Date: 2015-06-13.13:31:56
I think there are two cases to consider here:

If the lock is only temporarily unavailable, the proposed solution is nice.

But if the lock is unavailable due to some error or due to a
misconfiguration, I think it is not realistic to expect the user to keep
darcs hanging in the loop all the time it takes to resolve any
underlying issue.

So: yes, this is a partial solution but we should also offer a way to
save the current selection to a file which can then be read in by a
subsequent record. Similar for other commands that require patch selection.
msg18486 (view) Author: alain91 Date: 2015-06-13.20:42:35
1/ the patch fix the problem describe in issue
2/ I would have a look for an improvement and offer to user the choice 
to save selection in file. And then a command to reply from file.
msg18487 (view) Author: alain91 Date: 2015-06-13.20:43:28
1/ the patch fix the problem describe in issue2327
2/ I would have a look for an improvement and offer to user the choice 
to save selection in file. And then a command to reply from file.
msg18488 (view) Author: bfrk Date: 2015-06-13.21:24:39
Re: 1/ the patch fix the problem describe in issue2327

The repo locking code looks as if it does not "instantly give up" but
rather tries to lock for a number of times (30) using a delay of 2
seconds in between. That means it should try to get the lock for one
minute before giving up. Is that consistent with your experience?

Note: I am minded to accept the patch, just want to understand the
issues involved a bit better.

BTW, what was the reason for the failed lock in your case?
msg18489 (view) Author: bfrk Date: 2015-06-13.21:36:34
Converted this to a darcs patch.

BTW: you can use 'darcs send -O' to create darcs patch bundle with an
auto-generated file name, then attach the file to an email and send it with
your favourite mail program.

1 patch for repository http://darcs.net/screened:

patch b30f729b3499d31e1beb461cec229f4bf5e8c4ee
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Sat Jun 13 23:30:53 CEST 2015
  * resolve issue2327: ask user to keep trying if locking fails
Attachments
msg18580 (view) Author: ganesh Date: 2015-06-22.05:39:21
Seems reasonable. One slight worry is that could cause problems for 
scripts that weren't expecting darcs to ask for user input at this 
point and would have preferred it to just fail. But I think that's 
marginal.
msg18592 (view) Author: alain91 Date: 2015-06-22.19:08:14
What about using the --(no-)force option to allow a script to control 
the behaviour of the command ?

or creating a new option --no-asking ?
msg18594 (view) Author: bfrk Date: 2015-06-22.20:04:47
Adding an extra option means such scripts have to be changed to add the
new option. If they have to do this, they can as well change the call to

 yes|darcs <args>

Given that each additional option makes the UI more complex, I think the
net win is negative.

However: we could decide to ask the user only if --interactive is in
effect (or rather, if isInteractive returns True). This would cover all
scenarios where the user gets hurt by darcs giving up, while not
punishing scripts, at least not those that turn interactivity off
(either explicity or implicitly).
msg18610 (view) Author: ganesh Date: 2015-06-23.06:43:52
I'm marking this as accepted as the current situation is good enough, but 
feel free to add a followup to either this patch or as a fresh one.
msg18613 (view) Author: alain91 Date: 2015-06-23.20:21:32
It seems difficult to add access to /"isInteractive" /because this 
function needs the list of flags as argument. But options list are not 
accessible in the arguments.

I don't want to destabilize the code by adding new argument to manage 
/"isInteractive"

/I'd rather to stay with the current patch//and I stop to investigate on 
this issue.


Le 22/06/2015 22:04, Ben Franksen a écrit :
> Ben Franksen <benjamin.franksen@helmholtz-berlin.de> added the comment:
>
> Adding an extra option means such scripts have to be changed to add the
> new option. If they have to do this, they can as well change the call to
>
>   yes|darcs <args>
>
> Given that each additional option makes the UI more complex, I think the
> net win is negative.
>
> However: we could decide to ask the user only if --interactive is in
> effect (or rather, if isInteractive returns True). This would cover all
> scenarios where the user gets hurt by darcs giving up, while not
> punishing scripts, at least not those that turn interactivity off
> (either explicity or implicitly).
>
> __________________________________
> Darcs bug tracker <bugs@darcs.net>
> <http://bugs.darcs.net/patch1333>
> __________________________________
Attachments
msg18616 (view) Author: bfrk Date: 2015-06-23.22:57:25
Yes, the thought occurred to me, too. Anyway the correct solution would
be to evaluate isInteractive in the command implementations
(Darcs.UI.Command.*) and pass on a boolean flag (or perhaps a new Type
in Darcs.Repository.Flags) to the Repository functions. That would
indeed mean many, many changes with small benefit.
History
Date User Action Args
2015-05-07 21:38:27alain91create
2015-05-07 21:38:45alain91setassignedto: alain91
nosy: + alain91
2015-05-09 20:25:36alain91setassignedto: alain91 ->
2015-06-11 16:59:49bfrksetmessages: + msg18445
title: resolve issue 2327 -> resolve issue2327
2015-06-13 13:31:57bfrksetstatus: needs-screening -> in-discussion
messages: + msg18475
2015-06-13 20:42:35alain91setmessages: + msg18486
2015-06-13 20:43:28alain91setmessages: + msg18487
2015-06-13 21:24:40bfrksetmessages: + msg18488
2015-06-13 21:36:34bfrksetfiles: + patch-preview.txt, resolve-issue2327_-ask-user-to-keep-trying-if-locking-fails.dpatch, unnamed
messages: + msg18489
title: resolve issue2327 -> resolve issue 2327
2015-06-21 20:57:08bfrksetstatus: in-discussion -> needs-review
2015-06-22 05:39:22ganeshsetstatus: needs-review -> accepted-pending-tests
messages: + msg18580
2015-06-22 19:08:14alain91setmessages: + msg18592
2015-06-22 20:04:47bfrksetmessages: + msg18594
2015-06-23 06:43:52ganeshsetstatus: accepted-pending-tests -> accepted
messages: + msg18610
2015-06-23 20:21:33alain91setfiles: + unnamed
messages: + msg18613
2015-06-23 22:57:25bfrksetmessages: + msg18616