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

Re: svn commit: r1387696 - /subversion/trunk/subversion/libsvn_wc/conflicts.c

From: Hyrum K Wright <hyrum_at_hyrumwright.org>
Date: Thu, 11 Oct 2012 12:20:32 -0400

On Wed, Sep 19, 2012 at 2:06 PM, <stsp_at_apache.org> wrote:
> Author: stsp
> Date: Wed Sep 19 18:06:13 2012
> New Revision: 1387696
>
> URL: http://svn.apache.org/viewvc?rev=1387696&view=rev
> Log:
> * subversion/libsvn_wc/conflicts.c
> (read_prop_conflicts): New helper to prepare for the times when new
> conflict storage is enabled. Currently, this describes each property
> conflict in a separate svn_wc_conflict_description2_t, within the
> limitations of svn_wc_conflict_description2_t. These limitations
> and suggested improvements have been recorded in comments.
> (svn_wc__read_conflicts): Use the new read_prop_conflicts() helper
> instead of creating just one conflict description.
>
> Modified:
> subversion/trunk/subversion/libsvn_wc/conflicts.c
>
> Modified: subversion/trunk/subversion/libsvn_wc/conflicts.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/conflicts.c?rev=1387696&r1=1387695&r2=1387696&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_wc/conflicts.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/conflicts.c Wed Sep 19 18:06:13 2012
> @@ -1907,6 +1907,153 @@ svn_wc__conflict_invoke_resolver(svn_wc_
> return SVN_NO_ERROR;
> }
>
> +/* Read all property conflicts contained in CONFLICT_SKEL into
> + * individual conflict descriptions, and append those descriptions
> + * to the CONFLICTS array. Allocate results in RESULT_POOL.
> + * SCRATCH_POOL is used for temporary allocations. */
> +static svn_error_t *
> +read_prop_conflicts(apr_array_header_t *conflicts,
> + svn_wc__db_t *db,
> + const char *local_abspath,
> + svn_skel_t *conflict_skel,
> + apr_pool_t *result_pool,
> + apr_pool_t *scratch_pool)
> +{
> + const char *prop_reject_file;
> + apr_hash_t *my_props;
> + apr_hash_t *their_old_props;
> + apr_hash_t *their_props;
> + apr_hash_t *conflicted_props;
> + apr_hash_index_t *hi;
> + apr_pool_t *iterpool;
> +
> + SVN_ERR(svn_wc__conflict_read_prop_conflict(&prop_reject_file,
> + &my_props,
> + &their_old_props,
> + &their_props,
> + &conflicted_props,
> + db, local_abspath,
> + conflict_skel,
> + scratch_pool, scratch_pool));
> +
> + if (apr_hash_count(conflicted_props) == 0)
> + {
> + /* Legacy prop conflict with only a .reject file. */
> + svn_wc_conflict_description2_t *desc;
> +
> + desc = svn_wc_conflict_description_create_prop2(local_abspath,
> + svn_node_unknown,
> + "", result_pool);
> +
> + /* ### This should be changed. The prej file should be stored
> + * ### separately from the other files. We need to rev the
> + * ### conflict description struct for this. */
> + desc->their_abspath = apr_pstrdup(result_pool, prop_reject_file);
> +
> + APR_ARRAY_PUSH(conflicts, svn_wc_conflict_description2_t*) = desc;
> +
> + return SVN_NO_ERROR;
> + }
> +
> + iterpool = svn_pool_create(scratch_pool);
> + for (hi = apr_hash_first(scratch_pool, conflicted_props);
> + hi;
> + hi = apr_hash_next(hi))
> + {
> + const char *propname = svn__apr_hash_index_key(hi);
> + svn_string_t *old_value;
> + svn_string_t *my_value;
> + svn_string_t *their_value;
> + svn_wc_conflict_description2_t *desc;
> +
> + svn_pool_clear(iterpool);
> +
> + desc = svn_wc_conflict_description_create_prop2(local_abspath,
> + svn_node_unknown,
> + propname,
> + result_pool);
> +
> + desc->property_name = apr_pstrdup(result_pool, propname);
> +
> + my_value = apr_hash_get(my_props, propname, APR_HASH_KEY_STRING);
> + their_value = apr_hash_get(their_props, propname,
> + APR_HASH_KEY_STRING);
> +
> + /* Compute the incoming side of the conflict ('action'). */
> + if (their_value == NULL)
> + desc->action = svn_wc_conflict_action_delete;
> + else if (my_value == NULL)
> + desc->action = svn_wc_conflict_action_add;

Wrong enum type assignment here, the source is an
svn_wc_conflict_reason_t, but the destination is an
svn_wc_conflict_action_t. While this might work now, it feels like a
real opportunity for obscure bugs later. (And my compiler complains
about it. :) )

> + else
> + desc->action = svn_wc_conflict_action_edit;

Same.

> +
> + /* Compute the local side of the conflict ('reason'). */
> + if (my_value == NULL)
> + desc->reason = svn_wc_conflict_reason_deleted;
> + else if (their_value == NULL)
> + desc->action = svn_wc_conflict_reason_added;
> + else
> + desc->action = svn_wc_conflict_reason_edited;
> +
> + /* ### This should be changed. The prej file should be stored
> + * ### separately from the other files. We need to rev the
> + * ### conflict description struct for this. */
> + desc->their_abspath = apr_pstrdup(result_pool, prop_reject_file);
> +
> + /* ### This should be changed. The conflict description for
> + * ### props should contain these values as svn_string_t,
> + * ### rather than in temporary files. We need to rev the
> + * ### conflict description struct for this. */
> + if (my_value)
> + {
> + svn_stream_t *s;
> + apr_size_t len;
> +
> + SVN_ERR(svn_stream_open_unique(&s, &desc->my_abspath, NULL,
> + svn_io_file_del_on_pool_cleanup,
> + result_pool, iterpool));
> + len = my_value->len;
> + SVN_ERR(svn_stream_write(s, my_value->data, &len));
> + SVN_ERR(svn_stream_close(s));
> + }
> +
> + if (their_value)
> + {
> + svn_stream_t *s;
> + apr_size_t len;
> +
> + /* ### Currently, their_abspath is used for the prop reject file.
> + * ### Put their value into merged instead...
> + * ### We need to rev the conflict description struct to fix this. */
> + SVN_ERR(svn_stream_open_unique(&s, &desc->merged_file, NULL,
> + svn_io_file_del_on_pool_cleanup,
> + result_pool, iterpool));
> + len = their_value->len;
> + SVN_ERR(svn_stream_write(s, their_value->data, &len));
> + SVN_ERR(svn_stream_close(s));
> + }
> +
> + old_value = apr_hash_get(their_old_props, propname, APR_HASH_KEY_STRING);
> + if (old_value)
> + {
> + svn_stream_t *s;
> + apr_size_t len;
> +
> + SVN_ERR(svn_stream_open_unique(&s, &desc->base_abspath, NULL,
> + svn_io_file_del_on_pool_cleanup,
> + result_pool, iterpool));
> + len = old_value->len;
> + SVN_ERR(svn_stream_write(s, old_value->data, &len));
> + SVN_ERR(svn_stream_close(s));
> + }
> +
> + APR_ARRAY_PUSH(conflicts, svn_wc_conflict_description2_t*) = desc;
> + }
> + svn_pool_destroy(iterpool);
> +
> + return SVN_NO_ERROR;
> +}
> +
> svn_error_t *
> svn_wc__read_conflicts(const apr_array_header_t **conflicts,
> svn_wc__db_t *db,
> @@ -1942,21 +2089,8 @@ svn_wc__read_conflicts(const apr_array_h
> sizeof(svn_wc_conflict_description2_t*));
>
> if (prop_conflicted)
> - {
> - svn_wc_conflict_description2_t *desc;
> - desc = svn_wc_conflict_description_create_prop2(local_abspath,
> - svn_node_unknown,
> - "",
> - result_pool);
> -
> - SVN_ERR(svn_wc__conflict_read_prop_conflict(&desc->their_abspath,
> - NULL, NULL, NULL, NULL,
> - db, local_abspath,
> - conflict_skel,
> - result_pool, scratch_pool));
> -
> - APR_ARRAY_PUSH(cflcts, svn_wc_conflict_description2_t*) = desc;
> - }
> + SVN_ERR(read_prop_conflicts(cflcts, db, local_abspath, conflict_skel,
> + result_pool, scratch_pool));
>
> if (text_conflicted)
> {
>
>
Received on 2012-10-11 18:21:09 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.