Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: Implement provides/requires for extras packages + Rework error messages (#1546)

2021-04-21 Thread torsava
Closed #1546.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1546#event-4626456817___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: Implement provides/requires for extras packages + Rework error messages (#1546)

2021-04-21 Thread torsava
I'm closing this PR as it has been reopened against the newly separate 
python-rpm-packaging project: 
https://github.com/rpm-software-management/python-rpm-packaging/pull/7

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1546#issuecomment-824244556___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Clarify %check script use-case by executing it before %install (#1618)

2021-04-07 Thread torsava
This will seriously complicate my life as a maintainer. There are many good use 
cases for doing %check on the content of %install.

Your suggestion is to do a fake install in %build and check that, but that 1. 
won't test the actually installed stuff, 2. make me copy a possibly lots of 
code (not all %install sections are just %make_install!!!) and keep it with 
only slight changes in two places. That will be a spec file from hell.

Please don't do this.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1618#issuecomment-81450___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: Print Provides-Extra entries for Python packa… (#1014)

2021-03-26 Thread torsava
> We have it ready in Fedora: 
> https://fedoraproject.org/wiki/Changes/PythonExtras
> 
> We want to backport it here but we have been swamped with many other things.
> 
> cc @torsava

I've opened a PR to upstream our Fedora full-blown handling of Extras: 
https://github.com/rpm-software-management/rpm/pull/1546

@gordonmessmer Please take a look, I think it implements what this PR does and 
more.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1014#issuecomment-808206129___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Remove remaining Python helpers and scripts from the repo (#1607)

2021-03-26 Thread torsava
Indeed both choices are not perfect. However, the Python parts are getting 
rather complex. We even have a full blown test suite for them in Fedora that we 
would be able to finally upstream if there's a separate repo. So overall, :+1: 
from me.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1607#issuecomment-808204484___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


[Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: Implement provides/requires for extras packages + Rework error messages (#1546)

2021-02-18 Thread torsava
In Fedora we have adapted the pythondistdeps.py script (that generates 
python3.Xdist(foo) dependencies) to also generate requirements on Python extras 
(e.g. python3.Xdist(foo[bar])) whenever upstream metadata indicate such 
dependency.

The script was also adapted to be able to process a new Python extras packages 
that have the format `base_package+extras_name` (e.g. 
`python3-setuptools_scm+toml`) and generate provides and requires for them.

This has been implemented in Fedora 33: 
https://fedoraproject.org/wiki/Changes/PythonExtras

The change also includes an improvement of the error messages: Previously the 
error messages were printed to stdout, they were part of the output of the 
script, and thus the words were treated like dependencies, sorted 
alphabetically, thus rendering the message unreadable. This change prints a 
single keyword indicating the error and details are outputted to stderr. It 
also outputs *** on purpose to ensure the dependency would not be valid, 
causing a failure of the build.
You can view, comment on, or merge this pull request online at:

  https://github.com/rpm-software-management/rpm/pull/1546

-- Commit Summary --

  * scripts/pythondistdeps: Rework error messages
  * scripts/pythondistdeps: Implement provides/requires for extras packages

-- File Changes --

M scripts/pythondistdeps.py (113)

-- Patch Links --

https://github.com/rpm-software-management/rpm/pull/1546.patch
https://github.com/rpm-software-management/rpm/pull/1546.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1546
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Extras (#1541)

2021-02-16 Thread torsava
Closed #1541.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1541#event-4337554786___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Extras (#1541)

2021-02-16 Thread torsava
Apologies, I've made a mistake. I wanted to open a PR against my own fork to 
hammer out the kinks first. Closing for now.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1541#issuecomment-780015648___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Extras (#1541)

2021-02-16 Thread torsava
All 166 tests are passing. @hroncok Take a look please.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1541#issuecomment-780013285___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Extras (#1541)

2021-02-16 Thread torsava
@torsava pushed 2 commits.

84c0a7f0101125eed9c350c428626ceaf84b2e02  [DO NOT MERGE THIS] Our downstream 
test suite
3190112709309e7b2dbd744e8d74404b0418052d  [DO NOT MERGE THIS] New test for 
extras


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1541/files/50080e4dd18b361c691fac67dd50c18ad7f304b5..3190112709309e7b2dbd744e8d74404b0418052d
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


[Rpm-maint] [rpm-software-management/rpm] Extras (#1541)

2021-02-16 Thread torsava
First two commits are upstreaming of relevant Fedora changes:

- 
https://src.fedoraproject.org/rpms/python-rpm-generators/c/d1a02fdda78743c796fe5c01cbb5287a8f3c873b?branch=rawhide
- 
https://src.fedoraproject.org/rpms/python-rpm-generators/c/098c48d46d7f9eceb4b41eddd0b20b31611c8e41?branch=rawhide

The extras implementation is a combination of 2 commits:

- 
https://src.fedoraproject.org/rpms/python-rpm-generators/c/3b1100ba1f5cbf1b1ebde65adec82d13d6cc62a6?branch=rawhide
- 
https://src.fedoraproject.org/rpms/python-rpm-generators/c/0c9665427c3dd51ee94a5757f1a49db77a8e53b5?branch=rawhide

You can view, comment on, or merge this pull request online at:

  https://github.com/rpm-software-management/rpm/pull/1541

-- Commit Summary --

  * scripts/pythondistdeps: Rework error messages
  * scripts/pythondistdeps: Adapt Python version marker workaround for 
setuptools 42+
  * scripts/pythondistdeps: Implement provides/requires for extras packages

-- File Changes --

M scripts/pythondistdeps.py (108)

-- Patch Links --

https://github.com/rpm-software-management/rpm/pull/1541.patch
https://github.com/rpm-software-management/rpm/pull/1541.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1541
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)

2020-12-21 Thread torsava
So, who has the power to merge this? @ffesti ?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1317#issuecomment-748928961___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)

2020-11-10 Thread torsava
> Oh, I've been alerted by @torsava that my suggestions have been addressed. 
> Nice, thanks! The code looks good to me. If the external tests still pass, 
> let's ship it?
> 
> We can probably test this in Fedora before merging if we want more testing 
> (but this is not necessary).
> 
> Thank you @s-t-e-v-e-n-k!

I'd prefer to merge it here, we've delayed the PR long enough :) We'll test it 
in Fedora in any case, and we can address possible issues later.

Thank you @s-t-e-v-e-n-k for having patience with us :)

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1317#issuecomment-724772529___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)

2020-10-30 Thread torsava
@torsava commented on this pull request.



> +try:
+from importlib.metadata import PathDistribution
+except ImportError:
+from importlib_metadata import PathDistribution
+
+try:
+from pathlib import Path
+except ImportError:
+from pathlib2 import Path
+
+
+class Req(Requirement):
+def __init__(self, requirement_string):
+super(Req, self).__init__(requirement_string)
+self.normalized_name = normalize_name(self.name)
+self.legacy_normalized_name = legacy_normalize_name(self.name)

With a change this big, I wouldn't worry about that.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1317#discussion_r515074212___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)

2020-10-14 Thread torsava
@torsava approved this pull request.

Looks good to me!



-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1317#pullrequestreview-508091467___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)

2020-10-13 Thread torsava
@torsava commented on this pull request.



> @@ -142,9 +210,23 @@ def convert(name, operator, version_id):
 
 
 def normalize_name(name):

@s-t-e-v-e-n-k Thinking more on this, since we have to keep this function, and 
since the class/static methods you currently use are used inside 2 different 
classes anyway, how about implementing the functionality as normal global 
functions? That way they could be used outside this script easily, and we would 
not need this wrapper.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1317#discussion_r503744048___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)

2020-10-13 Thread torsava
@torsava commented on this pull request.



> @@ -142,9 +210,23 @@ def convert(name, operator, version_id):
 
 
 def normalize_name(name):

Ah, right. In that case, yeah, a comment would be great.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1317#discussion_r503734008___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)

2020-10-12 Thread torsava
@torsava commented on this pull request.



> +@classmethod
+def normalize_name(klass, name):
+"""https://www.python.org/dev/peps/pep-0503/#normalized-names""";
+return re.sub(r'[-_.]+', '-', name).lower()
+
+@classmethod
+def legacy_normalize_name(klass, name):
+"""Like pkg_resources Distribution.key property"""
+return re.sub(r'[-_]+', '-', name).lower()

> I wonder why it is a classmethod and not a staticmethod (when it does not use 
> the "kalss" attribute at all). I would make it a property, but initialized 
> value is fine as well.

It's also used outside of this class, so it can't be a property. But a static 
method/function makes sense.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1317#discussion_r503360841___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)

2020-10-12 Thread torsava
@torsava commented on this pull request.

Besides the two mostly cosmetic comments, I think it looks good!

> @@ -142,9 +210,23 @@ def convert(name, operator, version_id):
 
 
 def normalize_name(name):

This function is now completely unused and can be removed.

> +@classmethod
+def normalize_name(klass, name):
+"""https://www.python.org/dev/peps/pep-0503/#normalized-names""";
+return re.sub(r'[-_.]+', '-', name).lower()
+
+@classmethod
+def legacy_normalize_name(klass, name):
+"""Like pkg_resources Distribution.key property"""
+return re.sub(r'[-_]+', '-', name).lower()

Nitpick: I'm not excited about the `klass` name instead of the standard `cls`.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1317#pullrequestreview-506555419___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)

2020-09-29 Thread torsava
@torsava commented on this pull request.



>  from warnings import warn
 
+try:
+from importlib.metadata import PathDistribution
+except ImportError:
+from importlib_metadata import PathDistribution
+
+try:
+from pathlib import Path
+except ImportError:
+from pathlib2 import Path
+
+
+class Req(Requirement):
+@property
+def key(self):
+return self.name.lower().replace('_', '-')

And I agree with unifying the two ways to get a normalized name.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1317#discussion_r496527810___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)

2020-09-29 Thread torsava
@torsava commented on this pull request.



>  from warnings import warn
 
+try:
+from importlib.metadata import PathDistribution
+except ImportError:
+from importlib_metadata import PathDistribution
+
+try:
+from pathlib import Path
+except ImportError:
+from pathlib2 import Path
+
+
+class Req(Requirement):
+@property
+def key(self):
+return self.name.lower().replace('_', '-')

@s-t-e-v-e-n-k I wasn't against ceasing to use the `key` property, I was 
against passing different values in it's place. The name of the 
property/function is immaterial.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1317#discussion_r496524935___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)

2020-09-25 Thread torsava
@torsava commented on this pull request.



>  from warnings import warn
 
+try:
+from importlib.metadata import PathDistribution
+except ImportError:
+from importlib_metadata import PathDistribution
+
+try:
+from pathlib import Path
+except ImportError:
+from pathlib2 import Path
+
+
+class Req(Requirement):
+@property
+def key(self):
+return self.name.lower().replace('_', '-')

I'm also confused as to what that means..?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1317#discussion_r494836924___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)

2020-09-24 Thread torsava
@torsava commented on this pull request.



>  from warnings import warn
 
+try:
+from importlib.metadata import PathDistribution
+except ImportError:
+from importlib_metadata import PathDistribution
+
+try:
+from pathlib import Path
+except ImportError:
+from pathlib2 import Path
+
+
+class Req(Requirement):
+@property
+def key(self):
+return self.name.lower().replace('_', '-')

Ah, I see. Yeah, that should be fixed as well. @s-t-e-v-e-n-k please take a 
look.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1317#discussion_r494236346___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)

2020-09-24 Thread torsava
@torsava commented on this pull request.



>  from warnings import warn
 
+try:
+from importlib.metadata import PathDistribution
+except ImportError:
+from importlib_metadata import PathDistribution
+
+try:
+from pathlib import Path
+except ImportError:
+from pathlib2 import Path
+
+
+class Req(Requirement):
+@property
+def key(self):
+return self.name.lower().replace('_', '-')

To clarify a possible confusion, I think `key` always returned the same thing. 
But this script had two modes, in legacy mode it used the `key` value, and in 
pep503 mode it used the `normalize_name` function.

So yes, there are two different methods to convert the name, but that is 
handled by this script, not by `Requirement.key`.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1317#discussion_r494230415___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)

2020-09-24 Thread torsava
@torsava commented on this pull request.



>  from warnings import warn
 
+try:
+from importlib.metadata import PathDistribution
+except ImportError:
+from importlib_metadata import PathDistribution
+
+try:
+from pathlib import Path
+except ImportError:
+from pathlib2 import Path
+
+
+class Req(Requirement):
+@property
+def key(self):
+return self.name.lower().replace('_', '-')

Oh, how come? You think the [`Requirement.key` 
property](https://setuptools.readthedocs.io/en/latest/pkg_resources.html) gave 
different results in different situations?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1317#discussion_r494227693___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)

2020-09-24 Thread torsava
@torsava commented on this pull request.



>  if normalized_names_require_pep503:
-dep_normalized_name = normalize_name(dep.project_name)
+dep_normalized_name = normalize_name(dep.name)
 else:
 dep_normalized_name = dep.key

> Be careful with manual replaces for normalized names. There are two regexes 
> that this code supported before. The PEP 503 regex and the regex imported 
> from setuptools (described in here).

I wasn't exactly sure what you meant by this comment, so I asked if the new 
code looks good to you to be sure.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1317#discussion_r494224179___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)

2020-09-24 Thread torsava
@torsava commented on this pull request.



>  if normalized_names_require_pep503:
-dep_normalized_name = normalize_name(dep.project_name)
+dep_normalized_name = normalize_name(dep.name)
 else:
 dep_normalized_name = dep.key

@hroncok But the code continues to preserve both the legacy and pep503 
conventions. So I'm not sure I understand what you mean.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1317#pullrequestreview-495425833___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)

2020-09-24 Thread torsava
@torsava commented on this pull request.



>  else:
-dep_normalized_name = dep.key
+dep_normalized_name = dep.key.replace('_', '-')

The newest version looks good to me, and tests are passing.

The [documentation for 
pkg_resources](https://setuptools.readthedocs.io/en/latest/pkg_resources.html) 
says about `key`: An all-lowercase version of the project_name, useful for 
comparison or indexing.
>From there you have to look up `project_name` and then `safe_name` function 
>which generates it. So if we go by documentation, dots are not allowed in 
>`key`. However, if you actually test it, they are there.

What do you think, @hroncok ?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1317#discussion_r494166167___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)

2020-09-23 Thread torsava
@torsava commented on this pull request.



> -print('Conflicts:\t{} {} {}'.format(dep.key, 
> '==', spec[1]))
+for dep in dist.requirements_for_extra(extra):
+for spec in dep.specifier:
+if spec.operator == '!=':
+print('Conflicts:\t{} {} {}'.format(dep.name, 
'==', spec.version))
 else:
-print('Requires:\t{} {} {}'.format(dep.key, 
spec[0], spec[1]))
+print('Requires:\t{} {} {}'.format(dep.name, 
spec.operator, spec.version))

@s-t-e-v-e-n-k Here you still use `dep.name` as a replacement for `dep.key`, is 
there a reason?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1317#discussion_r493363804___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)

2020-09-23 Thread torsava
@torsava commented on this pull request.



>  else:
-dep_normalized_name = dep.key
+dep_normalized_name = dep.key.replace('_', '-')

In my testing, the previous setuptools version already has underscores replaced 
in the `key` property, so I would suggest doing it too.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1317#discussion_r493362675___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)

2020-09-23 Thread torsava
@torsava commented on this pull request.



>  else:
-dep_normalized_name = dep.key
+dep_normalized_name = dep.key.replace('_', '-')

Why do you still do the `replace` here, but not in the other places where you 
use `dep.key`? I'm not saying it's necessarily wrong, but I don't understand it.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1317#pullrequestreview-494464867___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)

2020-09-23 Thread torsava
@torsava commented on this pull request.



> +if isdir(path):
+path = dirname(path)
+res = re.search(r"/python(?P\d+\.\d+)/", path)

This breaks detection of Python version from the name of the egg-info file. And 
I think you misunderstood me, I wasn't doubting the usefulness of the variable, 
I was asking why do you do `dirname` at all.
```suggestion
res = re.search(r"/python(?P\d+\.\d+)/", path)
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1317#pullrequestreview-494457340___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)

2020-09-22 Thread torsava
> Latest push now has every test successfully run the script, however that one 
> test still fails because there's a difference in behaviour between the script 
> in this repo and in python-rpm-generators.
> 
> ```
> -if '>' == operator:
> -# distutils does not behave this way, but this is
> -# their recommendation
> -# 
> https://mail.python.org/archives/list/distutils-...@python.org/thread/NWEQVTCX5CR2RKW2LT4H77PJTEINSX7P/
> +if operator == '>':
> +# distutils will allow a prefix match with '>'
>  operator = '>='
> -version.increment()
> +if operator == '<=':
> +# distutils will not allow a prefix match with '<='
> +operator = '<'
> ```
> 
> The differences in that block are the cause of the test failure as far as I 
> can work out.

I believe the behaviour has been changed and you have the new correct version, 
but I'll double check.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1317#issuecomment-696766915___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)

2020-09-22 Thread torsava
@torsava requested changes on this pull request.

Hi @s-t-e-v-e-n-k ,
first of all, I'm really sorry the review took so long :(

I've pointed out some issues in the comments, mostly in the less-or-not-used 
parts of the code.

The PR as a whole looks fantastic, great piece of engineering. And all our 
tests are passing. Thanks!

> +if isdir(path):
+path_item = dirname(path)
+else:
+path_item = path

Why not just `path_item = path` (or just use `path` below instead)?

> -dep_normalized_name = dep.key
+dep_normalized_name = dep.name.lower().replace('_', 
'-')
 
 if args.legacy:
-name = 'pythonegg({})({})'.format(pyver_major, dep.key)
+name = 'pythonegg({})({})'.format(pyver_major, 
dep.name.lower())

I've never used the legacy mode, I'm not even sure if anyone uses it. But for 
consistency: Why is `dep.key` being replaced by slightly different expressions 
in these two places?

> -print('Conflicts:\t{} {} {}'.format(dep.key, 
> '==', spec[1]))
+for dep in dist.requirements_for_extra(extra):
+for spec in dep.specifier:
+if spec.operator == '!=':
+print('Conflicts:\t{} {} {}'.format(dep.name, 
'==', spec.version))
 else:
-print('Requires:\t{} {} {}'.format(dep.key, 
spec[0], spec[1]))
+print('Requires:\t{} {} {}'.format(dep.name, 
spec.operator, spec.version))

Another two places where `dep.key` is being replaced by yet different 
expression. Maybe it would be worth creating the `key` property for 
`Distribution` and unifying it.

> -for dep in dist.requires(extras=dist.extras):
-name = dep.key
-for spec in dep.specs:
-if spec[0] == '!=':
+for dep in dist.requirements + dist.extra_requirements:
+for spec in dep.specifier:
+if spec.operator == '!=':
 if name not in py_deps:
-py_deps[name] = []
-spec = ('==', spec[1])
+py_deps[dep.name] = []
+spec = ('==', spec.version)
 if spec not in py_deps[name]:
-py_deps[name].append(spec)
+py_deps[dep.name].append(spec)

`name` is not initialized here, and `dep.name` is another replacement for 
`dep.key` which is not consistent with the others.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1317#pullrequestreview-493449129___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)

2020-08-26 Thread torsava
> Okay, this new push solves all but one of the test failures, which has the 
> script failing to parse the requires.txt for pyreq2rpm.tests since it 
> contains comments.

Ah, I was glad that pkg_resources allowed those comments for easier 
orientation, but we can remove them if we must.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1317#issuecomment-68048___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)

2020-08-11 Thread torsava
> Any updates on the test suite investigation? Is there anything I can do to 
> help?

@s-t-e-v-e-n-k Apologies, I got buried under different priorities. I'll try to 
get this in a few days.

If you'd like to see on your own, the test suite for `pythondistdeps.py` is 
currently only in our Fedora repo here: 
https://src.fedoraproject.org/rpms/python-rpm-generators/blob/master/f/tests 
(file `test_scripts_pythondistdeps.py` and folder 
`data/scripts_pythondistdeps`).

Apologies for the location, the test suite was rejected here in the RPM repo 
for complexity. A new upstream repo for Python RPM scripts is planned (but that 
hasn't been created yet), so I'll be adding it there when possible.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1317#issuecomment-672061507___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)

2020-07-23 Thread torsava
There are test failures, but mainly do to different capitalization of names. 
Plus I haven't been able to run the comparison test metadata for some reason. 
I'll take a deeper look.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1317#issuecomment-662924340___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps.py: Adapt Python version marker workaround for setuptools 42+ (#1308)

2020-07-10 Thread torsava
Looks good to me!

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1308#issuecomment-656673366___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Fix python(abi) requires generator, it picked files from almost good directories (#1272)

2020-06-25 Thread torsava
The change looks good to me. And we've been using this in RHEL for a while with 
good results.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1272#issuecomment-649454084___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add _without_check macro (#1256)

2020-06-08 Thread torsava
> I think this should set _with_check unless _without_check is defined already. 
> Basically to have `%bcond_without check` by default without having to put it 
> in all spec files. But still need to make sure that somebody defines 
> `%bcond_without check`, this code won't override it.

That would be absolutely perfect, if there's a way to do it.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1256#issuecomment-640515842___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add _without_check macro (#1256)

2020-06-08 Thread torsava
I would like to avoid `nocheck`, because the common way to conditinalize would 
be `%if %{without nocheck}` and that's just confusing to read.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1256#issuecomment-640514577___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] RFE: add way to set macro for --nocheck in rpmbuild (#316)

2020-06-05 Thread torsava
+1 this would be a great way to standardize on disabling tests!

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/316#issuecomment-639545601___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] RPM generators errors are ignored (#1183)

2020-06-02 Thread torsava
> There are 3 things I'd like to see fixed:
> 
> * the traceback should say: `Cannot process Python package version: 0+unknown`
> * the build should abort on errors
> * the version is [actually 
> valid](https://www.python.org/dev/peps/pep-0440/#local-version-identifiers)

First and last issue have been fixed in 
https://github.com/rpm-software-management/rpm/pull/1242.

What remains is that the build should abort on errors.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/1183#issuecomment-637692530___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] RFE: split language specifics out of rpm core (#1199)

2020-05-27 Thread torsava
And are you planning to include the relevant scripts from these different repos 
into the releases of rpm?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/1199#issuecomment-634525298___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: Various updates and fixes (no test suite) (#1242)

2020-05-27 Thread torsava
Thank you too, @pmatilai!

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1242#issuecomment-634522612___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: Various updates and fixes (no test suite) (#1242)

2020-05-26 Thread torsava
> This pull request **introduces 1 alert** when merging 
> [592a6d5](https://github.com/rpm-software-management/rpm/commit/592a6d5980010c63eb76c31ddd8954fba9cbaa92)
>  into 
> [8734c1b](https://github.com/rpm-software-management/rpm/commit/8734c1b97e39e3c7d3ac8396c4d6a2733852545c)
>  - [view on 
> LGTM.com](https://lgtm.com/projects/g/rpm-software-management/rpm/rev/pr-b6979aa46105339f8a0843eae399433dc33d6444)
> 
> **new alerts:**
> 
> * 1 for Module is imported more than once

It's a lazy import so that if it's not needed, it doesn't slow down the 
execution.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1242#issuecomment-634119166___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: New test suite and various updates and fixes (#1195)

2020-05-26 Thread torsava
I have modified the commits to take out the test suite and opened it as a new 
PR https://github.com/rpm-software-management/rpm/pull/1242.

Am therefore closing this PR in favour of the new one.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1195#issuecomment-634114538___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: New test suite and various updates and fixes (#1195)

2020-05-26 Thread torsava
Closed #1195.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1195#event-3374229415___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


[Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: Various updates and fixes (no test suite) (#1242)

2020-05-26 Thread torsava
This is a modification of PR 
https://github.com/rpm-software-management/rpm/pull/1195, where I've 
deleted all mentions of the test suite.

CC @pmatilai, @Conan-Kudo, @ffesti, @hroncok 
You can view, comment on, or merge this pull request online at:

  https://github.com/rpm-software-management/rpm/pull/1242

-- Commit Summary --

  * scripts/pythondistdeps: Also provide pythonXdist() with PEP 503 normalized 
names
  * scripts/pythondistdeps: "Fix" support of environment markers
  * scripts/pythondistdeps: Notes from an attempted rewrite to 
importlib.metadata
  * scripts/pythondistdeps: Sort generated provides/requires
  * scripts/pythondistdeps: Add option to generate major-version provides only 
for specified Python versions
  * scripts/pythondistdeps: Implement --normalized-name-* options
  * scripts/pythondistdeps: Do anything only when called as a main script
  * scripts/pythondistdeps: Version handling exception with better information
  * scripts/pythondistdeps: Modify handling of dev versions

-- File Changes --

M scripts/pythondistdeps.py (466)

-- Patch Links --

https://github.com/rpm-software-management/rpm/pull/1242.patch
https://github.com/rpm-software-management/rpm/pull/1242.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1242
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: New test suite and various updates and fixes (#1195)

2020-05-26 Thread torsava
> Being an autotools project has little to do with it. Rpm's test-suite looks 
> like voodoo because of the fakechroot integration, but that aside the 
> autotest cases are nothing but shell script snippets followed by expected 
> stdin/stderr + return code. What you do in that shell script space is totally 
> up to you, and you can add arbitrary helper/wrappers to suit purpose. Our 
> test-suite actually has wrappers to allow eg native Python code directly in 
> the tests, take a look at 
> https://github.com/rpm-software-management/rpm/blob/master/tests/rpmpython.at 
> and 
> https://github.com/rpm-software-management/rpm/blob/master/tests/rpmvercmp.at 
> sometime.

So if we integrated the pythondistdeps test suite into the RPM test suite, you 
would be ok with merging it as well? The test suite would need to remain 
written in pytest of course, but from what you say, I can figure out to how to 
integrate a bash snippet that runs it.

Because we (at least I) are not interested in creating a new upstream for 
pythondistdeps.py, so I would probably end up creating a repo only for the test 
suite, which is not great.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1195#issuecomment-633945967___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: New test suite and various updates and fixes (#1195)

2020-05-26 Thread torsava
Ok, sounds good. I'll separate out the test suite.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1195#issuecomment-633908867___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: New test suite and various updates and fixes (#1195)

2020-05-20 Thread torsava
I would like to work on some more changes, for example to handle "extras", but 
I'm not sure it's a good idea before this is merged.

CC @Conan-Kudo, @pmatilai, @ffesti

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1195#issuecomment-631572000___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: New test suite and various updates and fixes (#1195)

2020-05-04 Thread torsava
These proposed changes have been merged to Fedora rawhide: 
https://src.fedoraproject.org/rpms/python-rpm-generators/pull-request/15

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1195#issuecomment-623426212___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Discussion: splitting language specifics out of rpm core (#1199)

2020-05-04 Thread torsava
> That's what I'm been mulling over here. Pulling stuff from multiple places 
> does complicate release-cutting considerably though (which tends to be hard 
> enough as it is), and will mean that bugs will get reported on rpm, but the 
> code will be someplace else, which I suspect would be a frustrating situation 
> for all involved. So I don't think that will work as such, but maybe there's 
> something else in that direction.

I gotta say I'm sceptical that we'll find a better solution than keeping the 
scripts in rpm.

However, the bugs being reported wrong could be solved by some more capable 
issue/bug-tracking software. For example a Bugzilla, where a bug can be easily 
reassigned to a different component (say `scripts/pythondistdeps`) that is 
administered by a different group of people.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/1199#issuecomment-623424253___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: New test suite and various updates and fixes (#1195)

2020-04-29 Thread torsava
I've merged the first fixup. Leaving the second one pending depending on 
whether we decide to merge pythondistdeps here or split off to a new package.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1195#issuecomment-621351551___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: New test suite and various updates and fixes (#1195)

2020-04-29 Thread torsava
While the [discussion about possibly moving pythondistdeps to a different 
repo](https://github.com/rpm-software-management/rpm/issues/1199) goes on:

Any technical issues or concerns with the PR?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1195#issuecomment-621108332___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Discussion: splitting language specifics out of rpm core (#1199)

2020-04-28 Thread torsava
I can see some benefits of having some standalone domain-specific repos, like a 
repo for Python scripts. However, I think Python is one of the special cases, 
splitting out all the scripts would seem to me an overkill. And if we split 
only some and leave the rest in rpm, it's more mess than we started with.

On the other hand, I don't really see the benefits of using rpm-extras. It has 
the problems of not being in the rpm repo with no benefits of the standalone 
repos.

So overall, I acknowledge the problems, but to me the proposed cures are worse 
than the disease (to use a popular turn of phrase).

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/1199#issuecomment-620630859___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: New test suite and various updates and fixes (#1195)

2020-04-27 Thread torsava
@torsava commented on this pull request.



> @@ -11,6 +11,10 @@
 # RPM python dependency generator, using .egg-info/.egg-link/.dist-info data
 #
 
+# Please know:
+# - Notes from an attempted rewrite from pkg_resources to importlib.metadata in
+#   2020 can be found in the message of the commit that added this line.

> I see this by a wrong comment, but +1.

What do you mean by that?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1195#discussion_r415943934___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: New test suite and various updates and fixes (#1195)

2020-04-27 Thread torsava
@torsava commented on this pull request.



> @@ -11,6 +11,10 @@
 # RPM python dependency generator, using .egg-info/.egg-link/.dist-info data
 #
 
+# Please know:
+# - Notes from an attempted rewrite from pkg_resources to importlib.metadata in
+#   2020 can be found in the message of the commit that added this line.

@Conan-Kudo @hroncok WDYT?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1195#discussion_r415926110___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: New test suite and various updates and fixes (#1195)

2020-04-27 Thread torsava
> Any new sources need to be EXTRA_DIST in Makefile.am's.

I've added the checked-in files to the `EXTRA_DIST` in the fixup commit. I 
didn't add the automatically-downloaded metadata, as the test script will 
automatically fetch them if missing. If it's desirable to have them inside 
EXTRA_DIST as well, let me know.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1195#issuecomment-620050790___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: New test suite and various updates and fixes (#1195)

2020-04-27 Thread torsava
@torsava pushed 1 commit.

e7cc87a083366439c0450c720ed36475b09d  fixup! scripts/pythondistdeps: Add 
the tests to the readme and makefile


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1195/files/96be70abdb8f5b7633fb41829a8b5d496a466e03..e7cc87a083366439c0450c720ed36475b09d
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: New test suite and various updates and fixes (#1195)

2020-04-27 Thread torsava
@torsava commented on this pull request.



> @@ -11,6 +11,10 @@
 # RPM python dependency generator, using .egg-info/.egg-link/.dist-info data
 #
 
+# Please know:
+# - Notes from an attempted rewrite from pkg_resources to importlib.metadata in
+#   2020 can be found in the message of the commit that added this line.

Implemented to support both ways, see the latest fixup commit. And I've added 
tests for these cases:

--provides --majorver-provides-versions 3.9 --majorver-provides-versions 2.7
--provides --majorver-provides-versions 3.9,2.7
--provides --majorver-provides-versions 3.9,2.7 
--majorver-provides-versions 3.10
--provides --majorver-provides-versions 3.9

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1195#discussion_r415748782___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: New test suite and various updates and fixes (#1195)

2020-04-27 Thread torsava
@torsava pushed 1 commit.

96be70abdb8f5b7633fb41829a8b5d496a466e03  fixup! scripts/pythondistdeps: Add 
option to generate major-version provides only for specified Python versions


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1195/files/dbc365b59207f4b00624241ae25542dd3f0a83cd..96be70abdb8f5b7633fb41829a8b5d496a466e03
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: New test suite and various updates and fixes (#1195)

2020-04-27 Thread torsava
@torsava commented on this pull request.



> +
+
+if __name__ == "__main__":
+"""To allow this script to be importable (and its classes/functions
+   reused), actions are performed only when run as a main script."""
+
+parser = argparse.ArgumentParser(prog=argv[0])
+group = parser.add_mutually_exclusive_group(required=True)
+group.add_argument('-P', '--provides', action='store_true', help='Print 
Provides')
+group.add_argument('-R', '--requires', action='store_true', help='Print 
Requires')
+group.add_argument('-r', '--recommends', action='store_true', help='Print 
Recommends')
+group.add_argument('-C', '--conflicts', action='store_true', help='Print 
Conflicts')
+group.add_argument('-E', '--extras', action='store_true', help='Print 
Extras')
+group_majorver = parser.add_mutually_exclusive_group()
+group_majorver.add_argument('-M', '--majorver-provides', 
action='store_true', help='Print extra Provides with Python major version only')
+group_majorver.add_argument('--majorver-provides-versions', action='store',

> I'm mostly thinking of potential field parsing bugs. I'm thinking of if 
> people call it twice with two comma separated lists. I don't know if we 
> really want to make that case possible...

The reason I'm suggesting it is that `--majorver-provides-versions` doesn't 
have a short form (and it's so specific that I don't think it should), and it 
would result in very long calls, e.g. in Fedora we would need:

--provides --majorver-provides-versions 2.7 --majorver-provides-versions 
3.8 --normalized-names-format pep503 --normalized-names-provide-both

I can actually make a test for all the possible ways to call 
`--majorver-provides-versions` so it should be safe to implement it both ways.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1195#discussion_r415696889___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: New test suite and various updates and fixes (#1195)

2020-04-27 Thread torsava
> Any new sources need to be EXTRA_DIST in Makefile.am's. For a moment I was 
> puzzling as to how come this is passing dist-check when the added sources are 
> missing in the release tarball, but then realized the added tests are not 
> getting run in _our_ test-suite. Which to me kinda steps over a certain line.

That is not on purpose, I would love if these tests were also run as part of 
the upstream test-suite. I thought about hooking it into `make check`. I didn't 
do it for 2 reasons:

1. These tests require new dependencies, and I wasn't sure if you would want to 
tie the rpm test suite to the pythondistdeps testing dependencies. This way 
there's a specific way to run the pythondistdeps test (and perhaps other 
scripts/ tests later) using `make check-scripts` that can be just added to the 
CI bot if desired.
 
2. The makefile machinery for `make test` is a bit too confusing to me to 
easily add to.

However, a separate repo would also work.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1195#issuecomment-619883458___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: New test suite and various updates and fixes (#1195)

2020-04-27 Thread torsava
@torsava commented on this pull request.



> +
+
+if __name__ == "__main__":
+"""To allow this script to be importable (and its classes/functions
+   reused), actions are performed only when run as a main script."""
+
+parser = argparse.ArgumentParser(prog=argv[0])
+group = parser.add_mutually_exclusive_group(required=True)
+group.add_argument('-P', '--provides', action='store_true', help='Print 
Provides')
+group.add_argument('-R', '--requires', action='store_true', help='Print 
Requires')
+group.add_argument('-r', '--recommends', action='store_true', help='Print 
Recommends')
+group.add_argument('-C', '--conflicts', action='store_true', help='Print 
Conflicts')
+group.add_argument('-E', '--extras', action='store_true', help='Print 
Extras')
+group_majorver = parser.add_mutually_exclusive_group()
+group_majorver.add_argument('-M', '--majorver-provides', 
action='store_true', help='Print extra Provides with Python major version only')
+group_majorver.add_argument('--majorver-provides-versions', action='store',

`append` shouldn't be a problem. `extend` however, swallows unlimited number of 
arguments, which could interfere with positional arguments, so I'd rather avoid 
it. How about using `append` and also keeping the option for comma-separated 
entries?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1195#discussion_r415683331___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: New test suite and various updates and fixes (#1195)

2020-04-24 Thread torsava
> **new alerts:**
> 
> * 1 for Module is imported more than once

It's a lazy import so that if it's not needed, it doesn't slow down the 
execution.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1195#issuecomment-619013601___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


[Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: New test suite and various updates and fixes (#1195)

2020-04-24 Thread torsava
Please do a review commit-by-commit. I changed indentation of a large part of 
the code, so the overall diff is hard to read.
You can view, comment on, or merge this pull request online at:

  https://github.com/rpm-software-management/rpm/pull/1195

-- Commit Summary --

  * scripts/pythondistdeps: Also provide pythonXdist() with PEP 503 normalized 
names
  * scripts/pythondistdeps: "Fix" support of environment markers
  * scripts/pythondistdeps: Notes from an attempted rewrite to 
importlib.metadata
  * scripts/pythondistdeps: Add option to generate major-version provides only 
for specified Python versions
  * scripts/pythondistdeps: Implement --normalized-name-* options
  * scripts/pythondistdeps: Add tests
  * scripts/pythondistdeps: Do anything only when called as a main script
  * scripts/pythondistdeps: Version handling exception with better information
  * scripts/pythondistdeps: Add the tests to the readme and makefile
  * scripts/pythondistdeps: Modify handling of dev versions

-- File Changes --

M .gitignore (1)
M scripts/pythondistdeps.py (463)
M tests/Makefile.am (4)
M tests/README (22)
A 
tests/data/scripts_pythondistdeps/pyreq2rpm.tests-2020.04.07.024dab0-py3.9.egg-info/PKG-INFO
 (21)
A 
tests/data/scripts_pythondistdeps/pyreq2rpm.tests-2020.04.07.024dab0-py3.9.egg-info/requires.txt
 (102)
A tests/data/scripts_pythondistdeps/test-data.yaml (1174)
A tests/data/scripts_pythondistdeps/test-requires.yaml (97)
A tests/test_scripts_pythondistdeps.py (242)

-- Patch Links --

https://github.com/rpm-software-management/rpm/pull/1195.patch
https://github.com/rpm-software-management/rpm/pull/1195.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1195
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] RPM generators errors are ignored (#1183)

2020-04-20 Thread torsava
> @torsava OK, let's wait for the next unexpected error. But wrapping 
> everything in try-except and failing with useful error message is not a bad 
> idea.

Ok, I'll add it to my PR.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/1183#issuecomment-616518564___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] RPM generators errors are ignored (#1183)

2020-04-20 Thread torsava
> the exception when the conversion fails is not telling anything useful

As this was an unexpected error, we don't know at what point in the code the 
next unexpected error would strike. So the way I see it we would have to wrap 
the whole machinery in `try..except` which isn't great, or do you propose a 
different solution, @hroncok ?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/1183#issuecomment-616507483___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Python dist RPM generators errors are silently ignored + 0 version fail (#1183)

2020-04-20 Thread torsava
Addressed here: https://github.com/rpm-software-management/rpm/pull/1184

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/1183#issuecomment-616497780___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: Switch to argparse (#1181)

2020-04-16 Thread torsava
This very much breaks my pull request I'm preparing — it seems that once you 
start working on pythondistdeps, everyone gets notification about it and starts 
working on it too :) — but it's beautiful and I'm all for it!

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1181#issuecomment-614615287___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: Improved python version and operator handling. (#1015)

2020-04-09 Thread torsava
@torsava approved this pull request.

Code looks good, tests are passing.



-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1015#pullrequestreview-391203820___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: Improved python version and operator handling. (#1015)

2020-04-09 Thread torsava
> I've pushed a change that should resolve the issues with trailing .0 versions.

And now all tests pass! +1

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1015#issuecomment-611808976___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: Improved python version and operator handling. (#1015)

2020-04-09 Thread torsava
@gordonmessmer As for my tests, I'm preparing a few changes to 
pythondistdeps.py, so I've added tests as well. The PR is still WIP.

https://github.com/torsava/rpm/pull/1

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1015#issuecomment-611623657___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: Improved python version and operator handling. (#1015)

2020-04-09 Thread torsava
> This change incorporates code from 
> https://github.com/gordonmessmer/pyreq2rpm/ which has a test suite of its own.

That's exactly where I took my test data. I have tested your PR and 84 out of 
86 tests passed! These are the only 2 issues:

1. Input: foobar4~=2.0
Expected output: (python3.7dist(foobar4) >= 2 with python3.7dist(foobar4) < 
3)
Actual output: Invalid requirement: python3.7dist(foobar4) ~= 2
 [crashes pythondistdeps]

2. Input: foobar1~=2.4.8.0
Expected output: (python3.7dist(foobar1) >= 2.4.8 with 
python3.7dist(foobar1) < 2.4.9)
Actual output: (python3.7dist(foobar1) >= 2.4.8 with python3.7dist(foobar1) 
< 2.5)

However, you did not introduce these issues, the improper handling of a 
trailing ".0" in some cases was already present in the original version.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1015#issuecomment-611614414___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: Improved python version and operator handling. (#1015)

2020-04-09 Thread torsava
> It would be nice to have the tests for these... But from the first glance 
> this looks good to me.

I'm actually writing a test suite for `scripts/pythondistdeps`, I'll report 
soon with the results.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1015#issuecomment-611481865___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] RFE: Syntax sugar for bconds with inherited defaults (#941)

2019-11-18 Thread torsava
Brainstorming:

* bcond_default_same + bcond_default_opposite

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/941#issuecomment-555008238___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] RFE: Syntax sugar for bconds with inherited defaults (#941)

2019-11-18 Thread torsava
I would find it useful as well. However, we should really consider the naming, 
as bconds are already confusing enough, this could become completely unreadable 
to people not-in-the-know.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/941#issuecomment-55487___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Change the shebang of pythondistdeps.py to Python 3 (#212)

2017-05-15 Thread torsava
So seeing as no one has the resources to guide the larger patch, then unless 
you find the simple shebang change acceptable, I suggest we close the issue.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/212#issuecomment-301459911___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Change the shebang of pythondistdeps.py to Python 3 (#212)

2017-05-10 Thread torsava
So, to conclude the discussion: I don't really have the know-how and time to 
push through the more complicated change that @proyvind proposed. Would you be 
interested in leading that, @proyvind ? I would help where I can.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/212#issuecomment-300476838___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Change the shebang of pythondistdeps.py to Python 3 (#212)

2017-05-05 Thread torsava
> As distros completes their migration to python 3 and no python 2 package 
> installed on system, any py3k compatible script from packages in distro using 
> /usr/bin/python as shebang will be left broken, for which there surely are 
> quite a few.

That's exactly what you want and why Fedora, RHEL, openSUSE and Mageia follow 
upstream recommendation in PEP 394. This way you'll know about scripts that 
aren't ported yet and you can fix them. Otherwise you would be running old 
Python 2 scripts on Python 3 which might run fine in 99% of cases and fail only 
in weird corner cases. And that's a huge pain to debug as you know.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/212#issuecomment-299409590___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Change the shebang of pythondistdeps.py to Python 3 (#212)

2017-05-04 Thread torsava
> Welcome back to 2017, Mandriva is dead ;)

Ah, that explains it :)

> Actually, in Fedora we should patch that to system-python..

Someone tried changing the shebang patch to `system-python`, but it had to be 
reverted back to Python 3 because the `pythondistdeps` needs `setuptools`, 
which `system-python` does not support.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/212#issuecomment-299243649___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Change the shebang of pythondistdeps.py to Python 3 (#212)

2017-05-04 Thread torsava
> First of all, while PEP 394 recommends this, it's not what's common practice 
> amongst distros, where Arch not even being a RPM based distro makes it less 
> relevant.
> 
> I know of no rpm based distros where /usr/bin/python isn't pointing to the 
> default python interpreter binary version.

I've just checked and the following RPM-based distros are keeping with the 
upstream recommendation of keeping `/usr/bin/python` always pointing to Python 
2 (and if Python 2 isn't installed then`/usr/bin/python` doesn't exist):

 * Fedora
 * RHEL
 * openSUSE
 * Mandriva (though here the container I found and tested was a bit old)

I would argue that's a solid chunk of the RPM-based distros.

> However, if you really think this is a necessity (where forcing distros still 
> using python2 to patch the shebang themself would be considered less 
> acceptable than not adhering to PEP 394), 

Right now the above distros are being forced to patch it. I think the burden of 
a patch should lie with the Python 2–only distros nowadays, this close to 
Python 2 EOL.

> the patch betlow would be the more proper way to achieve your goal.
> This one also ensures that this $PYTHON is used for both %__python and the 
> python bindings as well.

The patch looks interesting, though I'm not nearly experienced enough with the 
RPM code base to vet it. My only suggestion would be to set the default to 
`/usr/bin/python3`.


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/212#issuecomment-299224600___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Change the shebang of pythondistdeps.py to Python 3 (#212)

2017-05-04 Thread torsava
> This makes no sense, /usr/bin/python is usually the linked to the binary of 
> the default python version in use on distros, so hardcoding the version makes 
> no sense.

That is a common misconception. While some distros went ahead and switched 
`/usr/bin/python` to Python 3, most notably Arch Linux, most are keeping it 
pointing to Python 2 and will continue to do so at least until Python 2 EOL in 
2020. Upstream Python issued a recommendation to that effect as well: [PEP 394 
-- The "python" Command on Unix-Like 
Systems](https://www.python.org/dev/peps/pep-0394/#abstract)

So while most distros that will be using the latest RPM already switched to 
Python 3 being default, the binary `/usr/bin/python` still points to Python 2. 
That means that most distros will need to patch this shebang to 
`/usr/bin/python3` before deploying.

I propose that we switch the shebang to explicitly Python 3 by default. There 
possibly might be some distros that will be using the latest RPM version but 
still haven't switched to Python 3 being default (and they will need to patch 
this shebang back to Python 2), but I'd wager it will be a minuscule number 
compared to Python 3–default distros.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/212#issuecomment-299131961___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


[Rpm-maint] [rpm-software-management/rpm] Change the shebang of pythondistdeps.py to Python 3 (#212)

2017-05-03 Thread torsava
The `pythondistdeps.py` file is still carrying a Python 2 shebang 
(`/usr/bin/python`).

In Fedora we already patch it to Python 3, so I'm proposing to make the same 
change in upstream as well. Python 2 is EOL in 2 years and 11 months.
You can view, comment on, or merge this pull request online at:

  https://github.com/rpm-software-management/rpm/pull/212

-- Commit Summary --

  * Change the shebang of pythondistdeps.py to Python 3

-- File Changes --

M scripts/pythondistdeps.py (2)

-- Patch Links --

https://github.com/rpm-software-management/rpm/pull/212.patch
https://github.com/rpm-software-management/rpm/pull/212.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/212
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Fix pythondistdeps.py --provides for Python wheels (#154)

2017-02-15 Thread torsava
Thank *you!*

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/154#issuecomment-280033726___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Fix pythondistdeps.py --provides for Python wheels (#154)

2017-02-15 Thread torsava
torsava commented on this pull request.



>  if not dist.py_version:
-warn("Version for {!r} has not been found".format(dist), 
RuntimeWarning)
-continue
+# Try to parse the Python version from the path the metadata
+# resides at (e.g. /usr/lib/pythonX.Y/site-packages/...)
+import re
+res = re.search("/python(?P\d+\.\d)/", path_item)

Indeed I did, it works. Raw string is great if you want to use known escape 
codes verbatim (\n, \r and the like), here it's superfluous. But it's a good 
practice so I added it now.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/154___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Fix pythondistdeps.py --provides for Python wheels (#154)

2017-02-15 Thread torsava
I've changed it to `\d+\.\d` so we don't get a Python Y2K equivalent. :)

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/154#issuecomment-279997276___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Fix pythondistdeps.py --provides for Python wheels (#154)

2017-02-15 Thread torsava
torsava commented on this pull request.



>  if not dist.py_version:
-warn("Version for {!r} has not been found".format(dist), 
RuntimeWarning)
-continue
+# Try to parse the Python version from the path the metadata
+# resides at (e.g. /usr/lib/pythonX.Y/site-packages/...)
+import re
+res = re.search("/python(?P[0-9.]{3})/", path_item)

Absolutely!

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/154___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


[Rpm-maint] [rpm-software-management/rpm] Fix pythondistdeps.py --provides for Python wheels (#154)

2017-02-14 Thread torsava
As Python wheels do not contain targetted Python version in the directory/file
name of their metadata like Python eggs do, and since the Python version is not
contained in the metadata either, it is necessary to get it from elsewhere.

Here it is parsed from the path the metadata resides at
(e.g. /usr/lib/pythonX.Y/site-packages/...)

For more information, [bugzilla report for 
Fedora](https://bugzilla.redhat.com/show_bug.cgi?id=1421776).
You can view, comment on, or merge this pull request online at:

  https://github.com/rpm-software-management/rpm/pull/154

-- Commit Summary --

  * Fix pythondistdeps.py --provides for Python wheels

-- File Changes --

M scripts/pythondistdeps.py (13)

-- Patch Links --

https://github.com/rpm-software-management/rpm/pull/154.patch
https://github.com/rpm-software-management/rpm/pull/154.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/154
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint