Philip Martin <philip.mar...@wandisco.com> writes:
> Noorul Islam K M <noo...@collab.net> writes:
>
>> Philip Martin <philip.mar...@wandisco.com> writes:
>>
>>> Noorul Islam K M <noo...@collab.net> writes:
>>>
>>>> +static svn_error_t *
>>>> +props_only_receiver(void *baton, svn_log_entry_t *log_entry, apr_pool_t
>>>> *pool)
>>>> +{
>>>> + props_only_receiver_baton_t *rb = baton;
>>>> +
>>>> + if (log_entry->changed_paths2)
>>>> + {
>>>> + apr_array_header_t *sorted_paths;
>>>> + int i;
>>>> + svn_boolean_t text_modified = FALSE;
>>>> +
>>>> + /* Get an array of sorted hash keys. */
>>>> + sorted_paths = svn_sort__hash(log_entry->changed_paths2,
>>>> + svn_sort_compare_items_as_paths,
>>>> pool);
>>>
>>> Why sort?
>>>
>>
>> I just used an existing function. If this is going to be performance hit
>> then I can modify this part.
>
> Sorting when not necessary is a performance penalty. Why not simply
> iterate over the hash?
>
>>
>>>> +
>>>> + for (i = 0; i < sorted_paths->nelts; i++)
>>>> + {
>>>> + svn_sort__item_t *item = &(APR_ARRAY_IDX(sorted_paths, i,
>>>> + svn_sort__item_t));
>>>> + svn_log_changed_path2_t *log_item
>>>> + = apr_hash_get(log_entry->changed_paths2, item->key,
>>>> item->klen);
>>>> +
>>>> + if (log_item->text_modified == svn_tristate_true)
>>>> + {
>>>> + text_modified = TRUE;
>>>> + break;
>>>> + }
>>>> +
>>>> + }
>>>> +
>>>> + if ((text_modified && rb->props_only)
>>>> + || (! text_modified && rb->ignore_props_only))
>>>> + return SVN_NO_ERROR;
>>>> + }
>>>> +
>>>> + if (! rb->discover_changed_paths)
>>>> + log_entry->changed_paths2 = NULL;
>>>
>>> Set changed_paths as well?
>>>
>>
>> This is necessary because, if the user is not passing the verbose option
>> then they are not requesting this particular detail.
>>
>> But I have to pass discover_changed_paths as TRUE for these new options
>> because changed_paths2 information is required to filter.
>
> If you set changed_paths2 to NULL I think you need to set changed_paths
> to NULL as well.
Thank you! I agree with what you said. I updated the patch incorporating
you review comments.
Log
[[[
Make 'svn log' take --ignore-props-only and --props-only options.
If passed --ignore-props-only, log will ignore revisions that has only
property changes.
If passed --props-only, log will retrieve revisions that has only
property changes.
* subversion/svn/cl.h
(svn_cl__opt_state_t): New props_only and ignore_props_only members.
* subversion/svn/main.c
(svn_cl__longopt_t): Add opt_props_only and opt_ignore_props_only
members.
(svn_cl__options): Document new flags.
(svn_cl__cmd_table): Accept new flags for log operations.
(main): If given --ignore-props-only/--props-only, set the option
flags to TRUE. Disallow simultaneous use of both --ignore-props and
--props-only.
* subversion/include/svn_client.h
(svn_client_log6): New log variant with new flags.
(svn_client_log5): Deprecate.
* subversion/libsvn_client/deprecated.c
(svn_client_log5): New deprecated wrapper.
* subversion/libsvn_client/log.c
(props_only_receiver_baton_t): Struct for props_only_receiver.
(props_only_receiver): Receiver which implements --props-only and
--ignore-props-only functionality.
(svn_client_log5): Rename.
(svn_client_log6): New API with new flags.
* subversion/svn/log-cmd.c
(svn_cl__log): Call the new client API, with new flags.
* subversion/tests/cmdline/log_tests.py
(log_props_only): New test.
(test_list): Add new test.
* subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
(): add --props-only and --ignore-props-only help text.
Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
]]]
Index: subversion/tests/cmdline/log_tests.py
===================================================================
--- subversion/tests/cmdline/log_tests.py (revision 1148663)
+++ subversion/tests/cmdline/log_tests.py (working copy)
@@ -2036,6 +2036,41 @@
svntest.actions.run_and_verify_svn(None, None, expected_error,
'log', '-q', bad_path_default_rev)
+def log_props_only(sbox):
+ "svn log --ignore-props-only and --props-only"
+ sbox.build()
+ wc_dir = sbox.wc_dir
+ iota_file = os.path.join(wc_dir, 'iota')
+ svntest.main.run_svn(None, 'propset', 'foo', 'bar', iota_file)
+ svntest.main.run_svn(None, 'ci', '-m',
+ 'Set property "foo" to "bar" on A/iota', wc_dir)
+
+ svntest.main.run_svn(None, 'propset', 'bar', 'foo', iota_file)
+ svntest.main.run_svn(None, 'ci', '-m',
+ 'Set property "bar" to "foo" on A/iota', wc_dir)
+
+ svntest.main.file_write(iota_file, "Add dummy contents.\n")
+ svntest.main.run_svn(None, 'propset', 'dummy', 'dummy_value', iota_file)
+ svntest.main.run_svn(None, 'ci', '-m',
+ 'Modify iota and add property dummy', wc_dir)
+ svntest.main.run_svn(None, 'up', wc_dir)
+
+ exit_code, output, error = svntest.main.run_svn(0, 'log',
+ '--ignore-props-only',
+ wc_dir)
+
+ expected_output_re = re.compile(".*Set property.*")
+ if expected_output_re.search("".join(output), re.MULTILINE):
+ raise svntest.Failure('Log with --ignore-props-only failed')
+
+ exit_code, output, error = svntest.main.run_svn(0, 'log',
+ '--props-only',
+ wc_dir)
+
+ expected_output_re = re.compile(".*Modify iota.*revision 1.*")
+ if expected_output_re.search("".join(output), re.MULTILINE):
+ raise svntest.Failure('Log with --props-only failed')
+
########################################################################
# Run the tests
@@ -2075,6 +2110,7 @@
merge_sensitive_log_ignores_cyclic_merges,
log_with_unrelated_peg_and_operative_revs,
log_on_nonexistent_path_and_valid_rev,
+ log_props_only,
]
if __name__ == '__main__':
Index: subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
===================================================================
--- subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
(revision 1148663)
+++ subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
(working copy)
@@ -81,6 +81,8 @@
Ignore changes in EOL style.
-p (--show-c-function):
Show C function name in diff output.
+ --ignore-props-only : ignore property only revisions
+ --props-only : retrieve property only revisions
Global options:
--username ARG : specify a username ARG
Index: subversion/svn/cl.h
===================================================================
--- subversion/svn/cl.h (revision 1148663)
+++ subversion/svn/cl.h (working copy)
@@ -230,6 +230,10 @@
svn_boolean_t internal_diff; /* override diff_cmd in config file */
svn_boolean_t use_git_diff_format; /* Use git's extended diff format */
svn_boolean_t allow_mixed_rev; /* Allow operation on mixed-revision WC */
+ svn_boolean_t ignore_props_only; /* Ignore revisions with only property
+ changes */
+ svn_boolean_t props_only; /* Retrieve revisions with only property
+ changes */
} svn_cl__opt_state_t;
Index: subversion/svn/log-cmd.c
===================================================================
--- subversion/svn/log-cmd.c (revision 1148663)
+++ subversion/svn/log-cmd.c (working copy)
@@ -721,7 +721,7 @@
if (!opt_state->quiet)
APR_ARRAY_PUSH(revprops, const char *) = SVN_PROP_REVISION_LOG;
}
- SVN_ERR(svn_client_log5(targets,
+ SVN_ERR(svn_client_log6(targets,
&peg_revision,
opt_state->revision_ranges,
opt_state->limit,
@@ -729,6 +729,8 @@
opt_state->stop_on_copy,
opt_state->use_merge_history,
revprops,
+ opt_state->ignore_props_only,
+ opt_state->props_only,
log_entry_receiver_xml,
&lb,
ctx,
@@ -744,7 +746,7 @@
APR_ARRAY_PUSH(revprops, const char *) = SVN_PROP_REVISION_DATE;
if (!opt_state->quiet)
APR_ARRAY_PUSH(revprops, const char *) = SVN_PROP_REVISION_LOG;
- SVN_ERR(svn_client_log5(targets,
+ SVN_ERR(svn_client_log6(targets,
&peg_revision,
opt_state->revision_ranges,
opt_state->limit,
@@ -752,6 +754,8 @@
opt_state->stop_on_copy,
opt_state->use_merge_history,
revprops,
+ opt_state->ignore_props_only,
+ opt_state->props_only,
log_entry_receiver,
&lb,
ctx,
Index: subversion/svn/main.c
===================================================================
--- subversion/svn/main.c (revision 1148663)
+++ subversion/svn/main.c (working copy)
@@ -125,6 +125,8 @@
opt_internal_diff,
opt_use_git_diff_format,
opt_allow_mixed_revisions,
+ opt_ignore_props_only,
+ opt_props_only,
} svn_cl__longopt_t;
@@ -340,6 +342,10 @@
"Use of this option is not recommended!\n"
" "
"Please run 'svn update' instead.")},
+ {"ignore-props-only", opt_ignore_props_only, 0,
+ N_("ignore property only revisions")},
+ {"props-only", opt_props_only, 0,
+ N_("retrieve property only revisions")},
/* Long-opt Aliases
*
@@ -653,7 +659,8 @@
" svn log http://www.example.com/repo/project@50 foo.c bar.c\n"),
{'r', 'q', 'v', 'g', 'c', opt_targets, opt_stop_on_copy, opt_incremental,
opt_xml, 'l', opt_with_all_revprops, opt_with_no_revprops,
opt_with_revprop,
- opt_depth, opt_diff, opt_diff_cmd, opt_internal_diff, 'x'},
+ opt_depth, opt_diff, opt_diff_cmd, opt_internal_diff, 'x',
+ opt_ignore_props_only, opt_props_only},
{{opt_with_revprop, N_("retrieve revision property ARG")},
{'c', N_("the change made in revision ARG")}} },
@@ -2025,6 +2032,12 @@
case opt_allow_mixed_revisions:
opt_state.allow_mixed_rev = TRUE;
break;
+ case opt_ignore_props_only:
+ opt_state.ignore_props_only = TRUE;
+ break;
+ case opt_props_only:
+ opt_state.props_only = TRUE;
+ break;
default:
/* Hmmm. Perhaps this would be a good place to squirrel away
opts that commands like svn diff might need. Hmmm indeed. */
@@ -2204,6 +2217,16 @@
return svn_cmdline_handle_exit_error(err, pool, "svn: ");
}
+ /* Disallow simultaneous use of both --ignore-props-only and
+ --props-only. */
+ if (opt_state.props_only && opt_state.ignore_props_only)
+ {
+ err = svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+ _("--props-only and --ignore-props-only "
+ "are mutually exclusive"));
+ return svn_cmdline_handle_exit_error(err, pool, "svn: ");
+ }
+
/* Ensure that 'revision_ranges' has at least one item, and make
'start_revision' and 'end_revision' match that item. */
if (opt_state.revision_ranges->nelts == 0)
Index: subversion/include/svn_client.h
===================================================================
--- subversion/include/svn_client.h (revision 1148663)
+++ subversion/include/svn_client.h (working copy)
@@ -2436,6 +2436,12 @@
* If @a revprops is NULL, retrieve all revprops; else, retrieve only the
* revprops named in the array (i.e. retrieve none if the array is empty).
*
+ * If @a ignore_props_only is set, log information for revisions which
+ * has property changes alone will be ignored.
+ *
+ * If @a props_only is set, log information for revisions which
+ * has property changes alone will be returned.
+ *
* Use @a pool for any temporary allocation.
*
* @par Important:
@@ -2448,8 +2454,33 @@
* If @a ctx->notify_func2 is non-NULL, then call @a ctx->notify_func2/baton2
* with a 'skip' signal on any unversioned targets.
*
+ * @since New in 1.8.
+ */
+svn_error_t *
+svn_client_log6(const apr_array_header_t *targets,
+ const svn_opt_revision_t *peg_revision,
+ const apr_array_header_t *revision_ranges,
+ int limit,
+ svn_boolean_t discover_changed_paths,
+ svn_boolean_t strict_node_history,
+ svn_boolean_t include_merged_revisions,
+ const apr_array_header_t *revprops,
+ svn_boolean_t ignore_props_only,
+ svn_boolean_t props_only,
+ svn_log_entry_receiver_t receiver,
+ void *receiver_baton,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool);
+
+/**
+ * Similar to svn_client_log6(), but with @a ignore_props_only
+ * and @a props_only always @c FALSE.
+ *
+ * @deprecated Provided for compatibility with the 1.6 API.
+ *
* @since New in 1.6.
*/
+SVN_DEPRECATED
svn_error_t *
svn_client_log5(const apr_array_header_t *targets,
const svn_opt_revision_t *peg_revision,
Index: subversion/libsvn_client/deprecated.c
===================================================================
--- subversion/libsvn_client/deprecated.c (revision 1148663)
+++ subversion/libsvn_client/deprecated.c (working copy)
@@ -1272,6 +1272,26 @@
/*** From log.c ***/
svn_error_t *
+svn_client_log5(const apr_array_header_t *targets,
+ const svn_opt_revision_t *peg_revision,
+ const apr_array_header_t *revision_ranges,
+ int limit,
+ svn_boolean_t discover_changed_paths,
+ svn_boolean_t strict_node_history,
+ svn_boolean_t include_merged_revisions,
+ const apr_array_header_t *revprops,
+ svn_log_entry_receiver_t receiver,
+ void *receiver_baton,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool)
+{
+ return svn_client_log6(targets, peg_revision, revision_ranges, limit,
+ discover_changed_paths, strict_node_history,
+ include_merged_revisions, revprops, FALSE, FALSE,
+ receiver, receiver_baton, ctx, pool);
+}
+
+svn_error_t *
svn_client_log4(const apr_array_header_t *targets,
const svn_opt_revision_t *peg_revision,
const svn_opt_revision_t *start,
Index: subversion/libsvn_client/log.c
===================================================================
--- subversion/libsvn_client/log.c (revision 1148663)
+++ subversion/libsvn_client/log.c (working copy)
@@ -262,11 +262,59 @@
}
+
+/* properties only receiver */
+typedef struct props_only_receiver_baton_t
+{
+ svn_boolean_t props_only;
+ svn_boolean_t ignore_props_only;
+ svn_boolean_t discover_changed_paths;
+ svn_log_entry_receiver_t receiver;
+ void *baton;
+} props_only_receiver_baton_t;
+
+static svn_error_t *
+props_only_receiver(void *baton, svn_log_entry_t *log_entry, apr_pool_t *pool)
+{
+ props_only_receiver_baton_t *rb = baton;
+
+ if (log_entry->changed_paths2)
+ {
+ apr_hash_index_t *hi;
+ svn_boolean_t text_modified = FALSE;
+
+ for (hi = apr_hash_first(pool, log_entry->changed_paths2);
+ hi; hi = apr_hash_next(hi))
+ {
+ svn_log_changed_path2_t *log_item = svn__apr_hash_index_val(hi);
+
+ if (log_item->text_modified == svn_tristate_true)
+ {
+ text_modified = TRUE;
+ break;
+ }
+
+ }
+
+ if ((text_modified && rb->props_only)
+ || (! text_modified && rb->ignore_props_only))
+ return SVN_NO_ERROR;
+ }
+
+ if (! rb->discover_changed_paths)
+ {
+ log_entry->changed_paths2 = NULL;
+ log_entry->changed_paths = NULL;
+ }
+
+ return rb->receiver(rb->baton, log_entry, pool);
+}
+
/*** Public Interface. ***/
svn_error_t *
-svn_client_log5(const apr_array_header_t *targets,
+svn_client_log6(const apr_array_header_t *targets,
const svn_opt_revision_t *peg_revision,
const apr_array_header_t *revision_ranges,
int limit,
@@ -274,6 +322,8 @@
svn_boolean_t strict_node_history,
svn_boolean_t include_merged_revisions,
const apr_array_header_t *revprops,
+ svn_boolean_t ignore_props_only,
+ svn_boolean_t props_only,
svn_log_entry_receiver_t real_receiver,
void *real_receiver_baton,
svn_client_ctx_t *ctx,
@@ -562,6 +612,7 @@
svn_log_entry_receiver_t passed_receiver;
void *passed_receiver_baton;
const apr_array_header_t *passed_receiver_revprops;
+ props_only_receiver_baton_t pob;
svn_pool_clear(iterpool);
@@ -608,6 +659,19 @@
passed_receiver_baton = &lb;
}
+ if (ignore_props_only || props_only)
+ {
+ pob.props_only = props_only;
+ pob.ignore_props_only = ignore_props_only;
+ pob.discover_changed_paths = discover_changed_paths;
+ discover_changed_paths = TRUE;
+ pob.receiver = passed_receiver;
+ pob.baton = passed_receiver_baton;
+
+ passed_receiver = props_only_receiver;
+ passed_receiver_baton = &pob;
+ }
+
SVN_ERR(svn_ra_get_log2(ra_session,
condensed_targets,
start_revnum,
