On Fri, May 21, 2010 at 11:42:13AM +0200, Bert Huijben wrote:
> > -----Original Message-----
> > From: stsp_at_apache.org [mailto:stsp_at_apache.org]
> > Sent: donderdag 20 mei 2010 17:11
> > To: commits_at_subversion.apache.org
> > Subject: svn commit: r946661 -
> > /subversion/trunk/subversion/libsvn_wc/adm_crawler.c
> >
> > Author: stsp
> > Date: Thu May 20 15:11:07 2010
> > New Revision: 946661
> >
> > URL: http://svn.apache.org/viewvc?rev=946661&view=rev
> > Log:
> > Fix issue #2267, "support uncommitted svn:externals properties".
> >
> > * subversion/libsvn_wc/adm_crawler.c
> > (read_traversal_info): Rename to ...
> > (read_externals_info): ... this. We've been using an external_func
> > callback instead of a traversal info for some time.
> > (report_revisions_and_depths): Rename local variable CHILDREN to
> > BASE_CHILDREN for clarity (the children come from the BASE tree).
> > If the caller provided an EXTERNAL_FUNC callback, check locally
> > added directories for svn:externals as well and pass them to the
> > callback. The caller will then pull externals into the added directory.
>
> Hi,
>
> I'm not sure if this really fixes this issue for the common use cases.
What are the common use cases you have in mind?
The one I have in mind is where you want to test externals you have
configured on a locally added directory, before commit.
I tried the following and could not get svn to misbehave:
- reverting the locally added dir
- removing the svn:externals propery from the locally added dir
- removing an external from the svn:externals property
- adding a new external to the svn:externals property
- running svn status in all above cases
I could not get svn to behave differently when I tried the
above on a directory I had already committed.
> (And it introduces libsvn_client specific support for updates in a
> libsvn_wc common function that is used for more than just this update
> scenario. E.g. svn status -u).
>
> The current code (well; before your patch) applies changes on
> svn:externals on update. By comparing the old and new versions of the
> svn:externals property it can add/remove/switch externals.
When does it do that? I cannot get trunk to physically remove an
external which was removed from svn:external. Here's what I tried:
$ cd svn-sandbox/trunk
$ ls
alpha beta epsilon/ gamma/
$ svn mkdir d
A d
$ svn ci -mm
Adding d
Committed revision 3.
$ svn ps svn:externals '^/trunk/gamma gamma' d
property 'svn:externals' set on 'd'
$ svn up
Fetching external item into 'd/gamma'
A d/gamma/delta
Updated external to revision 3.
Updated to revision 3.
$ svn ci
Sending d
Committed revision 4.
$ svn ps svn:externals '^/trunk/epsilon epsilon' d
property 'svn:externals' set on 'd'
$ svn up
Fetching external item into 'd/epsilon'
A d/epsilon/zeta
Updated external to revision 4.
Updated to revision 4.
$ svn diff
Property changes on: d
___________________________________________________________________
Modified: svn:externals
## -1 +1 ##
-^/trunk/gamma gamma
+^/trunk/epsilon epsilon
$ svn ci -mm
Sending d
Committed revision 5.
$ svn up
Fetching external item into 'd/epsilon'
External at revision 5.
At revision 5.
$ svn st
? d/gamma
X d/epsilon
The old external 'gamma' stays behind as a detached WC.
> Your new code can just add externals, and if the directory is later
> reverted the externals will stay as a detached working copy. (And if a
> different svn:external value is set for a directory you will get some
> hard to resolve issues, that you would never get into with just the
> old code).
The same happens with an already committed directory, see above.
> I think fixing issue ##2267 needs a good design instead of a quick and
> dirty fix in one of the fundamental editor helper functions. (E.g.
> does this break backwards compatibility of our APIs somewhere?)
I don't know. But we usually don't have APIs with strict promises
that would forbid svn to do something in addition to what it is
currently doing.
Note also that in the issue the reporter says:
Note that TSVN does fetch the externals in this situation
(a temp workaround for me at least).
This was in 2005. So I believe that if pulling externals into
newly added dirs causes real problems, we'd know about them by now.
> Looking further in the implementation: Why does the code walk the children of
> an added noded, but not the children of the children?
That's a fair point. We should make it crawl the added tree recursively.
Thanks,
Stefan
Received on 2010-05-21 12:05:48 CEST