[PATCH] bzrlib API to add pending revision properties to a working tree

[James Henstridge]

On 05/09/06, Aaron Bentley <aaron.bentley at utoronto.ca> wrote:
> James Henstridge wrote:
> > The revision properties on the working tree would be for the revision
> > that is going to be committed.  It is not related to revision
> > properties set on any previous revision.
>
> Some revision properties, like the branch nick, are long-lived.  For
> those properties, setting them for a revision is an alteration of their
> previous value.

The branch nick name is currently an unversioned branch property, with
the branch-nick revprop being a snapshot of its value at the time of
the commit.  I hadn't thought about managing them as persistent
revision properties.  I'm not sure revprops are a good fit for them.

> Perhaps that's a bad example, and most revision properties wouldn't be
> continuous.  But it might be nice to have a list of bugs fixed in a
> branch as a revision property.  A list of recently-fixed bugs would be
> cheaper to use than a revision property that listed only the bugs fixed
> in the current revision.

Tracking bug fixes is one of the uses of revision properties I had in
mind.  Having the annotation attached to the revision where you assert
the bug is fixed as opposed to a cumulative property has a number of
benefits, including:
 1. no need to worry about merging the revprop value on merges.
 2. If the revision with the bug fix annotation appears in the
ancestry of another branch, I know fix has been merged.


> But you're right that the current model is that properties must be set
> in each revision, so I'm probably attacking a strawman.
>
> >> I don't think the code would be much simpler using readlines:
>
> > Well, it could be written as:
> >    if hashfile.readline() != MERGE_MODIFIED_HEADER_1 + '\n':
> >        raise MergeModifiedFormatError()
> >    for s in RioReader(hashfile):
>
> Because the next method on file objects uses an internal buffer, the
> docs specifically recommend against mixing next and readline:
> http://docs.python.org/lib/bltin-file-objects.html
>
> I suspect in this case it would work.  But in general, I consider it bad
> form.

What you say was a concern in Python 2.1 and 2.2.  For those releases,
iter(fileobject) would return an xreadlines() object that maintained
an internal buffer to speed up reading lines from the file.  This led
to error prone code like:
  for line in fileobject:
      if some_condition:
         break
  # do something
  for line in fileobject:
      ...

This would actually lose data when the iterator for the first loop was
freed, since the data it had buffered disappeared with it.

In Python 2.3, the buffering was moved inside the file object and type
was updated to implement the iterator protocol directly, to get rid of
this class of errors.  It also means that the iterator protocol uses
the same buffering as the rest of the file object methods.

Given this, the readline use seems fairly sensible.


> > Lastly, do you have any thoughts on what should be considered valid
> > revision property names?
>
> I guess non-whitespace ascii would be a good start, and we could loosen
> the restrictions if/when interoperability demands it.

Okay.  I suppose Revision._check_properties() should be updated to
ensure this.  I'll make that change in my branch.

> It's worth noting that Subversion's properties may have binary values,
> which we don't support.  A quick glance doesn't tell me whether their
> names are unicode.

I see this in the Subversion book about file property names:

"""
There are some restrictions on the names you can use for properties. A
property name must start with a letter, a colon (:), or an underscore
(_); after that, you can also use digits, hyphens (-), and periods (.)
"""

I assume the same applies to Subversion revprops.  So non-whitespace
ASCII property names should be fine for representing SVN property
names (even if we can't represent binary property values).

James.