You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Tim Hill <dr...@comcast.net> on 2006/11/14 18:31:38 UTC

Feature Request: Revision Properties at Commit Time

Hi,

Wanted to open discussion for a new svn feature here...


FEATURE PROPOSAL
At present, Subversion supports "revision properties" (aka  
unversioned properties) that can be set against a specific revision.  
This feature is considered (correctly) to be potentially dangerous,  
since it retroactively changes a revision, with all the potential for  
chaos that this implies. For this reason, revision properties are  
disabled by default.

There exists, however, _one_ instant in time when revision properties  
can be safely attached to a revision: at the time the revision was  
created, i.e. at commit time. Since commits are atomic, this implies  
that the revision properties must be submitted during the commit.

Therefore, I am suggesting the following changes to svn:

(1) Add command-line syntax to the SVN COMMIT command to allow  
setting of arbitrary revision properties at commit.

(2) Making necessary API changes to support this feature in other  
tools/bindings.

(3) Relaxing the current rules that disable revision properties to  
allow (1) and (2) even if revision property modifications are  
disabled (pre-revprop-change & post-revprop-change hooks). The hooks  
should still be invoked during the checkin, *but* if they are not  
implemented, commit-time revprops are still allowed.

A possible command-line syntax might be:

svn commit .... --revpropset name="value"	# set revprop "name" to  
"value"
svn commit .... --revpropset name:file		# set revprop "name" to  
contents of "file"

Multiple --revpropset switches should be allowed in a single command.


RATIONALE FOR PROPOSAL
At present, the only revision-level metadata usable in a general way  
at commit time is the log message. Although intended as a "comment"  
field, the log message is frequently over-loaded with semi-formal  
syntax to assist in various workflows, including such items as bug  
numbers, build information, branch and merge tracking etc. The  
proliferation of these items leads to "prayer-based parsing" of the  
log messages and (of course) the elimination of this field for its  
original use (as a comment!). It will also be observed that the very  
proliferation of these second-class syntax tricks is itself an  
indication of the need for the kind of formalized revision-level  
metadata that revision properties provide.

Benefits include:
-- Elimination of "magic decoding ring" syndrome for log messages:  
they are plain-old comments again
-- Richer meta-data at commit time allows tools to record better  
audit trails (e.g. bug tracking)
-- Formal and semi-formal "revision labels" come for free
-- Better meta-data at commit allows pre-revprop-change hook to be  
smarter about custom commit processing


IMPACT OF FEATURE
The proposer (me) is not sufficiently familiar with the internals of  
Subversion to assess the development work required; presumably the  
biggest issues would be modification / enhancement of the various  
APIs to support the commit-time properties and changes to the commit  
protocol to the server to support these in a backward compatible  
manner (the command-line parsing changes are probably trivial by  
comparison). It is therefore difficult to characterize the cost/ 
benefit ratio from a development standpoint.

 From a user perspective, the feature is totally backward compatible:  
no changes to existing workflows, commands, scripts etc are required.


Comments? Does this suck, has it got legs?

--Tim


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Feature Request: Revision Properties at Commit Time

Posted by Tim Hill <dr...@comcast.net>.
On Nov 15, 2006, at 2:22 AM, Peter Lundblad wrote:

> I like this proposals with a few comments.
>
> Tim Hill writes:
>> (1) Add command-line syntax to the SVN COMMIT command to allow
>> setting of arbitrary revision properties at commit.
>>
> I guess you mean all commands that do a commit, including copy, move
> and delete as well as commit itself.

Yes -- so this would include import, copy etc.

>
>> (3) Relaxing the current rules that disable revision properties to
>> allow (1) and (2) even if revision property modifications are
>> disabled (pre-revprop-change & post-revprop-change hooks). The hooks
>> should still be invoked during the checkin, *but* if they are not
>> implemented, commit-time revprops are still allowed.
>>
> Why should these hooks be called here? That would be inconsistent with
> what happens to svn:log, svn:date and svn:author when committing  
> today and
> also would be redundant since the commit related hooks can always take
> action if they want to.

Frankly this is a study item, as I'm not familiar enough with the  
hooks. The *intent* here is for existing workflows and hooks to run  
without any change. If we already have a hook script that expects to  
be able to "see" all revision property activity, then a question  
arises: should it see the new revision property activity (retains  
behavior based on data input) or should it only be called as  
currently (retains exact semantics of current version but allows  
revision properties to slip through).

