Ping! This patch has not been revieved
On Fri, Mar 12, 2010 at 10:52:55PM +0100, Daniel Näslund wrote:
> On Fri, Mar 12, 2010 at 05:43:18AM -0500, Greg Stein wrote:
> > On Fri, Mar 12, 2010 at 03:21, <dannas_at_apache.org> wrote:
> > >...
> > > +++ subversion/trunk/subversion/libsvn_wc/revision_status.c Fri Mar 12 08:21:45 2010
> > >...
> > > {
> > > - struct status_baton *sb = baton;
> > > + struct walk_baton *wb = baton;
> > > + svn_revnum_t changed_rev;
> > > + svn_revnum_t revision;
> > > + svn_depth_t depth;
> > > + svn_wc__db_status_t status;
> > > + svn_boolean_t wc_root;
> > > + svn_boolean_t switched;
> >
> > wc_root and switched can be moved into a tighter scope.
>
> Fixed!
>
> > > - if (status->entry->depth != svn_depth_exclude)
> > > + /* Added files have a revision of no interest */
> > > + if (revision != SVN_INVALID_REVNUM)
> > > {
> > > - sb->result->switched |= status->switched;
> > > - sb->result->modified |= (status->text_status != svn_wc_status_normal);
> > > - sb->result->modified |= (status->prop_status != svn_wc_status_normal
> > > - && status->prop_status != svn_wc_status_none);
> > > + svn_revnum_t item_rev = (wb->committed
> > > + ? changed_rev
> > > + : revision);
> >
> > I think this may introduce a bug. Depending on wb->committed, we look
> > at different revision values. And it may be that REVISION is valid,
> > but CHANGED_REV is not. I would suggest moving the assignment of
> > ITEM_REV one block out, and using that in the primary if() test.
>
> Fixed, although I must admit that I don't truly understand how
> changed_rev and revision differs!
>
> My change of the caller svnversion/main.c in r922176 caused a problem
> when svnversion was invoked on a newly added path. It is under version
> control but has no revision number. At the moment '-1' is returned for
> such a path. I intend to fix that in a separate patch.
>
> [[[
> Follow-up to r922176. Fix that tree changes were not considered when
> determining if the wc has modifications.
>
> * subversion/libsvn_wc/revision_status.c
> (analyze_status): Determine from status if a path has been added or
> deleted. Do some optimizations to avoid having to do a text
> comparison for determining if a wc has modifications.
>
> Suggested by: gstein
> rhuijben
> Approved by: ?
> ]]]
> Index: subversion/libsvn_wc/revision_status.c
> ===================================================================
> --- subversion/libsvn_wc/revision_status.c (revision 922398)
> +++ subversion/libsvn_wc/revision_status.c (arbetskopia)
> @@ -41,7 +41,8 @@
> };
>
> /* An svn_wc__node_found_func_t callback function for analyzing the status
> - * of nodes */
> + * of nodes. Optimized to avoid text compares and unneccessary checks of
> + * already set values. */
> static svn_error_t *
> analyze_status(const char *local_abspath,
> void *baton,
> @@ -50,10 +51,9 @@
> struct walk_baton *wb = baton;
> svn_revnum_t changed_rev;
> svn_revnum_t revision;
> + svn_revnum_t item_rev;
> svn_depth_t depth;
> svn_wc__db_status_t status;
> - svn_boolean_t wc_root;
> - svn_boolean_t switched;
>
> SVN_ERR(svn_wc__db_read_info(&status, NULL, &revision, NULL,
> NULL, NULL, &changed_rev,
> @@ -71,24 +71,36 @@
> wb->result->sparse_checkout = TRUE;
> return SVN_NO_ERROR;
> }
> + else if (status == svn_wc__db_status_not_present)
> + {
> + return SVN_NO_ERROR;
> + }
> + else if (status == svn_wc__db_status_added
> + || status == svn_wc__db_status_obstructed_add
> + || status == svn_wc__db_status_deleted
> + || status == svn_wc__db_status_obstructed_delete)
> + {
> + wb->result->modified = TRUE;
> + }
>
> - if (status == svn_wc__db_status_not_present)
> - return SVN_NO_ERROR;
> -
> if (! wb->result->switched)
> {
> + svn_boolean_t wc_root;
> + svn_boolean_t switched;
> +
> SVN_ERR(svn_wc__check_wc_root(&wc_root, NULL, &switched, wb->db,
> local_abspath, scratch_pool));
>
> wb->result->switched |= switched;
> }
>
> + item_rev = (wb->committed
> + ? changed_rev
> + : revision);
> +
> /* Added files have a revision of no interest */
> - if (revision != SVN_INVALID_REVNUM)
> + if (item_rev != SVN_INVALID_REVNUM)
> {
> - svn_revnum_t item_rev = (wb->committed
> - ? changed_rev
> - : revision);
>
> if (wb->result->min_rev == SVN_INVALID_REVNUM
> || item_rev < wb->result->min_rev)
> @@ -101,22 +113,27 @@
>
> if (! wb->result->modified)
> {
> - svn_boolean_t text_mod;
> svn_boolean_t props_mod;
>
> SVN_ERR(svn_wc__props_modified(&props_mod, wb->db, local_abspath,
> scratch_pool));
> + wb->result->modified |= props_mod;
> + }
>
> + if (! wb->result->modified)
> + {
> + svn_boolean_t text_mod;
> +
> SVN_ERR(svn_wc__internal_text_modified_p(&text_mod, wb->db,
> local_abspath,
> FALSE,
> TRUE,
> scratch_pool));
> - wb->result->modified |= (text_mod || props_mod);
> + wb->result->modified |= text_mod;
> }
>
> - wb->result->sparse_checkout |= ((depth != svn_depth_infinity
> - && depth != svn_depth_unknown));
> + wb->result->sparse_checkout |= (depth != svn_depth_infinity
> + && depth != svn_depth_unknown);
> return SVN_NO_ERROR;
> }
>
Received on 2010-03-30 22:14:30 CEST