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

Re: [PATCH] - Fix issue 3690 - "svn log" with ignore property changes

From: Noorul Islam K M <noorul_at_collab.net>
Date: Wed, 20 Jul 2011 17:44:06 +0530

Philip Martin <philip.martin_at_wandisco.com> writes:

> Noorul Islam K M <noorul_at_collab.net> writes:
>
>> Philip Martin <philip.martin_at_wandisco.com> writes:
>>
>>> Noorul Islam K M <noorul_at_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,
Received on 2011-07-20 14:16:26 CEST

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