>
>> A possible command-line syntax might be:
>>
>> svn commit .... --revpropset name="value"	# set revprop "name" to
>> "value"
>
> You could be more precise regarding the syntax here. I imagine the  
> quotes
> are not really part of the syntax, but used to make the shell include
> the space in the argument, right?
>
>> svn commit .... --revpropset name:file		# set revprop "name" to
>> contents of "file"
>
> We support colons in prop names, so this doesn't work. Could have
> --revpropset-file instead.

Agreed. And the quotes in the above are as you noted.

>
>> -- Formal and semi-formal "revision labels" come for free
>
> Do you mean labels as in tags? Then, they don't, because we don't have
> a quick way too look up revisions matching certain revprops.  
> Anyway, that's
> another (possibly long) thread:-)

Yes, I was aware of that, and that it implied a much more database- 
like lookup/search facility (another day...).

>
>> IMPACT OF FEATURE
>> The proposer (me) is not sufficiently familiar with the internals of
>> Subversion to assess the development work required; presumably the
>> biggest issues would be modification / enhancement of the various
>> APIs to support the commit-time properties and changes to the commit
>> protocol to the server to support these in a backward compatible
>> manner (the command-line parsing changes are probably trivial by
>> comparison). It is therefore difficult to characterize the cost/
>> benefit ratio from a development standpoint.
>>
> I don't think extending the APIs is any hard at all. Just a bunch  
> of API
> revving and protocol work. We need to make sure the server throws an
> error if the feature is not supported, though.

Lack of familiarity with the code base, so I was erring on the  
cautious side. I would prefer if the client was smart enough to  
figure the server was down-level and not even try to send revprops;  
but again lack of internals knowledge at this point  means I don't  
know if thats possible.

--Tim

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Feature Request: Revision Properties at Commit Time

Posted by Tim Hill <dr...@comcast.net>.
Agreed.

There is, of course, another alternative to this approach:  
standardize the log message format and make it "known" to the tools.  
For example, make the log message XML. The existing "plain text" log  
message could then be bumped to a field in the XML and other fields  
could be defined using the same formal command-line syntax.

The advantage here of course is this is a client-only change: no  
server change and no protocol change. Backward compatibility would be  
more awkward, since the clients/api libs would need to distinguish  
legacy from xml comments and handle accordingly. In addition, there  
are no issues with revprop hooks compatibility (although, of course,  
log message parsing would have to change).

On the downside, XML always seems to get fat very fast, and I'm no  
great fan of some of the more cryptic & obscure formatting implied by  
this.

--Tim

On Nov 15, 2006, at 8:13 AM, Peter Lundblad wrote:

> Michael Haggerty writes:
>> Peter Lundblad wrote:
>> Speaking of which, should one be able to use this feature to set  
>> svn:log
>> (i.e., serving as an alias for the "-m" and "-F" options)?  What  
>> about
>> svn:date and svn:author (i.e., overriding the defaults for these  
>> options)?
>>
> Good points. For svn:log, I don't have an opinion, really. They
> are just redundant ways to do the same thing.
>
> For svn:date, no, because the date is reset when the transaction is
> finally committed and we depend on the dates being in order.
>
> svn:author: no, because this is determined by authentication in many
> cases.
>
> Regards,
> //Peter
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Feature Request: Revision Properties at Commit Time

Posted by Peter Lundblad <pl...@google.com>.
Michael Haggerty writes:
> Peter Lundblad wrote:
> Speaking of which, should one be able to use this feature to set svn:log
> (i.e., serving as an alias for the "-m" and "-F" options)?  What about
> svn:date and svn:author (i.e., overriding the defaults for these options)?
> 
Good points. For svn:log, I don't have an opinion, really. They
are just redundant ways to do the same thing.

For svn:date, no, because the date is reset when the transaction is
finally committed and we depend on the dates being in order.

svn:author: no, because this is determined by authentication in many
cases.

Regards,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Feature Request: Revision Properties at Commit Time

Posted by Michael Haggerty <mh...@alum.mit.edu>.
Peter Lundblad wrote:
> I like this proposals with a few comments.
> 
> Tim Hill writes:
>> (3) Relaxing the current rules that disable revision properties to  
>> allow (1) and (2) even if revision property modifications are  
>> disabled (pre-revprop-change & post-revprop-change hooks). The hooks  
>> should still be invoked during the checkin, *but* if they are not  
>> implemented, commit-time revprops are still allowed.
>>
> Why should these hooks be called here? That would be inconsistent with
> what happens to svn:log, svn:date and svn:author when committing today and
> also would be redundant since the commit related hooks can always take
> action if they want to.

