[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

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

From: Bert Huijben <bert_at_qqmail.nl>
Date: Sat, 14 May 2011 20:28:13 +0200

> -----Original Message-----
> From: Bert Huijben [mailto:bert_at_qqmail.nl]
> Sent: zaterdag 14 mei 2011 14:36
> To: Greg Stein
> Cc: 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
>
> They would have to live in a layer below base if we want them in nodes
> (op depth -1 or MAXINT?)

Or in Working, as that would be free. (The only valid presence would be
BASE-DELETED, which would be calculated via shadowing)

> And there are only a few places where we do want to see them. Certainly
> not as just another node in a directory, but only accessible directly
> via its path. They are 'external' and really from a different
> world/origin and unrelated for all wc handling except conflicts.

On almost every command we only want to process file externals if they are
the explicit target of the operation (never via depth or as children of a
parent)

  add - No
  blame -Only as explicit target
  cat -Only as explicit target
  changelist - No
  checkout - No
  cleanup - n/a
  commit -Only as explicit target
  copy -Only as explicit target
  delete - Not supported
  diff -Only as explicit target
  export -Only as explicit target or via definition as external
  import - No
  info -Only as explicit target
  list - n/a
  lock -Only as explicit target
  log - Only explicit
  merge - Only explicit
  mergeinfo - Only explicit
  mkdir - n/a
  move - No
  prop* -Only as explicit target
  resolve - Only as explicit target
  revert - (unknown)
  status - Reported as unversioned but/external?
  switch -n/a
  unlock -Only as explicit target
  update -Only as explicit target

> They don't belong in BASE, and keeping them there brings all kinds of
> problems. Just look at that list of issues and that isn't anywhere near
> complete.
Moving them to working or a reserved depth (+ applying special presence)
would fix this, but then we would still have to check for their presence in
all loops over the children of a directory.
 
A special TOP depth (>=WORKING) would allow reusing some NODES functions
that help on installing files, but this wouldn't work over obstructing
working copies. A depth < BASE would not allow supporting externals as the
same path of not-present, excluded and absent nodes, where we do support
file externals.

> Moving them out of BASE would also allow handling conflicts between
> them and a node that really lives at that place in the directory.
> (Currently unsolvable in 1.7 as we can't have two op depth 0 nodes at
> one local_abspath.

This would also apply to a not already used depth != BASE.

> But even with op depth -1 we need more work to fix the other
> limitations of file externals. Different repositories and file
> externals in subdirs are not going to work from nodes without adding
> checks EVERYWHERE.

This applies to all storage in nodes. Libsvn_client and many parts of
libsvn_wc outside wc_db just assume that all nodes in a directory are from
one and the same repository.

> With externals 100% in EXTERNALS that problem would be solved
> everywhere and we only need some additional checks in libsvn_client
> while libsvn_wc can just assume that a directory has its own children.

It looks like with one more helper the current test suite works with file
externals completely moved to the EXTERNALS table.

The reason I took this approach instead of storing them in a new way in
NODES is that this allows adding file externals scenarios later while making
the behavior of libsvn_wc more stable, by guaranteeing that a directory
without switched children always has a complete set of children. (Required
for stable merges, etc.).

If we store them in NODES all working copy code has to know about them and
we have to check everywhere, while only a small subset of operations wants
to look at them as working copy files. For the rest they are just
unversioned obstructions. Like how we handle directory externals.

Currently all major tree updaters (update, merge, the adm crawler) are
specialized to skip file externals and where we don't skip them we most
likely didn't find the problems yet. (Status is the only exception).
Enabling file externals from different repositories is impossible without
major rework which is far more involving than moving file externals to
EXTERNALS.

But I look forward to hearing your solution to the file-externals problems,
which resolves all these current problems and allows plugging the other
limitations of file externals... without adding conditions everywhere where
we use NODES.

[continuing in your original reply]
>
> Bert Huijben (Cell phone) From: Greg Stein
> Sent: zaterdag 14 mei 2011 14:01
> To: Bert Huijben
> Cc: 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 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.

Where would that be?

No, we don't want to look at them instead of a node. In all normal
operations we want the nodes in the working copy, not the unversioned and
external files that share the same path namespace.
Externals are 'external'.

Currently we copy all nodes in a directory, except file-externals.

When merging we apply non inheritable mergeinfo on the directory.. and then
mergeinfo on all nodes inside except the file external.

When we are crawling a working copy for revision information or switches
(the adm crawler; svnversion; etc.) we skip file externals.

> Special casing doesn't seem like a good approach.

We already do that *everywhere*.

I would like to get rid of that, by only checking for file externals where
we need that support. And that would (in most cases) be inside
libsvn_client, where we process externals. Not in libsvn_wc where we manage
a working copy and not the not working-copy files inside it.

> 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 don't think we can do that without severe performance penalties. Every
additional db read transaction in a loop is a slowdown and you are asking
for a lot of these here. We would have to start checking repos_id
everywhere.

And then we would still have the problem that switched nodes and file
'externals' are the same thing.

Moving them to a different layer might help... But it also makes NODES db
operations slower, while we usually don't want to see them anywhere. (We
should default to not seeing them, instead of to seeing them as a normal
node)

> I'd rather see file externals look more like regular nodes, instead of
> some ephemeral thing.

I would like to hear your solution. And a timescale on when we can get this
major blocker for 1.7 done.
I don't think we can release with the current status of file externals
ignored in the adm crawler, update, etc. We need a solution in this space.

I don't think EXTERNALS is the ultimate solution to this problem...
But it works and it is easy to maintain and extend in the future. We can
always move the data to somewhere else, but at least we don't pollute NODES
like we did with svn_wc_entry_t.
(Currently file externals break +- every NODES validation rule that we have:
BASE nodes with only WORKING directories....)

        Bert
>
> Cheers,
> -g
Received on 2011-05-14 20:29:30 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.