What revision should an added not yet commited node have?

[Daniel Näslund <da...@longitudo.com>]

Hi!

When trying to replace entries in the status code I got a couple of test
failures saying that the revision should be 0 for newly added nodes.
Greg pointed out that the entries code set the revision to 0 for those
cases while the revision returned from _read_info() sets it to -1.

Should we continue to use the 0 value? Is it well established as the
revision number of version controlled, not yet committed files or should
we tell 'svn info' and 'svn status' to not output any rev nr at all for
these nodes?

Thoughts?

Daniel

Re: What revision should an added not yet commited node have?

[neels <ne...@gmail.com>]

On 19 March 2010 07:37, Daniel Näslund <da...@longitudo.com> wrote:
> Btw, other diffs I've found between entries and db handling of
> revisions: (i) A missing dir has no revision value in the entries code,
> but it has in the db.

Cool, so no real backwards compat problems there. If all else fails,
we can check the 'missing' state and erase the revision number before
passing on to callers-from-the-past. But I expect it to just sort
itself out.

> (ii) A file copied from foreign url to a wc has
> the revision number set to its value in the foreign repo, although it is
> locally added w/o history in this wc and thus should have 0. That's for
> the entries. The db sets it to -1.

Perfect. I never thought of copies from *foreign* repositories. So the
db acts as if it was a normal add, not a copy, which is correct.

~Neels

Re: What revision should an added not yet commited node have?

[Daniel Näslund <da...@longitudo.com>]

On Fri, Mar 19, 2010 at 02:33:33AM +0100, Neels J Hofmeyr wrote:
> C. Michael Pilato wrote:
> > Stefan Sperling wrote:
> >> On Tue, Mar 16, 2010 at 09:01:28PM +0100, Daniel Näslund wrote:
> >>> Hi!
> >>>
> >>> When trying to replace entries in the status code I got a couple of test
> >>> failures saying that the revision should be 0 for newly added nodes.
> >>> Greg pointed out that the entries code set the revision to 0 for those
> >>> cases while the revision returned from _read_info() sets it to -1.
> >>>
> >>> Should we continue to use the 0 value? Is it well established as the
> >>> revision number of version controlled, not yet committed files or should
> >>> we tell 'svn info' and 'svn status' to not output any rev nr at all for
> >>> these nodes?
> >> I think -1 (invalid revnum) is more appropriate than 0.
> 
> Nice, I hit that same question like two weeks ago, with
> svn_client__get_revision_number() upon svn_opt_revision_base for added
> nodes. I found the same conclusion: it should have always returned -1.
> 
> I am at the point where I would trial to see how callers deal with a -1
> revision number ("would" because I need to study for an exam next week, bah).
> 
> Ideally, we will change the behaviour of this private function when
> switching it to wc-ng. I hope we don't have to mock up current behaviour for
> compat, especially because that depends on parent nodes sometimes.

I've decided to postpone the change of revisions set to zero to a
follow-up. Two reasons: (i) I'm _replacing_ revisions fetched from the
entries file with revisions fetched from the db, changing the behaviour
is a separate task that involves fiddling with a lot of test, causing me
to loose momentum with the svn_wc_status2_t work. (ii) The info code
uses zeroes too for newly added nodes. It would be better to change the
behavior of 'svn {info,status}' at the same time.

When it's time to replace the zeroes in the CLI, I opt for leaving those
fields blank. We have no revision, we show no revision. Plain and
simple!

Btw, other diffs I've found between entries and db handling of
revisions: (i) A missing dir has no revision value in the entries code,
but it has in the db. (ii) A file copied from foreign url to a wc has
the revision number set to its value in the foreign repo, although it is
locally added w/o history in this wc and thus should have 0. That's for
the entries. The db sets it to -1.

I have some quirks with switching locally added files but that's for
another post.

Daniel

Re: What revision should an added not yet commited node have?

[Daniel Näslund <da...@longitudo.com>]

On Fri, Mar 19, 2010 at 02:33:33AM +0100, Neels J Hofmeyr wrote:
> C. Michael Pilato wrote:
> > Stefan Sperling wrote:
> >> On Tue, Mar 16, 2010 at 09:01:28PM +0100, Daniel Näslund wrote:
> >>> Hi!
> >>>
> >>> When trying to replace entries in the status code I got a couple of test
> >>> failures saying that the revision should be 0 for newly added nodes.
> >>> Greg pointed out that the entries code set the revision to 0 for those
> >>> cases while the revision returned from _read_info() sets it to -1.
> >>>
> >>> Should we continue to use the 0 value? Is it well established as the
> >>> revision number of version controlled, not yet committed files or should
> >>> we tell 'svn info' and 'svn status' to not output any rev nr at all for
> >>> these nodes?
> >> I think -1 (invalid revnum) is more appropriate than 0.
> 
> Nice, I hit that same question like two weeks ago, with
> svn_client__get_revision_number() upon svn_opt_revision_base for added
> nodes. I found the same conclusion: it should have always returned -1.
> 
> I am at the point where I would trial to see how callers deal with a -1
> revision number ("would" because I need to study for an exam next week, bah).
> 
> Ideally, we will change the behaviour of this private function when
> switching it to wc-ng. I hope we don't have to mock up current behaviour for
> compat, especially because that depends on parent nodes sometimes.

I've decided to postpone the change of revisions set to zero to a
follow-up. Two reasons: (i) I'm _replacing_ revisions fetched from the
entries file with revisions fetched from the db, changing the behaviour
is a separate task that involves fiddling with a lot of test, causing me
to loose momentum with the svn_wc_status2_t work. (ii) The info code
uses zeroes too for newly added nodes. It would be better to change the
behavior of 'svn {info,status}' at the same time.