Speaking of which, should one be able to use this feature to set svn:log
(i.e., serving as an alias for the "-m" and "-F" options)?  What about
svn:date and svn:author (i.e., overriding the defaults for these options)?

Michael

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Feature Request: Revision Properties at Commit Time

Posted by David Glasser <gl...@mit.edu>.
On 11/15/06, Peter Lundblad <pl...@google.com> wrote:

> Or just tack it onto svn_ra_get_commit_editor2, replacing the passed
> in log message with a new hash of props. Right, this doesn't scale if
> people add thousands of megabyte revprops at commit time, but this is the
> case of our whole property implementation.

The main issue with this solution in my mind is not the "hashes can be
big" thing, but that for the sorts of applications of this feature
that come to mind (revision signing being a biggy) you really want to
decide on a revprop value after running through the tree.

--dave

-- 
David Glasser | glasser@mit.edu | http://www.davidglasser.net/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Feature Request: Revision Properties at Commit Time

Posted by Peter Lundblad <pl...@google.com>.
I like this proposals with a few comments.

Tim Hill writes:
> (1) Add command-line syntax to the SVN COMMIT command to allow  
> setting of arbitrary revision properties at commit.
> 
I guess you mean all commands that do a commit, including copy, move
and delete as well as commit itself.

> (3) Relaxing the current rules that disable revision properties to  
> allow (1) and (2) even if revision property modifications are  
> disabled (pre-revprop-change & post-revprop-change hooks). The hooks  
> should still be invoked during the checkin, *but* if they are not  
> implemented, commit-time revprops are still allowed.
> 
Why should these hooks be called here? That would be inconsistent with
what happens to svn:log, svn:date and svn:author when committing today and
also would be redundant since the commit related hooks can always take
action if they want to.

> A possible command-line syntax might be:
> 
> svn commit .... --revpropset name="value"	# set revprop "name" to  
> "value"

You could be more precise regarding the syntax here. I imagine the quotes
are not really part of the syntax, but used to make the shell include
the space in the argument, right?

> svn commit .... --revpropset name:file		# set revprop "name" to  
> contents of "file"

We support colons in prop names, so this doesn't work. Could have
--revpropset-file instead.

> -- Formal and semi-formal "revision labels" come for free

Do you mean labels as in tags? Then, they don't, because we don't have
a quick way too look up revisions matching certain revprops. Anyway, that's
another (possibly long) thread:-)

> IMPACT OF FEATURE
> The proposer (me) is not sufficiently familiar with the internals of  
> Subversion to assess the development work required; presumably the  
> biggest issues would be modification / enhancement of the various  
> APIs to support the commit-time properties and changes to the commit  
> protocol to the server to support these in a backward compatible  
> manner (the command-line parsing changes are probably trivial by  
> comparison). It is therefore difficult to characterize the cost/ 
> benefit ratio from a development standpoint.
> 
I don't think extending the APIs is any hard at all. Just a bunch of API
revving and protocol work. We need to make sure the server throws an
error if the feature is not supported, though.


Ben Collins-Sussman writes:
> On 11/14/06, David Glasser <gl...@mit.edu> wrote:
> > adding a "set_edit_props" callback to the delta editor vtable, which
> > at the very least involves rewriting a bunch of a code to use a new
> > version of the table.
> 
> +1, this is the Right Way To Do it.
> 

Or just tack it onto svn_ra_get_commit_editor2, replacing the passed
in log message with a new hash of props. Right, this doesn't scale if
people add thousands of megabyte revprops at commit time, but this is the
case of our whole property implementation.

//lundblad -- continuing his 18 month old fight to avoid having to rev the
editor;)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Feature Request: Revision Properties at Commit Time

Posted by Ben Collins-Sussman <su...@red-bean.com>.
On 11/14/06, David Glasser <gl...@mit.edu> wrote:
> On 11/14/06, Mark Phippard <ma...@softlanding.com> wrote:
> > Tim Hill <dr...@comcast.net> wrote on 11/14/2006 01:31:38 PM:
> >
> > > Comments? Does this suck, has it got legs?
> >
> > I just wanted to point out for reference purposes that there is an issue
> > filed and there have been past discussions:
> >
> > http://subversion.tigris.org/issues/show_bug.cgi?id=1976
>
> Step one towards this would be to allow revprops to be set during the
> commit over RA; you can do this now at the repos level but not at the
> RA level.  I posted a patch a couple of months ago that implemented
> this using a new RA call which could only be called between an
> ra_get_commit_editor and its ending close_edit; however, this felt
> like somewhat of a hack, and I think a better design would involve
> adding a "set_edit_props" callback to the delta editor vtable, which
> at the very least involves rewriting a bunch of a code to use a new
> version of the table.

