Re: [PATCH] Moving/copying added files and dirs

On 8/8/06, Paul Burba <paulb@softlanding.com> wrote:
> Paul Burba <paulb@softlanding.com> wrote on 07/27/2006 08:37:31 AM:
>
> > Ivan,
> >
> > Sorry, I wasn't able to look at this until yesterday afternoon. I'm
> > afraid it's not as simple as defeating sanity check 2 (i.e. in
> > copy_file_administratively(): /* Sanity check 2: You cannot make a copy
> of
> > something that's not in the repository unless it's a copy of an
> > uncommitted copy. */). Here are some of the problems I see:
> >
> > 1) An added file has no text base, so when copy_file_administratively()
> > tries to copy the pristine text base it fails.
> >
> > 2) If we are going to support copying an added file, it only makes sense
>
> > (?) to support the following as well:
> >
> > a) moving an added file
> > b) copying an added directory
> > c) moving an added directory
> >
> > This isn't so much a problem, as an added complication.
> >
> > 3) If we move an added file how do we get rid of the add entry in the
> > source directory? It's not like moving a versioned item, we don't want
> > any evidence left behind in entries that something once existed there
> and
> > was scheduled for addition...do we? I suppose the equivalent of svn rm
> > --force will work...
> >
> > Anyhow, I'm looking into this right now, hope to have some ideas
> shortly.
>
> ...and here here it is. This patch supports copying and moving of added
> paths within a WC or from a WC to the REPOS.
Hi, Paul!
Thank you for continue working on this usefull thing.

>
> Re issue 1 above, after looking at this I think the solution shouldn't
> reside in libsvn_wc, but rather in libsvn_client. Trying to teach
> svn_wc_copy2() to copy paths that are not versioned doesn't make a lot of
> sense. It seems all we really need to do is copy the added files without
> the administrative directories and then add them. The first task sounds
> an awful lot like an export, and with some borrowed/modified code from
> copy_versioned_files() in export.c that's what this patch does. Then a
> call to svn_client_add3() adds the copied files. Does this approach sound
> reasonable?
After look to code I agree with you. This approach reasonable. But I
see two problems with your current patch:
Now unversioned files in versioned commited directory copied when you
copy directory. I mean:
$ mkdir dir
$ svn add dir
$ svn ci -m ""
$ touch dir\foo
$ svn st dir
? dir\foo

$ svn cp dir dir2
$ svn st dir2
? dir2\foo

But your patch seems copy only versioned files, i.e.
$ mkdir dir
$ svn add dir
$ svn ci -m ""
$ touch dir\foo
$ svn st dir
A dir
? dir\foo

$ svn cp dir dir2
$ svn st dir2
A dir2

So there is inconsistency. I consider it's bad.

Second problem in setup_copy():
  if (!src_is_url)
    {
      /* Are we copying/moving a path that is scheduled for addition? */
      SVN_ERR(svn_wc_adm_probe_open3(&adm_access, NULL, src_path, FALSE, 0,
                                     ctx->cancel_func, ctx->cancel_baton,
                                     pool));
      SVN_ERR(svn_wc_entry(&entry, src_path, adm_access, FALSE, pool));
      SVN_ERR(svn_wc_adm_close(adm_access));
      src_is_add = (entry && entry->revision == 0) ? TRUE : FALSE;
    }
Might be I forget something, but AFAIK zero revision number is valid.
So you should check against SVN_INVALID_REVNUM or check url field. I
don't remember exactly.

>
> Also, there is one obvious oddity regarding my solution, that is the
> notification of 'A'dded 'D'eleted paths. Normally svn copy reports 'A'dds
> first then 'D'eletes, but this patch reports in the opposite order when
> moving/copying an added path (i.e. the Deletes are reported first). It's
> certainly possible to make tweak wc_to_wc_copy() to make the ordering the
> same, but it would require a bit more cumbersome logic and require
> svn_wc_adm_access_t *adm_access/src_access to be opened and closed twice,
> and I wasn't sure this is worth it(?).
For me it is not a problem.

>
> [[[
> Support copy and move of paths scheduled for addition.
>
> Follow-up to r20811.
>
> Suggested by: zhakov
>
> * subversion/libsvn_client/copy.c
> (svn_pools.h): Include.
> (copy_added_path): Helper function for wc_to_wc_copy() which copies
> paths scheduled for addition to a new WC location.
> (wc_to_wc_copy): Add new argument indicating if src_path is not under
> version control but is scheduled for addition. Use copy_added_path()
> rather than svn_wc_copy2() when copying added paths, then add these
> with svn_client_add3().
> (setup_copy): Determine if we are copying a path that is scheduled
> for addition by looking at it's revision and pass that info to
> wc_to_wc_copy().
>
> * subversion/tests/cmdline/copy_tests.py
> (copy_move_added_paths, copy_added_paths_to_URL): New tests.
> (test_list): Run new tests.
> ]]]
>
>
>
>

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