Re: [PATCH] cvsimport: rewrite to use cvsps 3.x to fix major bugs

[Junio C Hamano <gitster@pobox.com>]

"Eric S. Raymond" <esr@thyrsus.com> writes:

> Junio C Hamano <gitster@pobox.com>:
> ...
> The other is a design-level problem - these options were a bad idea to
> begin with.  In earlier list mail I said
>
>     An example of the batchiness mistake close to home is the -m and -M
>     options in the old version of cvsimport.  It takes human judgment
>     looking at the whole commit DAG in gitspace to decide what merge
>     points would best express the (as you say, sometimes ambiguous) CVS
>     history - what's needed is a scalpel and sutures in a surgeon's hand,
>     not a regexp hammer.
>
> One specific problem with the regexp hammer is false-positive matches
> leading to unintended merges.

Yeah, it is OK to _discourage_ its use, but to me it looks like that
the above is a fairly subjective policy decision, not something I
should let you impose on the users of the old cvsimport, which you
do not seem to even treat as your users.

>> Having the code to die when it sees options the rewritten version
>> does not yet support before it calls the fallback makes the fallback
>> much less effective, no?
>
> Only to the extent that -o/-m/-M are really important, which I doubt.
> But that might be fixable, and I'll put it on the to-do list.
>
>> Not very impressed (yet).  The advertised "fix major bugs" sounds
>> more like "trade major bugs with different ones with a couple of
>> feature removals" at this point.
>
> If you think that, you have failed to understand just how broken and
> dangerous the old combination is.  None of the details you've called
> out are "major" by any stretch of the imagination compared to it
> silently botching the translation of repositories.

The "major" in my sentence was from your description (the bugs you
fixed), and not about the new ones you still have in this draft.  I
did not mean to say that you are trading fixes to "major" bugs with
different "major" bugs.

Insecure quoting of parameters is much easier to fix; it does need
to be addressed, though.

It is just that looking at the state of the patch as submitted left
me with a feeling that this topic needs a lot more time to mature
than I previously was led to believe by your earlier messages, which
made me someaht sad.