Stefan Sperling wrote on Fri, Oct 26, 2012 at 12:45:49 +0200:
> On Fri, Oct 26, 2012 at 11:57:12AM +0200, Daniel Shahaf wrote:
> > Using terminology from <http://s.apache.org/xy-problem>: if X is "I am
> > not comfortable writings loops in DOS batch", I don't think applying
> > your patch to trunk is the correct Y.
>
> I still don't get why you're so much objected to this.
>
I objected to this because the only reason given so far for including
the patch was "It is easier to implement in C than in DOS batch",
I didn't consider that a good enough reason, and I didn't see any other
reason.
> I'd like to have this feature because it will make my work a little
> easier should I ever be tasked again with repairing broken repositories.
> When I last had to repair a couple of dozen revisions in a broken
> repository I was rather annoyed at the way 'svnadmin verify' currently
> works. The current behaviour of 'svnadmin verify' is not very useful for
> answering the question "which revisions are broken?". It only answers
> the question "which is the first broken revision?", which so far has
> never been sufficient information when I had to verify a repository.
>
OK, what information would you want?
> Usually I need to quickly assess the overall impact of repository damage,
> and also quickly assess the overall impact of fixes I make to repair the
> repository. The number of the first broken revision isn't enough information.
>
Right. But Julian already pointed out that the implementation of this
patch will report a single corruption multiple times, and I hinted at
one of my mails that the way to avoid this problem is to push the logic
into a DAG-aware tree walk. Have you actually tried this patch yet?
Can you get the information you need from svnadmin-with-this-patch
despite the shortcoming Julian has pointed out?
> We usually don't require people to write loops anywhere else to obtain
> useful behaviour from our command line tools.
> For example, 'svn diff' accepts multiple targets. By your logic it is
> just as easy to call it in a loop to diff all targets in turn, isn't it?
> But that would be just as annoying.
>
The first thing I asked in this thread is why everyone thinks it'll be
better to implement the loop in C than in Python bindings. I did not
have that opinion a priori, but I'm willing to be convinced. [and
that's what the rest of your mail is trying to do]
> A proper 'svnadmin verify' loop will also collect errors reported and
> show a summary at the end. That's additional overhead if coding this as
> a loop on the command line. The repos library can easily collect this
> information internally and pass it as the final notification.
I am not sure that I understand this argument.
> I won't require Prabhu's patch to print a summary but the functionality
> Prabhu is adding is a required step anyway if we want to print a summary.
> I'll happily add the summary part myself if nobody else does it.
>
Sounds like you're evaluating Prabhu's patch against a list of features
you'd like 'svnadmin verify' to grow someday. I think it would be
helpful if you shared that list --- it would let people share their
thoughts on the whole context/plan, not just on this patch.
> And I don't think that even a published wrapper script in our tools/
> directory would be as useful as having this feature built-in.
> If you're on some customer's server you're usually constrained and cannot
> install some random script interpreter. Not every Subversion server in
> the world has python or similar programs installed. But they all have
> the 'svnadmin' program.
Sure, "not all Unix servers have Python" is a fair point.
>
Thanks for your mail, you've made some good points.
Daniel
Received on 2012-10-26 19:08:26 CEST