On Fri, Aug 12, 2011 at 02:56:28AM +0200, Neels J Hofmeyr wrote:
> Hi stsp,
>
> check this WIP patch out, attached. (I've also appended a wild test script
> that runs all sorts of things. Reading its bottom-most output may suffice.)
>
> I'd prefer printing the paths relative to the "current working dir" instead
> of relative to the WC root (as this patch does now).
>
> Say there was an
> [[[
> ~/wc$ svn mv x/alpha a/b/beta
> ]]]
>
> currently:
> [[[
> ~/wc$ cd a/b/
> ~/wc/a/b$ svn st
> A + beta
> > moved from x/alpha
> ]]]
> (relative to WC_ROOT)
>
> rather:
> [[[
> ~/wc/a/b$ svn st
> A + beta
> > moved from ../../x/alpha
> ]]]
>
> but I haven't figured out whether there's code that does this already. And,
> to that end, I think those batons should rather pass abspaths (see comments
> in the code).
>
> What do you say?
Thanks!
How badly does changing this output break "make check"?
See inline comments below.
>
> ~Neels
> First steps to show moved-to and moved-from in 'svn status'.
>
> * subversion/include/svn_client.h
>
> * subversion/include/svn_wc.h
>
> * subversion/libsvn_client/status.c
> (svn_client_status_dup):
> (svn_client__create_status):
>
> * subversion/libsvn_wc/status.c
> (assemble_status):
> (svn_wc_dup_status3):
>
> * subversion/svn/status.c
> (print_status):
> Index: subversion/include/svn_client.h
> ===================================================================
> --- subversion/include/svn_client.h (revision 1156828)
> +++ subversion/include/svn_client.h (working copy)
> @@ -2206,6 +2206,18 @@ typedef struct svn_client_status_t
> * to other data in future versions. */
> const void *backwards_compatibility_baton;
>
> + /** Set to @c true if this file or directory is scheduled for
> + * addition-with-history that is part of an explicit move operation (or
> part
I think we should just say "copied" instead of "add-with-history".
It doesn't matter how the copy operation is implemented, it is still a copy.
> + * of a subtree that is scheduled as such).
> + */
The boolean variable isn't declared. (Maybe the above comment should
not be here?)
> + /** Set to the path (relative to the working copy root) that this node was
> + * moved from, if this file or directory has been moved here locally. */
> + const char *moved_from_relpath;
As you said, in the APIs we should always use absolute paths.
I used relpaths in svn_wc APIs at one point, giving a bad example,
but they've since been converted to abspaths.
The 'svn' client should perform an abspath->relpath conversion
for display if needed.
> +
> + /** Set to the path (relative to the working copy root) that this node was
> + * moved to, if this file or directory has been moved away locally. */
> + const char *moved_to_relpath;
As above.
> +
> /* NOTE! Please update svn_client_status_dup() when adding new fields
> here. */
> } svn_client_status_t;
>
> Index: subversion/include/svn_wc.h
> ===================================================================
> --- subversion/include/svn_wc.h (revision 1156828)
> +++ subversion/include/svn_wc.h (working copy)
> @@ -3619,6 +3619,14 @@ typedef struct svn_wc_status3_t
>
> /** @} */
>
> + /* Set to the move source path if this item has been explicitly moved here
> + * (path relative to the working copy root) */
> + const char *moved_from_relpath;
Same, please use abspaths.
> +
> + /* Set to the move destination path if this item has been explicitly moved
> + * away (path relative to the working copy root) */
> + const char *moved_to_relpath;
> +
> /* NOTE! Please update svn_wc_dup_status3() when adding new fields here. */
> } svn_wc_status3_t;
>
> Index: subversion/libsvn_client/status.c
> ===================================================================
> --- subversion/libsvn_client/status.c (revision 1156828)
> +++ subversion/libsvn_client/status.c (working copy)
> @@ -563,6 +563,12 @@ svn_client_status_dup(const svn_client_s
> result_pool);
> }
>
> + if (status->moved_from_relpath)
> + st->moved_from_relpath = apr_pstrdup(result_pool,
> status->moved_from_relpath);
> +
> + if (status->moved_to_relpath)
> + st->moved_to_relpath = apr_pstrdup(result_pool,
> status->moved_to_relpath);
> +
> return st;
> }
>
> @@ -667,6 +673,9 @@ svn_client__create_status(svn_client_sta
> (*cst)->node_status = svn_wc_status_conflicted;
> }
>
> + (*cst)->moved_from_relpath = status->moved_from_relpath;
> + (*cst)->moved_to_relpath = status->moved_to_relpath;
> +
> return SVN_NO_ERROR;
> }
>
> Index: subversion/libsvn_wc/status.c
> ===================================================================
> --- subversion/libsvn_wc/status.c (revision 1156828)
> +++ subversion/libsvn_wc/status.c (working copy)
> @@ -403,6 +403,8 @@ assemble_status(svn_wc_status3_t **statu
> const char *repos_relpath;
> const char *repos_root_url;
> const char *repos_uuid;
> + const char *moved_from_relpath = NULL;
> + const char *moved_to_relpath = NULL;
> svn_filesize_t filesize = (dirent && (dirent->kind == svn_node_file))
> ? dirent->filesize
> : SVN_INVALID_FILESIZE;
> @@ -608,6 +610,42 @@ assemble_status(svn_wc_status3_t **statu
> }
> }
>
> + /* Get moved_to info. */
"moved-away" (for consistency)
> + if (info->status == svn_wc__db_status_deleted)
> + {
> + const char *moved_to_abspath = NULL;
> + SVN_ERR(svn_wc__db_scan_deletion(NULL,
> + &moved_to_abspath,
> + NULL, NULL,
> + db, local_abspath,
> + result_pool, scratch_pool));
> + if (moved_to_abspath)
> + /* ### This should probably be different, possibly returning a
> + * ### local_abspath and letting the client construct a pwd-relative
> + * ### path. */
Yes :)
> + SVN_ERR(svn_wc__db_to_relpath(&moved_to_relpath, db, local_abspath,
> + moved_to_abspath,
> + result_pool, scratch_pool));
> +
> + }
> +
> + /* Get moved_here info. */
"moved-here" (for consistency)
> + if (info->status == svn_wc__db_status_added)
> + {
Rest looks fine.
