> -----Original Message-----
> From: C. Michael Pilato [mailto:[email protected]]
> Sent: woensdag 20 april 2011 19:45
> To: [email protected]
> 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.
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).
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
