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

[0.5 PATCH] svn up ignore svn:externals + svn_io_make_dir_recursively bug

From: Vladimir Prus <ghost_at_cs.msu.su>
Date: 2002-11-01 14:05:11 CET

Vladimir Prus wrote:
> Vladimir Prus wrote:
>
>>
>> Hello,
>> somehow I've assumed that if I have svn:externals set, then
>> "svn up" would update directies mentioned in that properties, as well.
>> Seems like it's not the case (and in fact, the subversion book
>> source say so).

I think I know where's the bug.

Consider libsvn_client/externals.c, function svn_client__handle_externals.
If first calls svn_wc_edited_externals and then runs svn_hash_diff on
unmodified and current values. The parameter 'update_unchanged' control
whether we should update external items, for which corresponding line
in svn:externals property has not changed, and this is TRUE, when called
from update.c.

Suppose that svn:externals for a directory did not change. Comment on
'svn_wc_edited_externals' read:

     Directories whose value
     of the property did not change show the same value in each hash.

But they do not! update editors's close_directory method does nothing
at all if property has not changed, take a look at
libsvn_wc/update_editor.c:797.

I set out to fix this, and found another problem. svn_io_make_dir_recursively
will infinitely recurse if called with "", and it can indeed be called with
empty path, for example, from subversion/libsvn_client/externals.c:467 ---
"svn_path_split_nts (path, &parent, NULL, ib->pool);" sets 'parent' to ""
when 'path' is "lib". I don't know what's the best solution. I made
the function do nothing when passed empty dir. Maybe, new function
"... ensure_parent_dir" is needed.

I think I've fixed the "svn up" problem and make_dir problem with the attached patch.
The update editor is modified to fill in 'traversal_info' info in all cases.
I don't know if there are other editors that needs changing, and I have not written
any tests, but I probably can do it, if the general approach is OK.

- Volodya

Index: subversion/libsvn_wc/update_editor.c
===================================================================
--- subversion/libsvn_wc/update_editor.c (revision 3607)
+++ subversion/libsvn_wc/update_editor.c (working copy)
@@ -815,54 +815,6 @@
                                       (APR_WRITE | APR_CREATE), /* not excl */
                                       db->pool));
 
- /* If recording traversal info, then see if the
- SVN_PROP_EXTERNALS property on this directory changed,
- and record before and after for the change. */
- if (db->edit_baton->traversal_info)
- {
- svn_wc_traversal_info_t *ti = db->edit_baton->traversal_info;
- const svn_prop_t *change = externals_prop_changed (db->propchanges);
-
- if (change)
- {
- const svn_string_t *new_val_s = change->value;
- const svn_string_t *old_val_s;
-
- SVN_ERR (svn_wc_prop_get
- (&old_val_s, SVN_PROP_EXTERNALS,
- db->path, db->pool));
-
- if ((new_val_s == NULL) && (old_val_s == NULL))
- ; /* No value before, no value after... so do nothing. */
- else if (new_val_s && old_val_s
- && (svn_string_compare (old_val_s, new_val_s)))
- ; /* Value did not change... so do nothing. */
- else /* something changed, record the change */
- {
- /* We can't assume that ti came pre-loaded with the
- old values of the svn:externals property. Yes,
- most callers will have already initialized ti by
- sending it through svn_wc_crawl_revisions, but we
- shouldn't count on that here -- so we set both the
- old and new values again. */
-
- if (old_val_s)
- apr_hash_set (ti->externals_old,
- apr_pstrdup (ti->pool, db->path),
- APR_HASH_KEY_STRING,
- apr_pstrmemdup (ti->pool, old_val_s->data,
- old_val_s->len));
-
- if (new_val_s)
- apr_hash_set (ti->externals_new,
- apr_pstrdup (ti->pool, db->path),
- APR_HASH_KEY_STRING,
- apr_pstrmemdup (ti->pool, new_val_s->data,
- new_val_s->len));
- }
- }
- }
-
       /* Merge pending properties into temporary files and detect
          conflicts. */
       SVN_ERR_W (svn_wc__merge_prop_diffs (&prop_state, &prop_conflicts,
@@ -928,6 +880,53 @@
 
       /* Run the log. */
       SVN_ERR (svn_wc__run_log (adm_access, db->pool));
+ }
+
+ /* If recording traversal info, then see if the
+ SVN_PROP_EXTERNALS property on this directory changed,
+ and record before and after for the change. */
+ if (db->edit_baton->traversal_info)
+ {
+ svn_wc_traversal_info_t *ti = db->edit_baton->traversal_info;
+ const svn_prop_t *change = externals_prop_changed (db->propchanges);
+
+ const svn_string_t *new_val_s;
+ const svn_string_t *old_val_s;
+
+ SVN_ERR (svn_wc_prop_get
+ (&old_val_s, SVN_PROP_EXTERNALS,
+ db->path, db->pool));
+
+ if (change)
+ new_val_s = change->value;
+ else
+ new_val_s = old_val_s;
+
+ if ((new_val_s == NULL) && (old_val_s == NULL))
+ ; /* No value before, no value after... so do nothing. */
+ else /* something changed, record the change */
+ {
+ /* We can't assume that ti came pre-loaded with the
+ old values of the svn:externals property. Yes,
+ most callers will have already initialized ti by
+ sending it through svn_wc_crawl_revisions, but we
+ shouldn't count on that here -- so we set both the
+ old and new values again. */
+
+ if (old_val_s)
+ apr_hash_set (ti->externals_old,
+ apr_pstrdup (ti->pool, db->path),
+ APR_HASH_KEY_STRING,
+ apr_pstrmemdup (ti->pool, old_val_s->data,
+ old_val_s->len));
+
+ if (new_val_s)
+ apr_hash_set (ti->externals_new,
+ apr_pstrdup (ti->pool, db->path),
+ APR_HASH_KEY_STRING,
+ apr_pstrmemdup (ti->pool, new_val_s->data,
+ new_val_s->len));
+ }
     }
 
 
Index: subversion/libsvn_subr/io.c
===================================================================
--- subversion/libsvn_subr/io.c (revision 3607)
+++ subversion/libsvn_subr/io.c (working copy)
@@ -382,6 +382,9 @@
   apr_status_t apr_err;
   char *dir;
 
+ if (svn_path_is_empty_nts(path))
+ return SVN_NO_ERROR;
+
   SVN_ERR (svn_utf_cstring_from_utf8 (&path_native, path, pool));
 
 #if 0

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Nov 1 14:04:57 2002

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.