On Wed, 2011-04-20 at 21:24 +0200, Bert Huijben wrote:
>
> > -----Original Message-----
> > From: C. Michael Pilato [mailto:cmpilato_at_collab.net]
> > Sent: woensdag 20 april 2011 19:45
> > To: dev_at_subversion.apache.org
> > Subject: Re: svn commit: r1095130 - in /subversion/trunk/subversion:
> > libsvn_client/client.h libsvn_client/externals.c libsvn_client/switch.c
> > libsvn_client/update.c libsvn_wc/adm_crawler.c
> > tests/cmdline/externals_tests.py
> >
> > On 04/20/2011 01:32 PM, Stefan Sperling wrote:
> > > For the record, Bert and I discussed this on IRC today.
> > >
> > > The conclusion is that this feature will be revived as it was implemented,
> > > with the addition of a new flag to svn update that allows users to enable
> > > external definitions on locally added directories. Without this flag,
> > > external definitions on locally added directories will be ignored.
> > >
> > > There is a performance overhead involved in crawling the DB for external
> > > definitions. Bert's opinion is that adding this overhead to every update
> > > operation is not acceptable. I don't think that is a big problem.
> > > But the command line switch will make both of us happy.
> > >
> > > This way, users can test their externals definitions locally before
> > > committing them, which is the goal of issue #2267. But normal updates
> > > will not have to endure additional overhead.
> >
> > This just sounds like a truly odd thing to hook to a command-line option.
> > That said, I confess my ignorance of the details of the issue. We already
> > have to crawl for external definitions as part of 'svn update', don't we?
> > Isn't that what the external_func/external_baton pair passed to
> > svn_wc_crawl_revisions5() is for?
>
> For the crawling and the update process we look at the op_depth 0
> (also known as BASE) tree. This extension (since 1.7.0) explicitly
> adds support for externals on nodes that are not yet committed to the
> repository. These newly added nodes are never touched by the update
> processor (unless they cause a tree conflict).
> So there is not much to gain by building this added directory support
> in the update processing in libsvn_wc.
>
> Another topic we discussed is that the update processor explicitly
> handles *the changes* in svn:externals properties.
>
> The update processing of svn:externals doesn't just take the last set
> of properties. Instead it takes the old and the new definitions of the
> svn:externals and then performs a diff on them.
> * Externals that are removed are then removed from the working copy.
> (After verifying that they were from the right location)
> * Externals that are changed are switched or deleted and re-added if
> they are from a different repository (After verification)
> * And new externals are added.
Here it looks like you're talking about incoming changes, where
OLD==BASE and NEW==update-rev.
> One problem with externals on added nodes is that we can only look at
> the last definition of the externals.
> So instead we can only:
> * Delete if some path looks like an old external (guessing) and is
> reused
> * Add new externals
> And old externals are left as is (see issue #3351 for a real problem).
But here it looks like you're talking about a different kind of change:
not an incoming change from the repo, but rather it sounds like you want
to detect a change that has been made locally since ... some time in the
past. Is that so?
If so, that's an entirely different thing.
In both cases (existing dir, added dir), we can detect incoming changes
[1].
In both cases (existing dir, added dir), we *might* want to look at the
local 'actual' value of the property, and act on it so the user can try
out the changes before committing. When the user changes this actual
value, we can't explicitly detect the change to the property but
whatever we might want to do applies equally well to added dirs and
existing dirs.
It looks like the first problem here is trying to treat added nodes
specially.
- Julian
[1] In practice, an incoming change on a locally added node can't be
anything other than an add, which would result in a tree conflict, but
the point is that in principle we can detect it and there's no need for
any special option to handle this part.
> For non-file externals its usually easy to fix these problems yourself
> (Something wrong: just delete it and it will get fixed on the next
> update). But for file externals you will have to hack the database or
> call libsvn_wc yourself.
>
> So applying these uncommitted svn:externals by default opens new
> options to shoot yourself in the foot and break your wc. (When used
> with precaution and without file externals it is certainly useful. But
> we can't tell the user that when they are just prop-editing their
> property. )
>
> I was busy writing another mail on possible solutions:
> The best solution would be to store the currently applied externals
> somewhere outside the properties hash so we can still apply
> 'differences' instead of 'the last svn:externals' value.
>
> But when we go that far, we should also look at more viewspec options.
>
> Bert
>
Received on 2011-04-21 20:42:35 CEST