On Fri, Jul 30, 2010 at 01:28:39PM -0000, phi...@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).

Reply via email to