[PATCH] Don't close_edit in svn_ra_replay

[David Glasser <gl...@mit.edu>]

svn_ra_replay and svn_repos_replay2 do essentially the same thing at
different layers... except that svn_ra_replay calls close_edit on its
editor after the replay is done, and svn_repos_replay2 doesn't.  At
the very least, this inconsistency should be documented in 1.4.

I'd prefer, though, that ra_replay just shouldn't call close_edit at
all.  Not only is this more consistent, but it also is more flexible;
for example, in the theoretical future where we can set revprops
atomically over RA, we might want to slip set-revprop calls in between
ra_replay and close_edit.

The following patch implements this. It is mostly a matter of removing
close_edit calls, and adding a close_edit to svnsync (the only client
of svn_ra_replay).  This means that you can test this patch by running
the svnsync command-line tests; I've done this for ra_local, ra_svn,
and ra_dav, but not for ra_serf, because I don't have serf installed
on this machine yet.

The one tricky bit is that in order for svnserve to tell the client
that the replay is done without asking it to close (or abort) the
edit, I've added a "finish-replay" command which just switches from
the current editor session back to whichever command set was running
before it; this required upgrading an svn_ra_svn.h API to allow
specifying whether or not a given editor session is a replay session,
so that finish-replay can be an error outside of replay.

--dave

[[[
Don't close_edit at the end of svn_ra_replay, for consistency
with svn_repos_replayN.

* subversion/include/svn_ra_svn.h
  (svn_ra_svn_drive_editor2): Upgraded API taking a "for_replay"
   argument, specifying whether or not finish-replay should
   be accepted in this session.

* subversion/libsvn_ra_dav/replay.c
  (end_element): Don't close_edit at the end of a replay.

* subversion/libsvn_ra_local/ra_plugin.c
  (end_element): Don't close_edit at the end of a replay.

* subversion/libsvn_ra_serf/replay.c
  (end_replay): Don't close_edit at the end of a replay.

* subversion/libsvn_ra_svn/client.c
  (ra_svn_replay): Call new svn_ra_svn_drive_editor2 with
   TRUE for_replay.

* subversion/libsvn_ra_svn/editor.c
  (ra_svn_driver_state_t): Add a for_replay field.
  (ra_svn_handle_finish_replay): New function to finish
   an editor session without close_edit.
  (ra_svn_edit_commands): Register finish-replay.
  (svn_ra_svn_drive_editor2): Take new for_replay argument.

* subversion/libsvn_ra_svn/editorp.c
  (ra_svn_driver_state_t): Add a for_replay field.
  (ra_svn_handle_finish_replay): New function to finish
   an editor session without close_edit.
  (ra_svn_edit_commands): Register finish-replay.
  (svn_ra_svn__drive_editorp): Take new for_replay argument.

* subversion/libsvn_ra_svn/protocol
  Document finish-replay.

* subversion/libsvn_ra_svn/ra_svn.h
  (svn_ra_svn__drive_editorp): Take for_replay argument.

* subversion/mod_dav_svn/reports/replay.c
  (end_report): New function to add the closing
   </S:editor-report> to a replay report.
  (close_edit): This shouldn't be called any more,
   but in any case make it do nothing.
  (dav_svn__replay_report): Call end_report instead
   of close_edit.

* subversion/svnserve/serve.c
  (replay): Send a finish-replay command instead of
   calling close_edit.

* subversion/svnsync/main.c
  (do_synchronize): Explicitly close_edit after invoking
   ra_replay.
]]]



-- 
David Glasser | glasser@mit.edu | http://www.davidglasser.net/

Re: [PATCH] Don't close_edit in svn_ra_replay

[David Glasser <gl...@mit.edu>]

On 8/15/06, Garrett Rooney <ro...@electricjellyfish.net> wrote:
> It's going to need a backport branch for us to vote on it, since the
> diff doesn't apply to the version of mod_dav_svn in 1.4.x (maxb's
> refactoring in trunk moved the location of the code for the replay
> report, among other things).  I may not get around to that in the next
> few hours, as I need to head to the airport fairly soon, but if it
> hasn't been taken care of by the time I get to a network connection on
> the other end of the trip I'll handle it.

Thanks for looking into this.

I would help with the merge to 1.4.x, but I just shipped my
development MacBook back to Apple for repairs :(

--dave

-- 
David Glasser | glasser@mit.edu | http://www.davidglasser.net/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Don't close_edit in svn_ra_replay

[Garrett Rooney <ro...@electricjellyfish.net>]

On 8/14/06, David Glasser <gl...@mit.edu> wrote:
> svn_ra_replay and svn_repos_replay2 do essentially the same thing at
> different layers... except that svn_ra_replay calls close_edit on its
> editor after the replay is done, and svn_repos_replay2 doesn't.  At
> the very least, this inconsistency should be documented in 1.4.
>
> I'd prefer, though, that ra_replay just shouldn't call close_edit at
> all.  Not only is this more consistent, but it also is more flexible;
> for example, in the theoretical future where we can set revprops
> atomically over RA, we might want to slip set-revprop calls in between
> ra_replay and close_edit.
>
> The following patch implements this. It is mostly a matter of removing
> close_edit calls, and adding a close_edit to svnsync (the only client
> of svn_ra_replay).  This means that you can test this patch by running
> the svnsync command-line tests; I've done this for ra_local, ra_svn,
> and ra_dav, but not for ra_serf, because I don't have serf installed
> on this machine yet.
>
> The one tricky bit is that in order for svnserve to tell the client
> that the replay is done without asking it to close (or abort) the
> edit, I've added a "finish-replay" command which just switches from
> the current editor session back to whichever command set was running
> before it; this required upgrading an svn_ra_svn.h API to allow
> specifying whether or not a given editor session is a replay session,
> so that finish-replay can be an error outside of replay.

This seems good to me.  I've committed it to trunk (with a few minor
tweaks, mostly breaking overly long lines) and proposed it for
backport to 1.4.x.

It's going to need a backport branch for us to vote on it, since the
diff doesn't apply to the version of mod_dav_svn in 1.4.x (maxb's
refactoring in trunk moved the location of the code for the replay
report, among other things).  I may not get around to that in the next
few hours, as I need to head to the airport fairly soon, but if it
hasn't been taken care of by the time I get to a network connection on
the other end of the trip I'll handle it.

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org