[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] remove svn_wc_entry_t from export.c

From: Paul Burba <ptburba_at_gmail.com>
Date: Wed, 30 Sep 2009 13:26:01 -0400

On Tue, Sep 29, 2009 at 8:57 AM, Stefan Sperling <stsp_at_elego.de> wrote:
> Dave,
>
> thanks for the patch. Some comments below.
>
> On Fri, Sep 25, 2009 at 11:22:25AM -0700, Dave Brown wrote:
>> The added/deleted accessors are not perfect, they don't go through
>> all the logic that entries.c does to populate entry->schedule.  I
>> kept wanting to look at svn_wc__de_status_t values.  But the accessors
>> suffice for export.c use.
>
> I think the "skip export" logic is too simplistic. E.g. it does not
> consider schedule-replace nodes. However this problem has existed
> before your patch, and can be fixed in another patch.
> We should list all possible states a node can be in and define skipping
> behaviour for each case, then update your skip_export_node() function.
>
>> [[[
>> Remove wc_entry_t usage from export.c.  Make a couple of accessor api's
>> in svn_wc_private.h to fetch from wc_db the equivalent of the old test
>> entry->schedule == (schdule_add,schedule_delete).
>
> s/schdule/schedule/
>
>> * include/private/svn_wc_private.h:
>>    (svn_wc__node_is_delete, svn_wc__node_is_added): New functions
>>
>> * libsvn_wc/node.c:
>>    (svn_wc__node_is_delete, svn_wc__node_is_added): New functions
>>
>> * libsvn_client/export.c:
>>    (skip_export_node): add static helper function to decide if a node
>>    should be exported or not
>
> You can indent additional lines in each (...) entry by one or two spaces.
>
>>    (copy_one_versioned_file,copy_versioned_files): Remove wc_entry_t
>>    usage.  Use svn_wc_private.h api's to fetch specific information:
>>    url,depth,added or deleted status.
>> ]]]
>>
>> ------------------------------------------------------
>> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2400350
>
>> Index: subversion/include/private/svn_wc_private.h
>> ===================================================================
>> --- subversion/include/private/svn_wc_private.h       (revision 39608)
>> +++ subversion/include/private/svn_wc_private.h       (working copy)
>> @@ -434,6 +434,17 @@
>>                             void *cancel_baton,
>>                             apr_pool_t *scratch_pool);
>>
>> +/** Equivalent to the old notion of "entry->schedule == schedule_delete"  */
>> +svn_error_t *
>> +svn_wc__node_is_delete(svn_boolean_t *deleted, svn_wc_context_t *wc_ctx,
>> +                        const char *abspath, apr_pool_t *scratch_pool);
>
> This should be called "svn_wc__node_is_deleted" (past tense).
> There is a function with that name in entries.c already.
> The existing function expects a wc_db rather than a wc_ctx.
> and has a comment saying that it wants to be updated to wc_db.
>
> Looks like your new function is almost a drop-in replacement.

Hi Stefan,

libsvn_wc/entries.c:svn_wc__node_is_deleted() is actually asking about
the 'deleted' member of svn_wc_entry_t not the 'schedule' member.

entry->deleted and entry->schedule == svn_wc_schedule_delete are
different beasts:

  /** The directory containing this entry had a versioned child of this
   * name, but this entry represents a different revision or a switched
   * path at which no item exists in the repository. This typically
   * arises from committing or updating to a deletion of this entry
   * without committing or updating the parent directory.
   *
   * The schedule can be 'normal' or 'add'. */
  svn_boolean_t deleted;

  /** scheduling (add, delete, replace ...) */
  svn_wc_schedule_t schedule;

Paul

> If we cannot remove the existing function because it has too many users
> internal to libsvn_wc relying on the wc_db parameter, we should update
> the existing function with your code and provide a thin wrapper that takes
> a wc_ctx for use by libsvn_client, with a different name (but more different
> than just a single character).

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2402191
Received on 2009-09-30 19:26:08 CEST

This is an archived mail posted to the Subversion Dev mailing list.