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

Re: [PATCH v2] Replace entries in revisision_status.c

From: Daniel Näslund <daniel_at_longitudo.com>
Date: Wed, 10 Mar 2010 11:51:37 +0100

Hi!

Here's my new patch.

On Tue, Mar 09, 2010 at 12:52:23AM +0100, Bert Huijben wrote:
> > Several points:
> > * use svn_wc__internal_text_modified_p(); then you won't need wc_ctx
> > (just the db)

Done!

> > * read_info can return NULL for the repos_* values

I'm checking that the values are null before setting using them for
svn_uri_join().

> > * typo in analyze_status docstring (the typename)

Fixed!

> > * status_deleted nodes have no revision info; you may simply want to
> > test for SVN_INVALID_REVNUM, since read_info() will return that when
> > it is "not interesting". IOW, it does the proper work for you

I'm checking that revision is not SVN_INVALID_REVNUM. As I've understood
it, that happens when we have something in WORKING, it has been deleted
or added and in that case there's no revision info for us to use. Right?

> > * do you need wc_root? how about just using
> > svn_wc__internal_path_switched() ?

I first tried to use the child_disjoint.*() func of libsvn_wc/lock.c but
it felt strange to use that code when we already have
svn_wc__check_wc_root() that checks if a node is switched. I can
separate the switch part into a separate func in a follow up and
replace the call to svn_wc__check_wc_root() with that one then. But I'm
so terribly slow at getting my patches finished that I thought it was
best to use _check_wc_root() as is for now.

> * You could optimize the checking for modifications to stop checking when
> you find the first change. (You are not reporting specific changes)

Done that!

I've removed the comments in the end about sparse_checkout not detecting
all cases. The walker reaches all nodes beneath local_abspath and if we
check for absent too I can't come up with any more cases where we won't
detect sparse_checkouts. Is that assumption correct?

I'm catching the _WC_PATH_NOT_FOUND error. Before my changes the code
didn't return that error. BUT, is that the _right_ thing to do? Are we
counting on the svn_wc_.* functions to always return _WC_PATH_NOT_FOUND
error for unversioned resources and such? Since it's a new revision I
could just as well return it, right?

[[[
As part of WC-NG, remove some uses of svn_wc_entry_t.

* subversion/libsvn_wc/revision_status.c
  (status_baton): Rename this...
  (walk_baton): .. to this.
  (analyze_status): Change this to implement svn_wc__node_func_t and use
    WC-NG funcs instead of information in svn_wc_entry_t.
  (svn_wc_revision_status2): Use svn_wc__node_walk_children() instead of
    svn_wc_walk_status() to spare us from the overhead of invoking an
    editor.
]]]

Daniel

Received on 2010-03-10 11:52:18 CET

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