+1, this is the Right Way To Do it.

>
> I got a little lazy and hoped to piggyback off of Garrett's
> atomic-moves branch which also revs the delta editor :)

Eek, I wouldn't wait for that branch.  :-)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Feature Request: Revision Properties at Commit Time

Posted by Tim Hill <dr...@comcast.net>.
I'd me more than happy to help out with this work. Caveats: While I  
have over 25 years of working in OS internals and vast amounts of C  
coding experience, I know zilch about the svn code base and internal  
architecture, so this offer of *help* might be more of a hinderance  
than anything :)


--Tim

On Nov 14, 2006, at 6:04 PM, David Glasser wrote:

> On 11/14/06, Mark Phippard <ma...@softlanding.com> wrote:
>> Tim Hill <dr...@comcast.net> wrote on 11/14/2006 01:31:38 PM:
>>
>> > Comments? Does this suck, has it got legs?
>>
>> I just wanted to point out for reference purposes that there is an  
>> issue
>> filed and there have been past discussions:
>>
>> http://subversion.tigris.org/issues/show_bug.cgi?id=1976
>
> Step one towards this would be to allow revprops to be set during the
> commit over RA; you can do this now at the repos level but not at the
> RA level.  I posted a patch a couple of months ago that implemented
> this using a new RA call which could only be called between an
> ra_get_commit_editor and its ending close_edit; however, this felt
> like somewhat of a hack, and I think a better design would involve
> adding a "set_edit_props" callback to the delta editor vtable, which
> at the very least involves rewriting a bunch of a code to use a new
> version of the table.
>
> I got a little lazy and hoped to piggyback off of Garrett's
> atomic-moves branch which also revs the delta editor :)  One of these
> days I'll get back to it, but for now I'm pretty busy.  Or folks could
> take another look at my patch and decide it's not as hacky as I
> thought then.
>
> --dave
>
> -- 
> David Glasser | glasser@mit.edu | http://www.davidglasser.net/
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Feature Request: Revision Properties at Commit Time

Posted by David Glasser <gl...@mit.edu>.
On 11/14/06, Mark Phippard <ma...@softlanding.com> wrote:
> Tim Hill <dr...@comcast.net> wrote on 11/14/2006 01:31:38 PM:
>
> > Comments? Does this suck, has it got legs?
>
> I just wanted to point out for reference purposes that there is an issue
> filed and there have been past discussions:
>
> http://subversion.tigris.org/issues/show_bug.cgi?id=1976

Step one towards this would be to allow revprops to be set during the
commit over RA; you can do this now at the repos level but not at the
RA level.  I posted a patch a couple of months ago that implemented
this using a new RA call which could only be called between an
ra_get_commit_editor and its ending close_edit; however, this felt
like somewhat of a hack, and I think a better design would involve
adding a "set_edit_props" callback to the delta editor vtable, which
at the very least involves rewriting a bunch of a code to use a new
version of the table.

I got a little lazy and hoped to piggyback off of Garrett's
atomic-moves branch which also revs the delta editor :)  One of these
days I'll get back to it, but for now I'm pretty busy.  Or folks could
take another look at my patch and decide it's not as hacky as I
thought then.

--dave

-- 
David Glasser | glasser@mit.edu | http://www.davidglasser.net/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Feature Request: Revision Properties at Commit Time

Posted by Tim Hill <dr...@comcast.net>.
Probably because I raised this issue some time ago <g>. Wanted to  
flag again to test the waters for interest, as it were. The issue  
seems moribund at present.

--Tim

On Nov 14, 2006, at 10:57 AM, Mark Phippard wrote:

> Tim Hill <dr...@comcast.net> wrote on 11/14/2006 01:31:38 PM:
>
>> Comments? Does this suck, has it got legs?
>
> I just wanted to point out for reference purposes that there is an  
> issue
> filed and there have been past discussions:
>
> http://subversion.tigris.org/issues/show_bug.cgi?id=1976
>
> Mark
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Feature Request: Revision Properties at Commit Time

Posted by Mark Phippard <ma...@softlanding.com>.
Tim Hill <dr...@comcast.net> wrote on 11/14/2006 01:31:38 PM:

> Comments? Does this suck, has it got legs?

I just wanted to point out for reference purposes that there is an issue 
filed and there have been past discussions:

http://subversion.tigris.org/issues/show_bug.cgi?id=1976

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org