-----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
Received on Sat May 26 18:29:46 2007