On Aug 10, 2009, at 1:48 AM, HuiHuang wrote:
> Hey,
>
> My previous patch *remove adm_access in svn_wc_translated_file3*
> cause several
> tests failed. I fix the issue and put them together in this patch. I
> have run the whole
> tests and it fails in the following tests:
> db_tests.exe
> patch_tests.py
> svnlook_tests.py
> switch_tests.py
>
> But these tests also fail when I run tests without the patch. It
> seems some
> Windows-special error happened:
> WindowsError: [Error 32] : 'svn-test-work\\local_tmp\\tmp3ncn20'
>
> Maybe it is not the fault of this patch. Would you mind to review
> this patch
> and test it again on Linux? Thank you!
I get three test failures on OS X with and without your patch, so it
doesn't look like it creates any new ones.
>
> Log:
> [[[
> Rip out some adm_access usage in svn_wc_translated_file3().
>
> * subversion/include/svn_wc.h
> (svn_wc_translated_file3): New.
> (svn_wc_translated_file2): Deprecate.
>
> * subversion/libsvn_wc/deprecated.c
> (svn_wc_translated_file2): Reimplement as a wrapper.
>
> * subversion/libsvn_wc/translate.c
> (svn_wc__internal_translated_file): New.
> (svn_wc_translated_file3): New.
> (svn_wc_translated_file2): Remove.
>
> * subversion/libsvn_wc/translate.h
> (svn_wc__internal_translated_file): New.
>
> * subversion/libsvn_wc/log.c
> (loggy_path): Add a apr_pool_t * parameter and make it able to deal
> with absolute paths.
> ]]]
>
> Modified:
> trunk/subversion/include/svn_wc.h
> trunk/subversion/libsvn_wc/deprecated.c
> trunk/subversion/libsvn_wc/log.c
> trunk/subversion/libsvn_wc/translate.c
> trunk/subversion/libsvn_wc/translate.h
>
>
>
> Index: subversion/include/svn_wc.h
> ===================================================================
> --- subversion/include/svn_wc.h (revision 38658)
> +++ subversion/include/svn_wc.h (working copy)
> @@ -5842,8 +5842,8 @@
>
> /** Set @a xlated_path to a translated copy of @a src
> * or to @a src itself if no translation is necessary.
> - * That is, if @a versioned_file's properties indicate newline
> conversion or
> - * keyword expansion, point @a *xlated_path to a copy of @a src
> + * That is, if @a versioned_abspath's properties indicate newline
> conversion
> + * or keyword expansion, point @a *xlated_path to a copy of @a src
> * whose newlines and keywords are converted using the translation
> * as requested by @a flags.
> *
> @@ -5857,20 +5857,37 @@
> * @c SVN_WC_TRANSLATE_FORCE_COPY flag in @a flags.
> *
> * This function is generally used to get a file that can be compared
> - * meaningfully against @a versioned_file's text base, if
> - * @c SVN_WC_TRANSLATE_TO_NF is specified, against @a
> versioned_file itself
> + * meaningfully against @a versioned_abspath's text base, if
> + * @c SVN_WC_TRANSLATE_TO_NF is specified, against @a
> versioned_abspath itself
> * if @c SVN_WC_TRANSLATE_FROM_NF is specified.
> *
> * Output files are created in the temp file area belonging to
> - * @a versioned_file. By default they will be deleted at pool
> cleanup.
> + * @a versioned_abspath. By default they will be deleted at
> scratch_pool cleanup.
Below, you create the tempfile in result_pool, so this comment should
reflect that.
> *
> * If @c SVN_WC_TRANSLATE_NO_OUTPUT_CLEANUP is specified, the default
> - * pool cleanup handler to remove @a *xlated_path is not registered.
> + * result_pool cleanup handler to remove @a *xlated_path is not
> registered.
> *
> * If an error is returned, the effect on @a *xlated_path is
> undefined.
> *
> - * @since New in 1.4
> + * @since New in 1.7.
> + */
> +svn_error_t *
> +svn_wc_translated_file3(const char **xlated_path,
> + const char *src,
Are src and xlated_path absolute or relative? If relative, relative
to what?
I personally think we should make them absolute, since that better
fits in with our goal of using absolute paths within the working copy
library.
> + svn_wc_context_t *wc_ctx,
> + const char *versioned_abspath,
> + apr_uint32_t flags,
> + apr_pool_t *result_pool,
> + apr_pool_t *scratch_pool);
> +
> +
> +/** Similar to svn_wc_translated_file3(), but with an adm_access
> baton
> + * and relative paths instead of a wc_context and absolute paths.
> + *
> + * @since New in 1.4.
> + * @deprecated Provided for compatibility with the 1.6 API
> */
> +SVN_DEPRECATED
> svn_error_t *
> svn_wc_translated_file2(const char **xlated_path,
> const char *src,
> Index: subversion/libsvn_wc/deprecated.c
> ===================================================================
> --- subversion/libsvn_wc/deprecated.c (revision 38658)
> +++ subversion/libsvn_wc/deprecated.c (working copy)
> @@ -2100,6 +2100,28 @@
> return svn_error_return(svn_wc_context_destroy(wc_ctx));
> }
>
> +svn_error_t *
> +svn_wc_translated_file2(const char **xlated_path,
> + const char *src,
> + const char *versioned_file,
> + svn_wc_adm_access_t *adm_access,
> + apr_uint32_t flags,
> + apr_pool_t *pool)
> +{
> + const char *versioned_abspath;
> + svn_wc_context_t *wc_ctx;
> +
> + SVN_ERR(svn_dirent_get_absolute(&versioned_abspath,
> versioned_file, pool));
> + SVN_ERR(svn_wc__context_create_with_db(&wc_ctx, NULL,
> +
> svn_wc__adm_get_db(adm_access),
> + pool));
> +
> + SVN_ERR(svn_wc_translated_file3(xlated_path, src, wc_ctx,
> versioned_abspath,
> + flags, pool, pool));
If svn_wc_translated_file3() does return an absolute path, we may need
to adjust it here before returning it to callers of
svn_wc_translated_file2().
> +
> + return svn_error_return(svn_wc_context_destroy(wc_ctx));
> +}
> +
> /*** From relocate.c ***/
> svn_error_t *
> svn_wc_relocate3(const char *path,
> Index: subversion/libsvn_wc/log.c
> ===================================================================
> --- subversion/libsvn_wc/log.c (revision 38658)
> +++ subversion/libsvn_wc/log.c (working copy)
> @@ -1729,12 +1729,31 @@
> * directory. PATH must not be outside that directory. */
> static const char *
> loggy_path(const char *path,
> - svn_wc_adm_access_t *adm_access)
> + svn_wc_adm_access_t *adm_access,
> + apr_pool_t *pool)
> {
> + const char *adm_abspath = NULL;
> + const char *abspath = NULL;
> const char *adm_path = svn_wc_adm_access_path(adm_access);
> - const char *local_path = svn_dirent_is_child(adm_path, path, NULL);
> + const char *local_path = NULL;
> + svn_error_t *err1 = NULL, *err2 = NULL;
>
> - if (! local_path && strcmp(path, adm_path) == 0)
> + err1 = svn_dirent_get_absolute(&adm_abspath, adm_path, pool);
> + if (err1)
> + {
> + svn_error_clear(err1);
> + return NULL;
> + }
> + err2 = svn_dirent_get_absolute(&abspath, path, pool);
> + if (err2)
> + {
> + svn_error_clear(err2);
> + return NULL;
> + }
> +
> + local_path = svn_dirent_is_child(adm_abspath, abspath, NULL);
> +
> + if (! local_path && strcmp(abspath, adm_abspath) == 0)
> local_path = SVN_WC_ENTRY_THIS_DIR;
>
> return local_path;
> @@ -1751,9 +1770,9 @@
> svn_xml_self_closing,
> SVN_WC__LOG_APPEND,
> SVN_WC__LOG_ATTR_NAME,
> - loggy_path(src, adm_access),
> + loggy_path(src, adm_access, pool),
> SVN_WC__LOG_ATTR_DEST,
> - loggy_path(dst, adm_access),
> + loggy_path(dst, adm_access, pool),
> NULL);
>
> return SVN_NO_ERROR;
> @@ -1768,7 +1787,7 @@
> {
> svn_xml_make_open_tag(log_accum, pool, svn_xml_self_closing,
> SVN_WC__LOG_COMMITTED,
> - SVN_WC__LOG_ATTR_NAME, loggy_path(path,
> adm_access),
> + SVN_WC__LOG_ATTR_NAME, loggy_path(path,
> adm_access, pool),
> SVN_WC__LOG_ATTR_REVISION,
> apr_psprintf(pool, "%ld", revnum),
> NULL);
> @@ -1783,8 +1802,8 @@
> apr_pool_t *pool)
> {
> return loggy_move_copy_internal(log_accum, FALSE, adm_access,
> - loggy_path(src_path, adm_access),
> - loggy_path(dst_path, adm_access),
> + loggy_path(src_path, adm_access,
> pool),
> + loggy_path(dst_path, adm_access,
> pool),
> pool);
> }
>
> @@ -1799,9 +1818,9 @@
> svn_xml_make_open_tag
> (log_accum, pool, svn_xml_self_closing,
> SVN_WC__LOG_CP_AND_TRANSLATE,
> - SVN_WC__LOG_ATTR_NAME, loggy_path(src, adm_access),
> - SVN_WC__LOG_ATTR_DEST, loggy_path(dst, adm_access),
> - SVN_WC__LOG_ATTR_ARG_2, loggy_path(versioned, adm_access),
> + SVN_WC__LOG_ATTR_NAME, loggy_path(src, adm_access, pool),
> + SVN_WC__LOG_ATTR_DEST, loggy_path(dst, adm_access, pool),
> + SVN_WC__LOG_ATTR_ARG_2, loggy_path(versioned, adm_access, pool),
> NULL);
>
> return SVN_NO_ERROR;
> @@ -1815,7 +1834,7 @@
> {
> svn_xml_make_open_tag(log_accum, pool, svn_xml_self_closing,
> SVN_WC__LOG_DELETE_ENTRY,
> - SVN_WC__LOG_ATTR_NAME, loggy_path(path,
> adm_access),
> + SVN_WC__LOG_ATTR_NAME, loggy_path(path,
> adm_access, pool),
> NULL);
>
> return SVN_NO_ERROR;
> @@ -1829,7 +1848,7 @@
> {
> svn_xml_make_open_tag(log_accum, pool, svn_xml_self_closing,
> SVN_WC__LOG_DELETE_LOCK,
> - SVN_WC__LOG_ATTR_NAME, loggy_path(path,
> adm_access),
> + SVN_WC__LOG_ATTR_NAME, loggy_path(path,
> adm_access, pool),
> NULL);
>
> return SVN_NO_ERROR;
> @@ -1843,7 +1862,7 @@
> {
> svn_xml_make_open_tag(log_accum, pool, svn_xml_self_closing,
> SVN_WC__LOG_DELETE_CHANGELIST,
> - SVN_WC__LOG_ATTR_NAME, loggy_path(path,
> adm_access),
> + SVN_WC__LOG_ATTR_NAME, loggy_path(path,
> adm_access, pool),
> NULL);
>
> return SVN_NO_ERROR;
> @@ -1988,7 +2007,7 @@
> return SVN_NO_ERROR;
>
> apr_hash_set(prop_hash, SVN_WC__LOG_ATTR_NAME,
> - APR_HASH_KEY_STRING, loggy_path(path, adm_access));
> + APR_HASH_KEY_STRING, loggy_path(path, adm_access,
> pool));
>
> svn_xml_make_open_tag_hash(log_accum, pool,
> svn_xml_self_closing,
> @@ -2010,7 +2029,7 @@
> svn_xml_make_open_tag(log_accum, pool, svn_xml_self_closing,
> SVN_WC__LOG_MODIFY_WCPROP,
> SVN_WC__LOG_ATTR_NAME,
> - loggy_path(path, adm_access),
> + loggy_path(path, adm_access, pool),
> SVN_WC__LOG_ATTR_PROPNAME,
> propname,
> SVN_WC__LOG_ATTR_PROPVAL,
> @@ -2027,8 +2046,8 @@
> apr_pool_t *pool)
> {
> return loggy_move_copy_internal(log_accum, TRUE, adm_access,
> - loggy_path(src_path, adm_access),
> - loggy_path(dst_path, adm_access),
> + loggy_path(src_path, adm_access,
> pool),
> + loggy_path(dst_path, adm_access,
> pool),
> pool);
> }
>
> @@ -2042,7 +2061,7 @@
> pool,
> svn_xml_self_closing,
> SVN_WC__LOG_MAYBE_EXECUTABLE,
> - SVN_WC__LOG_ATTR_NAME, loggy_path(path,
> adm_access),
> + SVN_WC__LOG_ATTR_NAME, loggy_path(path,
> adm_access, pool),
> NULL);
>
> return SVN_NO_ERROR;
> @@ -2059,7 +2078,7 @@
> svn_xml_self_closing,
> SVN_WC__LOG_MAYBE_READONLY,
> SVN_WC__LOG_ATTR_NAME,
> - loggy_path(path, adm_access),
> + loggy_path(path, adm_access, pool),
> NULL);
>
> return SVN_NO_ERROR;
> @@ -2076,7 +2095,7 @@
> svn_xml_self_closing,
> SVN_WC__LOG_MODIFY_ENTRY,
> SVN_WC__LOG_ATTR_NAME,
> - loggy_path(path, adm_access),
> + loggy_path(path, adm_access, pool),
> SVN_WC__ENTRY_ATTR_TEXT_TIME,
> SVN_WC__TIMESTAMP_WC,
> NULL);
> @@ -2095,7 +2114,7 @@
> svn_xml_self_closing,
> SVN_WC__LOG_MODIFY_ENTRY,
> SVN_WC__LOG_ATTR_NAME,
> - loggy_path(path, adm_access),
> + loggy_path(path, adm_access, pool),
> SVN_WC__ENTRY_ATTR_WORKING_SIZE,
> SVN_WC__WORKING_SIZE_WC,
> NULL);
> @@ -2114,7 +2133,7 @@
> svn_xml_self_closing,
> SVN_WC__LOG_READONLY,
> SVN_WC__LOG_ATTR_NAME,
> - loggy_path(path, adm_access),
> + loggy_path(path, adm_access, pool),
> NULL);
>
> return SVN_NO_ERROR;
> @@ -2132,7 +2151,7 @@
> svn_xml_self_closing,
> SVN_WC__LOG_SET_TIMESTAMP,
> SVN_WC__LOG_ATTR_NAME,
> - loggy_path(path, adm_access),
> + loggy_path(path, adm_access, pool),
> SVN_WC__LOG_ATTR_TIMESTAMP,
> timestr,
> NULL);
> @@ -2152,7 +2171,7 @@
> svn_xml_self_closing,
> SVN_WC__LOG_RM,
> SVN_WC__LOG_ATTR_NAME,
> - loggy_path(path, adm_access),
> + loggy_path(path, adm_access, pool),
> NULL);
>
> return SVN_NO_ERROR;
> Index: subversion/libsvn_wc/translate.c
> ===================================================================
> --- subversion/libsvn_wc/translate.c (revision 38658)
> +++ subversion/libsvn_wc/translate.c (working copy)
> @@ -169,26 +169,27 @@
>
>
> svn_error_t *
> -svn_wc_translated_file2(const char **xlated_path,
> - const char *src,
> - const char *versioned_file,
> - svn_wc_adm_access_t *adm_access,
> - apr_uint32_t flags,
> - apr_pool_t *pool)
> +svn_wc__internal_translated_file(const char **xlated_path,
> + const char *src,
> + svn_wc__db_t *db,
> + const char *versioned_abspath,
> + apr_uint32_t flags,
> + apr_pool_t *result_pool,
> + apr_pool_t *scratch_pool)
> {
> svn_subst_eol_style_t style;
> const char *eol;
> apr_hash_t *keywords;
> svn_boolean_t special;
> - svn_wc__db_t *db = svn_wc__adm_get_db(adm_access);
> - const char *versioned_abspath;
>
> - SVN_ERR(svn_dirent_get_absolute(&versioned_abspath,
> versioned_file, pool));
> +
Unneeded extra vertical whitespace.
> + SVN_ERR_ASSERT(svn_dirent_is_absolute(versioned_abspath));
> +
> SVN_ERR(svn_wc__get_eol_style(&style, &eol, db, versioned_abspath,
> - pool, pool));
> - SVN_ERR(svn_wc__get_keywords(&keywords, db, versioned_abspath,
> NULL, pool,
> - pool));
> - SVN_ERR(svn_wc__get_special(&special, db, versioned_abspath,
> pool));
> + result_pool, scratch_pool));
The result doesn't need to be available beyond this function, so use
scratch_pool for both pool parameters here.
> + SVN_ERR(svn_wc__get_keywords(&keywords, db, versioned_abspath,
> NULL,
> + result_pool, scratch_pool));
Same.
> + SVN_ERR(svn_wc__get_special(&special, db, versioned_abspath,
> result_pool));
Again, you can use scratch_pool here.
>
> if (! svn_subst_translation_required(style, eol, keywords,
> special, TRUE)
> && (! (flags & SVN_WC_TRANSLATE_FORCE_COPY)))
> @@ -207,14 +208,15 @@
> if (flags & SVN_WC_TRANSLATE_USE_GLOBAL_TMP)
> tmp_dir = NULL;
> else
> - tmp_dir =
> svn_wc__adm_child(svn_dirent_dirname(versioned_file, pool),
> - SVN_WC__ADM_TMP, pool);
> + tmp_dir = svn_wc__adm_child(
> + svn_dirent_dirname(versioned_abspath,
> result_pool),
scratch_pool
> + SVN_WC__ADM_TMP, scratch_pool);
>
> SVN_ERR(svn_io_open_unique_file3(NULL, &tmp_vfile, tmp_dir,
> (flags & SVN_WC_TRANSLATE_NO_OUTPUT_CLEANUP)
> ? svn_io_file_del_none
> : svn_io_file_del_on_pool_cleanup,
> - pool, pool));
> + result_pool, scratch_pool));
>
> /* ### ugh. the repair behavior does NOT match the docstring.
> bleah.
> ### all of these translation functions are crap and should go
> @@ -244,7 +246,7 @@
> keywords,
> expand,
> special,
> - pool));
> + result_pool));
>
> *xlated_path = tmp_vfile;
> }
> @@ -252,7 +254,21 @@
> return SVN_NO_ERROR;
> }
>
> +svn_error_t *
> +svn_wc_translated_file3(const char **xlated_path,
> + const char *src,
> + svn_wc_context_t *wc_ctx,
> + const char *versioned_abspath,
> + apr_uint32_t flags,
> + apr_pool_t *result_pool,
> + apr_pool_t *scratch_pool)
> +{
> + return svn_wc__internal_translated_file(xlated_path, src, wc_ctx-
> >db,
> + versioned_abspath, flags,
> + result_pool,scratch_pool);
> +}
>
> +
> svn_error_t *
> svn_wc__get_eol_style(svn_subst_eol_style_t *style,
> const char **eol,
> Index: subversion/libsvn_wc/translate.h
> ===================================================================
> --- subversion/libsvn_wc/translate.h (revision 38658)
> +++ subversion/libsvn_wc/translate.h (working copy)
> @@ -133,7 +133,17 @@
> apr_pool_t *result_pool,
> apr_pool_t *scratch_pool);
>
> +/* Internal version of svn_wc_translated_file3(). */
> +svn_error_t *
> +svn_wc__internal_translated_file(const char **xlated_path,
> + const char *src,
> + svn_wc__db_t *db,
> + const char *versioned_abspath,
> + apr_uint32_t flags,
> + apr_pool_t *result_pool,
> + apr_pool_t *scratch_pool);
>
> +
> #ifdef __cplusplus
> }
> #endif /* __cplusplus */
-Hyrum
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2382095
Received on 2009-08-10 16:37:16 CEST