[Svnmerge] [PATCH] fix bugs and tests from r22788
lsuvkne at onemodel.org
lsuvkne at onemodel.org
Sat Jul 7 19:32:59 PDT 2007
Short version: Updated patch attached that fixes the comments
and takes advantage of Raman Gupta's patch, to no longer call
"svn resolved" in a test.
-----------------------------------
Long version:
I fixed the comments (which Dustin can still tweak if needed).
>>> On Sat, Jul 7, 2007 at 7:03 pm, lsuvkne at onemodel.org wrote:
>>> @@ ‑834,12 +853,24 @@
>>>
>>> # Merge into trunk
>>> self.svnmerge("merge ‑vv ‑S branch2",
>>> ‑ match=r"merge ‑r 18:19")
>>> + match=r"merge ‑‑force ‑r 15:19")
>>> +
>>> + # Before this file was modified in r22788, trunk was initialized
[snip]
>>> + self.launch("svn resolved .", match="Resolved conflicted state of
>> '.'")
>>
>> This case baffled me for a while, and while the above explanation is
[snip]
>
> (While I haven't studied them closely, this sounds related to the recent
> discussions
> on transitive/graph/"multi-hop"... merges.)
Indeed, r25683 (Raman Gupta's patch that Dustin just applied) makes the
need go away, so I've removed the call to "svn resolved" altogether, and
the "svn commit" now gets no errors, and we have no need for an ugly
comment.
So, new patch file attached w/ those two changes. Here again for convenience
is the candidate log message:
[[[
Fix bug & related test failures caused by r22788, to improve default revision
range
set by "svnmerge init" if none provided by user, for scenario where merge
source
is a copy of the merge target (i.e., merging from branch back to trunk).
* contrib/client‑side/svnmerge/svnmerge.py
(get_containing_logentry_xml_text): New function.
(get_copyfrom): Add copy_committed_in_rev to return values; refactor to
clarify.
(action_init): Use copy_committed_in_rev from get_copyfrom call for
correct
conditional default revision range; refactor to clarify.
* contrib/client‑side/svnmerge/svnmerge_test.py: update affected tests
Patch by: Luke Call <lsuvkne at onemodel.org>
Reviewed by: Dustin J. Mitchell <dustin at zmanda.com> (Dustin clarified
comments.)
]]]
Thanks again,
-Luke
-------------- next part --------------
Index: contrib/client-side/svnmerge/svnmerge_test.py
===================================================================
--- contrib/client-side/svnmerge/svnmerge_test.py (revision 25684)
+++ contrib/client-side/svnmerge/svnmerge_test.py (working copy)
@@ -499,7 +499,10 @@
def testBlocked(self):
# Initialize svnmerge
- self.svnmerge("init")
+ # (Initializing explicitly to versions 1-6; see comments under
+ # testBasic() for why.)
+ self.svnmerge("init -r 1-6")
+
self.launch("svn commit -F svnmerge-commit-message.txt",
match=r"Committed revision")
@@ -599,7 +602,10 @@
self.assertEqual(p, '/branches/test-branch:18')
def testBasic(self):
- self.svnmerge("init")
+ # specify revisions explicitly to avoid incidentally testing automatic
+ # revision guessing
+ self.svnmerge("init -r 1-6")
+
p = self.getproperty()
self.assertEqual("/trunk:1-6", p)
@@ -743,7 +749,10 @@
def testTrimmedAvailMerge(self):
"""Check that both avail and merge do not search for phantom revs too hard."""
- self.svnmerge("init")
+ # (Initializing explicitly to versions 1-6; see comments under
+ # testBasic() for why.)
+ self.svnmerge("init -r 1-6")
+
self.svnmerge("avail -vv -r8-9", match=r"svn log.*-r8:9")
self.svnmerge("merge -F -vv -r8-9", match=r"svn log.*-r8:9")
self.svnmerge("avail -vv -r2", nonmatch=r"svn log")
@@ -751,7 +760,11 @@
def testMergeRecordOnly(self):
"""Check that flagging revisions as manually merged works."""
- self.svnmerge("init")
+ """Check that both avail and merge do not search for phantom revs too hard."""
+ # (Initializing explicitly to versions 1-6; see comments under
+ # testBasic() for why.)
+ self.svnmerge("init -r 1-6")
+
self.svnmerge("avail -vv -r9", match=r"svn log.*-r9:9")
self.svnmerge("merge --record-only -F -vv -r9",
nonmatch=r"svn merge -r 8:9")
@@ -785,7 +798,7 @@
self.launch("svn commit -m \"Change to test1 on trunk\"",
match=r"Committed revision 16")
- self.svnmerge("integrated", match=r"^13-14$")
+ self.svnmerge("integrated", match=r"^13$")
os.chdir("..")
os.chdir("test-branch")
@@ -794,7 +807,7 @@
self.launch("svn update", match=r"At revision 16")
self.svnmerge("avail -vv --bidirectional", match=r"16$")
- self.svnmerge("merge -vv --bidirectional", match=r"merge -r 15:16")
+ self.svnmerge("merge -vv --bidirectional", match=r"merge --force -r 15:16")
p = self.getproperty()
self.assertEqual("/trunk:1-16", p)
self.svnmerge("integrated", match=r"^3-16$")
@@ -820,7 +833,7 @@
# Now check reflected revision is excluded with --bidirectional flag.
self.svnmerge("avail -vv --bidirectional", match=r"18$")
- self.svnmerge("merge -vv --bidirectional", match=r"merge -r 17:18")
+ self.svnmerge("merge -vv --bidirectional", match=r"merge --force -r 17:18")
p = self.getproperty()
self.assertEqual("/branches/test-branch:1-18", p)
@@ -882,12 +895,13 @@
# Merge into trunk
self.svnmerge("merge -vv -S branch2",
- match=r"merge -r 18:19")
+ match=r"merge --force -r 15:19")
+
p = self.getproperty()
- self.assertEqual("/branches/test-branch:1-16 /branches/test-branch2:1-19", p)
+ self.assertEqual("/branches/test-branch:1-13 /branches/test-branch2:1-19", p)
self.svnmerge("integrated -S branch2", match=r"^14-19$")
- self.svnmerge("integrated -S ../test-branch", match=r"^13-16$")
+ self.svnmerge("integrated -S ../test-branch", match=r"^13$")
self.launch("svn commit -F svnmerge-commit-message.txt",
match=r"Committed revision 20")
@@ -903,7 +917,7 @@
# should be available for test-branch with --bidirectional flag.
self.svnmerge("avail -vv --bidirectional", match=r"20$")
- self.svnmerge("merge -vv --bidirectional", match=r"merge -r 17:20")
+ self.svnmerge("merge -vv --bidirectional", match=r"merge --force -r 17:20")
p = self.getproperty()
self.assertEqual("/trunk:1-20", p)
Index: contrib/client-side/svnmerge/svnmerge.py
===================================================================
--- contrib/client-side/svnmerge/svnmerge.py (revision 25684)
+++ contrib/client-side/svnmerge/svnmerge.py (working copy)
@@ -757,22 +757,79 @@
assert url[:len(root)] == root, "url=%r, root=%r" % (url, root)
return url[len(root):]
+def get_containing_logentry_xml_text(logText, containedPatternMatchObject):
+ # (get the text of the single logentry xml element surrounding the
+ # pattern that we previously matched in the caller.
+ # (Not sure why the
+ # non-greedy quantifiers in a regexp "logEntryContainingFoundAddActionPattern"
+ # didn't seem to work for this--i.e., putting in '.*?' just before and
+ # after the mention of "actionAddPathPattern" in the section of
+ # get_copyfrom() right after this is called, caused it to get the
+ # whole regex which contains many log entries--not just the single
+ # log entry I want. (todo: use an xml parser instead? no matter for now)
+ start = 0
+ while start != -1:
+ # find the latest occurrence of "<logentry" before the
+ # preceding pattern match (in effect, search from it backwards
+ # to find its containing element).
+ mightBeStart = logText.find("<logentry", start + 1) # " + 1" to prevent looping forever; this string isn't first in the file, so we won't miss the first instance.
+ if mightBeStart == -1 or mightBeStart > containedPatternMatchObject.start():
+ break
+ start = mightBeStart
+
+ end = logText.find("</logentry>", containedPatternMatchObject.end()) + 11
+ if start == 0 or end == -1:
+ raise Exception, "Did not find " + \
+ "element as expected: 'A' log entry for " + \
+ "'" + repos_path + "' should be surrounded by " + \
+ "'logentry' xml element, in log text '" + \
+ + logText + "'"
+ return logText[start:end]
+
def get_copyfrom(dir):
"""Get copyfrom info for a given target (it represents the directory from
where it was branched). NOTE: repos root has no copyfrom info. In this case
- None is returned."""
+ None is returned.
+
+ Returns the:
+ - source file or directory from which the copy was made
+ - the revision from which that source was copied
+ - the revision in which the copy was committed
+ """
repos_path = target_to_repos_relative_path(dir)
- out = launchsvn('log -v --xml --stop-on-copy "%s"' % dir,
+ actionAddPattern = r'(<path\s[^>]*action="A"[^>]*>%s</path>)'
+ logText = launchsvn('log -v --xml --stop-on-copy "%s"' % dir,
split_lines=False)
- out = out.replace("\n", " ")
+ logText = logText.replace("\n", " ")
+
+ # first get the source; abort if unavailable (not a copy)
try:
- m = re.search(r'(<path\s[^>]*action="A"[^>]*>%s</path>)'
- % re.escape(repos_path), out)
- source = re.search(r'copyfrom-path="([^"]*)"', m.group(1)).group(1)
- rev = re.search(r'copyfrom-rev="([^"]*)"', m.group(1)).group(1)
- return source, rev
+ actionAddPatternWithPath = actionAddPattern % re.escape(repos_path)
+ addPathPatternMatch = re.search(actionAddPatternWithPath, logText)
+ source = re.search(r'copyfrom-path="([^"]*)"', \
+ addPathPatternMatch.group(1)).group(1)
except AttributeError:
- return None,None
+ return None,None,None
+
+ # get the revision from which source was copied (if the above re.search
+ # succeeded, yet this throws an exception, we probably want to know).
+ rev = re.search(r'copyfrom-rev="([^"]*)"',
+ addPathPatternMatch.group(1)).group(1)
+
+ # Get the revision in which the copy was committed.
+ logentryElementText = get_containing_logentry_xml_text(logText, addPathPatternMatch)
+ logEntryPatternContainingFoundAddAction = \
+ '\<logentry\s+revision\=\"(\d+)\".*' + \
+ actionAddPatternWithPath + \
+ '.*\</logentry\>'
+ match = re.search(logEntryPatternContainingFoundAddAction, logentryElementText)
+ if match == None:
+ raise Exception, "pattern: " + \
+ logEntryPatternContainingFoundAddAction + " ...not found in " + \
+ logentryElementText
+ copy_committed_in_rev = match.group(1)
+
+ return source, rev, copy_committed_in_rev
def get_latest_rev(url):
"""Get the latest revision of the repository of which URL is part."""
@@ -1013,43 +1070,44 @@
else:
assert False, "unhandled display style: %s" % display_style
-def action_init(branch_dir, branch_props):
+def action_init(target_dir, target_props):
"""Initialize a branch for merges."""
# Check branch directory is ready for being modified
- check_dir_clean(branch_dir)
-
+ check_dir_clean(target_dir)
+
# If the user hasn't specified the revisions to use, see if the
# "source" is a branch from the current tree and if so, we can use
# the version data obtained from it.
- if not opts["revision"]:
- cf_source, cf_rev = get_copyfrom(opts["source-url"])
- branch_path = target_to_repos_relative_path(branch_dir)
+ revisionRange = opts["revision"]
+ if not revisionRange:
+ cf_source_path, cf_rev, copy_committed_in_rev = get_copyfrom(opts["source-url"])
+ target_path = target_to_repos_relative_path(target_dir)
- # If the branch_path is the source path of "source",
+ # If the target_path is the same path from which "source" was
+ # copied when it was created,
# then "source" was branched from the current working tree
# and we can use the revisions determined by get_copyfrom
- if branch_path == cf_source:
+ if target_path == cf_source_path:
report('the source "%s" is a branch of "%s"' %
- (opts["source-url"], branch_dir))
- opts["revision"] = "1-" + cf_rev
+ (opts["source-url"], target_dir))
+ revisionRange = "1-" + copy_committed_in_rev
# Get the initial revision set if not explicitly specified.
- revs = opts["revision"] or "1-" + get_latest_rev(opts["source-url"])
- revs = RevisionSet(revs)
+ revisionRange = revisionRange or "1-" + get_latest_rev(opts["source-url"])
+ revs = str(RevisionSet(revisionRange))
report('marking "%s" as already containing revisions "%s" of "%s"' %
- (branch_dir, revs, opts["source-url"]))
-
- revs = str(revs)
+ (target_dir, revisionRange, opts["source-url"]))
+
# If the source-path already has an entry in the svnmerge-integrated
# property, simply error out.
- if not opts["force"] and branch_props.has_key(opts["source-path"]):
+ if not opts["force"] and target_props.has_key(opts["source-path"]):
error('%s has already been initialized at %s\n'
- 'Use --force to re-initialize' % (opts["source-path"], branch_dir))
- branch_props[opts["source-path"]] = revs
+ 'Use --force to re-initialize' % (opts["source-path"], target_dir))
+ target_props[opts["source-path"]] = revs
# Set property
- set_merge_props(branch_dir, branch_props)
+ set_merge_props(target_dir, target_props)
# Write out commit message if desired
if opts["commit-file"]:
@@ -1898,11 +1956,11 @@
report("calculate source path for the branch")
if not source:
if str(cmd) == "init":
- cf_source, cf_rev = get_copyfrom(branch_dir)
- if not cf_source:
+ cf_source_path, cf_rev, copy_committed_in_rev = get_copyfrom(branch_dir)
+ if not cf_source_path:
error('no copyfrom info available. '
'Explicit source argument (-S/--source) required.')
- opts["source-path"] = cf_source
+ opts["source-path"] = cf_source_path
if not opts["revision"]:
opts["revision"] = "1-" + cf_rev
else:
More information about the Svnmerge
mailing list