> -----Original Message-----
> From: Branko Čibej [mailto:brane_at_xbc.nu] On Behalf Of Branko Cibej
> Sent: zondag 15 mei 2011 3:59
> To: dev_at_subversion.apache.org
> Subject: Re: svn commit: r1102912 - in /subversion/trunk/subversion:
> include/private/svn_wc_private.h libsvn_client/prop_commands.c
> libsvn_wc/externals.c libsvn_wc/wc_db.c tests/libsvn_wc/db-test.c
>
> On 14.05.2011 14:01, Greg Stein wrote:
> > On Sat, May 14, 2011 at 04:00, Bert Huijben <bert_at_qqmail.nl> wrote:
> >>
> >>> -----Original Message-----
> >>> From: Greg Stein [mailto:gstein_at_gmail.com]
> >>> Sent: zaterdag 14 mei 2011 7:06
> >>> To: dev_at_subversion.apache.org
> >>> Subject: Re: svn commit: r1102912 - in /subversion/trunk/subversion:
> >>> include/private/svn_wc_private.h libsvn_client/prop_commands.c
> >>> libsvn_wc/externals.c libsvn_wc/wc_db.c tests/libsvn_wc/db-test.c
> >>>
> >>> On Fri, May 13, 2011 at 18:23, <rhuijben_at_apache.org> wrote:
> >>>> Author: rhuijben
> >>>> Date: Fri May 13 22:22:59 2011
> >>>> New Revision: 1102912
> >>>>
> >>>> URL: http://svn.apache.org/viewvc?rev=1102912&view=rev
> >>>> Log:
> >>>> Make the svn_client_proplist3 funtion capable of reading properties
> from
> >>>> new style file externals.
> >>> Huh? I thought file externals had NODES and ACTUAL_NODE rows?
> >>> Shouldn't the properties be available there? What are we doing
> >>> *special casing* externals like this?
> >> See the document you asked me to write: notes/wc-ng/externals
> >>
> >> They live in NODES now, but not after we bump to format 29. (They
> can/will
> >> probably survive in ACTUAL_NODES as they share the same path anyway)
> >>
> >> After we bump to format 29 file externals will be "just like normal
> >> externals". They won't be part of the parent working copy NODES tree.
> (And
> >> can come from different repositories; can be placed in subdirs; etc. etc.).
> >>
> >> But of course they will still share the parent wc's db and pristine store as
> >> they can't have their own administrative area.
> > I just read it, and I think that I disagree with the approach.
> >
> > It now seems that we are going to have to special-check *everywhere*
> > to see if a node exists as a file external. Every single place we look
> > for a node, will now require a check to see if a file external lives
> > at that spot.
> >
> > Special casing doesn't seem like a good approach.
> >
> > Couldn't you have used a new presence value in the database to note
> > these? The NODES schema already allows for different repositories
> > (repos_id). All the data would live in NODES just like every other
> > item. No need to go and look in a different table (which already looks
> > much like NODES).
> >
> > I'd rather see file externals look more like regular nodes, instead of
> > some ephemeral thing.
>
> Couldn't agree more. The direction we should be taking is integrating
> /all/ externals into NODES (eventually), not moving them out. This is
> going to make all queries into the wc-db even more complex (and slower).
The point is: We don't want to see file externals if we call into wc-db. We filter them *everywhere*. Why would we want to add them?
Where do we look at directory externals?
Note that *by definition* you can't assume that they are anywhere related. If they were related they weren't external.
So just assume they are from a different repository...
Where would you like to see them?
In merge?
No
We have quite a bit of code to filter them here and most likely have bugs where we don't test properly.
While performing a copy?
No
We have quite a bit of code to filter them here and most likely have bugs where we don't test properly.
(Copying a file external breaks tagging, etc.)
While updating?
No
We have quite a bit of code to filter them here and most likely have bugs where we don't test properly.
(Updating file externals with the wc breaks revision locked externals)
While performing a propset?
No
We have quite a bit of code to filter them here and most likely have bugs where we don't test properly.
(Changing file externals is not recommended and breaks revision locked externals)
While ....?
No
We have quite a bit of code to filter them here and most likely have bugs where we don't test properly.
(File externals are not part of the working copy... They are.... external)
When you look at what is at a specific local path?
Yes
(And this is really the only case where you want them.)
Why integrate something that you *don't* want to see?
Do you have file externals in your day-to-day working copy?
Does that work as expected?
I think any almost every user answers both questions with opposite answers: Or they don't use them and they appear to work just fine while testing, as would a switched file....
Or they do use them and have problems with them in different places.
Your and Greg's suggestion looks nice if you don't look at the consequences.
But instead of removing complexitiy by not checking for externals it requires us to check for externals everywhere...
Especially where you would least expect it (like in that merge code).
Any users that brings in a patch would have to answer the question: Does this work with file externals properly? Just like we have to do with the old 'hidden' nodes concept.
Every loop would have to check if the repository matches the parent directory... and/or for file externals. Review capacity for the current code welcome.
(And suggestions welcome on how not to bring down the performance down by a factor of two as we would have to add +- 1 join or a few c statements to every query)
Bert
Received on 2011-05-15 09:07:47 CEST