Conversation
|
Debian also has |
|
Debian Build-Depends should be fixed now by 20a8be8#diff-e1680efd70b630a38d8132340a9f77468b13b34f3b4162ceb1a8749bc1267665R8 |
|
@M0ses Hi, do you have time to rebase this to fix the conflicts? |
but has 'None' value
As described in openSUSE#183 '~=' is not a valid version specifier for rpm and deb packages. This patch converts the specifier to valid version specifiers for rpm/deb. Examples: `abc ~= 1.1.1` * in rpm: ``` BuildRequires: %{python_module abc >= 1.1.1} # Only for suse Requires: python-abc >= 1.1.1, python-abc < 1.2 ``` * in deb: ``` Depends: python-abc (>= 1.1.1), python-abc (<< 1.2) ``` FIXES: openSUSE#183 An schould also fix FIXES: openSUSE#93
danigm
left a comment
danigm
left a comment
There was a problem hiding this comment.
Looks mostly good to me, I've left some comments to try to simplify the code, but the new functionality looks good.
The change of the requires list to a list of list with a [PACKAGE, OP, VERSION] is asking for a change to create a new class to define a requirement and move all the operations to that class, but that's something that can be done after merging, just OOP stuff.
| if "entry_points" not in data or not data['entry_points']: | ||
| return [] | ||
| if isinstance(data["entry_points"], str): | ||
| eps = EntryPoints(EntryPoints._from_text(data["entry_points"])) | ||
| elif isinstance(data["entry_points"], dict): |
There was a problem hiding this comment.
This change break the following code, because entry_points is not defined and it's used below.
And I think this change is not needed, it's doing the same. If data['entry_points'] is None the if isinstance(data["entry_points"], str): and elif isinstance(data["entry_points"], dict): will be false so it'll go to the else clause and will return []
| req += [req[0], '<', ".".join(v)] | ||
| out_list.append(req) | ||
|
|
||
| return out_list |
There was a problem hiding this comment.
If I'm understanding this code correctly, the return now is a list of lists, so we should update the documentation of this method to match the new way of producing the output.
| v = req[2].split('.') | ||
| v.pop() |
There was a problem hiding this comment.
The version in a ~= requirement could be something without dots (abc ~= 3)? In that case, this can remove the version completely and will crash in the following line. But I'm not sure if that case is something that makes sense.
| data['build_requires'] += ['wheel'] | ||
|
|
||
| data["build_requires"] = [['setuptools']] | ||
| if _search_requires(data['build_requires'], 'setuptools') and not _search_requires(data['build_requires'], 'wheel'): |
There was a problem hiding this comment.
The search_requires function is not needed, this can be done with a list comprehension in a more pythonic way:
# Just the list of requires packages, without version information
require_names = [r[0] for r in data['build_requires']]
if 'setuptools' in require_names and 'wheel' not in require_names:
py2pack/__init__.py
Outdated
| # setup jinja2 environment with custom filters | ||
| env = jinja2.Environment(loader=jinja2.FileSystemLoader(template_dir)) | ||
|
|
||
| def _rpm_format_requires(req): |
There was a problem hiding this comment.
nitpick: This function does complex require formatting, it could be great to have some comment with examples of what it does, input -> output so it will be easier to understand in the future.
| part2 = ", " + " ".join(req[3:]) | ||
| return part1 + part2 | ||
|
|
||
| def _parenthesize_version(req): |
There was a problem hiding this comment.
nitpick: This function does complex require formatting, it could be great to have some comment with examples of what it does, input -> output so it will be easier to understand in the future.
3 participants
As described in #183 '~=' is not a valid version specifier for rpm and
deb packages.
This patch converts the specifier to valid version specifiers for
rpm/deb.
Examples:
abc ~= 1.1.1FIXES: #183
An schould also fix
FIXES: #93