> -----Original Message-----
> From: Ramkumar Ramachandra [mailto:artagnon_at_gmail.com]
> Sent: zondag 1 augustus 2010 18:31
> To: Subversion-dev Mailing List
> Subject: [PATCH] Fix a historical detail in the commit editor
>
> Hi,
>
> The patch should be pretty much self-explanatory. It makes no sense
> for the caller of the commit editor to find out the repository's URL.
> This information is actually accessible but hidden inside the
> `edit_baton` structure; but this structure is not public and the
> caller simply sees a void pointer. As long as the path relative to the
> repository's root is present in the repository, it should be enough
> for the commit editor. I assume that it's currently comparing against
> the full URL for some historical reasons.
But I think you forget that before creating a commit editor that user had to
setup an ra session. And every ra session allows retrieving the repository
root (in most cases it is even cached before the first ra operation). So you
can just store this in the initial commit editor baton.
I think this is a compromise between keeping an option on allowing to record
copies from different repositories (maybe compatibility with DeltaV?) and
otherwise the option to hide the real repository root. (We now need it in
several places so it is available everywhere, but around 1.0 the repository
root was mostly optional information).
But if we would rev this function today, I would recommend switching to a
repos_relpath.
(And we should certainly check how we are going to handle this case with the
new function of automatic relocation).
Bert
> I realize that this patch will break too many things, and hence it is
> not intended for inclusion. However, I just thought it would be nice
> to point this out so anyone starting out to build a new commit editor
> will be able to use this information.
>
> Index: commit.c
> ===================================================================
> --- commit.c (revision 981223)
> +++ commit.c (working copy)
> @@ -307,6 +307,7 @@ add_directory(const char *path,
> svn_fs_root_t *copy_root;
> svn_node_kind_t kind;
> size_t repos_url_len;
> + svn_boolean_t is_dir;
>
> /* Copy requires recursive write access to the destination path
> and write access to the parent path. */
> @@ -324,16 +325,17 @@ add_directory(const char *path,
> if ((kind != svn_node_none) && (! pb->was_copied))
> return out_of_date(full_path, kind);
>
> - /* For now, require that the url come from the same repository
> - that this commit is operating on. */
> + /* Check that copy_path is present in the repository */
> copy_path = svn_path_uri_decode(copy_path, subpool);
> repos_url_len = strlen(eb->repos_url);
> - if (strncmp(copy_path, eb->repos_url, repos_url_len) != 0)
> + svn_fs_is_dir(&is_dir, eb->txn_root, copy_path, subpool);
> + if (!is_dir)
> return svn_error_createf
> (SVN_ERR_FS_GENERAL, NULL,
> - _("Source url '%s' is from different repository"),
> copy_path);
> + _("Directory '%s' cannot be found in this repository"),
> + copy_path);
>
> - fs_path = apr_pstrdup(subpool, copy_path + repos_url_len);
> + fs_path = apr_pstrdup(subpool, copy_path);
>
> /* Now use the "fs_path" as an absolute path within the
> repository to make the copy from. */
> @@ -452,6 +454,7 @@ add_file(const char *path,
> svn_fs_root_t *copy_root;
> svn_node_kind_t kind;
> size_t repos_url_len;
> + svn_boolean_t is_file;
>
> /* Copy requires recursive write to the destination path and
> parent path. */
> @@ -468,16 +471,17 @@ add_file(const char *path,
> if ((kind != svn_node_none) && (! pb->was_copied))
> return out_of_date(full_path, kind);
>
> - /* For now, require that the url come from the same repository
> - that this commit is operating on. */
> + /* Check that copy_path is present in the repository */
> copy_path = svn_path_uri_decode(copy_path, subpool);
> repos_url_len = strlen(eb->repos_url);
> - if (strncmp(copy_path, eb->repos_url, repos_url_len) != 0)
> + svn_fs_is_file(&is_file, eb->txn_root, copy_path, subpool);
> + if (!is_file)
> return svn_error_createf
> (SVN_ERR_FS_GENERAL, NULL,
> - _("Source url '%s' is from different repository"),
> copy_path);
> + _("File '%s' cannot be found in this repository"),
> + copy_path);
>
> - fs_path = apr_pstrdup(subpool, copy_path + repos_url_len);
> + fs_path = apr_pstrdup(subpool, copy_path);
>
> /* Now use the "fs_path" as an absolute path within the
> repository to make the copy from. */
Received on 2010-08-01 22:50:30 CEST