You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Neels J Hofmeyr <ne...@elego.de> on 2010/04/24 14:37:01 UTC
wc-ng: determine if a node is versioned
Hi all,
I see a potential problem in the use of svn_wc__node_get_kind(): incomplete
state due to an interrupted operation.
Let's see the return values for svn_wc__node_get_kind(). It is quite
indistinguishable whether a node is not versioned, or whether it is
versioned but in incomplete state:
BASE or | one or | the | node is | show_ | Returned value
WORKING | both are | kind | hidden | hidden |
node | incomplete | is set | | arg is |
exists | | | | TRUE |
-------------------------------------------------------------------
1| no - - - * | svn_node_unknown
2| yes incomplete no - * | svn_node_unknown
3| yes complete yes hidden no | svn_node_none
4| yes complete yes hidden yes | _dir/_file
5| yes complete yes not hidden * | _dir/_file
6| yes incomplete yes - * | _dir/_file (relevant?)
At first glance, it appears that different code paths want to handle
incomplete nodes differently. Some want to act as if the node was 'normal'
== versioned. Most appear to want to act as if incomplete nodes were
non-versioned -- but maybe they just do that now because a cleanly
non-versioned node returns the same value as an incomplete node. And that's
the problem -- the API tempts hackers to just ignore the incomplete state
without reflection.
Maybe svn_wc__node_get_kind() should use determine if a node is incomplete
and throw an error if so, including a boolean arg allow_incomplete to bypass
that error?
What do the wc-ng gurus think about this?
Thanks,
~Neels
Re: wc-ng: determine if a node is versioned
Posted by Neels J Hofmeyr <ne...@elego.de>.
Neels J Hofmeyr wrote:
> Greg Stein wrote:
>> On Sat, Apr 24, 2010 at 10:37, Neels J Hofmeyr <ne...@elego.de> wrote:
>>> ...
>>> At first glance, it appears that different code paths want to handle
>>> incomplete nodes differently. Some want to act as if the node was 'normal'
>>> == versioned. Most appear to want to act as if incomplete nodes were
>>> non-versioned -- but maybe they just do that now because a cleanly
>>> non-versioned node returns the same value as an incomplete node. And that's
>>> the problem -- the API tempts hackers to just ignore the incomplete state
>>> without reflection.
>>>
>>> Maybe svn_wc__node_get_kind() should use determine if a node is incomplete
>>> and throw an error if so, including a boolean arg allow_incomplete to bypass
>>> that error?
>> The node_get_kind API is there for simplicity. Don't throw errors.
>>
>> Adjust return values instead. I would recommend returning none for
>> unversioned and hidden nodes (the idea behind hidden is "it isn't
>> there", so these are the same concept). And reserve unknown for
>> incomplete nodes.
>
> Ok cool. I was at first thinking the same and then thought that maybe the
> 'unknown' was intended to mean something like "it's definitely not in the
> WC, but who knows which kind it may be in the repos".
>
> But with that said, I agree totally.
>
> One thing I'd like to clarify though -- this function could technically also
> be used to determine the kind that may have been noted in an incomplete
> node. I assume that's not worth considering for a simplicity-API.
Just for the records of this thread: My assumption was wrong, as I was told
on #svn-dev. The idea of this function is that even if a node is marked
incomplete, it will return the correct node kind if available.
As of r937735, the only situation in which svn_wc__node_get_kind() returns
svn_node_unknown is when the node *has* WC-metadata but has no kind set (and
is hence incomplete).
When the function returns a _file or _dir node kind, the node may or may not
be marked incomplete, and there is other API to tell the difference.
~Neels
Re: wc-ng: determine if a node is versioned
Posted by Neels J Hofmeyr <ne...@elego.de>.
Greg Stein wrote:
> On Sat, Apr 24, 2010 at 10:37, Neels J Hofmeyr <ne...@elego.de> wrote:
>> ...
>> At first glance, it appears that different code paths want to handle
>> incomplete nodes differently. Some want to act as if the node was 'normal'
>> == versioned. Most appear to want to act as if incomplete nodes were
>> non-versioned -- but maybe they just do that now because a cleanly
>> non-versioned node returns the same value as an incomplete node. And that's
>> the problem -- the API tempts hackers to just ignore the incomplete state
>> without reflection.
>>
>> Maybe svn_wc__node_get_kind() should use determine if a node is incomplete
>> and throw an error if so, including a boolean arg allow_incomplete to bypass
>> that error?
>
> The node_get_kind API is there for simplicity. Don't throw errors.
>
> Adjust return values instead. I would recommend returning none for
> unversioned and hidden nodes (the idea behind hidden is "it isn't
> there", so these are the same concept). And reserve unknown for
> incomplete nodes.
Ok cool. I was at first thinking the same and then thought that maybe the
'unknown' was intended to mean something like "it's definitely not in the
WC, but who knows which kind it may be in the repos".
But with that said, I agree totally.
One thing I'd like to clarify though -- this function could technically also
be used to determine the kind that may have been noted in an incomplete
node. I assume that's not worth considering for a simplicity-API.
~Neels
Re: wc-ng: determine if a node is versioned
Posted by Greg Stein <gs...@gmail.com>.
On Sat, Apr 24, 2010 at 10:37, Neels J Hofmeyr <ne...@elego.de> wrote:
>...
> At first glance, it appears that different code paths want to handle
> incomplete nodes differently. Some want to act as if the node was 'normal'
> == versioned. Most appear to want to act as if incomplete nodes were
> non-versioned -- but maybe they just do that now because a cleanly
> non-versioned node returns the same value as an incomplete node. And that's
> the problem -- the API tempts hackers to just ignore the incomplete state
> without reflection.
>
> Maybe svn_wc__node_get_kind() should use determine if a node is incomplete
> and throw an error if so, including a boolean arg allow_incomplete to bypass
> that error?
The node_get_kind API is there for simplicity. Don't throw errors.
Adjust return values instead. I would recommend returning none for
unversioned and hidden nodes (the idea behind hidden is "it isn't
there", so these are the same concept). And reserve unknown for
incomplete nodes.
Cheers,
-g