Opened 14 years ago
Closed 14 years ago
#9230 closed defect (fixed)
OverflowError when viewing ticket
Reported by: | gbarberi | Owned by: | Remy Blank |
---|---|---|---|
Priority: | normal | Milestone: | 0.12 |
Component: | ticket system | Version: | 0.12dev |
Severity: | major | Keywords: | |
Cc: | Christian Boos | Branch: | |
Release Notes: | |||
API Changes: | |||
Internal Changes: |
Description
How to Reproduce
I got this error while viewing a ticket right after doing a push on Mercurial and having an automated script issue a "trac-admin changeset added…" wit the CommitTicketUpdater.
This was the "target" variable in the Local Variable table: u'/gameclient@92:569251789268' which looks suspicious to me.
Please let me know if I can provide any more information. I can not view the ticket page at all due to this error but I'm not 100% sure how this can be reproduced at will.
System Information
Trac | 0.12dev-r9476
|
Genshi | 0.6dev-r1096
|
Mercurial | 1.5
|
mod_wsgi | 2.7 (WSGIProcessGroup WSGIApplicationGroup %{GLOBAL})
|
pysqlite | 2.4.1
|
Python | 2.6.5 (r265:79096, Mar 19 2010, 21:48:26) [MSC v.1500 32 bit (Intel)]
|
setuptools | 0.6c11
|
SQLite | 3.5.9
|
Subversion | 1.6.6 (r40053)
|
jQuery | 1.4.2
|
Enabled Plugins
tracaccountmanager | 0.2.1dev-r7737
|
tracmercurial | 0.12.0.21dev-r9248
|
Python Traceback
Traceback (most recent call last): File "build\bdist.win32\egg\trac\web\main.py", line 512, in _dispatch_request dispatcher.dispatch(req) File "build\bdist.win32\egg\trac\web\main.py", line 255, in dispatch content_type) File "build\bdist.win32\egg\trac\web\chrome.py", line 864, in render_template stream.render(method, doctype=doctype, out=buffer) File "build\bdist.win32\egg\genshi\core.py", line 183, in render return encode(generator, method=method, encoding=encoding, out=out) File "build\bdist.win32\egg\genshi\output.py", line 58, in encode for chunk in iterator: File "build\bdist.win32\egg\genshi\output.py", line 339, in __call__ for kind, data, pos in stream: File "build\bdist.win32\egg\genshi\output.py", line 826, in __call__ for kind, data, pos in stream: File "build\bdist.win32\egg\genshi\output.py", line 670, in __call__ for kind, data, pos in stream: File "build\bdist.win32\egg\genshi\output.py", line 771, in __call__ for kind, data, pos in chain(stream, [(None, None, None)]): File "build\bdist.win32\egg\genshi\output.py", line 586, in __call__ for ev in stream: File "build\bdist.win32\egg\genshi\core.py", line 288, in _ensure for event in stream: File "build\bdist.win32\egg\genshi\core.py", line 288, in _ensure for event in stream: File "build\bdist.win32\egg\trac\web\chrome.py", line 968, in _strip_accesskeys for kind, data, pos in stream: File "build\bdist.win32\egg\genshi\core.py", line 288, in _ensure for event in stream: File "build\bdist.win32\egg\trac\web\chrome.py", line 957, in _generate for kind, data, pos in stream: File "build\bdist.win32\egg\genshi\template\base.py", line 592, in _include for event in stream: File "build\bdist.win32\egg\genshi\template\markup.py", line 381, in _match ctxt, start=idx + 1, **vars): File "build\bdist.win32\egg\genshi\template\markup.py", line 381, in _match ctxt, start=idx + 1, **vars): File "build\bdist.win32\egg\genshi\template\markup.py", line 330, in _match for event in stream: File "build\bdist.win32\egg\genshi\template\base.py", line 532, in _flatten for kind, data, pos in stream: File "build\bdist.win32\egg\genshi\core.py", line 288, in _ensure for event in stream: File "build\bdist.win32\egg\genshi\path.py", line 588, in _generate subevent = next() File "build\bdist.win32\egg\genshi\template\base.py", line 592, in _include for event in stream: File "build\bdist.win32\egg\genshi\template\markup.py", line 319, in _strip event = next() File "build\bdist.win32\egg\genshi\template\base.py", line 532, in _flatten for kind, data, pos in stream: File "build\bdist.win32\egg\genshi\core.py", line 288, in _ensure for event in stream: File "build\bdist.win32\egg\genshi\path.py", line 588, in _generate subevent = next() File "build\bdist.win32\egg\genshi\template\base.py", line 592, in _include for event in stream: File "build\bdist.win32\egg\genshi\template\markup.py", line 319, in _strip event = next() File "build\bdist.win32\egg\genshi\template\base.py", line 552, in _flatten result = _eval_expr(data, ctxt, vars) File "build\bdist.win32\egg\genshi\template\base.py", line 277, in _eval_expr retval = expr.evaluate(ctxt) File "build\bdist.win32\egg\genshi\template\eval.py", line 178, in evaluate return eval(self.code, _globals, {'__data__': data}) File "c:/sites/projects/share/trac/egg-cache\trac-0.12dev_r9476-py2.6.egg-tmp\trac\ticket\templates\ticket_change.html", line 61, in <Expression u'wiki_to_html(context, change.comment, escape_newlines=preserve_newlines)'> <py:otherwise>${wiki_to_html(context, change.comment, escape_newlines=preserve_newlines)}</py:otherwise> File "build\bdist.win32\egg\trac\wiki\formatter.py", line 1490, in format_to_html return HtmlFormatter(env, context, wikidom).generate(escape_newlines) File "build\bdist.win32\egg\trac\wiki\formatter.py", line 1445, in generate escape_newlines) File "build\bdist.win32\egg\trac\wiki\formatter.py", line 1226, in format result = re.sub(self.wikiparser.rules, self.replace, line) File "C:\Python26\Lib\re.py", line 151, in sub return _compile(pattern, 0).sub(repl, string, count) File "build\bdist.win32\egg\trac\wiki\formatter.py", line 1149, in replace replacement = self.handle_match(fullmatch) File "build\bdist.win32\egg\trac\wiki\formatter.py", line 1142, in handle_match return external_handler(self, match, fullmatch) File "build\bdist.win32\egg\trac\versioncontrol\web_ui\log.py", line 330, in <lambda> lambda x, y, z: self._format_link(x, 'log1', y[1:-1], y, z)) File "build\bdist.win32\egg\trac\versioncontrol\web_ui\log.py", line 377, in _format_link if revranges: OverflowError: long int too large to convert to int
Attachments (2)
Change History (20)
comment:1 by , 14 years ago
comment:2 by , 14 years ago
Owner: | set to |
---|
That's highly suspicious indeed. I'll try to reproduce the issue. Could you please provide the exact revision hash for which this issue happened (taken directly from Mercurial)? Thanks.
comment:3 by , 14 years ago
hg log -r 92
changeset: 92:569251789268
branch: xxxxxxxxx
user: xxxxxxxxx
date: Wed Apr 14 21:10:39 2010 -0400
summary: Test 2. Refs #109.
The following entry was in the ticket_change table that caused the error:
In [92:569251789268/gameclient]: {{{ #!CommitTicketReference repository="gameclient" revision="92:569251789268" Test 2. Refs #109. }}}
I was able to change the first line to (notice I replaced the last digit with an "a"):
In [92:56925178926a/gameclient]:
and the problem went away.
comment:4 by , 14 years ago
Funny, it seems that all-numerical hashes make trouble.
Also, I'm not sure if it's a good idea that the ticket updater component includes the revision number. It should probably rather use [569251789268/gameclient]
as a revision link.
comment:5 by , 14 years ago
Heh, actually that's exactly the problem: [92:569251789268]
is interpreted as a revision range, not a single revision. This must be fixed in the commit updater.
comment:6 by , 14 years ago
Cc: | added |
---|
This is tricky: the value contained in changeset.rev
in the case of TracMercurial is dependent on the node_format
and show_rev
settings. Using normalize_rev()
on that doesn't change anything, as it also depends on those settings.
So there are two issues:
- I don't know how I'm supposed to get the full revision hash for a changeset, to be used in the
revision=
argument of theCommitTicketReference
macro. - The "composite"
{num}:{hash}
revisions as generated by TracMercurial are ambiguous when put between[]
, and can be confused with revision ranges in the case of all-numerical hashes. For reference, the th:GitPlugin doesn't use the revision numbers and only ever displays hashes.
Thoughts?
comment:7 by , 14 years ago
Maybe we could use shortrev here, or introduce yet another method (canonicalrev
?), with a fallback implementation that would work for Subversion and Git? (Git has no concept of repository local revision numbers, btw).
follow-up: 9 comment:8 by , 14 years ago
Not sure I understand:
normalize_rev()
would return a "readable" revision number (for human consumption), including the local revision number for Mercurial (if configured).canonicalrev()
would return the full hash or revision number.
Or did you mean to say the reverse?
What I'm worried about is that normalize_rev()
is used throughout Trac for both making "readable" revisions and for making internal revisions for e.g. storing in the cache. So how about the following:
normalize_rev()
always returns the full hash.readable_rev()
returns a (configurable) readable revision for display.
The default for readable_rev()
would be to simply call normalize_rev()
.
follow-up: 10 comment:9 by , 14 years ago
Replying to rblank:
Not sure I understand:
normalize_rev()
would return a "readable" revision number (for human consumption), including the local revision number for Mercurial (if configured).canonicalrev()
would return the full hash or revision number.
Yes, that was the idea, as at first I didn't want to change the way normalize_rev
was currently used.
What I'm worried about is that
normalize_rev()
is used throughout Trac for both making "readable" revisions and for making internal revisions for e.g. storing in the cache.
If you see a way out, for cleaning-up the current API without breaking things to much, I'm all ears :-)
So how about the following:
normalize_rev()
always returns the full hash.readable_rev()
returns a (configurable) readable revision for display.The default for
readable_rev()
would be to simply callnormalize_rev()
.
This could work, yes. Too bad for the naming convention that I picked "shortrev" instead of "short_rev" (or we could fix that as well, for the sake of consistency?)
follow-up: 11 comment:10 by , 14 years ago
Replying to cboos:
This could work, yes. Too bad for the naming convention that I picked "shortrev" instead of "short_rev" (or we could fix that as well, for the sake of consistency?)
It's already short_rev()
in api.py
:-)
But wait, if we already have short_rev()
, we don't need another readable_rev()
, do we? We can make normalize_rev()
always return the complete hash (which shouldn't break anything, as the full hash can always be used to get a changeset), and short_rev()
return a configurable short revision for display.
So in the end, maybe this should be fixed in TracMercurial (with maybe adding a few short_rev()
calls in core for display)?
comment:11 by , 14 years ago
Replying to rblank:
Replying to cboos:
This could work, yes. Too bad for the naming convention that I picked "shortrev" instead of "short_rev" (or we could fix that as well, for the sake of consistency?)
It's already
short_rev()
inapi.py
:-)
Ah, "bien Christian !" ;-)
But wait, if we already have
short_rev()
, we don't need anotherreadable_rev()
, do we?
I think we still need it, short_rev
is used in place we're really tight of space, typically diff column headings. So in Mercurial we always use the local revision number, in Git they might use a very short hash prefix, etc.
We can make
normalize_rev()
always return the complete hash (which shouldn't break anything, as the full hash can always be used to get a changeset),
Other suggestions: long_rev for symmetry with short_rev, display_rev for consistency with hg_display
in TracMercurial… but readable_rev
is also OK for me.
by , 14 years ago
Attachment: | 9230-display-rev-r9490.patch added |
---|
Introduce and use Repository.display_rev()
.
by , 14 years ago
Attachment: | 9230-display-rev-hg-r9248.patch added |
---|
Move configurable rev display to display_rev()
.
comment:12 by , 14 years ago
9230-display-rev-r9490.patch introduces Repository.display_rev()
and uses it every time a revision must be displayed. Internally, and in URLs as well, full revision hashes are used consistently. It also replaces a few occurrences of r%(rev)s
with [%(rev)s]
, as the former looks quite ugly when rev
is a hash.
9230-display-rev-hg-r9248.patch modifies mercurial-plugin
so that normalize_rev()
always returns a full hash, and full hashes are used consistently internally. It also implements display_rev()
to shorten the hashes according to the configured preferences.
Thoughts?
comment:15 by , 14 years ago
follow-up: 17 comment:16 by , 14 years ago
I wonder if some of the changes could then be applied on all the translations, like s/new_rev/new_drev/
?
Anyway, it looks like we're going to have to shift 0.12b1 a bit more, see follow-up on TracDev/ToDo.
comment:17 by , 14 years ago
Replying to cboos:
I wonder if some of the changes could then be applied on all the translations, like
s/new_rev/new_drev/
?
Actually, those didn't change, I have kept new_rev
. There were two missing markers, and some changes in how to display the revision (r%(rev)s -> [%(rev)s]
).
Anyway, it looks like we're going to have to shift 0.12b1 a bit more, see follow-up on TracDev/ToDo.
Ok, so I committed the new extraction and the update to the French translation in [9562].
comment:18 by , 14 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Change documented in TracDev/ApiChanges/0.12@31.
Something else I noticed. It appears the revision came out to be "569251789268" while usually a revision will have letters such as "6de7f54ca23f."
What I'm guessing is that the formatter is looking at the value and thinking it's an integer while in reality it should be a string.