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

Re: Improving support for copy and move

From: Ivan Zhakov <chemodax_at_gmail.com>
Date: 2006-05-25 14:39:40 CEST

On 5/25/06, Paul Burba <paulb@softlanding.com> wrote:
> "Ivan Zhakov" <chemodax@gmail.com> wrote on 05/25/2006 05:25:24 AM:
>
> > On 5/18/06, Paul Burba <paulb@softlanding.com> wrote:
> > > Paul Burba <paulb@softlanding.com> wrote on 05/16/2006 04:51:59 PM:
> > >
> > > > Hi Ivan,
> > > >
> > > > Your patch seems to work when moving/copying copied *files* only.
> Not
> > > > sure what exactly is going on, but trying to copy a copied directory
> > > > fails:
> > > >
> > > > svnadmin create /svn/repos/cm1
> > > >
> > > > svn co file:///svn/repos/cm1 /svn/wcs/cm1
> > > > Checked out revision 0.
> > > >
> > > > touch /svn/wcs/cm1/foo
> > > >
> > > > svn mkdir /svn/wcs/cm1/bar
> > > > A \SVN\WCS\cm1\bar
> > > >
> > > > svn add /svn/wcs/cm1/foo
> > > > A \SVN\WCS\cm1\foo
> > > >
> > > > svn ci -m "log message" /svn/wcs/cm1
> > > > Adding WCS\cm1\bar
> > > > Adding WCS\cm1\foo
> > > > Transmitting file data .
> > > > Committed revision 1.
> > > >
> > > > svn cp /svn/wcs/cm1/foo /svn/wcs/cm1/foo1
> > > > A \SVN\WCS\cm1\foo1
> > > >
> > > > svn cp /svn/wcs/cm1/foo1 /svn/wcs/cm1/foo2
> > > > A \SVN\WCS\cm1\foo2
> > > >
> > > > svn cp /svn/wcs/cm1/bar /svn/wcs/cm1/bar1
> > > > A \SVN\WCS\cm1\bar1
> > > >
> > > > svn cp /svn/wcs/cm1/bar1 /svn/wcs/cm1/bar2
> > > >
> > > > c:\svn\svn.trunk.copymove\src-trunk.collabnet.
> > > > trunk\subversion\libsvn_wc\adm_files.c:974:
> > > > (apr_err=155000)
> > > > svn: URL 'file:///svn/repos/cm1/bar' doesn't match existing URL
> > > > 'file:///svn/repos/cm1/bar1' in '/SVN/WCS/cm1/bar2/bar1'
> > > >
> > > > I'll look into this tomorrow, but in the meantime if you have any
> > > thoughts
> > > > on what is wrong please let me know.
> > > >
> > > > Thanks,
> > > >
> > > > Paul B.
> > >
> > > Ivan,
> > >
> > > Sorry for the delay. The problem starts in
> copy_dir_administratively()
> > > where, following the example above, call svn_io_copy_dir_recursively(
> > > src_path=="bar1", dst_basename=="bar2") is called. This results in
> the
> > > following fields in /svn/wcs/cm1/bar2/.svn/entries:
> > >
> > > URL: file:///svn/repos/cm1/bar1
> > > COPIED: copied
> > > COPYFROM_URL: file:///svn/repos/cm1/bar
> > >
> > > Later in copy_dir_administratively() your patch sets copyfrom_url to
> > > src_entry->copyfrom_url=="file:///svn/repos/cm1/bar". Then
> > > svn_wc_add2(copyfrom_url=="file:///svn/repos/cm1/bar") is called,
> which in
> > > turn calls svn_wc_ensure_adm2(path=="bar2", url=="
> > > file:///svn/repos/cm1/bar") which fails with an
> > > SVN_ERR_WC_OBSTRUCTED_UPDATE error since the given URL=="
> > > file:///svn/repos/cm1/bar" doesn't match the URL in the administrative
> > > area=="file:///svn/repos/cm1/bar1".
> > >
> > > A new patch is attached with a partial solution: If moving/copying a
> > > copied dir, it sets the URL field in entries to
> src_entry->copyfrom_url ("
> > > file:///svn/repos/cm1/bar" in the above example) after calling
> > > svn_io_copy_dir_recursively(). This prevents svn_wc_ensure_adm2()
> from
> > > failing. The correct URL is then set by svn_wc__do_update_cleanup().
> This
> > > is what happens during a "normal" versioned directory copy so it seems
> > > like the right thing to do...but I'm hardly certain it's the best
> > > approach.
> > >
> > > Anyway, as my original purpose was to create some tests for your
> patch,
> > > five new copy tests are included too:
> > >
> > > 1) copy_copied_file_and_dir - copy a file and dir, copy the copies,
> commit
> > >
> > > 2) move_copied_file_and_dir - copy a file and dir, move the copies,
> commit
> > >
> > > 3) move_moved_file_and_dir - move a file and dir, move them again,
> commit
> > >
> > > 4) move_file_within_moved_dir - move a dir, then move a file within
> that
> > > dir twice, commit
> > >
> > > 5) move_dir_within_moved_dir - move a dir, then move a dir within that
> dir
> > > twice, commit
> > >
> > > As I said, my patch is, at best, only a partial solution...tests 4 & 5
> > > still fail when attempting the last of the three moves. I don't want
> to
> > > proceed any further until some folks with more experience in this area
> > > weigh in. I'm just going in circles at this point :-(
> > >
> > > Please take a look, thanks,
> > >
> > > Paul B.
> > >
> > > [[[
> > > Allow copy/move files that already copied.
> > > * subversion/libsvn_wc/copy.c:
> > > (copy_file_administratively, copy_dir_administratively): Use
> source's
> > > copyfrom-url and copyfrom-rev instead of url and rev when copying or
> > > moving already copied files or directories. When copying
> directories,
> > > set the URL entry in the destination directories "this dir" entries
> > > record to the source's copyfrom-url.
> > >
> > > * subversion/tests/cmdline/copy_tests.py
> > > (copy_copied_file_and_dir, move_copied_file_and_dir,
> > > move_moved_file_and_dir, move_file_within_moved_dir,
> > > move_dir_within_moved_dir): New tests.
> > > ]]]
> > >
> > >
> > Paul, thanks for tests! I am looking to situation, it seems design
> > problem. I am going to send another email about it.
> >
> > --
> > Ivan Zhakov
>
> Ivan,
>
> Just in case you missed it, Garrett had some helpful changes to my this
> patch. I'll have another one ready shortly based on his ideas, with some
> additional/improved tests as well. Sorry for the delay, I've been spread
> thin the last few days.
>
Yes, I've seen Garett's changes, but I'm not sure that it's right way
to fix problem.

-- 
Ivan Zhakov
Received on Thu May 25 14:40:18 2006

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