Re: [PATCH v4 2/2] git-svn: allow git-svn fetching to work using serf

2013-07-18 Thread Jonathan Nieder
(cc-ing Eric Wong, who maintains git-svn and knows both it and
the libsvn perl bindings much better than I do)
Kyle J. McKay wrote:

 David Rothenberger daver...@acm.org has determined the cause to
 be that ra_serf does not drive the delta editor in a depth-first
 manner [...]. Instead, the calls come in this order:

Thanks.

Sorry to nitpick, but the problem is not depth-first versus
breadth-first versus random.  Blaming the traversal order makes this
completely confusing.  The actual problem is that the driver asks us
to keep multiple files open at a time.

The approach taken in this patch would be racy if the driver calls us
multiple times concurrently (since temp_acquire can fail).  I believe
it doesn't but haven't checked.

The approach is generally good.  I wanted to propose some clearer
documentation for temp_is_locked() but didn't end up finding a moment
for it, so... meh.  I'll be happy to help get the details right if
someone else finds time for that (hint, hint).

Hope that helps,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 2/2] git-svn: allow git-svn fetching to work using serf

2013-07-18 Thread Kyle J. McKay

On Jul 18, 2013, at 12:29, Jonathan Nieder wrote:

(cc-ing Eric Wong, who maintains git-svn and knows both it and
the libsvn perl bindings much better than I do)
Kyle J. McKay wrote:


David Rothenberger daver...@acm.org has determined the cause to
be that ra_serf does not drive the delta editor in a depth-first
manner [...]. Instead, the calls come in this order:


Thanks.

Sorry to nitpick, but the problem is not depth-first versus
breadth-first versus random.  Blaming the traversal order makes this
completely confusing.  The actual problem is that the driver asks us
to keep multiple files open at a time.


On Sun, 07 Jul 2013 18:00:40 GMT, Branko Čibej wrote:

On 07.07.2013 19:40, Jonathan Nieder wrote:

(cc-ing subversion's users@ list for advice)
Kyle McKay wrote:

On Jul 6, 2013, at 18:37, Jonathan Nieder wrote:





Kyle McKay wrote:

Begin forwarded message:

[2] http://subversion.tigris.org/issues/show_bug.cgi?id=2932



Ah, thanks for the context.

It's still not clear to me how we know that ra_serf driving the  
editor
in a non depth-first manner is the problem here.  Has that  
explanation

been confirmed somehow?

[...]
Since ra_serf makes multiple connections to the server (hard-coded
to 4 prior to svn 1.8, defaults to 4 in svn 1.8 but can be set to
between 1 and 8) it makes sense there would be multiple active calls
to apply_textdelta if processing is done as results are received on
the multiple connections.
Ah, that's worrisome.  Do I understand you correctly that to work  
with

ra_serf in skelta mode, callers need to make their apply_textdelta
callback thread-safe?


No; the editor drive is single-threaded, but the order of the  
operations

isn't strictly depth-first.



Brane also describes this as a non-depth-first traversal order which  
is the root of the problem.  If the order of operations were strictly  
depth-first, the previous file would end up being closed before the  
next one's opened.



The approach taken in this patch would be racy if the driver calls us
multiple times concurrently (since temp_acquire can fail).  I believe
it doesn't but haven't checked.


Brane says the editor drive is single-threaded so that doesn't seem  
like a problem.


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 2/2] git-svn: allow git-svn fetching to work using serf

2013-07-18 Thread Eric Wong
Jonathan Nieder jrnie...@gmail.com wrote:
 (cc-ing Eric Wong, who maintains git-svn and knows both it and
 the libsvn perl bindings much better than I do)

I doubt that's the case anymore.  I've hardly looked at SVN in many
years, now.

Anyways, if the patches:

1) do not introduce regressions
2) fixes real problems

I'm inclined to think they're OK...

I'm sorry I can't help more, I had access to better drugs back then.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html