On Fri, Mar 12, 2010 at 05:43:18AM -0500, Greg Stein wrote:
> On Fri, Mar 12, 2010 at 03:21,  <dan...@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;
 }
 

Reply via email to