[PATCH] Issue 1825: adm_crawler and report_everything

I've been looking at issue 1825 and I just committed an XFail
regression test. The issue contains an old patch which boils down to
this change:

Index: subversion/libsvn_wc/adm_crawler.c
===================================================================
--- subversion/libsvn_wc/adm_crawler.c (revision 11994)
+++ subversion/libsvn_wc/adm_crawler.c (working copy)
@@ -243,8 +243,8 @@
       /* If the entry is 'deleted' or 'absent', make sure the server
          knows it's gone... unless we're reporting everything, in
          which case it's already missing on the server. */
- if ((current_entry->deleted || current_entry->absent)
- && (! report_everything))
+ if (current_entry->deleted || (current_entry->absent
+ && (! report_everything)))
         {
           SVN_ERR (reporter->delete_path (report_baton, this_path, iterpool));
           continue;

In essence if the entry is in state deleted="true" then report it as
deleted irrespective of report_everything. Applying this patch
changes the XFail into an XPass, i.e. it fixes the bug.

However, the similarity between the handling of deleted="true" and
absent="true" led me to test the latter. I set up an mod_authz_svn
file as follows

[switch_tests-14:/A/B/E/alpha]
* =
[/]
* = r

and did

$ svn co -r1 http://localhost/repositories/switch_tests-14 wc
$ touch wc/A/B/E/alpha
$ svn sw http://localhost/repositories/switch_tests-14/A/B/Esave wc/A/B/E
$ rm wc/A/B/E/alpha
$ svn sw http://localhost/repositories/switch_tests-14/A/B/Esave wc/A/B/E

and this fails in exactly the same way as the deleted="true" case, and
can be fixed by the obvious extension of the deleted="true" fix,
namely

Index: subversion/libsvn_wc/adm_crawler.c
===================================================================
--- subversion/libsvn_wc/adm_crawler.c (revision 11994)
+++ subversion/libsvn_wc/adm_crawler.c (working copy)
@@ -243,8 +243,7 @@
       /* If the entry is 'deleted' or 'absent', make sure the server
          knows it's gone... unless we're reporting everything, in
          which case it's already missing on the server. */
- if ((current_entry->deleted || current_entry->absent)
- && (! report_everything))
+ if (current_entry->deleted || current_entry->absent)
         {
           SVN_ERR (reporter->delete_path (report_baton, this_path, iterpool));
           continue;

The problem is that this directly contradicts the "unless ..." part
of the comment just above the modified lines. Obviously I can change
the comment to match the code, but I only want to do this if the code
change is correct. As this bit of code is a bit hairy, and I suspect
the regression test coverage is not very good, I'd like a second
opinon, does this fix look correct?

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org