Re: svn commit: r980784 - in /subversion/trunk/subversion/libsvn_wc: adm_files.c node.c wc_db.h

On Fri, Jul 30, 2010 at 01:28:39PM -0000, philip_at_apache.org wrote:
> Author: philip
> Date: Fri Jul 30 13:28:39 2010
> New Revision: 980784
>
> 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
> @@ -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));
>
> - /* 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);
>
> /* 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? */

Previously urls were compared but now, we're comparing uuid, root_url
and repos_relpath. Those are all already canonicalized, right? Appears
not, since a bit higher up we're doing:

repos_relpath = svn_uri_is_child(repos_root_url, url, scratch_pool);
repos_relpath = svn_path_uri_decode(repos_relpath, scratch_pool);

We probably should add a svn_relpath_canonicalize() after those and
maybe the documentation on the top of svn_dirent_uri.h should mention
that a relpath is supposed to not be encoded as an uri. And shouldn't
all relpaths, uris and dirents be canonicalized when they reach
libsvn_wc?

> + 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(&copyfrom_root_url,
> + &copyfrom_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);
> }
> }

Cheers,
Daniel (who is just trying to learn to do code reviews).
Received on 2010-08-01 14:47:14 CEST