darcs

Issue 2042 repair should be a subcommand of check

Title repair should be a subcommand of check
Priority wishlist Status resolved
Milestone Resolved in
Superseder Nosy List gh, jaredj, mndrix
Assigned To
Topics ProbablyEasy, UI

Created on 2011-02-10.12:48:14 by gh, last changed 2012-03-31.15:53:31 by gh.

Messages
msg13662 (view) Author: gh Date: 2011-02-10.12:48:11
I suggest to make the repair command a subcommand of check, that should
be invoked with "darcs check --repair".

Conceptually, repairing something is a proper supertask of checking
whether it is broken. Otherwise, how could you know what to repair?

When it comes to the current darcs code, modules Darcs.Command.Check and
Darcs.Command.Repair have a lot of overlap, and Darcs.Commands.Repair is
quite small (without the boilerplate).

Making repair an option of check would only involve making sure tests
are not run (ie, --repair should imply --no-test).

This change would also get rid of the module Darcs.Command.Repair, for
the greatest cognitive relief of current and future darcs developers. :-)

And "darcs repair" should be added as an alias for "darcs check --repair".
msg14935 (view) Author: kowey Date: 2012-01-01.12:10:26
Counter-proposal: rename darcs check to darcs repair --dry-run

http://irclog.perlgeek.de/darcs/2012-01-01#i_4909698

12:06:13 - mornfall: Did we get to move darcs repair to darcs check --
repair, and trackdown to darcs test --trackdown / darcs test --bisect?
12:06:25 - mornfall: (And darcs check --test to darcs test.)
12:06:30 - kowey: no darcs test yet
12:06:52 - mornfall: (re issue2042)
12:06:52 - kowey: repair is still in help
12:06:58 - kowey: and check doesn't have the flag
12:07:19 - mornfall: I would support that reorganisation wrt 
checking/testing/repairing.
12:07:59 - kowey: not sure about the repair/check one, but I think we 
have broad agreement on darcs test: 
http://wiki.darcs.net/DefaultSwitches
12:08:20 - kowey: (but that's two of you on check --repair, I don't know 
either way)
12:08:25 - mornfall: Repair does sound superfluous. I would go as far as 
make --repair the default, btw.
12:08:33 - mornfall: And have check --no-repair an explicit command.
12:09:05 - kowey: hmm, that could get in the way of forensics
12:09:34 - kowey: another option
12:09:41 - kowey: rename darcs check to darcs repair --dry-run
12:10:03 - kowey: this avoids the "uh, what, I wasn't expecting you to 
actually do anything"
12:10:14 - kowey: because repair quite explicitly says (in its name) 
that it will do things
12:10:15 - mornfall: Sounds reasonable.
12:10:24 - mornfall: It also avoids the confusion between check and 
test.
msg14956 (view) Author: gh Date: 2012-01-04.12:37:23
I like the "darcs repair --dry-run" idea, indeed "repair"
distinguishes itself a lot more from a "test" subcommand than "check".

g.
msg14957 (view) Author: ml Date: 2012-01-05.06:28:12
On 01/01/2012 07:10 AM, Eric Kow wrote:
>
> Eric Kow<kowey@darcs.net>  added the comment:
>
> [...]
> 12:09:41 - kowey: rename darcs check to darcs repair --dry-run
> 12:10:03 - kowey: this avoids the "uh, what, I wasn't expecting you to
> actually do anything"
> 12:10:14 - kowey: because repair quite explicitly says (in its name)
> that it will do things
> [...]

On the flip side, I'm always afraid when I pass a --dry-run argument 
that it'll fail to do nothing somehow (e.g. via typo, or "dry-run" not 
actually being an argument for that command, or program that fails to 
implement --dry-run in a way that actually does nothing).  I think I 
must have been bitten by some program or I wouldn't be nervous...the 
nervousness is greater for commands like check/repair that I wouldn't 
run regularly and thus know how they behave.

It feels less dissonant in my mind to say (hyperbolized)
darcs look-at-problems
than
darcs fix-problems --no-never-mind-dont-actually-fix-problems
.

Also: Shouldn't check/repair be interactive by default, making no 
changes without a "y" answer?  And have "check", if it finds problems, 
suggest to type "darcs repair" (or perhaps ask "repair? y/n" directly)?

