You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ju...@apache.org on 2010/03/18 12:34:46 UTC

svn commit: r924722 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_client/switch.c libsvn_client/update.c libsvn_wc/deprecated.c libsvn_wc/update_editor.c

Author: julianfoad
Date: Thu Mar 18 11:34:45 2010
New Revision: 924722

URL: http://svn.apache.org/viewvc?rev=924722&view=rev
Log:
Clean up two revved WC API function signatures: move the wc_ctx parameter
before the first of the main input parameters, to keep those together and
to minimize the difference from the previous version.  Do some doc string
clean-ups too.

* subversion/include/svn_wc.h
  (svn_wc_get_status_editor5): Fix a parameter name typo. Note doc string
    out-of-dateness.
  (svn_wc_crawl_revisions5): Note doc string out-of-dateness.
  (svn_wc_get_update_editor4): Swap revision and wc_ctx parameters.
  (svn_wc_get_switch_editor4): Swap revision and wc_ctx parameters. Refer to
    svn_wc_get_update_editor4's doc string for most parameters to make it
    easy for the reader to see that there are no differences.

* subversion/libsvn_client/switch.c
  (switch_internal): Adjust calls.

* subversion/libsvn_client/update.c
  (update_internal): Adjust calls.

* subversion/libsvn_wc/deprecated.c
  (svn_wc_get_update_editor3, svn_wc_get_switch_editor3): Adjust calls.

* subversion/libsvn_wc/update_editor.c
  (svn_wc_get_update_editor4, svn_wc_get_switch_editor4): Adjust calls.

Modified:
    subversion/trunk/subversion/include/svn_wc.h
    subversion/trunk/subversion/libsvn_client/switch.c
    subversion/trunk/subversion/libsvn_client/update.c
    subversion/trunk/subversion/libsvn_wc/deprecated.c
    subversion/trunk/subversion/libsvn_wc/update_editor.c

