> -----Original Message----- > From: phi...@apache.org [mailto:phi...@apache.org] > Sent: vrijdag 30 juli 2010 15:29 > To: comm...@subversion.apache.org > Subject: svn commit: r980784 - in > /subversion/trunk/subversion/libsvn_wc: adm_files.c node.c wc_db.h > > Author: philip > Date: Fri Jul 30 13:28:39 2010 > New Revision: 980784 > > URL: http://svn.apache.org/viewvc?rev=980784&view=rev > Log: > Remove an svn_wc_entry_t.
This breaks diff_tests.py 13 on Windows, although I don't know why yet. And the error code message looks like we might have a different problem somewhere else: See http://ci.apache.org/builders/svn-slik-w2k3-x64-local/builds/975/steps/Test%20fsfs%2Blocal/logs/faillog (some inline review below) > > * subversion/libsvn_wc/adm_files.c > (svn_wc__internal_ensure_adm): Remove svn_wc_entry_t, use wc-ng > calls. > > * subversion/libsvn_wc/wc_db.h > (svn_wc__internal_get_copyfrom_info): New. > > * subversion/libsvn_wc/node.c > (svn_wc__internal_get_copyfrom_info): Renamed from > svn_wc__node_get_copyfrom_info, adjust recursive call. > (svn_wc__node_get_copyfrom_info): Now just a wrapper. > > Modified: > subversion/trunk/subversion/libsvn_wc/adm_files.c > subversion/trunk/subversion/libsvn_wc/node.c > subversion/trunk/subversion/libsvn_wc/wc_db.h > > Modified: subversion/trunk/subversion/libsvn_wc/adm_files.c > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_ > files.c?rev=980784&r1=980783&r2=980784&view=diff > ======================================================================= > ======= > --- subversion/trunk/subversion/libsvn_wc/adm_files.c (original) > +++ subversion/trunk/subversion/libsvn_wc/adm_files.c Fri Jul 30 > 13:28:39 2010 > @@ -597,9 +597,11 @@ svn_wc__internal_ensure_adm(svn_wc__db_t > svn_depth_t depth, > apr_pool_t *scratch_pool) > { > - const svn_wc_entry_t *entry; > int format; > const char *repos_relpath; > + svn_wc__db_status_t status; > + const char *db_repos_relpath, *db_repos_root_url, *db_repos_uuid; > + svn_revnum_t db_revision; > > SVN_ERR_ASSERT(svn_dirent_is_absolute(local_abspath)); > SVN_ERR_ASSERT(url != NULL); > @@ -622,40 +624,59 @@ svn_wc__internal_ensure_adm(svn_wc__db_t > repos_relpath, repos_root_url, > repos_uuid, > revision, depth, scratch_pool)); > > - /* Now, get the existing url and repos for PATH. */ > - SVN_ERR(svn_wc__get_entry(&entry, db, local_abspath, FALSE, > svn_node_unknown, > - FALSE, scratch_pool, scratch_pool)); > + SVN_ERR(svn_wc__db_read_info(&status, NULL, > + &db_revision, &db_repos_relpath, > + &db_repos_root_url, &db_repos_uuid, > + NULL, NULL, NULL, NULL, NULL, NULL, > NULL, > + NULL, NULL, NULL, NULL, NULL, NULL, > NULL, > + NULL, NULL, NULL, NULL, > + db, local_abspath, scratch_pool, > scratch_pool)); This only sets db_revision, db_repos_relpath, etc. if the node doesn't have a working node... And then only when the node doesn't inherit the location from its parent. In all other cases the values will have their default values. (svn_wc__db_scan_addition() (status=added || !have_base) && svn_wc__db_base_scan_repos() (have_base)) and will handle the other cases). Looking at the rest of the function it might be useful to immediately check for original_* here as you need those for the copyfrom data. The svn_wc__get_entry() call always retrieved the inherited values for you using these helpers, so this changes more than a few cases. > > - /* When the directory exists and is scheduled for deletion do not > - * check the revision or the URL. The revision can be any > + /* When the directory exists and is scheduled for deletion or is > not-present > + * do not check the revision or the URL. The revision can be any > * arbitrary revision and the URL may differ if the add is > * being driven from a merge which will have a different URL. */ > - if (entry->schedule != svn_wc_schedule_delete) > + if (status != svn_wc__db_status_deleted > + && status != svn_wc__db_status_obstructed_delete) > { > - if (entry->revision != revision) > + /* ### Should we match copyfrom_revision? */ > + if (db_revision != revision) > return > svn_error_createf(SVN_ERR_WC_OBSTRUCTED_UPDATE, NULL, > _("Revision %ld doesn't match existing " > "revision %ld in '%s'"), > - revision, entry->revision, local_abspath); > + revision, db_revision, local_abspath); This needs a svn_dirent_local_style() around local_abspath. (Not a new issue, but needs fixing anyway). > > /* The caller gives us a URL which should match the entry. > However, > some callers compensate for an old problem in entry->url and > pass > the copyfrom_url instead. See ^/notes/api-errata/wc002.txt. > As > a result, we allow the passed URL to match copyfrom_url if it > - does match the entry's primary URL. */ > - /** ### comparing URLs, should they be canonicalized first? */ > - if (strcmp(entry->url, url) != 0 > - && (entry->copyfrom_url == NULL > - || strcmp(entry->copyfrom_url, url) != 0) > - && (!svn_uri_is_ancestor(repos_root_url, entry->url) > - || strcmp(entry->uuid, repos_uuid) != 0)) > + does not match the entry's primary URL. */ > + /* ### comparing URLs, should they be canonicalized first? */ > + if (strcmp(db_repos_uuid, repos_uuid) > + || strcmp(db_repos_root_url, repos_root_url) > + || !svn_relpath_is_ancestor(db_repos_relpath, > repos_relpath)) > { > - return > - svn_error_createf(SVN_ERR_WC_OBSTRUCTED_UPDATE, NULL, > - _("URL '%s' doesn't match existing " > - "URL '%s' in '%s'"), > - url, entry->url, local_abspath); > + const char *copyfrom_root_url, *copyfrom_repos_relpath; > + > + > SVN_ERR(svn_wc__internal_get_copyfrom_info(©from_root_url, > + > ©from_repos_relpath, > + NULL, NULL, NULL, > + db, > local_abspath, > + scratch_pool, > + scratch_pool)); > + > + if (copyfrom_root_url == NULL > + || strcmp(copyfrom_root_url, repos_root_url) > + || strcmp(copyfrom_repos_relpath, repos_relpath)) > + return > + svn_error_createf(SVN_ERR_WC_OBSTRUCTED_UPDATE, NULL, > + _("URL '%s' doesn't match existing " > + "URL '%s' in '%s'"), > + url, > + svn_uri_join(db_repos_root_url, > + db_repos_relpath, > scratch_pool), > + local_abspath); You can't construct a url by just calling svn_uri_join(url, repos_relpath), as that doesn't handle escaping the relpath. Please use svn_path_url_add_component2() instead. > } > } > > > Modified: subversion/trunk/subversion/libsvn_wc/node.c > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/node > .c?rev=980784&r1=980783&r2=980784&view=diff > ======================================================================= > ======= > --- subversion/trunk/subversion/libsvn_wc/node.c (original) > +++ subversion/trunk/subversion/libsvn_wc/node.c Fri Jul 30 13:28:39 > 2010 > @@ -467,17 +467,16 @@ svn_wc__node_get_repos_relpath(const cha > } > > svn_error_t * > -svn_wc__node_get_copyfrom_info(const char **copyfrom_root_url, > - const char **copyfrom_repos_relpath, > - const char **copyfrom_url, > - svn_revnum_t *copyfrom_rev, > - svn_boolean_t *is_copy_target, > - svn_wc_context_t *wc_ctx, > - const char *local_abspath, > - apr_pool_t *result_pool, > - apr_pool_t *scratch_pool) > +svn_wc__internal_get_copyfrom_info(const char **copyfrom_root_url, > + const char > **copyfrom_repos_relpath, > + const char **copyfrom_url, > + svn_revnum_t *copyfrom_rev, > + svn_boolean_t *is_copy_target, > + svn_wc__db_t *db, > + const char *local_abspath, > + apr_pool_t *result_pool, > + apr_pool_t *scratch_pool) > { > - svn_wc__db_t *db = wc_ctx->db; > const char *original_root_url; > const char *original_repos_relpath; > svn_revnum_t original_revision; > @@ -544,11 +543,12 @@ svn_wc__node_get_copyfrom_info(const cha > > /* This is a copied node, so we should never fall off the > top of a > * working copy here. */ > - SVN_ERR(svn_wc__node_get_copyfrom_info(NULL, NULL, > - &parent_copyfrom_url, > - NULL, NULL, > - wc_ctx, > parent_abspath, > - scratch_pool, > scratch_pool)); > + SVN_ERR(svn_wc__internal_get_copyfrom_info(NULL, NULL, > + > &parent_copyfrom_url, > + NULL, NULL, > + db, > parent_abspath, > + scratch_pool, > + scratch_pool)); > > /* So, count this as a separate copy target only if the URLs > * don't match up, or if the parent isn't copied at all. */ > @@ -607,6 +607,28 @@ svn_wc__node_get_copyfrom_info(const cha > return SVN_NO_ERROR; > } > > +svn_error_t * > +svn_wc__node_get_copyfrom_info(const char **copyfrom_root_url, > + const char **copyfrom_repos_relpath, > + const char **copyfrom_url, > + svn_revnum_t *copyfrom_rev, > + svn_boolean_t *is_copy_target, > + svn_wc_context_t *wc_ctx, > + const char *local_abspath, > + apr_pool_t *result_pool, > + apr_pool_t *scratch_pool) > +{ > + return > + > svn_error_return(svn_wc__internal_get_copyfrom_info(copyfrom_root_url, > + > copyfrom_repos_relpath, > + copyfrom_url, > + copyfrom_rev, > + > is_copy_target, > + wc_ctx->db, > + local_abspath, > + scratch_pool, > + > scratch_pool)); > +} > > /* A recursive node-walker, helper for svn_wc__node_walk_children(). > * > > Modified: subversion/trunk/subversion/libsvn_wc/wc_db.h > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_d > b.h?rev=980784&r1=980783&r2=980784&view=diff > ======================================================================= > ======= > --- subversion/trunk/subversion/libsvn_wc/wc_db.h (original) > +++ subversion/trunk/subversion/libsvn_wc/wc_db.h Fri Jul 30 13:28:39 > 2010 > @@ -2536,6 +2536,17 @@ svn_wc__db_drop_root(svn_wc__db_t *db, > const char *local_abspath, > apr_pool_t *scratch_pool); > > +/* Internal version of svn_wc__node_get_copyfrom_info */ > +svn_error_t * > +svn_wc__internal_get_copyfrom_info(const char **copyfrom_root_url, > + const char > **copyfrom_repos_relpath, > + const char **copyfrom_url, > + svn_revnum_t *copyfrom_rev, > + svn_boolean_t *is_copy_target, > + svn_wc__db_t *db, > + const char *local_abspath, > + apr_pool_t *result_pool, > + apr_pool_t *scratch_pool); > /* @} */ > > >
