You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "Hyrum K. Wright" <hy...@mail.utexas.edu> on 2007/02/22 23:32:30 UTC

Auditing Design - v1 (was: Re: [merge-tracking] 'svn blame' auditing)

Hyrum K. Wright wrote:
...
> I've decided to investigate what needs to be done to implement 'svn
> blame' auditing support.  For those unfamiliar with the problem, the use
> case is simple.  Quoting [1]: "If you merge rN into a branch B, and rN
> was committed by author A, then svn blame should show the changed lines
> in B as last touched by A, even if the merge was committed by you and
> you are not A. This must also work when merging a range of revisions
> with different authors."

Thanks to all those who have commented on this thread so far.  This
document is a preliminary attempt to incorporate those comments into a
cohesive design.  Comprehensive review and suggestions are encouraged.

            Merge Tracking Commutative Author Reporting
                                 or
                       "How to pass the buck"

The Problem
-----------
With the advent of merge tracking, users want the ability to obtain
usable information about with revisions have been merged to a given
location, where a given revision has been merged to, and who the
original author of content within a file is.  Collectively, these
reporting operations are referred to as "Merge Auditing".

This design seeks to address only the third aspect of auditing:
reporting commutative authorship of a file by using 'blame', 'log',
'status --show-updates' and 'info', hereafter referred to as "reporting
commands".

This design does *not* attempt to address auditing in the scope of
merges themselves, determining what has been merged where, or where a
particular version of a file exists in the repository.  These are left
as an exercise for the reader.  :)


Current Behavior
----------------
Currently, the reporting commands treat a merge just like any other
commit: they report the contents changed in the merge as being modified
in the revision and by the person who performed the merge.  While this
is useful, we now have the ability to provide much more information to
the user upon request.


Proposed Behavior
-----------------
To maintain backward compatibility, all the reporting commands will
continue to to behave as they currently do.  A '--merge-sensitive' (we
can bikeshed about the name later) command-line switch will be added to
the client to enable the extra merge information requested.  This switch
will also work with '--xml' to include the additional merge information
in xml-ized output.

Proposed changes with the '--merge-sensitive' switch are:
'svn log':
The original log message, in the current format, with the addition of a
list of revisions that have been merged into the target.  The
'--verbose' flag would output the log information for the merged
revisions as well.
(Question:  What is the best way to visually represent all the data
being returned by 'svn log --merge-sensitive'?)

'svn blame':
Two additional columns for each line, one with the original revision
number, and one with the original author of that line.  Unlike other
commands, we do not need to worry about multiple source revisions,
because each line can have at most one author.