Yes, having two command names is unfortunate for a rare command.  But I 
think we can document them well enough in help text and interactive 
text.  Or merge it into one command name, at the expense of that "uh, 
what, I wasn't expecting you to actually do anything" effect because one 
name cannot serve both (valuable) purposes of sounding like it clearly 
has no effect and also sometimes having effects.  I'm inclined to think 
that if the commands asked first before making any changes, that would 
be good enough that we could choose any naming arrangement and be okay.

~

I just tried corrupting a simple test repo, and both check and repair said:

{{{
darcs: failed to read patch:
Thu Jan  5 00:33:24 EST 2012  Isaac Dupree <id@isaac.cedarswampstudios.org>
   * whatarv
Couldn't fetch 
`0000000137-c74a61479ee2523c27575190fc2e23ae9b50dd35fdcb47c51cd8fde2732a37ff'
in subdir patches from sources:

thisrepo:/Users/me/test/darrrr
cache:/Users/me/HOME/.darcs/cache
}}}

Repair should really state "giving up on repairing" at the least.  If it 
wanted to be informative it could then echo the text that's hidden at 
the bottom of repair --help, below all the generic advanced options, 
"[...] Currently [darcs repair] can only repair damage to the pristine 
tree, which is where most corruption occurs."

(I deleted a file in _darcs/patches/.)

% darcs --version
2.5.2 (release)

Worse, back in a consistent repo, I tried deleting a file in 
_darcs/pristine/, then

{{{
% darcs check
Looks like we have a difference...
Nothing

Inconsistent repository!

% darcs repair
The repository is already consistent, no changes made.

% darcs check
The repository is consistent!
}}}

LIES!!!

Do you need a test case?  This is a repo I just made up for the purpose 
that has one or two trivial patches in it.  IMHO, this is far more 
important than naming.  On further testing, it appears that 'darcs 
check' actually repaired the pristine tree without telling me (thus 
instantly rendering its "Inconsistent repository!" claim no longer 
true).  Do 'check' and 'repair' ever do anything non-identical (besides 
emitting different text to stdout)?
msg14958 (view) Author: ganesh Date: 2012-01-05.07:08:13
I tend to agree with the problems of repair --dry-run. I'm actually 
quite happy with two commands, but otherwise check --repair, or an 
interactive prompt, seems preferable. I'm not sure what to do about the 
ambiguity with testing, but for me check doesn't imply test and I've 
been surprised in the past when it did test.

Re check actually repairing, it's not supposed to, but there's a known 
bug in 2.5. I can't find an issue number, but it's fixed in HEAD by this 
patch: http://bugs.darcs.net/patch607
msg14959 (view) Author: kowey Date: 2012-01-05.09:17:39
Thanks, Isaac!

On 5 Jan 2012, at 06:28, Isaac Dupree wrote:
> On the flip side, I'm always afraid when I pass a --dry-run argument 
> that it'll fail to do nothing somehow (e.g. via typo, or "dry-run" not 
> actually being an argument for that command, or program that fails to 
> implement --dry-run in a way that actually does nothing).  I think I 
> must have been bitten by some program or I wouldn't be nervous...the 
> nervousness is greater for commands like check/repair that I wouldn't 
> run regularly and thus know how they behave.

What about existing commands that have a dry-run?  
What would make repair different?

The thing that makes the --dry-run option attractive is the idea of "few principles, broadly applicable" that darcs seems to use (eg. interactive patch selection, matchers everywhere).   Maybe I'm looking at it in too superficial a way?

> It feels less dissonant in my mind to say (hyperbolized)
> darcs look-at-problems
> than
> darcs fix-problems --no-never-mind-dont-actually-fix-problems

Hmm, makes me wonder if a darcs mark-conflicts --dry-run would be a good idea

As for the interactivity suggestion, maybe.  Darcs UI isn't interactive for interactiveness sake, though.  The interactiveness is mainly used as a device for navigating through a list of similar objects.  It's there so you can give darcs different instructions in a series for different subsets of the same thing.  Sure we also have a few "are you sure you want to do this?" style prompts (which I would like to do away with where possible).

Does it really add more than "now do X?"

-- 
Eric Kow <http://erickow.com>
msg14960 (view) Author: ml Date: 2012-01-05.17:57:58
On 01/05/2012 04:18 AM, Eric Kow wrote:
> Thanks, Isaac!
>
> On 5 Jan 2012, at 06:28, Isaac Dupree wrote:
>> On the flip side, I'm always afraid when I pass a --dry-run argument
>> that it'll fail to do nothing somehow (e.g. via typo, or "dry-run" not
>> actually being an argument for that command, or program that fails to
>> implement --dry-run in a way that actually does nothing).  I think I
>> must have been bitten by some program or I wouldn't be nervous...the
>> nervousness is greater for commands like check/repair that I wouldn't
>> run regularly and thus know how they behave.
>
> What about existing commands that have a dry-run?
> What would make repair different?

"repair" is something that people are likely to use when confused and 
terrified.  That's a reason I want to be extra careful about consent in 
its UI specifically.


Though on second thought, it would also be imperfect for 'repair' to ask 
the user.  I (a user) might think suspiciously "Wait, why am I being 
asked? Is there something dangerous about this?"[1]

You have a good point that my argument about "dry-run" is a general 
argument.  Dry-run features seem important to have, but I'm not sure 
what UI would make me happiest.  A common dry-run workflow is doing a 
dry-run command that will probably be followed immediately by a command 
to actually do it.  A flag like --ask-before-doing-anything would be 
more optimal for that workflow.[2]  Sadly it wouldn't be optimal for a 
workflow where I know I just want to look & would say "no, don't do 
anything".  Hmm, if *every* darcs command had a --dry-run option that 
made the command have no effects besides writing to stdout/stderr, that 
would help me trust the option more.[3]  And/or "darcs dry-run 
command-name" feels slightly better semantically to me, but not better 
enough to be worth the cost that that interface can't allow --dry-run at 
the end of the command-line where it is easier to add/delete while using 
shell history.


[1] *Is* there any possible danger here, by the way? Is it possible for 
'repair' to delete information or make things worse given the right 
combination of corruptions?  Whether that's true or false, are people 
going to think about it the same way they think about 'fsck', however 
that is?

[2] And it can be intrinsically safe against race conditions, unlike "do 
dry run, wait, do actual command" - what if something changed in 
between.  The default behavior of Linux command-line package managers 
like 'aptitude' and 'pacman' makes me happy - if you ask to install a 
package and it needs to do anything more than installing that one 
package, it shows you a list of everything it proposes to 
install/change, and their exact version numbers, and asks you whether 
you want to do that.

[3] Though I'm not sure what it would do with commands like interactive 
'record'.  Probably display warnings every step of the way saying that 
your choices will be discarded after the command is done. Hmph. Sounds 
like a worse extension of the fact that you can't save your 
hunk-selection work in regular interactive 'record', but that's an 
entirely different subject.
msg15357 (view) Author: gh Date: 2012-03-19.20:49:15
I'd like to go past the questions related to interactive UI and see if
we can go ahead with the idea of merging check and repair, in order to
produce something concrete from the discussions we already had here.

So, given that it would be nice to separate the purpose of the commands
repair and test, I suggest we make check a flag of repair (possibly
renaming it dry-run).  We will be able to continue discussing how the UI
will work later.
msg15365 (view) Author: mndrix Date: 2012-03-19.23:25:45
On Mon, Mar 19, 2012 at 2:49 PM, Guillaume Hoffmann <bugs@darcs.net> wrote:
> So, given that it would be nice to separate the purpose of the commands
> repair and test, I suggest we make check a flag of repair (possibly
> renaming it dry-run).  We will be able to continue discussing how the UI
> will work later.

I like this idea.  When first perusing darcs commands, these two
confused me.  I assumed they were doing something completely different
one from another and I just wasn't smart enough to see why they were
separate.  Having "repair --dry-run" would have been clearer to me.
msg15456 (view) Author: gh Date: 2012-03-31.15:53:30
Resolved now that check has becomre repair --dry-run.
History
Date User Action Args
2011-02-10 12:48:14ghcreate
2012-01-01 12:10:27koweysetmessages: + msg14935
2012-01-04 12:37:24ghsetmessages: + msg14956
2012-01-05 06:28:13mlsetmessages: + msg14957
2012-01-05 07:08:14ganeshsetmessages: + msg14958
2012-01-05 09:17:41koweysetmessages: + msg14959
2012-01-05 17:57:59mlsetmessages: + msg14960
2012-03-19 20:49:17ghsetmessages: + msg15357
2012-03-19 23:25:46mndrixsetmessages: + msg15365
2012-03-21 22:25:00mndrixsetnosy: + mndrix
2012-03-31 15:53:31ghsetstatus: unknown -> resolved
messages: + msg15456