[PATCH] Fix Issue #2986

[David Glasser <gl...@davidglasser.net>]

Now ready for review.  (Ben has promised me some in-person review time
tomorrow too :-) but the more eyes the better, as this is a big
change.)

A few outstanding questions:

There's this part where I filter out svn:entry:* props from being
copied.  How about svn:wc:* props?  I haven't tested this with DAV
yet.

Am I being careful to use revert-base instead of text-base everywhere
necessary?  I don't think I'm regressing anywhere but there might be
some pre-existing problems.  For example, should svn_wc__merge_props
be considering using revert-props?

Some stuff in add_file_with_history should be moved into a subpool and cleared.

There's still the issue in locate_copyfrom where we are making a brand
new adm_access instead of using the set we already have, and not
taking out a write lock.

[[[
Fix Issue #2986, by essentially rewriting the wc-side logic for
copy-on-update.

Previously, add_file_with_history in the WC update editor faked these
actions:

  * add_file
  * install something like the copied file
  * close_file
  * run logs early
  * open_file

This led to a bunch of problems; the biggest one was that the
premature log run could leave the WC in a state that corresponded to
no state of the repository.  Additionally, the extra log runs could be
a performance problem; some operations (like copying over the working
file) occured without using the loggy system; and the
text-base/revert-base distinction was not followed.

Now, add_file_with_history just runs a normal add_file; functions like
apply_textdelta and merge_props have been changed to be capable of
taking base and working data from the file baton instead of the
working copy.

* subversion/libsvn_wc/merge.c
  (svn_wc__merge_internal): Add a new copyfrom_text argument; if
   not NULL, the working text is taken from this file instead of from
   the actual version in the working copy. Allow the merge_target to
   not be under version control if this argument is given.
  (svn_wc_merge3): Update call to svn_wc__merge_internal.

* subversion/libsvn_wc/props.c
  (svn_wc__merge_props): Add new base_props and working_props
   arguments; if given, use them as properties for the file we're
   merging instead of reading them from the wc.
  (svn_wc_merge_props2): Update call to svn_wc__merge_props.


* subversion/libsvn_wc/update_editor.c
  (file_baton): New fields: booleans added_with_history and
   received_textdelta, paths copied_text_base and
   copied_working_text, and property hashes copied_base_props and
   copied_working_props.  Remove send_notification field (which was
   only necessary because of the extra close_file in the previous
   implementation's add_file_with_history).
  (choose_base_paths): New function extracted from apply_textdelta
   which assigns the text_base_path and new_text_base_path fields in
   the file_baton (to either text-base or revert-base paths), as well
   as returning some other relevant information.
  (apply_textdelta): Set received_textdelta when called.  Use the
   extracted choose_base_paths function.  If we have a copied text
   base, use that instead of the empty stream for the delta's source.
  (window_handler): When done, close the copied_text_base instead of
   the usual text-base if that's what you were using.
  (close_file): If this was an add_with_history but there were no text
   deltas, set up the base path fields, new text base file, and
   checksum as if a trivial delta had occured.  Remove reference to
   send_notification.
  (merge_file): Pass the two copied property hashes to
   svn_wc__merge_props via merge_props.  If we saved a copy of the
   working text in add_file_with_history, then consider us to have
   local mods.  Use .copied as conflict file extension if it is
   copied.  Use copied text-base path as the left argument to the
   merge it's there.  Pass copied working text path as
   svn_wc__merge_internal's new parameter.  Loggily clean up copied
   working text temp file after running the merge, and loggliy clean
   up the copied text-base temp file at the end.
  (add_file_with_history): Rewrite most of the function to set up all
   the new fields, copy a bunch of things to temp files, etc.  But
   don't run any logs!
  (copy_non_entry_props): New helper func copying a property hash but
   without svn:entry:* props.
  (merge_props): Add base_props and working_props arguments, passed
   straight through to svn_wc__merge_props.
  (make_file_baton): Don't initialize send_notification.
  (close_directory): Update call to svn_wc__merge_props.

* subversion/libsvn_wc/props.h
  (svn_wc__merge_props): Update declaration and documentation.

* subversion/libsvn_wc/wc.h
  (svn_wc__merge_internal): Update declaration and documentation.

* subversion/libsvn_wc/log.c
  (log_do_merge): Update call to svn_wc__merge_internal.
]]]


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

Re: [PATCH] Fix Issue #2986

[David Glasser <gl...@davidglasser.net>]

On 10/25/07, David Glasser <gl...@davidglasser.net> wrote:
> On 10/24/07, David Glasser <gl...@davidglasser.net> wrote:
> > Now ready for review.  (Ben has promised me some in-person review time
> > tomorrow too :-) but the more eyes the better, as this is a big
> > change.)
> >
> > A few outstanding questions:
> >
> > There's this part where I filter out svn:entry:* props from being
> > copied.  How about svn:wc:* props?  I haven't tested this with DAV
> > yet.
> >
> > Am I being careful to use revert-base instead of text-base everywhere
> > necessary?  I don't think I'm regressing anywhere but there might be
> > some pre-existing problems.  For example, should svn_wc__merge_props
> > be considering using revert-props?
> >
> > Some stuff in add_file_with_history should be moved into a subpool and cleared.
> >
> > There's still the issue in locate_copyfrom where we are making a brand
> > new adm_access instead of using the set we already have, and not
> > taking out a write lock.
>
> Oh, and of course: there should be more tests.  At the very least,
> something should be testing translations, and the replace/revert-base
> cases...

I committed this patch in r27377, because it does pass tests and that
will make it easier for me to start working on the ambient_depth
refactoring; however, the above points are still worth considering.

--dave

-- 
David Glasser | glasser@davidglasser.net | 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] Fix Issue #2986

[David Glasser <gl...@davidglasser.net>]

On 10/24/07, David Glasser <gl...@davidglasser.net> wrote:
> Now ready for review.  (Ben has promised me some in-person review time
> tomorrow too :-) but the more eyes the better, as this is a big
> change.)
>
> A few outstanding questions:
>
> There's this part where I filter out svn:entry:* props from being
> copied.  How about svn:wc:* props?  I haven't tested this with DAV
> yet.
>
> Am I being careful to use revert-base instead of text-base everywhere
> necessary?  I don't think I'm regressing anywhere but there might be
> some pre-existing problems.  For example, should svn_wc__merge_props
> be considering using revert-props?
>
> Some stuff in add_file_with_history should be moved into a subpool and cleared.
>
> There's still the issue in locate_copyfrom where we are making a brand
> new adm_access instead of using the set we already have, and not
> taking out a write lock.

Oh, and of course: there should be more tests.  At the very least,
something should be testing translations, and the replace/revert-base
cases...

--dave


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

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