Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 611838 - sys-apps/portage: regression causes app-portage/repoman-2.3.2 to break on brackets in URLs
Summary: sys-apps/portage: regression causes app-portage/repoman-2.3.2 to break on bra...
Status: RESOLVED FIXED
Alias: None
Product: Portage Development
Classification: Unclassified
Component: Repoman (show other bugs)
Hardware: All Linux
: Normal minor (vote)
Assignee: Portage team
URL:
Whiteboard:
Keywords: InVCS, REGRESSION
Depends on:
Blocks: 612780
  Show dependency tree
 
Reported: 2017-03-06 01:44 UTC by Mike Auty (RETIRED)
Modified: 2017-05-20 18:24 UTC (History)
0 users

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Auty (RETIRED) gentoo-dev 2017-03-06 01:44:29 UTC
Hi there,

For fun, I was trying to run "repoman full" on the root of the gentoo tree, but unfortunately it stopped halfway through due to the vdr-burn-templates ebuild which contains the following line:

HOMEPAGE="http://www.vdr-wiki.de/wiki/index.php/Vorlagen_(burn-plugin)"

Based on the error message, it suggests that the DEPEND/RDEPEND requirements on brackets are being applied to the HOMEPAGE variable.  Since brackets are valid identifiers in URLs, this probably isn't wanted.

Reproducible: Always

Steps to Reproduce:
1. cd /usr/portage/media-plugins/vdr-burn-templates
2. repoman full

Actual Results:  
RepoMan scours the neighborhood...
Traceback (most recent call last):
  File "/usr/lib/python-exec/python3.4/repoman", line 44, in <module>
    repoman_main(sys.argv[1:])
  File "/usr/lib64/python3.4/site-packages/repoman/main.py", line 118, in repoman_main
    scanner.scan_pkgs(can_force)
  File "/usr/lib64/python3.4/site-packages/repoman/scanner.py", line 363, in scan_pkgs
    self._scan_ebuilds(ebuildlist, dynamic_data)
  File "/usr/lib64/python3.4/site-packages/repoman/scanner.py", line 397, in _scan_ebuilds
    _continue = func(**self.set_func_kwargs(mod[0], dynamic_data))
  File "/usr/lib64/python3.4/site-packages/repoman/modules/scan/metadata/ebuild_metadata.py", line 71, in homepage_urischeme
    matchall=True,flat=True):
  File "/usr/lib64/python3.4/site-packages/portage/dep/__init__.py", line 680, in use_reduce
    missing_white_space_check(token, pos)
  File "/usr/lib64/python3.4/site-packages/portage/dep/__init__.py", line 504, in missing_white_space_check
    _("missing whitespace around '%s' at '%s', token %s") % (x, token, pos+1))
portage.exception.InvalidDependString: missing whitespace around ')' at 'http://www.vdr-wiki.de/wiki/index.php/Vorlagen_(burn-plugin)', token 1


Expected Results:  
RepoMan scours the neighborhood...
  <appropriate issues>

Note: use --include-dev (-d) to check dependencies for 'dev' profiles

RepoMan sez: "You're only giving me a partial QA payment?
              I'll take it this time, but I'm not happy."
Comment 1 Zac Medico gentoo-dev 2017-03-06 04:52:29 UTC
It looks like a regression triggered by this commit:

https://gitweb.gentoo.org/proj/portage.git/commit/?id=55dedaa865334543e51693838700dcf5e72754d2

We need to add_src_uri=True to the use_reduce function arguments.
Comment 2 Zac Medico gentoo-dev 2017-03-06 05:42:49 UTC
It still fails with is_src_uri=True, so it looks like we need to fix use_reduce to handle this.

Alternatively, I suppose we could percent encode the HOMEPAGE before we pass it to use_reduce. Parenthesis are among the percent-encoding reserved characters:

https://en.wikipedia.org/wiki/Percent-encoding#Percent-encoding_reserved_characters
Comment 3 Mike Auty (RETIRED) gentoo-dev 2017-03-06 09:43:12 UTC
Hmmmm,

So despite it apparently not being used in the tree ever, HOMEPAGE is included in the very last item [1] of the PMS description on Dependencies:

"A use-conditional group, which consists of an optional exclamation mark, followed by a use flag name, followed by a question mark, followed by whitespace, followed by an open parenthesis, followed by whitespace, followed by zero or more of (a dependency item of any kind followed by whitespace), followed by a close parenthesis. More formally: use-conditional ::= ’!’? flag-name ’?’ whitespace ’(’ whitespace (item whitespace)* ’)’. Permitted in all specification style variables."

I suppose the white-space is supposed to allow determining when the bracket is a part of the URL (and white-space in a URL would be percent encoded), which suggests percent encoding is the "right" way to solve this.  Unfortunately, we can't percent encode the entire thing, because the PMS says it should allow for use dependent HOMEPAGES (even though no ebuild in the entire tree uses it, and why would you have a homepage dependent on a USE flag?).

So perhaps repoman should barf on it, and anyone including brackets in their HOMEPAGE should be percent encoding them?  (I believe it's just this one instance in the entire tree).  So the primary repoman change would be the warning message detecting HOMEPAGE variables (and maybe even SRC_URI ones, since presumably they can legitimately contain brackets too) and suggest that either there has been a syntax error or some special characters need percent encoding?

Thanks for looking into it Zac!  5:)

Mike  5:)

[1] https://dev.gentoo.org/~ulm/pms/head/pms.html#x1-760008
Comment 4 Zac Medico gentoo-dev 2017-03-06 19:20:24 UTC
Note that use-dependency atoms can contain parenthesis too, so maybe we can handle it similarly.
Comment 5 Zac Medico gentoo-dev 2017-03-06 19:35:18 UTC
The missing_white_space_check function never matches use-atoms because they never end with right parenthesis:

def missing_white_space_check(token, pos):
	"""
	Used to generate good error messages for invalid tokens.
	"""
	for x in (")", "(", "||"):
		if token.startswith(x) or token.endswith(x):
			raise InvalidDependString(
				_("missing whitespace around '%s' at '%s', token %s") % (x, token, pos+1))

I suppose we could limit the missing_white_space_check usage to cases where token_class is available for token validation, and call it only if token_class raises an exception.
Comment 8 Zac Medico gentoo-dev 2017-05-20 18:24:23 UTC
Fixed in portage-2.3.5.