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

Re: svn commit: r32324 - in branches/issue-2843-dev/subversion: libsvn_repos libsvn_wc

From: Karl Fogel <kfogel_at_red-bean.com>
Date: Sun, 17 Aug 2008 19:45:39 -0400

firemeteor_at_tigris.org writes:
> --- branches/issue-2843-dev/subversion/libsvn_wc/update_editor.c
> +++ branches/issue-2843-dev/subversion/libsvn_wc/update_editor.c
> @@ -496,19 +496,22 @@ complete_directory(struct edit_baton *eb
> if (entry && entry->depth == svn_depth_exclude)
> {
> char * full_target;
> - entry->depth = svn_depth_infinity;
> - SVN_ERR(svn_wc__entries_write(entries, adm_access, pool));
> /* There is a small chance that the target is gone in the
> repository. We'd better get rid of the exclude flag now. */
> full_target = svn_path_join(eb->anchor, eb->target, pool);
> SVN_ERR(svn_wc__adm_retrieve_internal
> (&target_access, eb->adm_access, full_target, pool));
> - if (!target_access)
> + if (!target_access && entry->kind == svn_node_dir)
> {
> int log_number = 0;
> SVN_ERR(do_entry_deletion(eb, eb->anchor, eb->target,
> &log_number, pool));
> }
> + else
> + {
> + entry->depth = svn_depth_infinity;
> + SVN_ERR(svn_wc__entries_write(entries, adm_access, pool));
> + }
> }
> }
> return SVN_NO_ERROR;

I don't quite understand the conditional. Why does it matter if the
entry is a dir? If we got no target_access, then the entry is excluded,
and we should delete the entry, right? But in the current code, we
could get no target_access and still end up writing out an entry with
depth_infinity, if the entry is a file.

(There's probably something I'm not understanding here.)

Regarding the "log_number = 0" thing:

The log_number parameter to do_entry_deletion() was not documented (in
fact, the entire function was not documented). I made an attempt to
document it in r32516. Then I looked more carefully at the log message
for r9868, where that parameter was introduced, and got a better idea of
what log_number is for; now I'm not sure it's safe for us to just write
"log_number = 0" there, unless we're sure that no other log files are in
use right now. Are we sure of that?

(Note: I found r9868 by doing a binary search. Then I edited its log
message to contain the word "log_number", since that is the name of the
new struct field and parameter that r9868 added. If r9868 had mentioned
each new symbol name the way it should, I could have saved a lot of
time. Of course, none of this is your fault, I'm just pointing out for
others how our log message conventions do have a purpose :-) ).

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-08-18 01:45:55 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.