Modified: subversion/trunk/subversion/include/svn_wc.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_wc.h?rev=924722&r1=924721&r2=924722&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_wc.h (original)
+++ subversion/trunk/subversion/include/svn_wc.h Thu Mar 18 11:34:45 2010
@@ -3923,6 +3923,9 @@ svn_wc_walk_status(svn_wc_context_t *wc_
  * state in it.  (Caller should obtain @a traversal_info from
  * svn_wc_init_traversal_info().)
  *
+ * ### Since r879231 it's not traversal_info, it's external_func/
+ *     external_baton which is ...?
+ *
  * Allocate the editor itself in @a pool, but the editor does temporary
  * allocations in a subpool of @a pool.
  *
@@ -3942,7 +3945,7 @@ svn_wc_get_status_editor5(const svn_delt
                           const apr_array_header_t *ignore_patterns,
                           svn_wc_status_func4_t status_func,
                           void *status_baton,
-                          svn_wc_external_update_t external_update,
+                          svn_wc_external_update_t external_func,
                           void *external_baton,
                           svn_cancel_func_t cancel_func,
                           void *cancel_baton,
@@ -4997,6 +5000,9 @@ svn_wc_process_committed(const char *pat
  * state in it.  (Caller should obtain @a traversal_info from
  * svn_wc_init_traversal_info().)
  *
+ * ### Since r879231 it's not traversal_info, it's external_func/
+ *     external_baton which is ...?
+ *
  * @since New in 1.7.
  */
 svn_error_t *
@@ -5257,8 +5263,8 @@ svn_wc_get_actual_target(const char *pat
 svn_error_t *
 svn_wc_get_update_editor4(const svn_delta_editor_t **editor,
                           void **edit_baton,
-                          svn_revnum_t *target_revision,
                           svn_wc_context_t *wc_ctx,
+                          svn_revnum_t *target_revision,
                           const char *anchor_abspath,
                           const char *target_basename,
                           svn_boolean_t use_commit_times,
@@ -5369,70 +5375,22 @@ svn_wc_get_update_editor(svn_revnum_t *t
                          apr_pool_t *pool);
 
 /**
- * A variant of svn_wc_get_update_editor().
+ * A variant of svn_wc_get_update_editor4().
  *
  * Set @a *editor and @a *edit_baton to an editor and baton for "switching"
  * a working copy to a new @a switch_url.  (Right now, this URL must be
  * within the same repository that the working copy already comes
  * from.)  @a switch_url must not be @c NULL.
  *
- * @a anchor_abspath is a local working copy directory, with a fully recursive
- * write lock in @a wc_ctx, which will be used as the root of our editor.
- *
- * @a target_basename is the entry in @a anchor_abspath that will actually be
- * updated, or the empty string if all of @a anchor_abspath should be updated.
- *
- * The editor invokes @a notify_func with @a notify_baton as the switch
- * progresses, if @a notify_func is non-NULL.
- *
- * If @a cancel_func is non-NULL, the editor will invoke @a cancel_func with
- * @a cancel_baton as the switch progresses to see if it should continue.
- *
- * If @a conflict_func is non-NULL, then invoke it with @a
- * conflict_baton whenever a conflict is encountered, giving the
- * callback a chance to resolve the conflict before the editor takes
- * more drastic measures (such as marking a file conflicted, or
- * bailing out of the switch).
- *
- * If @a external_func is non-NULL, then invoke it with @a external_baton
- * whenever external changes are encountered, giving the callback a chance
- * to store the external information for processing.
- *
- * If @a fetch_func is non-NULL, then use it (with @a fetch_baton) as
- * a fallback for retrieving repository files whenever 'copyfrom' args
- * are sent into editor->add_file().
- *
- * If @a diff3_cmd is non-NULL, then use it as the diff3 command for
- * any merging; otherwise, use the built-in merge code.
- *
- * @a preserved_exts is an array of filename patterns which, when
- * matched against the extensions of versioned files, determine for
- * which such files any related generated conflict files will preserve
- * the original file's extension as their own.  If a file's extension
- * does not match any of the patterns in @a preserved_exts (which is
- * certainly the case if @a preserved_exts is @c NULL or empty),
- * generated conflict files will carry Subversion's custom extensions.
- *
- * @a target_revision is a pointer to a revision location which, after
- * successful completion of the drive of this editor, will be
- * populated with the revision to which the working copy was updated.
- *
- * If @a use_commit_times is TRUE, then all edited/added files will
- * have their working timestamp set to the last-committed-time.  If
- * FALSE, the working files will be touched with the 'now' time.
- *
- * @a depth and @a depth_is_sticky behave as for svn_wc_get_update_editor3().
- *
- * If @a allow_unver_obstructions is TRUE, then allow unversioned
- * obstructions when adding a path.
+ * All other parameters behave as for svn_wc_get_update_editor4().
  *
  * @since New in 1.7.
  */
 svn_error_t *
 svn_wc_get_switch_editor4(const svn_delta_editor_t **editor,
                           void **edit_baton,
-                          svn_revnum_t *target_revision,
                           svn_wc_context_t *wc_ctx,
+                          svn_revnum_t *target_revision,
                           const char *anchor_abspath,
                           const char *target_basename,
                           const char *switch_url,

Modified: subversion/trunk/subversion/libsvn_client/switch.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/switch.c?rev=924722&r1=924721&r2=924722&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/switch.c (original)
+++ subversion/trunk/subversion/libsvn_client/switch.c Thu Mar 18 11:34:45 2010
@@ -189,7 +189,7 @@ switch_internal(svn_revnum_t *result_rev
   efb.ambient_depths = apr_hash_make(pool);
   efb.result_pool = pool;
   SVN_ERR(svn_wc_get_switch_editor4(&switch_editor, &switch_edit_baton,
-                                    &revnum, ctx->wc_ctx, anchor_abspath,
+                                    ctx->wc_ctx, &revnum, anchor_abspath,
                                     target, switch_rev_url, use_commit_times,
                                     depth,
                                     depth_is_sticky, allow_unver_obstructions,

Modified: subversion/trunk/subversion/libsvn_client/update.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/update.c?rev=924722&r1=924721&r2=924722&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/update.c (original)
+++ subversion/trunk/subversion/libsvn_client/update.c Thu Mar 18 11:34:45 2010
@@ -221,7 +221,7 @@ update_internal(svn_revnum_t *result_rev
   /* Fetch the update editor.  If REVISION is invalid, that's okay;
      the RA driver will call editor->set_target_revision later on. */
   SVN_ERR(svn_wc_get_update_editor4(&update_editor, &update_edit_baton,
-                                    &revnum, ctx->wc_ctx, anchor_abspath,
+                                    ctx->wc_ctx, &revnum, anchor_abspath,
                                     target, use_commit_times, depth,
                                     depth_is_sticky, allow_unver_obstructions,
                                     diff3_cmd, preserved_exts,

Modified: subversion/trunk/subversion/libsvn_wc/deprecated.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/deprecated.c?rev=924722&r1=924721&r2=924722&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/deprecated.c (original)
+++ subversion/trunk/subversion/libsvn_wc/deprecated.c Thu Mar 18 11:34:45 2010
@@ -2751,8 +2751,8 @@ svn_wc_get_update_editor3(svn_revnum_t *
     }
 
   SVN_ERR(svn_wc_get_update_editor4(editor, edit_baton,
-                                    target_revision,
                                     wc_ctx,
+                                    target_revision,
                                     svn_wc__adm_access_abspath(anchor),
                                     target,
                                     use_commit_times,
@@ -2868,8 +2868,8 @@ svn_wc_get_switch_editor3(svn_revnum_t *
     }
 
   SVN_ERR(svn_wc_get_switch_editor4(editor, edit_baton,
-                                    target_revision,
                                     wc_ctx,
+                                    target_revision,
                                     svn_wc__adm_access_abspath(anchor),
                                     target, switch_url,
                                     use_commit_times,

Modified: subversion/trunk/subversion/libsvn_wc/update_editor.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/update_editor.c?rev=924722&r1=924721&r2=924722&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/update_editor.c (original)
+++ subversion/trunk/subversion/libsvn_wc/update_editor.c Thu Mar 18 11:34:45 2010
@@ -5352,8 +5352,8 @@ make_editor(svn_revnum_t *target_revisio
 svn_error_t *
 svn_wc_get_update_editor4(const svn_delta_editor_t **editor,
                           void **edit_baton,
-                          svn_revnum_t *target_revision,
                           svn_wc_context_t *wc_ctx,
+                          svn_revnum_t *target_revision,
                           const char *anchor_abspath,
                           const char *target_basename,
                           svn_boolean_t use_commit_times,
@@ -5390,8 +5390,8 @@ svn_wc_get_update_editor4(const svn_delt
 svn_error_t *
 svn_wc_get_switch_editor4(const svn_delta_editor_t **editor,
                           void **edit_baton,
-                          svn_revnum_t *target_revision,
                           svn_wc_context_t *wc_ctx,
+                          svn_revnum_t *target_revision,
                           const char *anchor_abspath,
                           const char *target_basename,
                           const char *switch_url,



Re: svn commit: r924722 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_client/switch.c libsvn_client/update.c libsvn_wc/deprecated.c libsvn_wc/update_editor.c

Posted by Julian Foad <ju...@wandisco.com>.
On Thu, 2010-03-18 at 20:21 -0400, Greg Stein wrote:
> On Thu, Mar 18, 2010 at 19:54, Julian Foad <ju...@wandisco.com> wrote:
> > On Thu, 2010-03-18, Greg Stein wrote:
> >> On Thu, Mar 18, 2010 at 07:34,  <ju...@apache.org> wrote:
> >> > Author: julianfoad
> >> > Date: Thu Mar 18 11:34:45 2010
> >> > New Revision: 924722
> >> >
> >> > URL: http://svn.apache.org/viewvc?rev=924722&view=rev
> >> > Log:
> >> > Clean up two revved WC API function signatures: move the wc_ctx parameter
> >> > before the first of the main input parameters, to keep those together and
> >> > to minimize the difference from the previous version.  Do some doc string
> >> > clean-ups too.
> >> >
> >> > * subversion/include/svn_wc.h
> >> >  (svn_wc_get_status_editor5): Fix a parameter name typo. Note doc string
> >> >    out-of-dateness.
> >> >  (svn_wc_crawl_revisions5): Note doc string out-of-dateness.
> >> >  (svn_wc_get_update_editor4): Swap revision and wc_ctx parameters.
> >> >  (svn_wc_get_switch_editor4): Swap revision and wc_ctx parameters. Refer to
> >> >    svn_wc_get_update_editor4's doc string for most parameters to make it
> >> >    easy for the reader to see that there are no differences.
> >>
> >> Why swap these parameters? REVISION (or TARGET_REVISION) is an OUT
> >> parameter. We always place OUT parameters first, and then the WC_CTX,
> >> then the other params.
> >
> > Bert discussed this with me on IRC too.  My log message said why, but
> > actually there's more to it.  I hadn't been able to discern a reason why
> > the rev-number param was moved away (relative to the previous version of
> > the API) from the other target parameters in the first place.  After
> > this commit, Bert said he'd done it for the reason you state - output
> > params come before input params.  But this parameter isn't an output
> > from this function, at least not in the usual sense.  It's an address
> > given to this function, an address for this function to store away but
> > definitely not to read from or write to.  A different function will
> > later retrieve that address and write a revision number to it.
> >
> > In that sense, the pointer is an input to this function, whereas in
> > another sense we could call it an "output pointer" meaning it's a
> > pointer that will only ever be written through (not read from), but it
> > is neither a classical "input pointer parameter" nor a classical "output
> > pointer parameter" for this function.  Indeed, the code sequence in the
> > present only caller of svn_wc_get_update_editor4() depends on this
> > function call NOT overwriting the pointed-to revision number.
> >
> > I don't know of a precedent or rule for this specific situation so I
> > feel either way is OK.  Bert said he was happy to call it an input
> > parameter.  If you want to declare that this kind of parameter should be
> > grouped with classical output pointer parameters, I would be fine with
> > that decision and would happily change it.
> 
> Conceptually, it is still an output parameter. You are saying "put the
> result HERE." That's an output parameter.

Output of the editor, not of this function, but OK, that's a fair point
of view.  I'll change these two back.

r925061.

- Julian


> Sure, it takes a while to get the value; you have to drive the editor.
> But that's just timing. The concept is still an OUT param.
> 
> I certainly have no problem with a redesign of the dataflow, but for
> now... I see it as an OUT parameter.
> 
> (note that we have some similar-style params with some
> checksum-computing functions, too)


Re: svn commit: r924722 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_client/switch.c libsvn_client/update.c libsvn_wc/deprecated.c libsvn_wc/update_editor.c

Posted by Greg Stein <gs...@gmail.com>.
On Thu, Mar 18, 2010 at 19:54, Julian Foad <ju...@wandisco.com> wrote:
> On Thu, 2010-03-18, Greg Stein wrote:
>> On Thu, Mar 18, 2010 at 07:34,  <ju...@apache.org> wrote:
>> > Author: julianfoad
>> > Date: Thu Mar 18 11:34:45 2010
>> > New Revision: 924722
>> >
>> > URL: http://svn.apache.org/viewvc?rev=924722&view=rev
>> > Log:
>> > Clean up two revved WC API function signatures: move the wc_ctx parameter
>> > before the first of the main input parameters, to keep those together and
>> > to minimize the difference from the previous version.  Do some doc string
>> > clean-ups too.
>> >
>> > * subversion/include/svn_wc.h
>> >  (svn_wc_get_status_editor5): Fix a parameter name typo. Note doc string
>> >    out-of-dateness.
>> >  (svn_wc_crawl_revisions5): Note doc string out-of-dateness.
>> >  (svn_wc_get_update_editor4): Swap revision and wc_ctx parameters.
>> >  (svn_wc_get_switch_editor4): Swap revision and wc_ctx parameters. Refer to
>> >    svn_wc_get_update_editor4's doc string for most parameters to make it
>> >    easy for the reader to see that there are no differences.
>>
>> Why swap these parameters? REVISION (or TARGET_REVISION) is an OUT
>> parameter. We always place OUT parameters first, and then the WC_CTX,
>> then the other params.
>
> Bert discussed this with me on IRC too.  My log message said why, but
> actually there's more to it.  I hadn't been able to discern a reason why
> the rev-number param was moved away (relative to the previous version of
> the API) from the other target parameters in the first place.  After
> this commit, Bert said he'd done it for the reason you state - output
> params come before input params.  But this parameter isn't an output
> from this function, at least not in the usual sense.  It's an address
> given to this function, an address for this function to store away but
> definitely not to read from or write to.  A different function will
> later retrieve that address and write a revision number to it.
>
> In that sense, the pointer is an input to this function, whereas in
> another sense we could call it an "output pointer" meaning it's a
> pointer that will only ever be written through (not read from), but it
> is neither a classical "input pointer parameter" nor a classical "output
> pointer parameter" for this function.  Indeed, the code sequence in the
> present only caller of svn_wc_get_update_editor4() depends on this
> function call NOT overwriting the pointed-to revision number.
>
> I don't know of a precedent or rule for this specific situation so I
> feel either way is OK.  Bert said he was happy to call it an input
> parameter.  If you want to declare that this kind of parameter should be
> grouped with classical output pointer parameters, I would be fine with
> that decision and would happily change it.

Conceptually, it is still an output parameter. You are saying "put the
result HERE." That's an output parameter.

Sure, it takes a while to get the value; you have to drive the editor.
But that's just timing. The concept is still an OUT param.

I certainly have no problem with a redesign of the dataflow, but for
now... I see it as an OUT parameter.

(note that we have some similar-style params with some
checksum-computing functions, too)

Cheers,
-g

Re: svn commit: r924722 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_client/switch.c libsvn_client/update.c libsvn_wc/deprecated.c libsvn_wc/update_editor.c

Posted by Julian Foad <ju...@wandisco.com>.
On Thu, 2010-03-18, Greg Stein wrote:
> On Thu, Mar 18, 2010 at 07:34,  <ju...@apache.org> wrote:
> > Author: julianfoad
> > Date: Thu Mar 18 11:34:45 2010
> > New Revision: 924722
> >
> > URL: http://svn.apache.org/viewvc?rev=924722&view=rev
> > Log:
> > Clean up two revved WC API function signatures: move the wc_ctx parameter
> > before the first of the main input parameters, to keep those together and
> > to minimize the difference from the previous version.  Do some doc string
> > clean-ups too.
> >
> > * subversion/include/svn_wc.h
> >  (svn_wc_get_status_editor5): Fix a parameter name typo. Note doc string
> >    out-of-dateness.
> >  (svn_wc_crawl_revisions5): Note doc string out-of-dateness.
> >  (svn_wc_get_update_editor4): Swap revision and wc_ctx parameters.
> >  (svn_wc_get_switch_editor4): Swap revision and wc_ctx parameters. Refer to
> >    svn_wc_get_update_editor4's doc string for most parameters to make it
> >    easy for the reader to see that there are no differences.
> 
> Why swap these parameters? REVISION (or TARGET_REVISION) is an OUT
> parameter. We always place OUT parameters first, and then the WC_CTX,
> then the other params.

Bert discussed this with me on IRC too.  My log message said why, but
actually there's more to it.  I hadn't been able to discern a reason why
the rev-number param was moved away (relative to the previous version of
the API) from the other target parameters in the first place.  After
this commit, Bert said he'd done it for the reason you state - output
params come before input params.  But this parameter isn't an output
from this function, at least not in the usual sense.  It's an address
given to this function, an address for this function to store away but
definitely not to read from or write to.  A different function will
later retrieve that address and write a revision number to it.

In that sense, the pointer is an input to this function, whereas in
another sense we could call it an "output pointer" meaning it's a
pointer that will only ever be written through (not read from), but it
is neither a classical "input pointer parameter" nor a classical "output
pointer parameter" for this function.  Indeed, the code sequence in the
present only caller of svn_wc_get_update_editor4() depends on this
function call NOT overwriting the pointed-to revision number.

I don't know of a precedent or rule for this specific situation so I
feel either way is OK.  Bert said he was happy to call it an input
parameter.  If you want to declare that this kind of parameter should be
grouped with classical output pointer parameters, I would be fine with
that decision and would happily change it.

- Julian


Re: svn commit: r924722 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_client/switch.c libsvn_client/update.c libsvn_wc/deprecated.c libsvn_wc/update_editor.c

Posted by Greg Stein <gs...@gmail.com>.
On Thu, Mar 18, 2010 at 07:34,  <ju...@apache.org> wrote:
> Author: julianfoad
> Date: Thu Mar 18 11:34:45 2010
> New Revision: 924722
>
> URL: http://svn.apache.org/viewvc?rev=924722&view=rev
> Log:
> Clean up two revved WC API function signatures: move the wc_ctx parameter
> before the first of the main input parameters, to keep those together and
> to minimize the difference from the previous version.  Do some doc string
> clean-ups too.
>
> * subversion/include/svn_wc.h
>  (svn_wc_get_status_editor5): Fix a parameter name typo. Note doc string
>    out-of-dateness.
>  (svn_wc_crawl_revisions5): Note doc string out-of-dateness.
>  (svn_wc_get_update_editor4): Swap revision and wc_ctx parameters.
>  (svn_wc_get_switch_editor4): Swap revision and wc_ctx parameters. Refer to
>    svn_wc_get_update_editor4's doc string for most parameters to make it
>    easy for the reader to see that there are no differences.

Why swap these parameters? REVISION (or TARGET_REVISION) is an OUT
parameter. We always place OUT parameters first, and then the WC_CTX,
then the other params.

>...

Cheers,
-g