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

On 8/10/06, Paul Burba <paulb@softlanding.com> wrote:
> Paul Burba <paulb@softlanding.com> wrote on 08/09/2006 02:22:01 PM:
>
> > "Ivan Zhakov" <chemodax@gmail.com> wrote on 08/09/2006 11:20:45 AM:
> >
> > > On 8/9/06, Paul Burba <paulb@softlanding.com> wrote:
> > > >
> > > > Hi All,
> > > >
> > > > Just to be clear on my goal in this chunk of code: it's to determine
>
> > if
> > > > the src_path arg to setup_copy is scheduled for addition as a result
>
> > of
> > > > svn add, as opposed to a WC->WC copy/move. We need to know this
> since
> > > > wc_to_wc_copy() handles these two cases differently. My comment is
> > > > unclear on this and I will improve it with something like this:
> > > >
> > > [..]
> > > > So we can't just look at the schedule value, it's add in both cases.
>
> > The
> > > > explanation of the revision field in
> > > > http://svn.collab.net/repos/svn/trunk/subversion/libsvn_wc/README
> is:
> > > >
> > > > revision:
> > > > The revision that the pristine text and properties of this entry
> > > > represent. Defaults to the revision of the this_dir entry, for
> > > > which it is required. Set to 0 for entries not yet in the
> > > > repository.
> > > >
> > > > Between this and the observed behavior above led me to think that
> > looking
> > > > at the rev is the way to differentiate between the two scenarios.
> The
> > > > docstring for svn_wc_add2() makes no promises regarding the revision
>
> > of
> > > > added paths, though it does explicitly set it to 0 on line 1184:
> > > >
> > > > tmp_entry.revision = 0;
> > > >
> > > > Maybe I'm making a dangerous assumption?
> > > >
> > > > Perhaps it's better to look at the schedule *and* copy fields to
> make
> > the
> > > > determination?
> > > >
> > > > if (!src_is_url)
> > > > {
> > > > /* Are we copying/moving a path that is scheduled for
> addition
> > > > as the result of svn add or svn mv/cp? */
> > > > 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));
> > > > /* If scheduled for addition but not copied, src_path is
> > > > result of svn add not svn cp/mv. */
> > > > src_is_add = (entry
> > > > && entry->schedule == svn_wc_schedule_add
> > > > && !(entry->copied))
> > > > ? TRUE : FALSE;
> > > > }
> > > >
> > > > Thoughts?
> > > >
> > > I consider that anyway condition should be the same that in
> > > copy_file_administratively() and copy_dir_administratively()
> >
> > I've taken a longer look at how entries are created and I'm fairly
> > confident that checking for revision == 0 in entries is a reliable way
> to
> > test for "svn added-ness". But using the entry's schedule and copied
> > fields as you suggest seems equally valid, so I'll defer to you on this
> > and use the latter.
> >
> > > But have
> > > same condition in several place is little bit dangerous,
> > > so might we should reconsider and move your code from libsvn_client to
> > > libsvn_wc?
> >
> > As I mentioned perviously in this thread, an added file has no text
> base,
> > so when copy_file_administratively() tries to copy the pristine text
> base
> > it fails. It's easy enough to work around that I guess, just use the
> > added file itself instead of the text-base. I recall some other
> problems,
> > but let me take a look again.
>
> Ivan,
>
> I took a second look at doing this in libsvn_wc rather than libsvn_client
> as you suggested. It doesn't take much to copy added files, but I'm
> getting nowhere fast when copying added directories. Is having similar
> svn add testing logic in libsvn_wc and libsvn_client really that
> "dangerous" as you say, why exactly?
>
I mean that it dangerous because introduce undocumented behavior
dependency between two layers (libsvn_client and libsvn_wc), which is
not right thing.
Just imagine: we have rewritten libsvn_wc using central database and
there is no problems to copy added files in this situation, but it
will require to change libsvn_client. This is violation of
incapsulation between layers.

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