'svn info':
Add two more pieces of information: 'Last Original Author:' and 'Last
Original Revision:'.  Note that these could either be set by a merge to
the node, or a commit to the node.  In the latter case, they would be
the same as the 'Last Author' and 'Last Revision'.
(Question:  Do we need '--merge-sensitive' to do this?  Should the 'Last
Original {Author,Revision}' lines appear if only different from 'Last
{Author, Revision}'?)

'svn status --show-updates':
Add additional columns, reflecting the last original authors and revisions.


Implementation
--------------
Each of the reporting commands, although separate, will need to access
the merge info in a similar way.  The implementation will provide a
clean interface, so that all of the client APIs can access the reporting
logic.  It is anticipated that most of this logic will live in
libsvn_repos, with appropriate parameters punched back through the RA
layer to the client.

[As the requirements and UI are fleshed out, this section will grow as
needed.]


Outstanding Question(s)
-----------------------
As this design is still preliminary, there are outstanding questions.

* From a UI perspective, how best do we handle multiple source revisions
in a single merge revision?  That's potentially *a lot* of data.  Do we
only need handle the most recent original revision in a merge?


Miscellany
----------
Although not required as part of the Subversion 1.5 "Merge Tracking"
release, auditing is a significant "nice to have" feature, which many
users will find useful.  Once the design is finalized, whatever help
people can contribute toward getting it implemented would be appreciated.


Hopefully, the above design accurately describes the Merge Tracking
Auditing scenario, and the design parameters already agreed upon.  It
also asks several questions, which I hope we can iron on-list.

I look forward to comments.

-Hyrum


Re: Auditing Design - v1

Posted by Giovanni Bajo <ra...@develer.com>.
On 24/02/2007 3.54, Daniel Rall wrote:

>> 'svn blame':
>> Two additional columns for each line, one with the original revision
>> number, and one with the original author of that line.  Unlike other
>> commands, we do not need to worry about multiple source revisions,
>> because each line can have at most one author.
> 
> Yup, but determine which of a merge's source revisions changed a line
> is going to be tricky.

But if you don't do this, anything else in this auditing design seems rather 
useless to me. If you just "go back" through single-revision merges, you're 
not helping *that* much. A merge revision with a commit log quoting the single 
revision that is being merged (like when you do it with svnmerge.py) is a 
reasonable thing to use. Even without auditing, you need a single svn log to 
find out the infos you might need.

On the other hand, auditing through a merge of many revisions is something 
which is very hard to do manually. At minimum, you have to find out the 
original branch URL, and the magic combination of peg-revs/operative-revs 
which will allow you to blame/cat/whatever the file in that branch at the 
point of merge.
-- 
Giovanni Bajo
Develer S.r.l.
http://www.develer.com


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

Re: Auditing Design - v1 (was: Re: [merge-tracking] 'svn blame' auditing)

Posted by David Glasser <gl...@mit.edu>.
On 2/23/07, Daniel Rall <dl...@collab.net> wrote:
> > This design seeks to address only the third aspect of auditing:
> > reporting commutative authorship of a file by using 'blame', 'log',
> > 'status --show-updates' and 'info', hereafter referred to as "reporting
> > commands".
>
> Are there any other commands which report user names and revisions
> from a previous commit?  I can't think of any.

There's "ls --verbose", though I'm not sure if that's worth making
merge-sensitive.

--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: Auditing Design - v1 (was: Re: [merge-tracking] 'svn blame' auditing)

Posted by Daniel Rall <dl...@collab.net>.
On Sun, 25 Feb 2007, Stefan Haller wrote:

> mark benedetto king <mb...@lowlatency.com> wrote:
> 
> > On Fri, Feb 23, 2007 at 06:54:04PM -0800, Daniel Rall wrote:
> >
> > > > 'svn blame':
> > > > Two additional columns for each line, one with the original revision
> > > > number, and one with the original author of that line.  Unlike other
> > > > commands, we do not need to worry about multiple source revisions,
> > > > because each line can have at most one author.
> > 
> > What should be done if the merge included revisions that were themselves
> > merges?  Should those revisions be treated as "original" or is the problem
> > recursive?  Should that question be answered on the command-line?
> 
> Treating those revisions as "original" doesn't seem very useful to me in
> practice.  I'd certainly want to see the "real" original revision in
> blame.

I tend to think that 'svn blame --merge-sensitive' should recurse
until we hit the true original revision.

Re: Auditing Design - v1 (was: Re: [merge-tracking] 'svn blame' auditing)

Posted by Stefan Haller <ha...@ableton.com>.
mark benedetto king <mb...@lowlatency.com> wrote:

> On Fri, Feb 23, 2007 at 06:54:04PM -0800, Daniel Rall wrote:
>
> > > 'svn blame':
> > > Two additional columns for each line, one with the original revision
> > > number, and one with the original author of that line.  Unlike other
> > > commands, we do not need to worry about multiple source revisions,
> > > because each line can have at most one author.
> 
> What should be done if the merge included revisions that were themselves
> merges?  Should those revisions be treated as "original" or is the problem
> recursive?  Should that question be answered on the command-line?

Treating those revisions as "original" doesn't seem very useful to me in
practice.  I'd certainly want to see the "real" original revision in
blame.


-- 
Stefan Haller
Ableton
http://www.ableton.com/

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

Re: Auditing Design - v1 (was: Re: [merge-tracking] 'svn blame' auditing)

Posted by mark benedetto king <mb...@lowlatency.com>.
On Fri, Feb 23, 2007 at 06:54:04PM -0800, Daniel Rall wrote:
> 
> > 'svn blame':
> > Two additional columns for each line, one with the original revision
> > number, and one with the original author of that line.  Unlike other
> > commands, we do not need to worry about multiple source revisions,
> > because each line can have at most one author.
> 

What should be done if the merge included revisions that were themselves
merges?  Should those revisions be treated as "original" or is the problem
recursive?  Should that question be answered on the command-line?

> Yup, but determine which of a merge's source revisions changed a line
> is going to be tricky.
> 

Tricky.  ("But can you do it?")  

> > 
> > Implementation
> > --------------
> > Each of the reporting commands, although separate, will need to access
> > the merge info in a similar way.  The implementation will provide a
> > clean interface, so that all of the client APIs can access the reporting
> > logic.  It is anticipated that most of this logic will live in
> > libsvn_repos, with appropriate parameters punched back through the RA
> > layer to the client.
> 

I think that the blame processing should be done client-side, mainly
because that's where all of the work is currently being done.

svn_client_blame() already keeps the revision author and date around;
it could easily be modified to keep the merge properties, too (since
file_rev_handler gets prop diffs).

Additionally, it would be useful to keep track of the original line
number of each blame block.

Once all of the "apparent" blame has been assigned, the blame
blocks with revisions that have merge metadata need to be further
inspected.

To determine the "true" source of a particular line, a process
similar to the existing blame routine would need to be followed,
except rather than diffing against one historical revision at
a time, the system would need to diff against one merged revision
at a time.

To restate: 

For each revision R of node N that is the apparent source of a
particular line of blame, if R is a merge revision then:

Let O represent the revision of N prior to R.
Let G represent the revisions of N merged in R.
Let M represent the manual changes performed in R.

Starting with O, apply the differences in G, keeping track of
the source revision of each change.

Either these changes yield the blamed line (in which case, we know
the "true" source), or they do not, meaning that the blamed line was
part of M.

If the problem is recursive (see above), then recurse. :-)