When it's time to replace the zeroes in the CLI, I opt for leaving those
fields blank. We have no revision, we show no revision. Plain and
simple!

Btw, other diffs I've found between entries and db handling of
revisions: (i) A missing dir has no revision value in the entries code,
but it has in the db. (ii) A file copied from foreign url to a wc has
the revision number set to its value in the foreign repo, although it is
locally added w/o history in this wc and thus should have 0. That's for
the entries. The db sets it to -1.

I have some quirks with switching locally added files but that's for
another post.

Daniel

Re: What revision should an added not yet commited node have?

[Neels J Hofmeyr <ne...@elego.de>]

C. Michael Pilato wrote:
> Stefan Sperling wrote:
>> On Tue, Mar 16, 2010 at 09:01:28PM +0100, Daniel Näslund wrote:
>>> Hi!
>>>
>>> When trying to replace entries in the status code I got a couple of test
>>> failures saying that the revision should be 0 for newly added nodes.
>>> Greg pointed out that the entries code set the revision to 0 for those
>>> cases while the revision returned from _read_info() sets it to -1.
>>>
>>> Should we continue to use the 0 value? Is it well established as the
>>> revision number of version controlled, not yet committed files or should
>>> we tell 'svn info' and 'svn status' to not output any rev nr at all for
>>> these nodes?
>> I think -1 (invalid revnum) is more appropriate than 0.

Nice, I hit that same question like two weeks ago, with
svn_client__get_revision_number() upon svn_opt_revision_base for added
nodes. I found the same conclusion: it should have always returned -1.

I am at the point where I would trial to see how callers deal with a -1
revision number ("would" because I need to study for an exam next week, bah).

Ideally, we will change the behaviour of this private function when
switching it to wc-ng. I hope we don't have to mock up current behaviour for
compat, especially because that depends on parent nodes sometimes.

~Neels

Re: What revision should an added not yet commited node have?

["C. Michael Pilato" <cm...@collab.net>]

Stefan Sperling wrote:
> On Tue, Mar 16, 2010 at 09:01:28PM +0100, Daniel Näslund wrote:
>> Hi!
>>
>> When trying to replace entries in the status code I got a couple of test
>> failures saying that the revision should be 0 for newly added nodes.
>> Greg pointed out that the entries code set the revision to 0 for those
>> cases while the revision returned from _read_info() sets it to -1.
>>
>> Should we continue to use the 0 value? Is it well established as the
>> revision number of version controlled, not yet committed files or should
>> we tell 'svn info' and 'svn status' to not output any rev nr at all for
>> these nodes?
> 
> I think -1 (invalid revnum) is more appropriate than 0.
> After all, the repository has a revision 0.
> We could make info and status print 'no known revision' or something
> like that for added files.

Bah.  Let 'status' print nothing in that column, and let 'info' print no
"Revision" or "Last Changed *" lines at all.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

Re: What revision should an added not yet commited node have?

[Stefan Sperling <st...@elego.de>]

On Tue, Mar 16, 2010 at 09:01:28PM +0100, Daniel Näslund wrote:
> Hi!
> 
> When trying to replace entries in the status code I got a couple of test
> failures saying that the revision should be 0 for newly added nodes.
> Greg pointed out that the entries code set the revision to 0 for those
> cases while the revision returned from _read_info() sets it to -1.
> 
> Should we continue to use the 0 value? Is it well established as the
> revision number of version controlled, not yet committed files or should
> we tell 'svn info' and 'svn status' to not output any rev nr at all for
> these nodes?

I think -1 (invalid revnum) is more appropriate than 0.
After all, the repository has a revision 0.
We could make info and status print 'no known revision' or something
like that for added files.

But I don't know how much work it is to fix up all the code expecting 0.

Stefan

Re: What revision should an added not yet commited node have?

["C. Michael Pilato" <cm...@collab.net>]

Daniel Näslund wrote:
> Hi!
> 
> When trying to replace entries in the status code I got a couple of test
> failures saying that the revision should be 0 for newly added nodes.
> Greg pointed out that the entries code set the revision to 0 for those
> cases while the revision returned from _read_info() sets it to -1.
> 
> Should we continue to use the 0 value? Is it well established as the
> revision number of version controlled, not yet committed files or should
> we tell 'svn info' and 'svn status' to not output any rev nr at all for
> these nodes?

Technically speaking, they shouldn't have a revision.  SVN_INVALID_REVNUM
makes the most sense.  The repository has a real revision 0, complete with
modifiable revprops, and so on, so it was a mistake for us to have ever
allowed 0 as a placeholder revision in the past.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

Re: What revision should an added not yet commited node have?

[Stefan Sperling <st...@elego.de>]

On Tue, Mar 16, 2010 at 09:01:28PM +0100, Daniel Näslund wrote:
> Hi!
> 
> When trying to replace entries in the status code I got a couple of test
> failures saying that the revision should be 0 for newly added nodes.
> Greg pointed out that the entries code set the revision to 0 for those
> cases while the revision returned from _read_info() sets it to -1.
> 
> Should we continue to use the 0 value? Is it well established as the
> revision number of version controlled, not yet committed files or should
> we tell 'svn info' and 'svn status' to not output any rev nr at all for
> these nodes?

I think -1 (invalid revnum) is more appropriate than 0.
After all, the repository has a revision 0.
We could make info and status print 'no known revision' or something
like that for added files.

But I don't know how much work it is to fix up all the code expecting 0.

Stefan