At 12:14 AM 06/07/2005 +0100, Philip Martin wrote:
>"Jonathan Gilbert" <o2w9gs702@sneakemail.com> writes:
>> When I added the command to the front-end of the command-line client, I
>> created an accompanying 'help' page. I preserved the same options that 'svn
>> checkout' takes, because my new 'svn takeover' is identical in behaviour to
>> 'svn checkout', with the exception of leaving existing files alone (and not
>> considering them a fatal error).
>
>It helps us review the patch if you write a log message, see the
>HACKING file.
>
>I think this is a feature a lot of people have been requesting, but
>I'm wondering why you made it a new command and not just a checkout
>(and update?) option?
Sorry, I do recall reading this but I guess it didn't sink in. Odd, too,
because I've contributed to other projects and always updated the ChangeLog
file :-)
>> Up until the code interfaces with the edit script driver & update editor,
>> the boolean indicating that a takeover is in progress is passed as an
>> additional parameter to functions (where applicable, I created a new
>> function, leaving the old interface intact with the old non-takeover
>> behaviour). Since the actual checking out of files during 'svn checkout' is
>> done by means of an update, after creating a skeleton .svn directory, the
>> flag gets passed there too. Once the code creates the svn_delta_editor_t
>> instance, it ceases to be a parameter; it is instead stored within the
>> struct. It is pulled back out of the struct and turned into a parameter
>> again only right before the add_file vtable entry is called, since add_file
>> does not receive a reference, direct or otherwise, to the editor.
>>
>> If I've missed any salient points, let me know, and I'll explain any other
>> changes I've made :-)
>
>Bad news :( I've had a very brief look at the patch and it looks like
>you have modified the ABI. The changes to svn_delta_editor_t and
>svn_wc_notify_action_t will break binary compatibility, again see the
>HACKING file, which means that the patch is not suitable for
>Subversion 1.x.
Okay, well, can you give me some advice here?
Firstly, if the new enum value were placed at the end of the enum where it
wouldn't disrupt the existing values, would that still be considered a
breaking change to the ABI?
And regarding the changes to svn_delta_editor_t: Can you think of any other
way to get that flag from the command-line interface to the update editor?
Is there any way to convey "edit"-global state to the editor? I'd think
global state would be inappropriate because it would preclude the
possibility of running multiple updates concurrently. And, of course,
whatever solution is used, it needs to perform well so that doesn't
interfere with the speed of large checkouts, e.g. a hash table for mapping
data to the current "edit" would be out of the question.
The two changes to the svn_delta_editor_t struct are:
1. A new parameter to add_file. I couldn't see any existing parameter in
which to squirrel away a boolean. Perhaps some odd scheme such as abusing
the name parameter could be devised, but this would be an ugly hack, and I
don't really like ugly code.
2. A new member to the struct itself. I also needed a way to convey the
flag to the various edit script drivers -- DAV, SVN, local -- so that they
could pass the parameter in to the add_file method.
Both of these changes can be avoided if another way to convey the flag to
the editor can be devised.
Another possibility that springs to mind is that an altered update_editor.c
could be made -- takeover_editor.c? -- with essentially the same behaviour
as update_editor.c but with the flag hardcoded. Then, the code path from
the main module for doing a takeover would invoke a takeover instead of an
update.
Does any of this make sense? :-)
Jonathan Gilbert
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jul 6 04:48:45 2005