Opened 16 years ago
Closed 15 years ago
#7706 closed defect (fixed)
ValueError: too many values to unpack
Reported by: | gwk | Owned by: | Remy Blank |
---|---|---|---|
Priority: | high | Milestone: | 0.12-multirepos |
Component: | version control | Version: | 0.12dev |
Severity: | normal | Keywords: | multirepos |
Cc: | gwk.rko@…, androsis@… | Branch: | |
Release Notes: | |||
API Changes: | |||
Internal Changes: |
Description
How to Reproduce
While doing a GET operation on /changeset/1032:e33a4c091c1e hg-v14
, Trac issued an internal error.
I think the problem is some invalid markup in the changeset comment. I tried to hyperlink to a comment within a ticket, and I used the syntax
...according to #507#comment:18 in order to...
While this might be invalid, I would like to get Trac to react
gracefully. I would even like to patch it myself but I can't
quite understand why it complains "too many values to unpack".
If you look at the variable value for lastp below, it does have
exactly 2 values, and the failing code is for k, v in lastp
.
That looks correct to me.
Could someone with more Python powers please teach my why it fails and maybe give a hint how to fix?
Request parameters:
{'new': u'1032:e33a4c091c1e', 'new_path': u'/hg-v14'}
User Agent was: Mozilla/5.0 (X11; U; Linux x86_64; de; rv:1.9.0.3) Gecko/2008092510 Ubuntu/8.04 (hardy) Firefox/3.0.3
System Information
Trac | 0.12multirepos
|
Python | 2.3.4 (#1, Jan 9 2007, 16:40:09) [GCC 3.4.6 20060404 (Red Hat 3.4.6-3)]
|
setuptools | 0.6c8
|
SQLite | 3.3.6
|
pysqlite | 2.3.3
|
Genshi | 0.5
|
Pygments | 0.10
|
Subversion | 1.4.6 (r28521)
|
Mercurial | 1.0.1
|
jQuery: | 1.2.3
|
Python Traceback
Most recent call last: * File "/usr/local/trac-multirepos/lib/python2.3/site-packages/Trac-0.12multirepos-py2.3.egg/trac/versioncontrol/templates/changeset.html", line 1, in <Expression u"wiki_to_html(context('changeset', (reponame, changeset.rev)), changeset.message, escape_newlines=True)"> Code fragment: 1. <!DOCTYPE html 2. PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" 3. "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"> 4. <html xmlns="http://www.w3.org/1999/xhtml" 5. xmlns:py="http://genshi.edgewall.org/" 6. xmlns:xi="http://www.w3.org/2001/XInclude"> Local variables: Name Value __data__ [{'changeset': <tracext.hg.backend.MercurialChangeset object at ... * File "/usr/local/trac-multirepos/lib/python2.3/site-packages/Trac-0.12multirepos-py2.3.egg/trac/util/compat.py", line 133, in newfunc Code fragment: 128. try: 129. from functools import partial 130. except ImportError: 131. def partial(func_, *args, **kwargs): 132. def newfunc(*fargs, **fkwargs): 133. return func_(*(args + fargs), **dict(kwargs, **fkwargs)) 134. newfunc.func = func_ 135. newfunc.args = args 136. newfunc.keywords = kwargs 137. try: 138. newfunc.__name__ = func_.__name__ Local variables: Name Value args (<trac.env.Environment object at 0x2a9722e450>,) fargs (<Context <Resource u"changeset:('hg-v14', '1032:e33a4c091c1e')">>, ... fkwargs {'escape_newlines': True} func_ <function format_to_html at 0x2a9720a578> kwargs {} * File "/usr/local/trac-multirepos/lib/python2.3/site-packages/Trac-0.12multirepos-py2.3.egg/trac/wiki/formatter.py", line 1107, in format_to_html Code fragment: 1102. return format_to_html(env, context, wikidom, **options) 1103. 1104. def format_to_html(env, context, wikidom, escape_newlines=False): 1105. if not wikidom: 1106. return Markup() 1107. return HtmlFormatter(env, context, wikidom).generate(escape_newlines) 1108. 1109. def format_to_oneliner(env, context, wikidom, shorten=False): 1110. if not wikidom: 1111. return Markup() 1112. return InlineHtmlFormatter(env, context, wikidom).generate(shorten) Local variables: Name Value context <Context <Resource u"changeset:('hg-v14', '1032:e33a4c091c1e')">> env <trac.env.Environment object at 0x2a9722e450> escape_newlines True wikidom u'refactor to introduce portal and portal instance model\n\nrefactor the ... * File "/usr/local/trac-multirepos/lib/python2.3/site-packages/Trac-0.12multirepos-py2.3.egg/trac/wiki/formatter.py", line 1066, in generate Code fragment: 1061. newlines in the wikidom will be preserved if `escape_newlines` is set. 1062. """ 1063. # FIXME: compatibility code only for now 1064. out = StringIO() 1065. Formatter(self.env, self.context).format(self.wikidom, out, 1066. escape_newlines) 1067. return Markup(out.getvalue()) 1068. 1069. 1070. class InlineHtmlFormatter(object): 1071. """Format parsed wiki text to inline elements HTML. Local variables: Name Value escape_newlines True out <StringIO.StringIO instance at 0x2a9bf10a70> self <trac.wiki.formatter.HtmlFormatter object at 0x2a9ba03d50> * File "/usr/local/trac-multirepos/lib/python2.3/site-packages/Trac-0.12multirepos-py2.3.egg/trac/wiki/formatter.py", line 862, in format Code fragment: 857. if escape_newlines: 858. line += ' [[BR]]' 859. self.in_list_item = False 860. self.in_quote = False 861. # Throw a bunch of regexps on the problem 862. result = re.sub(self.wikiparser.rules, self.replace, line) 863. 864. if not self.in_list_item: 865. self.close_list() 866. 867. if not self.in_quote: Local variables: Name Value escape_newlines True line u'refactor the code according to #507#comment:18 in order to introduce ... out <StringIO.StringIO instance at 0x2a9bf10a70> result u'refactor to introduce portal and portal instance model <br />' self <trac.wiki.formatter.Formatter object at 0x2a9ba03750> text u'refactor to introduce portal and portal instance model\n\nrefactor the ... * File "/usr/lib64/python2.3/sre.py", line 143, in sub Code fragment: 138. 139. def sub(pattern, repl, string, count=0): 140. """Return the string obtained by replacing the leftmost 141. non-overlapping occurrences of the pattern in string by the 142. replacement repl""" 143. return _compile(pattern, 0).sub(repl, string, count) 144. 145. def subn(pattern, repl, string, count=0): 146. """Return a 2-tuple containing (new_string, number). 147. new_string is the string obtained by replacing the leftmost 148. non-overlapping occurrences of the pattern in the source Local variables: Name Value count 0 pattern <_sre.SRE_Pattern object at 0xcd8e80> repl <bound method Formatter.replace of <trac.wiki.formatter.Formatter object ... string u'refactor the code according to #507#comment:18 in order to introduce ... * File "/usr/local/trac-multirepos/lib/python2.3/site-packages/Trac-0.12multirepos-py2.3.egg/trac/wiki/formatter.py", line 807, in replace Code fragment: 802. internal_handler = getattr(self, '_%s_formatter' % itype) 803. return internal_handler(match, fullmatch) 804. 805. def replace(self, fullmatch): 806. """Replace one match with its corresponding expansion""" 807. replacement = self.handle_match(fullmatch) 808. if replacement: 809. return _markup_to_unicode(replacement) 810. 811. def reset(self, source, out=None): 812. self.source = source Local variables: Name Value fullmatch <_sre.SRE_Match object at 0x12d2800> self <trac.wiki.formatter.Formatter object at 0x2a9ba03750> * File "/usr/local/trac-multirepos/lib/python2.3/site-packages/Trac-0.12multirepos-py2.3.egg/trac/wiki/formatter.py", line 803, in handle_match Code fragment: 798. if itype in self.wikiparser.external_handlers: 799. external_handler = self.wikiparser.external_handlers[itype] 800. return external_handler(self, match, fullmatch) 801. else: 802. internal_handler = getattr(self, '_%s_formatter' % itype) 803. return internal_handler(match, fullmatch) 804. 805. def replace(self, fullmatch): 806. """Replace one match with its corresponding expansion""" 807. replacement = self.handle_match(fullmatch) 808. if replacement: Local variables: Name Value fullmatch <_sre.SRE_Match object at 0x12d2800> internal_handler <bound method Formatter._shref_formatter of <trac.wiki.formatter.Formatter ... itype 'shref' match u'comment:18' self <trac.wiki.formatter.Formatter object at 0x2a9ba03750> * File "/usr/local/trac-multirepos/lib/python2.3/site-packages/Trac-0.12multirepos-py2.3.egg/trac/wiki/formatter.py", line 344, in _shref_formatter Code fragment: 339. return text 340. 341. def _shref_formatter(self, match, fullmatch): 342. ns = fullmatch.group('sns') 343. target = self._unquote(fullmatch.group('stgt')) 344. return self._make_link(ns, target, match, match, fullmatch) 345. 346. def _lhref_formatter(self, match, fullmatch): 347. rel = fullmatch.group('rel') 348. ns = fullmatch.group('lns') 349. target = self._unquote(fullmatch.group('ltgt')) Local variables: Name Value fullmatch <_sre.SRE_Match object at 0x12d2800> match u'comment:18' ns u'comment' self <trac.wiki.formatter.Formatter object at 0x2a9ba03750> target u'18' * File "/usr/local/trac-multirepos/lib/python2.3/site-packages/Trac-0.12multirepos-py2.3.egg/trac/wiki/formatter.py", line 381, in _make_link Code fragment: 376. def _make_link(self, ns, target, match, label, fullmatch): 377. # first check for an alias defined in trac.ini 378. ns = self.env.config['intertrac'].get(ns, ns) 379. if ns in self.wikiparser.link_resolvers: 380. return self.wikiparser.link_resolvers[ns](self, ns, target, 381. escape(label, False)) 382. elif target.startswith('//'): 383. return self._make_ext_link(ns+':'+target, label) 384. elif ns == "mailto": 385. from trac.web.chrome import Chrome 386. otarget = Chrome(self.env).format_emails(self.context, target) Local variables: Name Value fullmatch <_sre.SRE_Match object at 0x12d2800> label u'comment:18' match u'comment:18' ns u'comment' self <trac.wiki.formatter.Formatter object at 0x2a9ba03750> target u'18' * File "/usr/local/trac-multirepos/lib/python2.3/site-packages/Trac-0.12multirepos-py2.3.egg/trac/ticket/api.py", line 361, in _format_comment_link Code fragment: 356. else: 357. resource = formatter.resource 358. cnum = target 359. 360. if resource: 361. href = "%s#comment:%s" % (formatter.href.ticket(resource.id), cnum) 362. title = _("Comment %(cnum)s for Ticket #%(id)s", cnum=cnum, 363. id=resource.id) 364. return tag.a(label, href=href, title=title) 365. else: 366. return label Local variables: Name Value cnum u'18' formatter <trac.wiki.formatter.Formatter object at 0x2a9ba03750> label <Markup u'comment:18'> ns u'comment' resource <Resource u"changeset:('hg-v14', '1032:e33a4c091c1e')"> self <trac.ticket.api.TicketSystem object at 0x2a999f0f10> target u'18' * File "/usr/local/trac-multirepos/lib/python2.3/site-packages/Trac-0.12multirepos-py2.3.egg/trac/web/href.py", line 163, in <lambda> Code fragment: 158. 159. return href 160. 161. def __getattr__(self, name): 162. if not self._derived.has_key(name): 163. self._derived[name] = lambda *args, **kw: self(name, *args, **kw) 164. return self._derived[name] 165. 166. 167. if __name__ == '__main__': 168. import doctest, sys Local variables: Name Value args (('hg-v14', '1032:e33a4c091c1e'),) kw {} name 'ticket' self <trac.web.href.Href object at 0x2a9ba4a850> * File "/usr/local/trac-multirepos/lib/python2.3/site-packages/Trac-0.12multirepos-py2.3.egg/trac/web/href.py", line 142, in __call__ Code fragment: 137. if lastp and type(lastp) is dict: 138. for k,v in lastp.items(): 139. add_param(k, v) 140. args = args[:-1] 141. elif lastp and type(lastp) in (list, tuple): 142. for k,v in lastp: 143. add_param(k, v) 144. args = args[:-1] 145. 146. # build the path 147. path = '/'.join([unicode_quote(unicode(arg).strip('/')) for arg in args Local variables: Name Value add_param <function add_param at 0x129e0c8> args ('ticket', ('hg-v14', '1032:e33a4c091c1e')) href '/trac/SimManager' kw {} lastp ('hg-v14', '1032:e33a4c091c1e') params [] self <trac.web.href.Href object at 0x2a9ba4a850> File "/usr/local/trac-multirepos/lib/python2.3/site-packages/Trac-0.12multirepos-py2.3.egg/trac/versioncontrol/templates/changeset.html", line 1, in <Expression u"wiki_to_html(context('changeset', (reponame, changeset.rev)), changeset.message, escape_newlines=True)"> <!DOCTYPE html File "/usr/local/trac-multirepos/lib/python2.3/site-packages/Trac-0.12multirepos-py2.3.egg/trac/util/compat.py", line 133, in newfunc return func_(*(args + fargs), **dict(kwargs, **fkwargs)) File "/usr/local/trac-multirepos/lib/python2.3/site-packages/Trac-0.12multirepos-py2.3.egg/trac/wiki/formatter.py", line 1107, in format_to_html return HtmlFormatter(env, context, wikidom).generate(escape_newlines) File "/usr/local/trac-multirepos/lib/python2.3/site-packages/Trac-0.12multirepos-py2.3.egg/trac/wiki/formatter.py", line 1066, in generate escape_newlines) File "/usr/local/trac-multirepos/lib/python2.3/site-packages/Trac-0.12multirepos-py2.3.egg/trac/wiki/formatter.py", line 862, in format result = re.sub(self.wikiparser.rules, self.replace, line) File "/usr/lib64/python2.3/sre.py", line 143, in sub return _compile(pattern, 0).sub(repl, string, count) File "/usr/local/trac-multirepos/lib/python2.3/site-packages/Trac-0.12multirepos-py2.3.egg/trac/wiki/formatter.py", line 807, in replace replacement = self.handle_match(fullmatch) File "/usr/local/trac-multirepos/lib/python2.3/site-packages/Trac-0.12multirepos-py2.3.egg/trac/wiki/formatter.py", line 803, in handle_match return internal_handler(match, fullmatch) File "/usr/local/trac-multirepos/lib/python2.3/site-packages/Trac-0.12multirepos-py2.3.egg/trac/wiki/formatter.py", line 344, in _shref_formatter return self._make_link(ns, target, match, match, fullmatch) File "/usr/local/trac-multirepos/lib/python2.3/site-packages/Trac-0.12multirepos-py2.3.egg/trac/wiki/formatter.py", line 381, in _make_link escape(label, False)) File "/usr/local/trac-multirepos/lib/python2.3/site-packages/Trac-0.12multirepos-py2.3.egg/trac/ticket/api.py", line 361, in _format_comment_link href = "%s#comment:%s" % (formatter.href.ticket(resource.id), cnum) File "/usr/local/trac-multirepos/lib/python2.3/site-packages/Trac-0.12multirepos-py2.3.egg/trac/web/href.py", line 163, in <lambda> self._derived[name] = lambda *args, **kw: self(name, *args, **kw) File "/usr/local/trac-multirepos/lib/python2.3/site-packages/Trac-0.12multirepos-py2.3.egg/trac/web/href.py", line 142, in __call__ for k,v in lastp:
Attachments (6)
Change History (30)
comment:1 by , 16 years ago
Cc: | added |
---|
comment:2 by , 16 years ago
Component: | general → version control |
---|---|
Keywords: | multirepos added |
Milestone: | → not applicable |
Priority: | normal → high |
comment:4 by , 16 years ago
Cc: | added |
---|
comment:5 by , 16 years ago
The same error with Subversion backend and multirepos branch (0.12multirepos-r8028). In my case the offending comment in SVN is 'check #: 1234 done'. If I set wiki_format_messages = false in trac.ini then everything is fine. In regular wiki page this string does not cause exception. It looks like lastp variable is expected to have sequence of pairs, but gets plain list (in my case lastp=(,2)). The same repository is shown fine in the regular Trac 0.12dev-r8046.
follow-up: 8 comment:6 by , 15 years ago
Milestone: | not applicable → 0.12 |
---|
So this problem is about composite ids appearing in unsuspecting places.
I think turning the changeset and source resource into child resources of their repository resource would make this problem go away.
comment:7 by , 15 years ago
Milestone: | 0.12 → 0.12-multirepos |
---|
comment:8 by , 15 years ago
Owner: | set to |
---|
Replying to cboos:
I think turning the changeset and source resource into child resources of their repository resource would make this problem go away.
I'd like to try that.
follow-up: 10 comment:9 by , 15 years ago
Great, just post patches as you go, so I can comment early!
by , 15 years ago
Attachment: | 7706-repo-resource-r8633.patch added |
---|
Make source and changeset resources child resources of a repository resource.
follow-up: 11 comment:10 by , 15 years ago
Replying to cboos:
Great, just post patches as you go, so I can comment early!
Sure, here's one:
- Repositories become resources (with realm
'repository'
). 'source'
and'changeset'
resources become child resources of'repository'
.'source'
and'changeset'
resources in the default repository do not have a parent resource. This should allow for better backward compatibility with pre-multirepos plugins.
We could drop the last item to simplify the code a bit more, at the expense of more breakage in plugins (even when using a single default repository).
I have to say that I rather like the idea of a parent 'repository'
resource for VC resources, compared to the composite (reponame, path)
resource ID.
follow-up: 12 comment:11 by , 15 years ago
Replying to rblank:
…
'source'
and'changeset'
resources in the default repository do not have a parent resource. This should allow for better backward compatibility with pre-multirepos plugins.We could drop the last item to simplify the code a bit more, at the expense of more breakage in plugins (even when using a single default repository).
The main reason multirepos is not yet on trunk is that I wanted to be sure we could still make such breakage decisions if we wanted to. I think it makes sense to have changeset and source resources be always children resources of a parent repository resource. The current "0.12-multirepos" plugins will be broken anyway by moving away from composite ids, so let's do it the way we find the best.
follow-up: 13 comment:12 by , 15 years ago
Replying to cboos:
I think it makes sense to have changeset and source resources be always children resources of a parent repository resource.
Ok, will do. And I assume when we get to the multi-project functionality (in a later release), everything will be a child resource of a project resource? Sounds nice and clean.
follow-up: 14 comment:13 by , 15 years ago
Replying to rblank:
I assume when we get to the multi-project functionality (in a later release), everything will be a child resource of a project resource?
Except maybe for the "shared" resources. For the specific case of repositories (and their sub-resources), I think it makes more sense to keep them as independent entities, each project could have 0-n associated repositories. But deleting a project won't make the repository go away, as it might still be used in another project, or reused later in a new project. The behavior on deletion is my "rule of thumb" whether one thing should be a sub-resource of another (I know there's a fancy word for that property but I fail to recall it this morning… time to reach the coffee machine ;-) ).
comment:14 by , 15 years ago
Replying to cboos:
The behavior on deletion is my "rule of thumb" whether one thing should be a sub-resource of another (I know there's a fancy word for that property but I fail to recall it this morning… time to reach the coffee machine ;-) ).
Makes sense. Sounds like the "cascading" deletes in PostgreSQL.
by , 15 years ago
Attachment: | 7706-repo-resource-r8633.2.patch added |
---|
Make all source and changeset resources children of repository resources.
comment:15 by , 15 years ago
The second patch above makes VC resources always children of a repository resource. It also fixes many more occurrences of composite IDs that I had missed, in templates, sub-contexts and permission checks.
Testing welcome.
comment:16 by , 15 years ago
I have fixed the links for the revision log (which were completely broken on MultiRepos) in [8642] and slightly changed the links for changesets in [8643], so that both follow the same algorithm:
- If a VC resource is in the context, use it to find the
reponame
, and interpret the scope relative to that repository. - Otherwise, interpret the scope as absolute.
by , 15 years ago
Attachment: | 7706-repo-resource-r8643.patch added |
---|
Updated patch for current MultiRepos head.
follow-up: 18 comment:17 by , 15 years ago
Version: | → 0.12dev |
---|
I'm not sure split_vc_resource is that useful, e.g.
(self.reponame, self.path) = split_vc_resource(context.resource)
vs.
self.reponame = context.resource.parent.id self.path = context.resource.id
Otherwise I've tested the patch and it seems to work fine, except for the repository index. Patch follows.
by , 15 years ago
Attachment: | repo_index_repo_resources.diff added |
---|
fix for repository index, on top of 7706-repo-resource-r8643.patch
follow-up: 19 comment:18 by , 15 years ago
Replying to cboos:
I'm not sure split_vc_resource is that useful
It was useful for testing, as I had quite a few assert
s in there to check for consistency. Now, it's true that it doesn't seem to do much (I still find it slightly more readable than the inlined version, but that's cosmetic). I'll remove it.
Otherwise I've tested the patch and it seems to work fine, except for the repository index.
Yes, missed that one. Heh, I had worked hard to avoid introducing Resource
into the template data, and now you're putting it in anyway :-) Should I also replace all instances of repos_resource
with Resource('repository', reponame)
?
comment:19 by , 15 years ago
Replying to rblank:
Otherwise I've tested the patch and it seems to work fine, except for the repository index.
Yes, missed that one. Heh, I had worked hard to avoid introducing
Resource
into the template data, and now you're putting it in anyway :-)
Ok, I was not completely sold on that either. The alternative would have been to replace reponame
by repos_resource
in each tuple of the repositories
list. I'm also OK with that. See reworked patch, in which I also introduced Repository.resource
(we do something similar in other model classes).
comment:20 by , 15 years ago
That second patch doesn't work out well in the case of aliases, as repos.resource.id
will be the aliased repository name, whereas reponame
will be the name of the alias.
So I'm not sure the Repository.resource
property is a good idea, as it increases the likelihood that the wrong resource id is used.
by , 15 years ago
Attachment: | 7706-repo-resource-r8643.2.patch added |
---|
Combinded patch including attachment:repo_index_repo_resources.diff
comment:21 by , 15 years ago
This is the final combined patch, using your first suggestion for fixing the repository index. AFAICT, everything is working correctly.
Ok to apply?
comment:23 by , 15 years ago
Ah, yes, sorry. I had forgotten about that. It will be gone even before it reaches your working copy :-)
comment:24 by , 15 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Patch applied in [8652], and this indeed fixes the initially reported issue.
Interesting bug indeed.
The comment:18 is parsed in isolation, as
#123#comment:18
is not valid butcomment:18
is, so the formatter assumes the comment is attached to the current resource, here the changeset. Besides the fact that generic comments are not yet implemented (#2035), the problem here is that we use a "composite id" for the changeset resources, in order to encapsulate the extra repository name information.Composite ids can't be used as such, in a couple of places like this one (e.g. I've seen it when reporting missing permissions).
For the exception itself, nothing mysterious:
will give you the same exception when iterating over the second element, as it has more values (here 1-character strings) than the two expected…