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