You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2011/02/16 04:29:58 UTC

Re: svn commit: r1070980 - /subversion/trunk/subversion/svnrdump/load_editor.c

CC += artagnon

Ramkumar, I think the lesson from Mike and I's recent fixes is: "Mark
unreachable code with a run-time assertion, not with a source comment".

i.e., if those places were marked with SVN__NOT_IMPLEMENTED(), debugging
would have been easier.

(Tests pass if I add a SVN__NOT_IMPLEMENTED() to that function.)

cmpilato@apache.org wrote on Tue, Feb 15, 2011 at 17:22:32 -0000:
> Author: cmpilato
> Date: Tue Feb 15 17:22:32 2011
> New Revision: 1070980
> 
> URL: http://svn.apache.org/viewvc?rev=1070980&view=rev
> Log:
> Improve 'svnrdump load' by implementing a missing function (the one
> that handles fulltext file contents in dumpstreams).  We still can't
> fully handle non-delta dumpstreams due to remove_node_props() being a
> no-op.
> 
> * subversion/svnrdump/load_editor.c
>   (set_fulltext): Really implement this function.
> 
> Modified:
>     subversion/trunk/subversion/svnrdump/load_editor.c
> 
> Modified: subversion/trunk/subversion/svnrdump/load_editor.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnrdump/load_editor.c?rev=1070980&r1=1070979&r2=1070980&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/svnrdump/load_editor.c (original)
> +++ subversion/trunk/subversion/svnrdump/load_editor.c Tue Feb 15 17:22:32 2011
> @@ -464,7 +464,20 @@ static svn_error_t *
>  set_fulltext(svn_stream_t **stream,
>               void *node_baton)
>  {
> -  /* ### Not implemented */
> +  struct node_baton *nb;
> +  const struct svn_delta_editor_t *commit_editor;
> +  svn_txdelta_window_handler_t handler;
> +  void *handler_baton;
> +  apr_pool_t *pool;
> +
> +  nb = node_baton;
> +  commit_editor = nb->rb->pb->commit_editor;
> +  pool = nb->rb->pool;
> +  LDR_DBG(("Setting fulltext for %p\n", nb->file_baton));
> +  SVN_ERR(commit_editor->apply_textdelta(nb->file_baton, nb->base_checksum,
> +                                         pool, &handler, &handler_baton));
> +  *stream = svn_txdelta_target_push(handler, handler_baton,
> +                                    svn_stream_empty(pool), pool);
>    return SVN_NO_ERROR;
>  }
>  
> 
> 

Re: svn commit: r1070980 - /subversion/trunk/subversion/svnrdump/load_editor.c

Posted by "C. Michael Pilato" <cm...@collab.net>.
On 02/15/2011 10:40 PM, Daniel Shahaf wrote:
> Daniel Shahaf wrote on Wed, Feb 16, 2011 at 05:29:58 +0200:
>> (Tests pass if I add a SVN__NOT_IMPLEMENTED() to that function.)
>>
> 
> On the other hand, some tests fail if I add SVN__NOT_IMPLEMENTED() to
> remove_node_props().  Another one for the todo list...

Yeah, I went that route myself already.  :-)

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: svn commit: r1070980 - /subversion/trunk/subversion/svnrdump/load_editor.c

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Daniel Shahaf wrote on Wed, Feb 16, 2011 at 05:29:58 +0200:
> (Tests pass if I add a SVN__NOT_IMPLEMENTED() to that function.)
> 

On the other hand, some tests fail if I add SVN__NOT_IMPLEMENTED() to
remove_node_props().  Another one for the todo list...

> cmpilato@apache.org wrote on Tue, Feb 15, 2011 at 17:22:32 -0000:
> > Author: cmpilato
> > Date: Tue Feb 15 17:22:32 2011
> > New Revision: 1070980
> > 
> > URL: http://svn.apache.org/viewvc?rev=1070980&view=rev
> > Log:
> > Improve 'svnrdump load' by implementing a missing function (the one
> > that handles fulltext file contents in dumpstreams).  We still can't
> > fully handle non-delta dumpstreams due to remove_node_props() being a
> > no-op.

Re: svn commit: r1070980 - /subversion/trunk/subversion/svnrdump/load_editor.c

Posted by Ramkumar Ramachandra <ar...@gmail.com>.
Hi Daniel,

Daniel Shahaf writes:
> CC += artagnon
> 
> Ramkumar, I think the lesson from Mike and I's recent fixes is: "Mark
> unreachable code with a run-time assertion, not with a source comment".
> 
> i.e., if those places were marked with SVN__NOT_IMPLEMENTED(), debugging
> would have been easier.
> 
> (Tests pass if I add a SVN__NOT_IMPLEMENTED() to that function.)

Cool, thanks for pointing this out. Since this is such a common task,
maybe add this information to HACKING?

-- Ram