[PATCH] Issue 2848

Attached is a patch for issue 2848
http://subversion.tigris.org/issues/show_bug.cgi?id=2848. It makes the
start and end fields of svn_merge_range_t analogous to the command line
option -rS:E, i.e. if S < E then S is exclusive to the range described
and E inclusive. If E > S then the inverse is true. This assures that
we can identify an additive or subtractive merge of a single revision.

What about when S == E? Well this patch largely does away with that
possibility. Unfortunately there is one case where it still occurs, see
open issue B below.

Open Issues:

A) NOTIFICATION BIKESHED:

How should the notification look when reverse merging a single revision?
Previously the notifications for additive and subtractive merges of a
single revision were identical:

>svn merge -c5 %URL% WCPATH
 or
>svn merge -c-5 %URL% WCPATH
--- Merging r5:
...

This patch changes the latter subtractive merge notification to look
like this:
>svn merge -c-5 %URL% WCPATH
--- Merging r-5:
             ^

What do people think of this? Ben, I know you discussed this a lot on
list and IRC already, did you have a better option?

B) MERGE OF DIFFERENT URLS:

The one place we still find svn_merge_range_t start and end fields equal
is when merging two different URLS (for those not following merge
tracking closely, note that these types of merges have *no* effect on
svn:mergeinfo),

svn merge sourceURL1[@N] sourceURL2[@M] [WCPATH]

where N=M or N and M are missing and default to HEAD. In this case
grok_range_info_from_opt_revisions() returns a svn_merge_range_t with
start == end. In this case my patch does the notifications the way it
always has:

--- Merging rM:

Or, if N != M:

--- Merging rN+1 through rM:

Thinking about this a bit, I wonder if we shouldn't change the
notification in these cases to something like:

--- Merging different URLs:

Or something else, just let the user know that svn:mergeinfo is not
being set/updated.

Thoughts?

C) RANGELIST UTILITY LIMITATIONS:

Not really related to this patch, but thought I'd mention it...

Many of the svn_rangelist_* functions in libsvn_subr/mergeinfo.c won't
work properly with svn_merge_range_ts where start > end, for example:

svn_rangelist_to_stringbuf()
svn_rangelist_to_revs()
svn_rangelist_merge()...or anything dependent on combine_ranges(), which
means almost everything.

Currently this isn't a problem, as we are usually dealing with ranges
from or to become svn:mergeinfo where start <= end.

Should we tweak the docstrings to reflect this limitation or does anyone
see a need for these functions to handle start > end cases?

Paul

P.S. I'm on vacation Monday and Tuesday so my Internet access will be
intermittent at best (beach shack with no power) until Tuesday night.
Of course if it rains I might be back sooner...and it's supposed to rain
:-(

[[[
Make the svn_merge_range_t start field exclusive of the range being
described.

This will make svn_merge_range_t's start and end fields analogous to the
command line's "-rX:Y" option. Previously a command like 'svn merge
%URL%
WCPATH -rX:Y' resulted in a svn_merge_range_t with start == X+1 and end
== Y.
With this patch start == X and end == Y. This is particularly helpful
when
merging/reverse merging a single revision. Where before 'svn merge
%URL%
WCPATH -cZ' *and* 'svn merge %URL% WCPATH -c-Z' both resulted in
start == end == Z, now the former is start == Z-1, end == Z and the
latter
start == Z, end == Z - 1. This allows for more accurate merge
notifications.

Note that this patch in now way affects how we store svn:mergeinfo
props,
Those are still inclusive.

* subversion/libsvn_client/copy.c
  (get_implied_mergeinfo(): Make start rev of implied range exclusive.

* subversion/libsvn_client/merge.c
  (grok_range_info_from_opt_revisions): Make start rev range exclusive.
  (do_merge): Stop tweaking start and end of range for calls to
  svn_client__get_diff_editor(), svn_ra_do_diff3(), and
reporter->set_path(),
  the ranges from grok_range_info_from_opt_revisions() now work as-is.
  (do_single_file_merge): As above, but for calls to
  single_file_merge_get_file() and merge_file_changed().

* subversion/libsvn_repos/log.c
  (find_merge_source): Account for fact that range start is now
exclusive.

* subversion/libsvn_subr/mergeinfo.c
  (combine_ranges, parse_revlist, range_intersect,
  rangelist_intersect_or_remove, svn_rangelist_count_revs,
  svn_rangelist_to_revs, svn_range_to_stringbuf): Account for fact that
  range start is now exclusive or in the case of reverse merges that the
end
  range is exclusive. Make notification for reverse merge of a single
  revision discernable from a "forward" merge of the same revision.

* subversion/svn/notify.c
  (notify): Account for fact that range start is now exclusive.

* subversion/tests/cmdline/merge_tests.py
  (textual_merges_galore, merge_with_implicit_target_helper,
  merge_with_prev, merge_binary_with_common_ancestry,
merge_dir_branches,
  mergeinfo_elision, empty_rev_range_mergeinfo): Update calls to
  svntest.main.merge_notify_line() to pass a negative number when
reverse
  merging a single revision. No changes to merge_notify_line() are
required,
  it works as-is.

* subversion/tests/libsvn_subr/mergeinfo-test.c
  (mergeinfo_ranges, test_parse_combine_rangeinfo, test_diff_mergeinfo,
  test_rangelist_reverse, test_rangelist_intersect,
test_merge_mergeinfo,
  test_remove_mergeinfo): Account for fact that range start is now
exclusive.
]]]

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