Re: svn commit: r40321 - trunk/subversion/libsvn_wc

I think this is wrong on several counts, and that Paul's changes were
proper. This revision seems a complete step backwards.

First, "repos_relpath" is a horrible variable name for this.
read_info() has an output variable named repos_relpath which is
TOTALLY different from the original_repos_relpath that you're actually
querying. Same goes for the other variables. Paul kept the semantics
on the variable names, which keeps the code very readable and
understandable.

After the scan_addition, you join stuff to derive the *root* of the
copyfrom. Paul's change computed corresponding source node.

add_component2() will escape the repos_relpath before appending to the URL.

Cheers,
-g

On Sat, Oct 31, 2009 at 08:08, Stefan Sperling <stsp_at_elego.de> wrote:
> Author: stsp
> Date: Sat Oct 31 05:08:03 2009
> New Revision: 40321
>
> Log:
> Follow-up to r40306:
>
> * subversion/libsvn_wc/copy.c
>  (determine_copyfrom_info): Remove redundant SRC_COPYFROM_REV variable
>   and use REV directly. Rename SRC_COPYFROM_ROOT_URL and SRC_COPYFROM_RELPATH
>   variables to ROOT_URL and REPOS_RELPATH, respectively.
>   Only scan up the tree for copyfrom if the node's status indicates that
>   it was copied or moved, and re-use existing variables when doing so instead
>   of using new ones. Also, use svn_path_url_add_component2() instead of
>   svn_uri_join() to join the repository root URL with the repository
>   relpath. At the hackathon, gstein said it was better to use
>   svn_path_url_add_component2() for this, but I cannot remember why.
>
> Modified:
>   trunk/subversion/libsvn_wc/copy.c
>
> Modified: trunk/subversion/libsvn_wc/copy.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/copy.c?pathrev=40321&r1=40320&r2=40321
> ==============================================================================
> --- trunk/subversion/libsvn_wc/copy.c   Fri Oct 30 18:56:23 2009        (r40320)
> +++ trunk/subversion/libsvn_wc/copy.c   Sat Oct 31 05:08:03 2009        (r40321)
> @@ -273,26 +273,21 @@ determine_copyfrom_info(const char **cop
>  {
>   const char *url;
>   svn_revnum_t rev;
> -  const char *src_copyfrom_root_url;
> -  const char *src_copyfrom_relpath;
> -  svn_revnum_t src_copyfrom_rev;
> +  const char *root_url;
> +  const char *repos_relpath;
>   svn_wc__db_status_t status;
>
>   SVN_ERR(svn_wc__db_read_info(&status, NULL, NULL, NULL, NULL, NULL, NULL,
>                                NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> -                               NULL, &src_copyfrom_relpath,
> -                               &src_copyfrom_root_url, NULL,
> -                               &src_copyfrom_rev, NULL, NULL, NULL, NULL,
> +                               NULL, &repos_relpath, &root_url, NULL,
> +                               &rev, NULL, NULL, NULL, NULL,
>                                NULL, db, src_abspath, result_pool,
>                                scratch_pool));
> -
> -  if (src_copyfrom_root_url && src_copyfrom_relpath)
> +  if (root_url && repos_relpath)
>     {
>       /* When copying/moving a file that was already explicitly
>          copied/moved then we know the URL it was copied from... */
> -      url = svn_path_url_add_component2(src_copyfrom_root_url,
> -                                        src_copyfrom_relpath, result_pool);
> -      rev = src_copyfrom_rev;
> +      url = svn_path_url_add_component2(root_url, repos_relpath, scratch_pool);
>     }
>   else if (status == svn_wc__db_status_added
>            || status == svn_wc__db_status_obstructed_add)
> @@ -300,25 +295,16 @@ determine_copyfrom_info(const char **cop
>       /* ...But if this file is merely the descendant of an explicitly
>          copied/moved directory, we need to do a bit more work to
>          determine copyfrom_url and copyfrom_rev. */
> -      const char *op_root_abspath;
> -      const char *original_repos_relpath;
> -      const char *original_root_url;
> -      svn_revnum_t original_revision;
> -      const char *op_root_rel_path;
> -
> -      SVN_ERR(svn_wc__db_scan_addition(&status, &op_root_abspath,
> -                                       NULL, NULL, NULL,
> -                                       &original_repos_relpath,
> -                                       &original_root_url, NULL,
> -                                       &original_revision,
> -                                       db, src_abspath,
> +      SVN_ERR(svn_wc__db_scan_addition(&status, NULL, NULL, NULL, NULL,
> +                                       &repos_relpath, &root_url, NULL,
> +                                       &rev, db, src_abspath,
>                                        scratch_pool, scratch_pool));
> -      url = svn_uri_join(original_root_url, original_repos_relpath,
> -                         result_pool);
> -      op_root_rel_path = svn_dirent_is_child(op_root_abspath, src_abspath,
> -                                             scratch_pool);
> -      url = svn_uri_join(url, op_root_rel_path, scratch_pool);
> -      rev = original_revision;
> +      if (status == svn_wc__db_status_copied ||
> +          status == svn_wc__db_status_moved_here)
> +        {
> +          url = svn_path_url_add_component2(root_url, repos_relpath,
> +                                            scratch_pool);
> +        }
>     }
>   else
>     {
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=2413219
>

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2413230
Received on 2009-10-31 13:54:14 CET