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
