You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Rall <dl...@collab.net> on 2007/05/25 19:18:20 UTC
Re: svn commit: r25156 - in branches/merge-sensitive-log/subversion: svn tests/cmdline
Nice!! Time to merge to trunk.
On Fri, 25 May 2007, hwright@tigris.org wrote:
...
> On the merge-sensitive-log branch:
> Add command line client output of merge sensitive log information. Update test,
> and mark it as passing.
>
> * subversion/svn/log-cmd.c
> (log_receiver_baton): Add the merge_stack member.
> (merge_frame): New structure.
> (log_message_receiver): Check for nesting, and output extra "Result of merge"
> line if the message of interested in a child of another message. Also,
> check for children, and push state onto the merge stack if they exist.
> (svn_cl__log): Initialize data structures.
>
> * subversion/test/cmdline/log_tests.py
> (check_merge_results): Check the results of the parse of the "Result of merge"
> line with the expected value.
> (simple_merge_sensitive_log): Update the exepected_merges value to match
> the expected input to check_merge_results().
> (test_list): Mark test simple_merge_sensitive_log() test as passing.
...
> --- branches/merge-sensitive-log/subversion/svn/log-cmd.c (original)
> +++ branches/merge-sensitive-log/subversion/svn/log-cmd.c Fri May 25 11:48:55 2007
> @@ -50,6 +50,22 @@
>
> /* Don't print log message body nor its line count. */
> svn_boolean_t omit_log_message;
> +
> + /* Stack with keeps track of merge revision nesting. */
^^^^
Doc string should use "that" or "which". You might also want to
mention the data type that this list holds (struct merge_frame *'s).
> + apr_array_header_t *merge_stack;
> +};
> +
> +/* Structure to hold merging revisions, and the number of children they have
> + remaining. These structures get pushed and popped from
> + log_receiver_baton.merge_stack, and help implement the pre-order traversal
> + of the log message tree. */
> +struct merge_frame
> +{
> + /* The revision the merge occured in. */
> + svn_revnum_t merge_rev;
> +
> + /* The number of outstanding children. */
> + apr_uint64_t child_count;
We ended up calling this "nbr_children" elsewhere.
> };
...
I was wondering if we want to use an iterpool in the following loop;
but, it doesn't look like it's necessary.
> + if (lb->merge_stack->nelts > 0)
> + {
> + int i;
> + struct merge_frame *frame = APR_ARRAY_IDX(lb->merge_stack,
> + lb->merge_stack->nelts - 1,
> + struct merge_frame *);
> +
> + /* Print the result of merge line */
> + SVN_ERR(svn_cmdline_printf(pool, _("Result of a merge from:")));
> + for (i = 0; i < lb->merge_stack->nelts; i++)
> + {
> + struct merge_frame *output_frame = APR_ARRAY_IDX(lb->merge_stack, i,
> + struct merge_frame *);
> +
> + SVN_ERR(svn_cmdline_printf(pool, " r%ld%c", output_frame->merge_rev,
> + i == lb->merge_stack->nelts - 1 ?
> + '\n' : ','));
> + }
> +
> + /* Decrement the child_counter, and check to see if we have any more
> + children. If not, pop the stack. */
> + frame->child_count -= 1;
Or:
frame->child_count--;
> + if (frame->child_count == 0)
> + apr_array_pop(lb->merge_stack);
> + }
...
> + if (log_entry->nbr_children > 0)
> + {
> + struct merge_frame *frame = apr_palloc(pool, sizeof(*frame));
> +
> + frame->merge_rev = log_entry->revision;
> + frame->child_count = log_entry->nbr_children;
> +
> + APR_ARRAY_PUSH(lb->merge_stack, struct merge_frame *) = frame;
> + }
> +
> return SVN_NO_ERROR;
> }
>
> @@ -490,6 +542,7 @@
> lb.cancel_func = ctx->cancel_func;
> lb.cancel_baton = ctx->cancel_baton;
> lb.omit_log_message = opt_state->quiet;
> + lb.merge_stack = apr_array_make(pool, 1, sizeof(svn_revnum_t));
The element size for this list is incorrect. You want sizeof(struct
merge_frame *).
...
> --- branches/merge-sensitive-log/subversion/tests/cmdline/log_tests.py (original)
> +++ branches/merge-sensitive-log/subversion/tests/cmdline/log_tests.py Fri May 25 11:48:55 2007
> @@ -927,6 +927,13 @@
> for rev in expected_merges.keys():
> try:
> log = [x for x in log_chain if x['revision'] == rev][0]
> + actual = log['merges']
> + expected = expected_merges[rev]
> +
> + if actual != expected:
> + raise SVNUnexpectedLogs((("Merging revisions in rev %d not correct; " +
> + "expecting ") % rev) + str(expected) +
> + ", found " + str(actual), log_chain)
This string concat might look a little cleaner with format specifiers.
Also, the indentation looks a tad suspect.
> except IndexError:
> raise SVNUnexpectedLogs("Merged revision '%d' missing" % rev, log_chain)
>
> @@ -957,7 +964,7 @@
>
> log_chain = parse_log_output(output)
> expected_merges = {
> - 1 : 7, 3 : 7, 4 : 7, 5 : 7, 6 : 7,
> + 1 : [2], 3 : [7], 4 : [7], 6 : [7],
> }
> check_merge_results(log_chain, expected_merges)
...
Re: svn commit: r25156 - in branches/merge-sensitive-log/subversion:
svn tests/cmdline
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Daniel Rall wrote:
> Nice!! Time to merge to trunk.
Dan,
I implemented your suggested in r25165. Thanks for the review!
- -Hyrum
> On Fri, 25 May 2007, hwright@tigris.org wrote:
> ...
>> On the merge-sensitive-log branch:
>> Add command line client output of merge sensitive log information. Update test,
>> and mark it as passing.
>>
>> * subversion/svn/log-cmd.c
>> (log_receiver_baton): Add the merge_stack member.
>> (merge_frame): New structure.
>> (log_message_receiver): Check for nesting, and output extra "Result of merge"
>> line if the message of interested in a child of another message. Also,
>> check for children, and push state onto the merge stack if they exist.
>> (svn_cl__log): Initialize data structures.
>>
>> * subversion/test/cmdline/log_tests.py
>> (check_merge_results): Check the results of the parse of the "Result of merge"
>> line with the expected value.
>> (simple_merge_sensitive_log): Update the exepected_merges value to match
>> the expected input to check_merge_results().
>> (test_list): Mark test simple_merge_sensitive_log() test as passing.
> ...
>> --- branches/merge-sensitive-log/subversion/svn/log-cmd.c (original)
>> +++ branches/merge-sensitive-log/subversion/svn/log-cmd.c Fri May 25 11:48:55 2007
>> @@ -50,6 +50,22 @@
>>
>> /* Don't print log message body nor its line count. */
>> svn_boolean_t omit_log_message;
>> +
>> + /* Stack with keeps track of merge revision nesting. */
> ^^^^
> Doc string should use "that" or "which". You might also want to
> mention the data type that this list holds (struct merge_frame *'s).
>
>> + apr_array_header_t *merge_stack;
>> +};
>> +
>> +/* Structure to hold merging revisions, and the number of children they have
>> + remaining. These structures get pushed and popped from
>> + log_receiver_baton.merge_stack, and help implement the pre-order traversal
>> + of the log message tree. */
>> +struct merge_frame
>> +{
>> + /* The revision the merge occured in. */
>> + svn_revnum_t merge_rev;
>> +
>> + /* The number of outstanding children. */
>> + apr_uint64_t child_count;
>
> We ended up calling this "nbr_children" elsewhere.
We did, but this one is actually a counter which keeps track of how many
children we are expecting to see. I've renamed it to
"children_remaining", which is (hopefully) a bit more descriptive.
>> };
> ...
>
> I was wondering if we want to use an iterpool in the following loop;
> but, it doesn't look like it's necessary.
>
>> + if (lb->merge_stack->nelts > 0)
>> + {
>> + int i;
>> + struct merge_frame *frame = APR_ARRAY_IDX(lb->merge_stack,
>> + lb->merge_stack->nelts - 1,
>> + struct merge_frame *);
>> +
>> + /* Print the result of merge line */
>> + SVN_ERR(svn_cmdline_printf(pool, _("Result of a merge from:")));
>> + for (i = 0; i < lb->merge_stack->nelts; i++)
>> + {
>> + struct merge_frame *output_frame = APR_ARRAY_IDX(lb->merge_stack, i,
>> + struct merge_frame *);
>> +
>> + SVN_ERR(svn_cmdline_printf(pool, " r%ld%c", output_frame->merge_rev,
>> + i == lb->merge_stack->nelts - 1 ?
>> + '\n' : ','));
>> + }
>> +
>> + /* Decrement the child_counter, and check to see if we have any more
>> + children. If not, pop the stack. */
>> + frame->child_count -= 1;
>
> Or:
>
> frame->child_count--;
>
>> + if (frame->child_count == 0)
>> + apr_array_pop(lb->merge_stack);
>> + }
> ...
>> + if (log_entry->nbr_children > 0)
>> + {
>> + struct merge_frame *frame = apr_palloc(pool, sizeof(*frame));
>> +
>> + frame->merge_rev = log_entry->revision;
>> + frame->child_count = log_entry->nbr_children;
>> +
>> + APR_ARRAY_PUSH(lb->merge_stack, struct merge_frame *) = frame;
>> + }
>> +
>> return SVN_NO_ERROR;
>> }
>>
>> @@ -490,6 +542,7 @@
>> lb.cancel_func = ctx->cancel_func;
>> lb.cancel_baton = ctx->cancel_baton;
>> lb.omit_log_message = opt_state->quiet;
>> + lb.merge_stack = apr_array_make(pool, 1, sizeof(svn_revnum_t));
>
> The element size for this list is incorrect. You want sizeof(struct
> merge_frame *).
>
> ...
>> --- branches/merge-sensitive-log/subversion/tests/cmdline/log_tests.py (original)
>> +++ branches/merge-sensitive-log/subversion/tests/cmdline/log_tests.py Fri May 25 11:48:55 2007
>> @@ -927,6 +927,13 @@
>> for rev in expected_merges.keys():
>> try:
>> log = [x for x in log_chain if x['revision'] == rev][0]
>> + actual = log['merges']
>> + expected = expected_merges[rev]
>> +
>> + if actual != expected:
>> + raise SVNUnexpectedLogs((("Merging revisions in rev %d not correct; " +
>> + "expecting ") % rev) + str(expected) +
>> + ", found " + str(actual), log_chain)
>
> This string concat might look a little cleaner with format specifiers.
> Also, the indentation looks a tad suspect.
>
>> except IndexError:
>> raise SVNUnexpectedLogs("Merged revision '%d' missing" % rev, log_chain)
>>
>> @@ -957,7 +964,7 @@
>>
>> log_chain = parse_log_output(output)
>> expected_merges = {
>> - 1 : 7, 3 : 7, 4 : 7, 5 : 7, 6 : 7,
>> + 1 : [2], 3 : [7], 4 : [7], 6 : [7],
>> }
>> check_merge_results(log_chain, expected_merges)
> ...
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFGWGD3CwOubk4kUXwRAijCAJ41s+7BAunWJnMnnKWUGl/FjWA/+wCgqahA
j79P3xGjbyLbayHctqNW9Ho=
=IzYK
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org