Merge lp:~jameinel/bzr-builddeb/changelog-parser into lp:bzr-builddeb
- changelog-parser
- Merge into trunk
Status: | Merged | ||||||||
---|---|---|---|---|---|---|---|---|---|
Merged at revision: | not available | ||||||||
Proposed branch: | lp:~jameinel/bzr-builddeb/changelog-parser | ||||||||
Merge into: | lp:bzr-builddeb | ||||||||
Diff against target: |
609 lines (+271/-269) 2 files modified
merge_changelog.py (+104/-222) tests/test_merge_changelog.py (+167/-47) |
||||||||
To merge this branch: | bzr merge lp:~jameinel/bzr-builddeb/changelog-parser | ||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Robert Collins (community) | Approve | ||
Review via email: mp+18557@code.launchpad.net |
Commit message
Description of the change
John A Meinel (jameinel) wrote : | # |
- 402. By John A Meinel
-
Make sure that the blocks are in sorted order before we do anything else.
- 403. By John A Meinel
-
Fix bug #516060, implement 3-way changelog merge.
This uses a fairly crude 3-way changelog merge algorithm, but doesn't yet
introduce conflicts. That will be next, as it requires an api bump.
If left != right, then we check if the version is in base, and if
one side is identical to base, then we pick the other.
This doesn't:
a) Conflict when left != right != base
b) Allow deleting entries (present in base & this but not other, for instance)
etc. - 404. By John A Meinel
-
Conflict when appropriate.
We could try to do a textual merge, but it is just easier to
conflict on the whole region. Mostly because merge3.Merge3 is missing
a decent api for telling whether there was a conflict region :(.
John A Meinel (jameinel) wrote : | # |
I just updated this branch. I was going to submit a separate proposal, but I did find a bug here, and just fixed it while doing the rest.
Anyway, this now does 3-way merging of the blocks. The basic logic is:
1) Find all blocks in THIS, OTHER and BASE
2) For all blocks in THIS and OTHER, include them in the output in sorted order.
3) If THIS and OTHER both contain a block (matched by Version) with different content, compare against the BASE content
a) If THIS == BASE, chose OTHER
b) if OTHER == BASE, chose THIS
c) If BASE is not present, or BASE is different from both, conflict
What this is missing:
1) If THIS or OTHER deletes a block it will be restored. (I assume that if a version is in THIS or OTHER then you always want it in the output)
2) 3c could use merge3.Merge3() to try to do a textual merge of the conflicted region.
I can open bugs on those two bits if you want.
- 405. By John A Meinel
-
Simplify the conflict logic slightly.
Robert Collins (lifeless) wrote : | # |
On Wed, 2010-02-03 at 22:19 +0000, John A Meinel wrote:
>
> What this is missing:
>
> 1) If THIS or OTHER deletes a block it will be restored. (I assume
> that if a version is in THIS or OTHER then you always want it in the
> output)
> 2) 3c could use merge3.Merge3() to try to do a textual merge of the
> conflicted region.
>
> I can open bugs on those two bits if you want.
--
please do
-Rob
- 406. By John A Meinel
-
Trying to trigger the invalid code, only to find bugs in python-debian :(
Robert Collins (lifeless) wrote : | # |
review: approve
On Wed, 2010-02-03 at 20:19 +0000, John A Meinel wrote:
>
> + elif right_block is None:
> + next_block = left_block
> + left_block = step(left_blocks)
> ...
> + elif left_block.version > right_block.
> + # left comes first
> + next_block = left_block
> + left_block = step(left_blocks)
The None and > cases are the same - I think it would be shorter to group
them.
elif right_block_is None or left_block.version > right_block.
-Rob
John A Meinel (jameinel) wrote : | # |
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Robert Collins wrote:
> Review: Approve
> review: approve
>
> On Wed, 2010-02-03 at 20:19 +0000, John A Meinel wrote:
>> + elif right_block is None:
>> + next_block = left_block
>> + left_block = step(left_blocks)
>> ...
>
>> + elif left_block.version > right_block.
>> + # left comes first
>> + next_block = left_block
>> + left_block = step(left_blocks)
>
>
> The None and > cases are the same - I think it would be shorter to group
> them.
>
> elif right_block_is None or left_block.version > right_block.
>
> -Rob
>
>
I believe I've updated to that style in more recent patches. But I'm
also trying to address some of James's concerns about always being
active. I'm trying to get Strict handling and fallback to regular
processing working (only to find that Strict isn't properly triggering
in some edge cases... :(
John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://
iEYEARECAAYFAkt
50gAn02X+
=271q
-----END PGP SIGNATURE-----
- 407. By John A Meinel
-
Sort out some more details for the 3-way merge code.
The main problem I was running into was that the constructor always suppressed parse failures.
James Westby (james-w) wrote : | # |
I think what I would like to do is use strict=True and then let
merge3 do the merge if we can't parse it. That way we won't drop
text if we can't parse it.
Thanks,
James
James Westby (james-w) wrote : | # |
Oh, it already has code for that, but I don't understand the comment
# BASE lines don't end up in the output, so we allow strict=False
Would it also be good to note to the user when we tried to merge but
it failed due to parsing?
Thanks,
James
John A Meinel (jameinel) wrote : | # |
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
James Westby wrote:
> I think what I would like to do is use strict=True and then let
> merge3 do the merge if we can't parse it. That way we won't drop
> text if we can't parse it.
>
> Thanks,
>
> James
>
That is what I ended up with. I don't know if the current diff shows that.
John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://
iEYEARECAAYFAkt
Bh0Anit1wIn+
=h8Ig
-----END PGP SIGNATURE-----
Preview Diff
1 | === modified file 'merge_changelog.py' | |||
2 | --- merge_changelog.py 2010-01-29 10:51:45 +0000 | |||
3 | +++ merge_changelog.py 2010-02-04 16:34:14 +0000 | |||
4 | @@ -20,243 +20,125 @@ | |||
5 | 20 | 20 | ||
6 | 21 | from bzrlib import ( | 21 | from bzrlib import ( |
7 | 22 | merge, | 22 | merge, |
8 | 23 | merge3, | ||
9 | 23 | ) | 24 | ) |
10 | 24 | 25 | ||
11 | 26 | from debian_bundle import changelog | ||
12 | 27 | |||
13 | 25 | class ChangeLogFileMerge(merge.ConfigurableFileMerger): | 28 | class ChangeLogFileMerge(merge.ConfigurableFileMerger): |
14 | 26 | 29 | ||
15 | 27 | name_prefix = 'deb_changelog' | 30 | name_prefix = 'deb_changelog' |
16 | 28 | default_files = ['debian/changelog'] | 31 | default_files = ['debian/changelog'] |
17 | 29 | 32 | ||
18 | 30 | def merge_text(self, params): | 33 | def merge_text(self, params): |
25 | 31 | return 'success', merge_changelog(params.this_lines, params.other_lines) | 34 | return merge_changelog(params.this_lines, params.other_lines, |
26 | 32 | 35 | params.base_lines) | |
27 | 33 | 36 | ||
22 | 34 | ######################################################################## | ||
23 | 35 | # Changelog Management | ||
24 | 36 | ######################################################################## | ||
28 | 37 | 37 | ||
29 | 38 | # Regular expression for top of debian/changelog | 38 | # Regular expression for top of debian/changelog |
30 | 39 | CL_RE = re.compile(r'^(\w[-+0-9a-z.]*) \(([^\(\) \t]+)\)((\s+[-0-9a-z]+)+)\;', | 39 | CL_RE = re.compile(r'^(\w[-+0-9a-z.]*) \(([^\(\) \t]+)\)((\s+[-0-9a-z]+)+)\;', |
31 | 40 | re.IGNORECASE) | 40 | re.IGNORECASE) |
32 | 41 | 41 | ||
34 | 42 | def merge_changelog(left_changelog_lines, right_changelog_lines): | 42 | def merge_changelog(this_lines, other_lines, base_lines=[]): |
35 | 43 | """Merge a changelog file.""" | 43 | """Merge a changelog file.""" |
36 | 44 | 44 | ||
39 | 45 | left_cl = read_changelog(left_changelog_lines) | 45 | try: |
40 | 46 | right_cl = read_changelog(right_changelog_lines) | 46 | left_cl = read_changelog(this_lines) |
41 | 47 | right_cl = read_changelog(other_lines) | ||
42 | 48 | # BASE lines don't end up in the output, so we allow strict=False | ||
43 | 49 | base_cl = read_changelog(base_lines, strict=False) | ||
44 | 50 | except changelog.ChangelogParseError: | ||
45 | 51 | return ('not_applicable', None) | ||
46 | 47 | 52 | ||
47 | 48 | content = [] | 53 | content = [] |
76 | 49 | # TODO: This is not a 3-way merge, but a 2-way merge | 54 | def step(iterator): |
77 | 50 | # The resolution is currently 'if left and right have texts that have | 55 | try: |
78 | 51 | # the same "version" string, use left', aka "prefer-mine". | 56 | return iterator.next() |
79 | 52 | # We could introduce BASE, and cause conflicts, or appropriately | 57 | except StopIteration: |
80 | 53 | # resolve, etc. | 58 | return None |
81 | 54 | # Note also that this code is only invoked when there is a | 59 | left_blocks = dict((b.version, b) for b in left_cl._blocks) |
82 | 55 | # left-and-right change, so merging a pure-right change will take all | 60 | right_blocks = dict((b.version, b) for b in right_cl._blocks) |
83 | 56 | # changes. | 61 | # Unfortunately, while version objects implement __eq__ they *don't* |
84 | 57 | for right_ver, right_text in right_cl: | 62 | # implement __hash__, which means we can't do dict lookups properly, so |
85 | 58 | while len(left_cl) and left_cl[0][0] > right_ver: | 63 | # instead, we fall back on the version string instead of the object. |
86 | 59 | (left_ver, left_text) = left_cl.pop(0) | 64 | # Make sure never to try to use right_version in left_blocks because of |
87 | 60 | content.append(left_text) | 65 | # this. |
88 | 61 | content.append('\n') | 66 | # We lazily parse the base data, in case we never need it |
89 | 62 | 67 | base_blocks = dict((b.version.full_version, b) for b in base_cl._blocks) | |
90 | 63 | while len(left_cl) and left_cl[0][0] == right_ver: | 68 | left_order = iter(sorted(left_blocks.keys(), reverse=True)) |
91 | 64 | (left_ver, left_text) = left_cl.pop(0) | 69 | right_order = iter(sorted(right_blocks.keys(), reverse=True)) |
92 | 65 | 70 | left_version = step(left_order) | |
93 | 66 | content.append(right_text) | 71 | right_version = step(right_order) |
94 | 67 | content.append('\n') | 72 | |
95 | 68 | 73 | # TODO: Do we want to support the ability to delete a section? We could do | |
96 | 69 | for left_ver, left_text in left_cl: | 74 | # a first-pass algorithm that checks the versions in base versus the |
97 | 70 | content.append(left_text) | 75 | # versions in this and other, to determine what versions should be in |
98 | 71 | content.append('\n') | 76 | # the output. For now, we just assume that if a version is present in |
99 | 72 | 77 | # any of this or other, then we want it in the output. | |
100 | 73 | return content | 78 | conflict_status = 'success' |
101 | 74 | 79 | ||
102 | 75 | 80 | while left_version is not None or right_version is not None: | |
103 | 76 | def read_changelog(lines): | 81 | if (left_version is None or |
104 | 82 | (right_version is not None and right_version > left_version)): | ||
105 | 83 | next_content = str(right_blocks[right_version]) | ||
106 | 84 | right_version = step(right_order) | ||
107 | 85 | elif (right_version is None or | ||
108 | 86 | (left_version is not None and left_version > right_version)): | ||
109 | 87 | next_content = str(left_blocks[left_version]) | ||
110 | 88 | left_version = step(left_order) | ||
111 | 89 | else: | ||
112 | 90 | assert left_version == right_version | ||
113 | 91 | # Same version, step both | ||
114 | 92 | # TODO: Conflict if left_version != right | ||
115 | 93 | # Note: See above comment why we can't use | ||
116 | 94 | # right_blocks[left_version] even though they *should* be | ||
117 | 95 | # equivalent | ||
118 | 96 | left_content = str(left_blocks[left_version]) | ||
119 | 97 | right_content = str(right_blocks[right_version]) | ||
120 | 98 | if left_content == right_content: | ||
121 | 99 | # Identical content | ||
122 | 100 | next_content = left_content | ||
123 | 101 | else: | ||
124 | 102 | # Sides disagree, compare with base | ||
125 | 103 | base_content = str(base_blocks.get(left_version.full_version, | ||
126 | 104 | '')) | ||
127 | 105 | if left_content == base_content: | ||
128 | 106 | next_content = right_content | ||
129 | 107 | elif right_content == base_content: | ||
130 | 108 | next_content = left_content | ||
131 | 109 | else: | ||
132 | 110 | # TODO: We could use merge3.Merge3 to try a line-based | ||
133 | 111 | # textual merge on the content. However, for now I'm | ||
134 | 112 | # just going to conflict on the whole region | ||
135 | 113 | # Conflict names taken from merge.py | ||
136 | 114 | next_content = ('<<<<<<< TREE\n' | ||
137 | 115 | + left_content | ||
138 | 116 | + '=======\n' | ||
139 | 117 | + right_content | ||
140 | 118 | + '>>>>>>> MERGE-SOURCE\n' | ||
141 | 119 | ) | ||
142 | 120 | conflict_status = 'conflicted' | ||
143 | 121 | next_block = left_blocks[left_version] | ||
144 | 122 | left_version = step(left_order) | ||
145 | 123 | right_version = step(right_order) | ||
146 | 124 | content.append(next_content) | ||
147 | 125 | |||
148 | 126 | return conflict_status, content | ||
149 | 127 | |||
150 | 128 | |||
151 | 129 | def read_changelog(lines, strict=True): | ||
152 | 77 | """Return a parsed changelog file.""" | 130 | """Return a parsed changelog file.""" |
338 | 78 | entries = [] | 131 | # Note: There appears to be a bug in Changelog if you pass it an iterable |
339 | 79 | 132 | # of lines (like a file obj, or a list of lines). Specifically, it | |
340 | 80 | (ver, text) = (None, "") | 133 | # does not strip trailing newlines, and it adds ones back in, so you |
341 | 81 | for line in lines: | 134 | # get doubled blank lines... :( |
342 | 82 | match = CL_RE.search(line) | 135 | # So we just ''.join() the lines and don't worry about it |
343 | 83 | if match: | 136 | # Note: There is also a bug that the Changelog constructor suppresses parse |
344 | 84 | try: | 137 | # errors, so we want to always call parse_changelog separately |
345 | 85 | ver = Version(match.group(2)) | 138 | content = ''.join(lines) |
346 | 86 | except ValueError: | 139 | cl = changelog.Changelog() |
347 | 87 | ver = None | 140 | if content: |
348 | 88 | 141 | # We get a warning if we try to parse an empty changelog file, which in | |
349 | 89 | text += line | 142 | # strict mode is an error, so only parse when we have content |
350 | 90 | elif line.startswith(" -- "): | 143 | cl.parse_changelog(content, strict=strict) |
351 | 91 | if ver is None: | 144 | return cl |
167 | 92 | ver = Version("0") | ||
168 | 93 | |||
169 | 94 | text += line | ||
170 | 95 | entries.append((ver, text)) | ||
171 | 96 | (ver, text) = (None, "") | ||
172 | 97 | elif len(line.strip()) or ver is not None: | ||
173 | 98 | text += line | ||
174 | 99 | |||
175 | 100 | if len(text): | ||
176 | 101 | entries.append((ver, text)) | ||
177 | 102 | |||
178 | 103 | return entries | ||
179 | 104 | |||
180 | 105 | ######################################################################## | ||
181 | 106 | # Version parsing code | ||
182 | 107 | ######################################################################## | ||
183 | 108 | # Regular expressions make validating things easy | ||
184 | 109 | valid_epoch = re.compile(r'^[0-9]+$') | ||
185 | 110 | valid_upstream = re.compile(r'^[A-Za-z0-9+:.~-]*$') | ||
186 | 111 | valid_revision = re.compile(r'^[A-Za-z0-9+.~]+$') | ||
187 | 112 | |||
188 | 113 | # Character comparison table for upstream and revision components | ||
189 | 114 | cmp_table = "~ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz+-.:" | ||
190 | 115 | |||
191 | 116 | |||
192 | 117 | class Version(object): | ||
193 | 118 | """Debian version number. | ||
194 | 119 | |||
195 | 120 | This class is designed to be reasonably transparent and allow you | ||
196 | 121 | to write code like: | ||
197 | 122 | |||
198 | 123 | | s.version >= '1.100-1' | ||
199 | 124 | |||
200 | 125 | The comparison will be done according to Debian rules, so '1.2' will | ||
201 | 126 | compare lower. | ||
202 | 127 | |||
203 | 128 | Properties: | ||
204 | 129 | epoch Epoch | ||
205 | 130 | upstream Upstream version | ||
206 | 131 | revision Debian/local revision | ||
207 | 132 | """ | ||
208 | 133 | |||
209 | 134 | def __init__(self, ver): | ||
210 | 135 | """Parse a string or number into the three components.""" | ||
211 | 136 | self.epoch = 0 | ||
212 | 137 | self.upstream = None | ||
213 | 138 | self.revision = None | ||
214 | 139 | |||
215 | 140 | ver = str(ver) | ||
216 | 141 | if not len(ver): | ||
217 | 142 | raise ValueError | ||
218 | 143 | |||
219 | 144 | # Epoch is component before first colon | ||
220 | 145 | idx = ver.find(":") | ||
221 | 146 | if idx != -1: | ||
222 | 147 | self.epoch = ver[:idx] | ||
223 | 148 | if not len(self.epoch): | ||
224 | 149 | raise ValueError | ||
225 | 150 | if not valid_epoch.search(self.epoch): | ||
226 | 151 | raise ValueError | ||
227 | 152 | ver = ver[idx+1:] | ||
228 | 153 | |||
229 | 154 | # Revision is component after last hyphen | ||
230 | 155 | idx = ver.rfind("-") | ||
231 | 156 | if idx != -1: | ||
232 | 157 | self.revision = ver[idx+1:] | ||
233 | 158 | if not len(self.revision): | ||
234 | 159 | raise ValueError | ||
235 | 160 | if not valid_revision.search(self.revision): | ||
236 | 161 | raise ValueError | ||
237 | 162 | ver = ver[:idx] | ||
238 | 163 | |||
239 | 164 | # Remaining component is upstream | ||
240 | 165 | self.upstream = ver | ||
241 | 166 | if not len(self.upstream): | ||
242 | 167 | raise ValueError | ||
243 | 168 | if not valid_upstream.search(self.upstream): | ||
244 | 169 | raise ValueError | ||
245 | 170 | |||
246 | 171 | self.epoch = int(self.epoch) | ||
247 | 172 | |||
248 | 173 | def getWithoutEpoch(self): | ||
249 | 174 | """Return the version without the epoch.""" | ||
250 | 175 | str = self.upstream | ||
251 | 176 | if self.revision is not None: | ||
252 | 177 | str += "-%s" % (self.revision,) | ||
253 | 178 | return str | ||
254 | 179 | |||
255 | 180 | without_epoch = property(getWithoutEpoch) | ||
256 | 181 | |||
257 | 182 | def __str__(self): | ||
258 | 183 | """Return the class as a string for printing.""" | ||
259 | 184 | str = "" | ||
260 | 185 | if self.epoch > 0: | ||
261 | 186 | str += "%d:" % (self.epoch,) | ||
262 | 187 | str += self.upstream | ||
263 | 188 | if self.revision is not None: | ||
264 | 189 | str += "-%s" % (self.revision,) | ||
265 | 190 | return str | ||
266 | 191 | |||
267 | 192 | def __repr__(self): | ||
268 | 193 | """Return a debugging representation of the object.""" | ||
269 | 194 | return "<%s epoch: %d, upstream: %r, revision: %r>" \ | ||
270 | 195 | % (self.__class__.__name__, self.epoch, | ||
271 | 196 | self.upstream, self.revision) | ||
272 | 197 | |||
273 | 198 | def __cmp__(self, other): | ||
274 | 199 | """Compare two Version classes.""" | ||
275 | 200 | other = Version(other) | ||
276 | 201 | |||
277 | 202 | result = cmp(self.epoch, other.epoch) | ||
278 | 203 | if result != 0: return result | ||
279 | 204 | |||
280 | 205 | result = deb_cmp(self.upstream, other.upstream) | ||
281 | 206 | if result != 0: return result | ||
282 | 207 | |||
283 | 208 | result = deb_cmp(self.revision or "", other.revision or "") | ||
284 | 209 | if result != 0: return result | ||
285 | 210 | |||
286 | 211 | return 0 | ||
287 | 212 | |||
288 | 213 | |||
289 | 214 | def strcut(str, idx, accept): | ||
290 | 215 | """Cut characters from str that are entirely in accept.""" | ||
291 | 216 | ret = "" | ||
292 | 217 | while idx < len(str) and str[idx] in accept: | ||
293 | 218 | ret += str[idx] | ||
294 | 219 | idx += 1 | ||
295 | 220 | |||
296 | 221 | return (ret, idx) | ||
297 | 222 | |||
298 | 223 | def deb_order(str, idx): | ||
299 | 224 | """Return the comparison order of two characters.""" | ||
300 | 225 | if idx >= len(str): | ||
301 | 226 | return 0 | ||
302 | 227 | elif str[idx] == "~": | ||
303 | 228 | return -1 | ||
304 | 229 | else: | ||
305 | 230 | return cmp_table.index(str[idx]) | ||
306 | 231 | |||
307 | 232 | def deb_cmp_str(x, y): | ||
308 | 233 | """Compare two strings in a deb version.""" | ||
309 | 234 | idx = 0 | ||
310 | 235 | while (idx < len(x)) or (idx < len(y)): | ||
311 | 236 | result = deb_order(x, idx) - deb_order(y, idx) | ||
312 | 237 | if result < 0: | ||
313 | 238 | return -1 | ||
314 | 239 | elif result > 0: | ||
315 | 240 | return 1 | ||
316 | 241 | |||
317 | 242 | idx += 1 | ||
318 | 243 | |||
319 | 244 | return 0 | ||
320 | 245 | |||
321 | 246 | def deb_cmp(x, y): | ||
322 | 247 | """Implement the string comparison outlined by Debian policy.""" | ||
323 | 248 | x_idx = y_idx = 0 | ||
324 | 249 | while x_idx < len(x) or y_idx < len(y): | ||
325 | 250 | # Compare strings | ||
326 | 251 | (x_str, x_idx) = strcut(x, x_idx, cmp_table) | ||
327 | 252 | (y_str, y_idx) = strcut(y, y_idx, cmp_table) | ||
328 | 253 | result = deb_cmp_str(x_str, y_str) | ||
329 | 254 | if result != 0: return result | ||
330 | 255 | |||
331 | 256 | # Compare numbers | ||
332 | 257 | (x_str, x_idx) = strcut(x, x_idx, "0123456789") | ||
333 | 258 | (y_str, y_idx) = strcut(y, y_idx, "0123456789") | ||
334 | 259 | result = cmp(int(x_str or "0"), int(y_str or "0")) | ||
335 | 260 | if result != 0: return result | ||
336 | 261 | |||
337 | 262 | return 0 | ||
352 | 263 | 145 | ||
353 | === modified file 'tests/test_merge_changelog.py' | |||
354 | --- tests/test_merge_changelog.py 2010-01-29 10:51:45 +0000 | |||
355 | +++ tests/test_merge_changelog.py 2010-02-04 16:34:14 +0000 | |||
356 | @@ -1,5 +1,5 @@ | |||
357 | 1 | # Copyright (C) 2010 Canonical Ltd | 1 | # Copyright (C) 2010 Canonical Ltd |
359 | 2 | # | 2 | # |
360 | 3 | # This file is part of bzr-builddeb. | 3 | # This file is part of bzr-builddeb. |
361 | 4 | # | 4 | # |
362 | 5 | # bzr-builddeb is free software; you can redistribute it and/or modify | 5 | # bzr-builddeb is free software; you can redistribute it and/or modify |
363 | @@ -19,6 +19,10 @@ | |||
364 | 19 | 19 | ||
365 | 20 | """Tests for the merge_changelog code.""" | 20 | """Tests for the merge_changelog code.""" |
366 | 21 | 21 | ||
367 | 22 | import warnings | ||
368 | 23 | |||
369 | 24 | from debian_bundle import changelog | ||
370 | 25 | |||
371 | 22 | from bzrlib import ( | 26 | from bzrlib import ( |
372 | 23 | memorytree, | 27 | memorytree, |
373 | 24 | merge, | 28 | merge, |
374 | @@ -28,66 +32,181 @@ | |||
375 | 28 | from bzrlib.plugins.builddeb import merge_changelog | 32 | from bzrlib.plugins.builddeb import merge_changelog |
376 | 29 | 33 | ||
377 | 30 | 34 | ||
378 | 35 | v_111_2 = """\ | ||
379 | 36 | psuedo-prog (1.1.1-2) unstable; urgency=low | ||
380 | 37 | |||
381 | 38 | * New upstream release. | ||
382 | 39 | * Awesome bug fixes. | ||
383 | 40 | |||
384 | 41 | -- Joe Foo <joe@example.com> Thu, 28 Jan 2010 10:45:44 +0000 | ||
385 | 42 | |||
386 | 43 | """.splitlines(True) | ||
387 | 44 | |||
388 | 45 | |||
389 | 46 | v_111_2b = """\ | ||
390 | 47 | psuedo-prog (1.1.1-2) unstable; urgency=low | ||
391 | 48 | |||
392 | 49 | * New upstream release. | ||
393 | 50 | * Awesome bug fixes. | ||
394 | 51 | * But more is better | ||
395 | 52 | |||
396 | 53 | -- Joe Foo <joe@example.com> Thu, 28 Jan 2010 10:45:44 +0000 | ||
397 | 54 | |||
398 | 55 | """.splitlines(True) | ||
399 | 56 | |||
400 | 57 | |||
401 | 58 | v_111_2c = """\ | ||
402 | 59 | psuedo-prog (1.1.1-2) unstable; urgency=low | ||
403 | 60 | |||
404 | 61 | * New upstream release. | ||
405 | 62 | * Yet another content for 1.1.1-2 | ||
406 | 63 | |||
407 | 64 | -- Joe Foo <joe@example.com> Thu, 28 Jan 2010 10:45:44 +0000 | ||
408 | 65 | |||
409 | 66 | """.splitlines(True) | ||
410 | 67 | |||
411 | 68 | |||
412 | 69 | v_112_1 = """\ | ||
413 | 70 | psuedo-prog (1.1.2-1) unstable; urgency=low | ||
414 | 71 | |||
415 | 72 | * New upstream release. | ||
416 | 73 | * No bug fixes :( | ||
417 | 74 | |||
418 | 75 | -- Barry Foo <barry@example.com> Thu, 27 Jan 2010 10:45:44 +0000 | ||
419 | 76 | |||
420 | 77 | """.splitlines(True) | ||
421 | 78 | |||
422 | 79 | |||
423 | 80 | v_001_1 = """\ | ||
424 | 81 | psuedo-prog (0.0.1-1) unstable; urgency=low | ||
425 | 82 | |||
426 | 83 | * New project released!!!! | ||
427 | 84 | * No bugs evar | ||
428 | 85 | |||
429 | 86 | -- Barry Foo <barry@example.com> Thu, 27 Jan 2010 10:00:44 +0000 | ||
430 | 87 | |||
431 | 88 | """.splitlines(True) | ||
432 | 89 | |||
433 | 90 | |||
434 | 31 | class TestReadChangelog(tests.TestCase): | 91 | class TestReadChangelog(tests.TestCase): |
435 | 32 | 92 | ||
436 | 33 | def test_read_changelog(self): | 93 | def test_read_changelog(self): |
451 | 34 | lines = """\ | 94 | cl = merge_changelog.read_changelog(v_112_1) |
452 | 35 | psuedo-prog (1.1.1-2) unstable; urgency=low | 95 | self.assertEqual(1, len(cl._blocks)) |
453 | 36 | 96 | ||
454 | 37 | * New upstream release. | 97 | |
441 | 38 | * Awesome bug fixes. | ||
442 | 39 | |||
443 | 40 | -- Joe Foo <joe@example.com> Thu, 28 Jan 2010 10:45:44 +0000 | ||
444 | 41 | """.splitlines(True) | ||
445 | 42 | |||
446 | 43 | |||
447 | 44 | entries = merge_changelog.read_changelog(lines) | ||
448 | 45 | self.assertEqual(1, len(entries)) | ||
449 | 46 | |||
450 | 47 | |||
455 | 48 | class TestMergeChangelog(tests.TestCase): | 98 | class TestMergeChangelog(tests.TestCase): |
456 | 49 | 99 | ||
459 | 50 | def assertMergeChangelog(self, expected_lines, this_lines, other_lines): | 100 | def assertMergeChangelog(self, expected_lines, this_lines, other_lines, |
460 | 51 | merged_lines = merge_changelog.merge_changelog(this_lines, other_lines) | 101 | base_lines=[], conflicted=False): |
461 | 102 | status, merged_lines = merge_changelog.merge_changelog( | ||
462 | 103 | this_lines, other_lines, base_lines) | ||
463 | 104 | if conflicted: | ||
464 | 105 | self.assertEqual('conflicted', status) | ||
465 | 106 | else: | ||
466 | 107 | self.assertEqual('success', status) | ||
467 | 52 | self.assertEqualDiff(''.join(expected_lines), ''.join(merged_lines)) | 108 | self.assertEqualDiff(''.join(expected_lines), ''.join(merged_lines)) |
468 | 53 | 109 | ||
469 | 54 | def test_merge_by_version(self): | 110 | def test_merge_by_version(self): |
470 | 55 | v_111_2 = """\ | ||
471 | 56 | psuedo-prog (1.1.1-2) unstable; urgency=low | ||
472 | 57 | |||
473 | 58 | * New upstream release. | ||
474 | 59 | * Awesome bug fixes. | ||
475 | 60 | |||
476 | 61 | -- Joe Foo <joe@example.com> Thu, 28 Jan 2010 10:45:44 +0000 | ||
477 | 62 | |||
478 | 63 | """.splitlines(True) | ||
479 | 64 | |||
480 | 65 | v_112_1 = """\ | ||
481 | 66 | psuedo-prog (1.1.2-1) unstable; urgency=low | ||
482 | 67 | |||
483 | 68 | * New upstream release. | ||
484 | 69 | * No bug fixes :( | ||
485 | 70 | |||
486 | 71 | -- Barry Foo <barry@example.com> Thu, 27 Jan 2010 10:45:44 +0000 | ||
487 | 72 | |||
488 | 73 | """.splitlines(True) | ||
489 | 74 | |||
490 | 75 | v_001_1 = """\ | ||
491 | 76 | psuedo-prog (0.0.1-1) unstable; urgency=low | ||
492 | 77 | |||
493 | 78 | * New project released!!!! | ||
494 | 79 | * No bugs evar | ||
495 | 80 | |||
496 | 81 | -- Barry Foo <barry@example.com> Thu, 27 Jan 2010 10:00:44 +0000 | ||
497 | 82 | |||
498 | 83 | """.splitlines(True) | ||
499 | 84 | |||
500 | 85 | this_lines = v_111_2 + v_001_1 | 111 | this_lines = v_111_2 + v_001_1 |
501 | 86 | other_lines = v_112_1 + v_001_1 | 112 | other_lines = v_112_1 + v_001_1 |
502 | 87 | expected_lines = v_112_1 + v_111_2 + v_001_1 | 113 | expected_lines = v_112_1 + v_111_2 + v_001_1 |
503 | 88 | self.assertMergeChangelog(expected_lines, this_lines, other_lines) | 114 | self.assertMergeChangelog(expected_lines, this_lines, other_lines) |
504 | 89 | self.assertMergeChangelog(expected_lines, other_lines, this_lines) | 115 | self.assertMergeChangelog(expected_lines, other_lines, this_lines) |
505 | 90 | 116 | ||
506 | 117 | def test_this_shorter(self): | ||
507 | 118 | self.assertMergeChangelog(v_112_1 + v_111_2 + v_001_1, | ||
508 | 119 | this_lines=v_111_2, | ||
509 | 120 | other_lines=v_112_1 + v_001_1, | ||
510 | 121 | base_lines=[]) | ||
511 | 122 | self.assertMergeChangelog(v_112_1 + v_111_2 + v_001_1, | ||
512 | 123 | this_lines=v_001_1, | ||
513 | 124 | other_lines=v_112_1 + v_111_2, | ||
514 | 125 | base_lines=[]) | ||
515 | 126 | |||
516 | 127 | def test_other_shorter(self): | ||
517 | 128 | self.assertMergeChangelog(v_112_1 + v_111_2 + v_001_1, | ||
518 | 129 | this_lines=v_112_1 + v_001_1, | ||
519 | 130 | other_lines=v_111_2, | ||
520 | 131 | base_lines=[]) | ||
521 | 132 | self.assertMergeChangelog(v_112_1 + v_111_2 + v_001_1, | ||
522 | 133 | this_lines=v_112_1 + v_111_2, | ||
523 | 134 | other_lines=v_001_1, | ||
524 | 135 | base_lines=[]) | ||
525 | 136 | |||
526 | 137 | def test_unsorted(self): | ||
527 | 138 | # Passing in an improperly sorted text should result in a properly | ||
528 | 139 | # sorted one | ||
529 | 140 | self.assertMergeChangelog(v_111_2 + v_001_1, | ||
530 | 141 | this_lines = v_001_1 + v_111_2, | ||
531 | 142 | other_lines = [], | ||
532 | 143 | base_lines = []) | ||
533 | 144 | |||
534 | 145 | def test_3way_merge(self): | ||
535 | 146 | # Check that if one of THIS or OTHER matches BASE, then we select the | ||
536 | 147 | # other content | ||
537 | 148 | self.assertMergeChangelog(expected_lines=v_111_2, | ||
538 | 149 | this_lines=v_111_2, other_lines=v_111_2b, | ||
539 | 150 | base_lines=v_111_2b) | ||
540 | 151 | self.assertMergeChangelog(expected_lines=v_111_2b, | ||
541 | 152 | this_lines=v_111_2, other_lines=v_111_2b, | ||
542 | 153 | base_lines=v_111_2) | ||
543 | 154 | |||
544 | 155 | def test_3way_conflicted(self): | ||
545 | 156 | self.assertMergeChangelog( | ||
546 | 157 | expected_lines=['<<<<<<< TREE\n'] | ||
547 | 158 | + v_111_2b | ||
548 | 159 | + ['=======\n'] | ||
549 | 160 | + v_111_2c | ||
550 | 161 | + ['>>>>>>> MERGE-SOURCE\n'], | ||
551 | 162 | this_lines=v_111_2b, other_lines=v_111_2c, | ||
552 | 163 | base_lines=v_111_2, | ||
553 | 164 | conflicted=True) | ||
554 | 165 | self.assertMergeChangelog( | ||
555 | 166 | expected_lines=['<<<<<<< TREE\n'] | ||
556 | 167 | + v_111_2b | ||
557 | 168 | + ['=======\n'] | ||
558 | 169 | + v_111_2c | ||
559 | 170 | + ['>>>>>>> MERGE-SOURCE\n'], | ||
560 | 171 | this_lines=v_111_2b, other_lines=v_111_2c, | ||
561 | 172 | base_lines=[], | ||
562 | 173 | conflicted=True) | ||
563 | 174 | |||
564 | 175 | def test_not_valid_changelog(self): | ||
565 | 176 | invalid_changelog = """\ | ||
566 | 177 | psuedo-prog (1.1.1-2) unstable; urgency=low | ||
567 | 178 | |||
568 | 179 | * New upstream release. | ||
569 | 180 | * Awesome bug fixes. | ||
570 | 181 | |||
571 | 182 | -- Thu, 28 Jan 2010 10:45:44 +0000 | ||
572 | 183 | |||
573 | 184 | """.splitlines(True) | ||
574 | 185 | # Missing the author and we don't have allow_missing_author set | ||
575 | 186 | cl = changelog.Changelog() | ||
576 | 187 | self.assertRaises(changelog.ChangelogParseError, | ||
577 | 188 | cl.parse_changelog, ''.join(invalid_changelog), strict=True) | ||
578 | 189 | # If strict parsing fails, don't try to do special merging | ||
579 | 190 | self.assertEqual(('not_applicable', None), | ||
580 | 191 | merge_changelog.merge_changelog(invalid_changelog, v_111_2, | ||
581 | 192 | v_111_2)) | ||
582 | 193 | self.assertEqual(('not_applicable', None), | ||
583 | 194 | merge_changelog.merge_changelog(v_111_2, invalid_changelog, | ||
584 | 195 | v_111_2)) | ||
585 | 196 | # We are non-strict about parsing BASE, because its contents are not | ||
586 | 197 | # included in the output. | ||
587 | 198 | # This triggers a warning, but we don't want to clutter the test run | ||
588 | 199 | cur_filters = warnings.filters[:] | ||
589 | 200 | warnings.simplefilter('ignore', UserWarning) | ||
590 | 201 | try: | ||
591 | 202 | self.assertMergeChangelog(v_112_1 + v_111_2, | ||
592 | 203 | this_lines=v_111_2, | ||
593 | 204 | other_lines=v_112_1, | ||
594 | 205 | base_lines=invalid_changelog, | ||
595 | 206 | ) | ||
596 | 207 | finally: | ||
597 | 208 | warnings.filters = cur_filters[:] | ||
598 | 209 | |||
599 | 91 | 210 | ||
600 | 92 | class TestChangelogHook(tests.TestCaseWithMemoryTransport): | 211 | class TestChangelogHook(tests.TestCaseWithMemoryTransport): |
601 | 93 | 212 | ||
602 | @@ -122,6 +241,7 @@ | |||
603 | 122 | def test_changelog_merge_hook_successful(self): | 241 | def test_changelog_merge_hook_successful(self): |
604 | 123 | params, merger = self.make_params() | 242 | params, merger = self.make_params() |
605 | 124 | params.other_lines = [''] | 243 | params.other_lines = [''] |
606 | 244 | params.base_lines = [''] | ||
607 | 125 | file_merger = builddeb.changelog_merge_hook_factory(merger) | 245 | file_merger = builddeb.changelog_merge_hook_factory(merger) |
608 | 126 | result, new_content = file_merger.merge_text(params) | 246 | result, new_content = file_merger.merge_text(params) |
609 | 127 | self.assertEqual('success', result) | 247 | self.assertEqual('success', result) |
This removes the guts of the Changelog parsing code that I brought in, and replaces it by just importing debian_ bundle. changelog and having that do the parsing.
I don't really like depending on cl._blocks, but since import_dsc is doing so, I assumed that was the best we could do.
The tests all still pass, and the parsing is going to be as accurate as python-debian's parsing, which is what import_dsc uses anyway.