On 12.01.2012 20:07, Philip Martin wrote:
Stefan Küng<tortoise...@gmail.com> writes:
Could someone please review my patch? I'm sure you guys can see
immediately whether this is save or not. If it's ok, then I'll commit
it later.
Index: subversion/libsvn_client/patch.c
===================================================================
--- subversion/libsvn_client/patch.c (Revision 1230694)
+++ subversion/libsvn_client/patch.c (Arbeitskopie)
@@ -2920,8 +2920,9 @@
}
while (patch);
- /* Delete directories which are empty after patching, if any. */
- SVN_ERR(delete_empty_dirs(targets_info, ctx, dry_run, scratch_pool));
+ if (!dry_run)
+ /* Delete directories which are empty after patching, if any. */
+ SVN_ERR(delete_empty_dirs(targets_info, ctx, dry_run, scratch_pool));
SVN_ERR(svn_diff_close_patch_file(patch_file, iterpool));
svn_pool_destroy(iterpool);
It doesn't look right to me. Use dry_run to in two places, to determine
whether to call delete_empty_dirs and as a parameter to
delete_empty_dirs isn't sensible.
Question is: do we need the notifications for deleting empty dirs in a
dry-run? If yes, then this get's complicated:
the error is thrown from svn_wc_walk_status() called in check_dir_empty.
I could just not call that function in case of a dry-run, or catch that
specific error and go on. But then the notifications wouldn't be
completely accurate anymore.
So assuming we don't want to extend svn_wc_walk_status to take a dry-run
param, the notifications for deleting empty dirs wouldn't be accurate.
Should I just change delete_empty_dirs() to not use the dry_run param
and not get the notifications for those in a dry-run?
I think no notifications is better than inaccurate ones (at least in a
dry-run).
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
