Re: [PATCH] git-svn: allow disabling expensive broken symlink checks

[Eric Wong <normalperson@yhbt.net>]

Junio C Hamano <gitster@pobox.com> wrote:
> Eric Wong <normalperson@yhbt.net> writes:
> 
> > Since dbc6c74d0858d77e61e092a48d467e725211f8e9, git-svn has had
> > an expensive check for broken symlinks that exist in some
> > repositories.  This leads to a heavy performance hit on
> > repositories with many empty blobs that are not supposed to be
> > symlinks.
> >
> > The workaround is enabled by default; and may be disabled via:
> >
> >   git config svn.brokenSymlinkWorkaround false
> >
> > Reported by Markus Heidelberg.
> >
> > Signed-off-by: Eric Wong <normalperson@yhbt.net>
> 
> How common is this breakage in people's subversion repositories that
> dbc6c74d (git-svn: handle empty files marked as symlinks in SVN,
> 2009-01-11) works around?

It's not common at all.  Some broken Windows clients were able to
create it.

> What's the way to recover from a broken import, when the subversion
> repository does have such a breakage, and the user used git-svn that
> predates dbc6c74?  Is it very involved, and it is much better to have the
> safety by default than to force everybody else who interacts with
> non-broken subversion repository suffer from this performance penalty?

Previously, git-svn would just stop importing and refuse to continue.
So allowing the user to enable it would be a problem; too.  I don't
recall the error being easy to distinguish from other errors.

> Because the fix (that is broken from the performance angle) is relatively
> recent, I am wondering if it makes more sense to turn it off by default,
> and allow people with such a broken history to optionally turn it on.

I'm considering disabling it by default, too.  It only gets triggered if
there are any empty blobs at all in the repository (which are fairly
rare, but not as rare as broken symlinks in SVN).  So for a "normal"
repository it would just be the (low) overhead of calling ls-files for
every revision we fetch (and the hash-object </dev/null, which could
even be hard-coded).

Perhaps just having a "try enabling this ..." type option on any
fetch failure would be better...

-- 
Eric Wong