On 10/3/05, Ivan Zhakov <chemodax@gmail.com> wrote:
> On 10/2/05, Erik Huelsmann <ehuels@gmail.com> wrote:
> > Hi Ivan!
> >
> > On 9/27/05, Ivan Zhakov <chemodax@gmail.com> wrote:
> > > Hi!
> > >
> > > There is patch to wc-replacements branch that makes wc->wc copy loggy.
> > > I am posting patch to the list because I don't sure about some tricks.
> > > Core idea is change svn_wc_add2() to svn_wc_add_repos_file() in
> > > subversion/libsvn_wc/copy.c:copy_file_administratively(). Because
> > > svn_wc_add_repos_file() already loggy and preserves text and prop
> > > base. It works and passes all tests. Of course there is possible
> > > optimizations: make svn_wc_add_repos_file() recieve new working text
> > > and prop. But I consider this could be done as next step.
> > >
> > > [[
> > > Make svn cp PATH PATH loggy.
> > >
> > > * subversion/libsvn_wc/copy.c
> > > (copy_file_administratively): Use svn_wc_add_repos_file() instead of
> > > svn_wc_add2().
> > > ]]
> >
> > Sorry it took me so long to review your patch.
> No problem. Take your time.
>
> > The main problem with this patch is that it does not combine all
> > changes to the working copy into 1 log-file:
> > copy_file_administratively copies the working copy props file after
> > svn_wc_add_repos_file has atomically completed all other changes to
> > the file.
> I know. As I said, it is only idea proposal. Copy working file in
> svn_wc_add_repos_file() would be simple fix.
Well, I guess I was saying the basic idea of the patch is good. If we
want to add a wcprops file to the svn_wc_add_repos_file() interface,
we need ....2()
> > What reasons did you have not to use svn_wc_add2() - other than that
> > it currently isn't loggy?
> > In other words, wouldn't it be the better solution to make svn_wc_add2() loggy?
>
> 1. svn_wc_add2() assumes that working and base file already exists at
> place. It is possibly because svn_wc_add2() have designed for
> implement svn add command.
> 2. svn_wc_add2() works with directories, it is really hard to
> implement loggy for directories recursively.
> 3. In svn_wc_add_repos_file() we already have done replacements logic.
> I didn't see reasons to duplicate it.
I don't see reasons to either, it's just that the functions internals
changed so much (as in: it was rewritten) that the diff wasn't really
helpfull in reviewing... That's the only reason I asked.
bye,
Erik.
Received on Mon Oct 3 10:53:17 2005