--ben

PS: it's been a long time since I've gotten into the source
so take all of the above with several grains of salt.

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

Re: Auditing Design - v1 (was: Re: [merge-tracking] 'svn blame' auditing)

Posted by Stefan Haller <ha...@ableton.com>.
Daniel Rall <dl...@collab.net> wrote:

> As far as the original log messages go (for 'log --merge-sensitive
> --verbose'), we could do something like svnmerge.py's generated log
> messages:
> 
> $ cat svnmerge-commit-message.txt 
> Merged revisions 23490 via svnmerge from 
> https://svn.collab.net/repos/svn/trunk
> 
> ........
>   r23490 | dlr | 2007-02-23 18:19:06 -0800 (Fri, 23 Feb 2007) | 1 line
>   
>   * www/merge-tracking/design.html (data-structures): Add new section.
> ........
> 
> It indents the original log message, and adds separators between them.
> This seems like a reasonable style of output to me.

Note, however, that the information can be recursive; if r23490 is in
turn a merge of some other revisions, the original revisions of that
merge would have to be printed too, with another level of indentation.
(This is not a contrived case, it happens in practice all the time for
us.)

My feeling is that it may be good enough to leave out r23490 itself in
that case, and only print all the original revisions (i.e. the leaf
nodes in the merge tree) in one flat list.  I'm not sure if all users
would agree with that though.


-- 
Stefan Haller
Ableton
http://www.ableton.com/

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

Re: Auditing Design - v1 (was: Re: [merge-tracking] 'svn blame' auditing)

Posted by Daniel Rall <dl...@collab.net>.
On Thu, 22 Feb 2007, Hyrum K. Wright wrote:

> Hyrum K. Wright wrote:
...
> Thanks to all those who have commented on this thread so far.  This
> document is a preliminary attempt to incorporate those comments into a
> cohesive design.  Comprehensive review and suggestions are encouraged.

Hyrum, this looks great, and asks all the right questions.  Once we
finalize things, I'll roll your work into the func spec, and any
nitty-gritty details into the design doc.

>             Merge Tracking Commutative Author Reporting
>                                  or
>                        "How to pass the buck"
> 
> The Problem
> -----------
> With the advent of merge tracking, users want the ability to obtain
> usable information about with revisions have been merged to a given
> location, where a given revision has been merged to, and who the
> original author of content within a file is.  Collectively, these
> reporting operations are referred to as "Merge Auditing".

The requirements/use cases can be found here:

http://subversion.tigris.org/merge-tracking/requirements.html#auditing

> This design seeks to address only the third aspect of auditing:
> reporting commutative authorship of a file by using 'blame', 'log',
> 'status --show-updates' and 'info', hereafter referred to as "reporting
> commands".

Are there any other commands which report user names and revisions
from a previous commit?  I can't think of any.

...
> Proposed Behavior
> -----------------
> To maintain backward compatibility, all the reporting commands will
> continue to to behave as they currently do.  A '--merge-sensitive' (we
> can bikeshed about the name later) command-line switch will be added to
> the client to enable the extra merge information requested.  This switch
> will also work with '--xml' to include the additional merge information
> in xml-ized output.

I like --merge-sensitive.  Alternatives (which I don't necessarilly
like better):

--merge-audit
--merge-report

What will --merge-sensitive do for reverts (e.g. commits which remove
merge info)?

> Proposed changes with the '--merge-sensitive' switch are:
> 'svn log':
> The original log message, in the current format, with the addition of a
> list of revisions that have been merged into the target.  The
> '--verbose' flag would output the log information for the merged
> revisions as well.

I addition to the list of merged revisions, we should provide a "merge
source" path.  A single merge could theoretically have multiple merge
sources, and each source could have multiple revisions (what's called
a "range list" on the merge-tracking branch).  I've added a
description of the merge-tracking branch's new data structures to the
design doc:

http://subversion.tigris.org/merge-tracking/design.html#data-structures


Showing all original log messages could be quite verbose in some cases
(e.g. after sync'ing a feature branch with trunk).  That said, I agree
that we should provide easy access to that information.

> (Question:  What is the best way to visually represent all the data
> being returned by 'svn log --merge-sensitive'?)

To represent the merge info, we probably want to use output consistent
with what we did for diffs of the "svn:mergeinfo" property.  We
currently just stringify the merge info, showing it similarly to how
we record it in the "svn:mergeinfo" property (but with an additional
"r", for some reason):

$ svn ps svn:mergeinfo "/trunk:3" .
$ ./subversion/svn/svn di -N .

Property changes on: .
___________________________________________________________________
Name: svn:mergeinfo
   Merged /trunk:r3

If we want to do something fancier for 'log', we should change 'diff'
as well.

As far as the original log messages go (for 'log --merge-sensitive
--verbose'), we could do something like svnmerge.py's generated log
messages:

$ cat svnmerge-commit-message.txt 
Merged revisions 23490 via svnmerge from 
https://svn.collab.net/repos/svn/trunk

........
  r23490 | dlr | 2007-02-23 18:19:06 -0800 (Fri, 23 Feb 2007) | 1 line
  
  * www/merge-tracking/design.html (data-structures): Add new section.
........

It indents the original log message, and adds separators between them.
This seems like a reasonable style of output to me.

> 'svn blame':
> Two additional columns for each line, one with the original revision
> number, and one with the original author of that line.  Unlike other
> commands, we do not need to worry about multiple source revisions,
> because each line can have at most one author.

Yup, but determine which of a merge's source revisions changed a line
is going to be tricky.

> 'svn info':
> Add two more pieces of information: 'Last Original Author:' and 'Last
> Original Revision:'.  Note that these could either be set by a merge to
> the node, or a commit to the node.  In the latter case, they would be
> the same as the 'Last Author' and 'Last Revision'.
> (Question:  Do we need '--merge-sensitive' to do this?  Should the 'Last
> Original {Author,Revision}' lines appear if only different from 'Last
> {Author, Revision}'?)

As the output must remain machine-parsable, we need --merge-sensitive
to toggle the expected format.

> 'svn status --show-updates':
> Add additional columns, reflecting the last original authors and revisions.
> 
> 
> Implementation
> --------------
> Each of the reporting commands, although separate, will need to access
> the merge info in a similar way.  The implementation will provide a
> clean interface, so that all of the client APIs can access the reporting
> logic.  It is anticipated that most of this logic will live in
> libsvn_repos, with appropriate parameters punched back through the RA
> layer to the client.

For all cases discussed above, we're looking at an API which takes one
or more paths and a revision (range?), and returns the merge info
which was added or removed in that revision.  For the 'blame' case, we
may also need to include some type of line number parameter, the
handling of which is going to get ugly, since our FS isn't based on a
weave.

http://web.mit.edu/ghudson/thoughts/file-versioning
http://en.wikipedia.org/wiki/Source_Code_Control_System

In svn_repos.h on the merge-tracking branch, we have this API:

/**
 * Fetch the merge info for @a paths at @rev, and save it to @a
 * mergeinfo.  @a mergeinfo is a mapping of @c char * target paths
 * (from @a paths) to textual (@c char *) representations of merge
 * info (as managed by svn_mergeinfo.h), or @c NULL if there is no
 * merge info visible or available.
 *
 * When @a include_parents is @c TRUE, include inherited merge info
 * from parent directories of @a paths.
 *
 * If @a revision is @c SVN_INVALID_REVNUM, it defaults to youngest.
 *
 * If optional @a authz_read_func is non-NULL, then use this function
 * (along with optional @a authz_read_baton) to check the readability
 * of each path which merge info was requested for (from @a paths).
 * Silently omit unreadable paths from the request for merge info.
 *
 * Use @a pool for temporary allocations.
 *
 * @since New in 1.5.
 */
svn_error_t *
svn_repos_fs_get_merge_info(apr_hash_t **mergeoutput,
                            svn_repos_t *repos,
                            const apr_array_header_t *paths,
                            svn_revnum_t revision,
                            svn_boolean_t include_parents,
                            svn_repos_authz_func_t authz_read_func,
                            void *authz_read_baton,
                            apr_pool_t *pool);

I think this doesn't do exactly what you want, because it gives you
the entire current state of the merge info for some paths, rather than
telling you only the merge info which changed in a given revision.  We
should already be recording the information you want, but you'll need
to examine the FS code changed on the merge-tracking branch to see how
to get it out of current sqlite schema.

This API might be a good place to look:

/* Get the merge info for the set of PATHS (an array of
   absolute-in-the-fs paths) under ROOT and return it in *MERGEINFO,
   mapping char * paths to char * strings with mergeinfo, allocated in
   POOL.  If INCLUDE_PARENTS is TRUE elide for mergeinfo.  If a path
   has no mergeinfo, just return no info for that path.  Return an
   error if the mergeinfo store does not exist or doesn't use the
   'mergeinfo' schema.  */
svn_error_t *
svn_fs_merge_info__get_merge_info(apr_hash_t **mergeinfo,
                                  svn_fs_root_t *root,
                                  const apr_array_header_t *paths,
                                  svn_boolean_t include_parents,
                                  apr_pool_t *pool);


...
> Outstanding Question(s)
> -----------------------
> As this design is still preliminary, there are outstanding questions.
> 
> * From a UI perspective, how best do we handle multiple source revisions
> in a single merge revision?  That's potentially *a lot* of data.  Do we
> only need handle the most recent original revision in a merge?

In most cases, we can represent multiple source revisions using a
range notation (e.g. 3-7).  However, this doesn't fix the case where a
merge cherry-picks a large number of revision ranges
(e.g. 3-7,11,17-20,42-50,71,80-132).  I think that this is something
that we are just going to have to live with.

Another question:

* How are we going to implement 'blame --merge-sensitive'?  It seems
like we need to look at each delta in our set of merge sources,
figure out which lines it changes, and use the most recent (youngest)
revision/author for each line of our blame output.

> Miscellany
> ----------
> Although not required as part of the Subversion 1.5 "Merge Tracking"
> release, auditing is a significant "nice to have" feature, which many
> users will find useful.  Once the design is finalized, whatever help
> people can contribute toward getting it implemented would be appreciated.

mbk mentioned on IRC that he required auditing features, and might be
available to help out with the work.  I've added him to the CC list.

Re: Auditing Design - v1

Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
Hyrum K. Wright wrote:
...
> Thanks to all those who have commented on this thread so far.  This
> document is a preliminary attempt to incorporate those comments into a
> cohesive design.  Comprehensive review and suggestions are encouraged.

To all interested parties:
I've moved the design for merge tracking auditing to the website.  You
can find it here:
http://subversion.tigris.org/merge-tracking/func-spec.html#commutative-reporting

As always, comments still appreciated.

-Hyrum


Re: Auditing Design - v1

Posted by Stefan Haller <ha...@ableton.com>.
Hyrum K. Wright <hy...@mail.utexas.edu> wrote:

> Also, in the indentation scheme, how would '--limit' function?  Just
> limit the native revisions?  Limit all revisions, including merged ones?
>  In the above example, it would seem incomplete to print one of the
> indented revisions and not the other, if the user gave '--limit 3'.

Good question.  After thinking about it, I'm afraid the only reasonable
answer is that the limit should apply only to top-level revisions (the
two alternatives would be applying the limit to the entire list of
top-level and indented revisions as a whole, or applying it again
recursively at each indentation level.  Neither seems useful in
practice).

Which means that with '--merge-sensitive' you have to be prepared for
getting lots of output even with a small --limit value.


-- 
Stefan Haller
Ableton
http://www.ableton.com/

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

Re: Auditing Design - v1

Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
Stefan Haller wrote:
> Hyrum K. Wright <hy...@mail.utexas.edu> wrote:
> 
>> This idea of representing the merged revisions inline when using
>> '--merge-sensitive' is starting to grow on me.  Should they be included
>> in all 'svn log' invocations, or just enabled by using '--verbose'?
> 
> No, '--verbose' would be the wrong flag for this.  '--merge-sensitive'
> should control whether the merged revisions show up at all; and if they
> do, '--verbose' should control whether the changed path information is
> shown for them, like for every log entry.

Upon thinking about it further, I agree.

>> I guess, without '--verbose' it may be difficult to determine that the
>> revision came in as a result of a merge.
> 
> I'm strongly in favour of indenting the log entries for the merged
> sub-revisions (with a new level of indentation for every merged revision
> that was in turn a merge).  This is particularly important with
> --merge-sensitive and --quiet, but without --verbose:
> 
> ------------------------------------------------------------------------
> r23493 | malcolm | 2007-02-24 09:24:26 +0100 (Sat, 24 Feb 2007)
> ------------------------------------------------------------------------
> r23474 | dlr | 2007-02-23 00:04:33 +0100 (Fri, 23 Feb 2007)
>   ----------------------------------------------------------------------
>   r23406 | blair | 2007-02-16 03:30:50 +0100 (Fri, 16 Feb 2007)
>   ----------------------------------------------------------------------
>   r23405 | blair | 2007-02-16 03:29:38 +0100 (Fri, 16 Feb 2007)
> ------------------------------------------------------------------------

Ah, but if you don't use '--merge-sensitive', the merged revisions won't
show up *at all*.  If you really wanted to know which revisions were
merge, and which were native, it would be possible to run 'svn log'
twice, once with '--merge-sensitive' and once without it.  The new
revisions would be pretty obvious (especially if combined with your
favorite diff tool.)

Also, in the indentation scheme, how would '--limit' function?  Just
limit the native revisions?  Limit all revisions, including merged ones?
 In the above example, it would seem incomplete to print one of the
indented revisions and not the other, if the user gave '--limit 3'.

-Hyrum


Re: Auditing Design - v1

Posted by Stefan Haller <ha...@ableton.com>.
Hyrum K. Wright <hy...@mail.utexas.edu> wrote:

> This idea of representing the merged revisions inline when using
> '--merge-sensitive' is starting to grow on me.  Should they be included
> in all 'svn log' invocations, or just enabled by using '--verbose'?

No, '--verbose' would be the wrong flag for this.  '--merge-sensitive'
should control whether the merged revisions show up at all; and if they
do, '--verbose' should control whether the changed path information is
shown for them, like for every log entry.

> I guess, without '--verbose' it may be difficult to determine that the
> revision came in as a result of a merge.

I'm strongly in favour of indenting the log entries for the merged
sub-revisions (with a new level of indentation for every merged revision
that was in turn a merge).  This is particularly important with
--merge-sensitive and --quiet, but without --verbose:

------------------------------------------------------------------------
r23493 | malcolm | 2007-02-24 09:24:26 +0100 (Sat, 24 Feb 2007)
------------------------------------------------------------------------
r23474 | dlr | 2007-02-23 00:04:33 +0100 (Fri, 23 Feb 2007)
  ----------------------------------------------------------------------
  r23406 | blair | 2007-02-16 03:30:50 +0100 (Fri, 16 Feb 2007)
  ----------------------------------------------------------------------
  r23405 | blair | 2007-02-16 03:29:38 +0100 (Fri, 16 Feb 2007)
------------------------------------------------------------------------


-- 
Stefan Haller
Ableton
http://www.ableton.com/

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

Re: Auditing Design - v1

Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
Giovanni Bajo wrote:
> On 23/02/2007 0.32, Hyrum K. Wright wrote:
...
>> 'svn log':
>> The original log message, in the current format, with the addition of a
>> list of revisions that have been merged into the target.  The
>> '--verbose' flag would output the log information for the merged
>> revisions as well.
>> (Question:  What is the best way to visually represent all the data
>> being returned by 'svn log --merge-sensitive'?)
> 
> I know Daniel proposed the output of svnmerge.py, but I think it's
> better to be more radical here. For instance, this is output of "svn
> log" in the 1.4.x branch today:
> 
> ------------------------------------------------------------------------
> r23493 | malcolm | 2007-02-24 09:24:26 +0100 (Sat, 24 Feb 2007) | 2 lines
> Changed paths:
>    M /branches/1.4.x/STATUS
> 
> * STATUS: Nominate r23491, r23492.
> 
> ------------------------------------------------------------------------
> r23474 | dlr | 2007-02-23 00:04:33 +0100 (Fri, 23 Feb 2007) | 5 lines
> Changed paths:
>    M /branches/1.4.x
>    M /branches/1.4.x/STATUS
>    M /branches/1.4.x/subversion/bindings/swig/ruby/svn/client.rb
>    M /branches/1.4.x/subversion/bindings/swig/ruby/svn/wc.rb
> 
> Backport r23405 and r23406 from trunk into the 1.4.x branch, fixing
> typos in Ruby bindings method name and documentation.
> 
> Approved by: +1: blair, dlr, kou
> 
> ------------------------------------------------------------------------
> 
> 
> And this is what I would like to see:
> 
> ------------------------------------------------------------------------
> r23493 | malcolm | 2007-02-24 09:24:26 +0100 (Sat, 24 Feb 2007) | 2 lines
> Changed paths:
>    M /branches/1.4.x/STATUS
> 
> * STATUS: Nominate r23491, r23492.
> 
> ------------------------------------------------------------------------
> r23474 | dlr | 2007-02-23 00:04:33 +0100 (Fri, 23 Feb 2007) | 5 lines
> Changed paths:
>    M /branches/1.4.x
>    M /branches/1.4.x/STATUS
>    M /branches/1.4.x/subversion/bindings/swig/ruby/svn/client.rb
>    M /branches/1.4.x/subversion/bindings/swig/ruby/svn/wc.rb
> 
> Backport r23405 and r23406 from trunk into the 1.4.x branch, fixing
> typos in Ruby bindings method name and documentation.
> 
> Approved by: +1: blair, dlr, kou
> 
> ------------------------------------------------------------------------
> r23406 | blair | 2007-02-16 03:30:50 +0100 (Fri, 16 Feb 2007) | 4 lines
> Changed paths:
>    M /trunk/subversion/bindings/swig/ruby/svn/wc.rb
> 
> * subversion/bindings/swig/ruby/svn/wc.rb:
>   (merge_prop_diffs):
>     Renamed from mrege_prop_diffs.
> 
> ------------------------------------------------------------------------
> r23405 | blair | 2007-02-16 03:29:38 +0100 (Fri, 16 Feb 2007) | 3 lines
> Changed paths:
>    M /trunk/subversion/bindings/swig/ruby/svn/client.rb
> 
> * subversion/bindings/swig/ruby/svn/client.rb:
>   Spelling fix: s/avaiable/available/g.
> 
> ------------------------------------------------------------------------
> 
> 
> I think this example might not even be the best one, because this is a
> release branch. But think of the output of svn log of SVN trunk after
> the merge-tracking branch is merged. You probably don't care about
> anything else *but* seeing the actual revisions that are the commits in
> the branch. Having them "quoted" like svnmerge.py does doesn't help in
> any way. It just highlights an implementation detail of how SVN merges
> work.
> 
> Notice that if you really need, you can still know that the commit was
> done in another branch by looking at the modifies paths (in the
> --verbose output shown above). I assume you really don't need this
> information most of the time, but still it's there.
> 
> Moreover, with the example above, it would be nice if "svn diff -c23405"
> did the right thing, even if you're sitting in the branch...

This idea of representing the merged revisions inline when using
'--merge-sensitive' is starting to grow on me.  Should they be included
in all 'svn log' invocations, or just enabled by using '--verbose'?

I guess, without '--verbose' it may be difficult to determine that the
revision came in as a result of a merge.

Thoughts?

-Hyrum


Re: Auditing Design - v1

Posted by Giovanni Bajo <ra...@develer.com>.
On 23/02/2007 0.32, Hyrum K. Wright wrote:

> Proposed Behavior
> -----------------
> To maintain backward compatibility, all the reporting commands will
> continue to to behave as they currently do.  A '--merge-sensitive' (we
> can bikeshed about the name later) command-line switch will be added to
> the client to enable the extra merge information requested.  

I'm not sure why we need to preserve backward compatibility. Merge support is 
new in 1.5, so I guess it is expected that more things will be done and the 
output of commands will change. The mere fact that a new property is modified 
*is* a change.

Honestly, I'd rather not to add another option and just add auditing behaviour 
as default. I think we should think the other way: if there is a legitimate 
use case for *disabling* merge auditing, we can add a --no-merge-sensitive 
later (even in SVN 1.6).

> 'svn log':
> The original log message, in the current format, with the addition of a
> list of revisions that have been merged into the target.  The
> '--verbose' flag would output the log information for the merged
> revisions as well.
> (Question:  What is the best way to visually represent all the data
> being returned by 'svn log --merge-sensitive'?)

I know Daniel proposed the output of svnmerge.py, but I think it's better to 
be more radical here. For instance, this is output of "svn log" in the 1.4.x 
branch today:

------------------------------------------------------------------------
r23493 | malcolm | 2007-02-24 09:24:26 +0100 (Sat, 24 Feb 2007) | 2 lines
Changed paths:
    M /branches/1.4.x/STATUS

* STATUS: Nominate r23491, r23492.

------------------------------------------------------------------------
r23474 | dlr | 2007-02-23 00:04:33 +0100 (Fri, 23 Feb 2007) | 5 lines
Changed paths:
    M /branches/1.4.x
    M /branches/1.4.x/STATUS
    M /branches/1.4.x/subversion/bindings/swig/ruby/svn/client.rb
    M /branches/1.4.x/subversion/bindings/swig/ruby/svn/wc.rb

Backport r23405 and r23406 from trunk into the 1.4.x branch, fixing
typos in Ruby bindings method name and documentation.

Approved by: +1: blair, dlr, kou

------------------------------------------------------------------------


And this is what I would like to see:

------------------------------------------------------------------------
r23493 | malcolm | 2007-02-24 09:24:26 +0100 (Sat, 24 Feb 2007) | 2 lines
Changed paths:
    M /branches/1.4.x/STATUS

* STATUS: Nominate r23491, r23492.

------------------------------------------------------------------------
r23474 | dlr | 2007-02-23 00:04:33 +0100 (Fri, 23 Feb 2007) | 5 lines
Changed paths:
    M /branches/1.4.x
    M /branches/1.4.x/STATUS
    M /branches/1.4.x/subversion/bindings/swig/ruby/svn/client.rb
    M /branches/1.4.x/subversion/bindings/swig/ruby/svn/wc.rb

Backport r23405 and r23406 from trunk into the 1.4.x branch, fixing
typos in Ruby bindings method name and documentation.

Approved by: +1: blair, dlr, kou

------------------------------------------------------------------------
r23406 | blair | 2007-02-16 03:30:50 +0100 (Fri, 16 Feb 2007) | 4 lines
Changed paths:
    M /trunk/subversion/bindings/swig/ruby/svn/wc.rb

* subversion/bindings/swig/ruby/svn/wc.rb:
   (merge_prop_diffs):
     Renamed from mrege_prop_diffs.

------------------------------------------------------------------------
r23405 | blair | 2007-02-16 03:29:38 +0100 (Fri, 16 Feb 2007) | 3 lines
Changed paths:
    M /trunk/subversion/bindings/swig/ruby/svn/client.rb

* subversion/bindings/swig/ruby/svn/client.rb:
   Spelling fix: s/avaiable/available/g.

------------------------------------------------------------------------


I think this example might not even be the best one, because this is a release 
branch. But think of the output of svn log of SVN trunk after the 
merge-tracking branch is merged. You probably don't care about anything else 
*but* seeing the actual revisions that are the commits in the branch. Having 
them "quoted" like svnmerge.py does doesn't help in any way. It just 
highlights an implementation detail of how SVN merges work.

Notice that if you really need, you can still know that the commit was done in 
another branch by looking at the modifies paths (in the --verbose output shown 
above). I assume you really don't need this information most of the time, but 
still it's there.

Moreover, with the example above, it would be nice if "svn diff -c23405" did 
the right thing, even if you're sitting in the branch...


> 'svn blame':
> Two additional columns for each line, one with the original revision
> number, and one with the original author of that line.  Unlike other
> commands, we do not need to worry about multiple source revisions,
> because each line can have at most one author.

Here too, I don't see why we need to confuse people with the two columns. I 
think by default 99% of the cases people just want to know the original 
revision in which the original commit is done, with the original log message 
and original author. They want the auditing behaviour *by default*.

If and only if a legitimate use case for *disabling* it is found, we could 
conceive a "svn blame --no-merge-sensitive" which shows the commits of the 
merges themselves.

> 'svn info':
> Add two more pieces of information: 'Last Original Author:' and 'Last
> Original Revision:'.  Note that these could either be set by a merge to
> the node, or a commit to the node.  In the latter case, they would be
> the same as the 'Last Author' and 'Last Revision'.

I'm fine with this. It's clear and simple.

> (Question:  Do we need '--merge-sensitive' to do this?  Should the 'Last

No, if you ask me :) It should be the default.

> Original {Author,Revision}' lines appear if only different from 'Last
> {Author, Revision}'?)

No, that's confusing, it would seem to me as if auditing wasn't working for 
some reason. I think there's nothing to gain and much to confuse.
-- 
Giovanni Bajo

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

Re: Auditing Design - v1 (was: Re: [merge-tracking] 'svn blame' auditing)

Posted by Daniel Rall <dl...@collab.net>.
On Sun, 25 Feb 2007, Stefan Haller wrote:

> Hyrum K. Wright <hy...@mail.utexas.edu> wrote:
> 
> > To maintain backward compatibility, all the reporting commands will
> > continue to to behave as they currently do.  A '--merge-sensitive' (we
> > can bikeshed about the name later) command-line switch will be added to
> > the client to enable the extra merge information requested.
> 
> Backward compatibility is good, but I expect that many users want this
> switch to be on by default all the time.  Should there be a way to make
> it the default, maybe with a .subversion/config option?

That sounds reasonable to me.  However, I'd pencil this in as a "nice
to have".  IMHO, it shouldn't be a blocker for this feature.

Re: Auditing Design - v1 (was: Re: [merge-tracking] 'svn blame' auditing)

Posted by Stefan Haller <ha...@ableton.com>.
Hyrum K. Wright <hy...@mail.utexas.edu> wrote:

> To maintain backward compatibility, all the reporting commands will
> continue to to behave as they currently do.  A '--merge-sensitive' (we
> can bikeshed about the name later) command-line switch will be added to
> the client to enable the extra merge information requested.

Backward compatibility is good, but I expect that many users want this
switch to be on by default all the time.  Should there be a way to make
it the default, maybe with a .subversion/config option?


-- 
Stefan Haller
Ableton
http://www.ableton.com/

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