[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r960858 - /subversion/trunk/subversion/libsvn_client/patch.c

From: Stefan Sperling <stsp_at_elego.de>
Date: Tue, 6 Jul 2010 12:14:44 +0200

On Tue, Jul 06, 2010 at 09:57:02AM -0000, rhuijben_at_apache.org wrote:
> Author: rhuijben
> Date: Tue Jul 6 09:57:02 2010
> New Revision: 960858
>
> URL: http://svn.apache.org/viewvc?rev=960858&view=rev
> Log:
> Following up on r960851, move code in a function by itself and resolve
> an error on checking files not in a wc.
>
> * subversion/libsvn_client/patch.c
> (resolve_target_wc_file_info): New function.
> (resolve_target_path): Extract eol and keywords check code and
> use only if the file is in the wc.
>
> Suggested by: stsp

> @@ -227,7 +280,7 @@ strip_path(const char **result, const ch
> * STRIP_COUNT specifies the number of leading path components
> * which should be stripped from target paths in the patch.
> * If the path is not skipped also obtain eol-style and keywords
> - * information and store this in *TARGET.
> + * information by calling resolve_target_wc_info() on TARGET.
> * Use RESULT_POOL for allocations of fields in TARGET. */
> static svn_error_t *
> resolve_target_path(patch_target_t *target,

As soon as we change the code again, the above docstring may become obsolete.
I'd suggest leaving the docstring as it was.
Callers of resolve_target_path() should not care in what way eol/keyword
information is obtained.

> @@ -334,49 +387,9 @@ resolve_target_path(patch_target_t *targ
> if (target->locally_deleted && target->kind_on_disk != svn_node_none)
> target->skipped = TRUE;
>
> - {
> - apr_hash_t *props;
> - svn_string_t *keywords_val, *eol_style_val;
> -
> - /* Handle svn:keyword and svn:eol-style properties. */
> - SVN_ERR(svn_wc_prop_list2(&props, wc_ctx, target->local_abspath,
> - scratch_pool, scratch_pool));
> - keywords_val = apr_hash_get(props, SVN_PROP_KEYWORDS,
> - APR_HASH_KEY_STRING);
> - if (keywords_val)
> - {
> - svn_revnum_t changed_rev;
> - apr_time_t changed_date;
> - const char *rev_str;
> - const char *author;
> - const char *url;
> -
> - SVN_ERR(svn_wc__node_get_changed_info(&changed_rev,
> - &changed_date,
> - &author, wc_ctx,
> - target->local_abspath,
> - scratch_pool,
> - scratch_pool));
> - rev_str = apr_psprintf(scratch_pool, "%"SVN_REVNUM_T_FMT,
> - changed_rev);
> - SVN_ERR(svn_wc__node_get_url(&url, wc_ctx,
> - target->local_abspath,
> - scratch_pool, scratch_pool));
> - SVN_ERR(svn_subst_build_keywords2(&target->keywords,
> - keywords_val->data,
> - rev_str, url, changed_date,
> - author, result_pool));
> - }
> -
> - eol_style_val = apr_hash_get(props, SVN_PROP_EOL_STYLE,
> - APR_HASH_KEY_STRING);
> - if (eol_style_val)
> - {
> - svn_subst_eol_style_from_value(&target->eol_style,
> - &target->eol_str,
> - eol_style_val->data);
> - }
> - }
> + if (target->kind_on_disk == svn_node_file)

Oh, and please don't call resolve_target_wc_file_info() on skipped
targets (look at target->skipped).

> + SVN_ERR(resolve_target_wc_file_info(wc_ctx, target->local_abspath,
> + target, result_pool, scratch_pool));
>
> return SVN_NO_ERROR;
> }
>
Received on 2010-07-06 12:15:27 CEST

This is an archived mail posted to the Subversion Dev mailing list.