Re: [Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)
@s-t-e-v-e-n-k commented on this pull request. > @@ -143,10 +201,30 @@ def convert(name, operator, version_id): def normalize_name(name): """https://www.python.org/dev/peps/pep-0503/#normalized-names""; -import re Indeed -- 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_r514807140___ 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)
@s-t-e-v-e-n-k 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) I'm trying to keep the diff small, but I can move the function definition up if you wish. -- 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_r514806181___ 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)
@s-t-e-v-e-n-k commented on this pull request. > @@ -142,9 +210,23 @@ def convert(name, operator, version_id): def normalize_name(name): It can't, and I wish it could be -- https://src.fedoraproject.org/rpms/python-rpm-generators/blob/master/f/pythonbundles.py#_41 -- 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_r503587276___ 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)
@s-t-e-v-e-n-k 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() Ah, good point! staticmethod makes much more 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_r503586786___ 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)
@s-t-e-v-e-n-k pushed 1 commit. 29f742a6b45b0d33463b963347cd083a1ebb2a71 pythondistdeps: Switch to importlib.metadata -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1317/files/20ad7b9b4845386ddb54ce2a88465e00d992fd0c..29f742a6b45b0d33463b963347cd083a1ebb2a71 ___ 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)
@s-t-e-v-e-n-k 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 sorry, I misread that completely and conflated it with the argument
parsing, my apologies.
This would also require massive changes everywhere to stop using the key
property, which @torsava has seemed against -- but I'm happy to do so.
Secondly, this doesn't solve it for Distribution, so anything we do for
requirements we should also do for the Distribution class -- my current idea is
a classmethod on Req that we call there and in Distribution. And a consequence
the `normalize_name()` function gets removed as well.
`re.sub('[^A-Za-z0-9.]+', '-', self.name).lower()` is what Distribution current
does, which is what `pkg_resources.safe_name()` does.
--
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_r496409925___
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)
@s-t-e-v-e-n-k 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('_', '-')
Which is a massive layering violation. I'll have a think on 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/1317#discussion_r494312871___
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)
@s-t-e-v-e-n-k pushed 1 commit. 20ad7b9b4845386ddb54ce2a88465e00d992fd0c pythondistdeps: Switch to importlib.metadata -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1317/files/8a5494c5d22b620a071b73d8b3dc31f495a7cdcd..20ad7b9b4845386ddb54ce2a88465e00d992fd0c ___ 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)
@s-t-e-v-e-n-k commented on this pull request.
> -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())
Because dep.key is from setuptools' pkg_resources.Requirement and
packaging.Requirement does not include it -- I'm not against creating a
subclass of packing.Requirement here and including a key property that returns
self.name.lower()
--
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_r493154379___
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)
@torsava Hi, do you have a chance over the next few days to check this over again? -- 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-686888640___ 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)
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. -- 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-682312470___ 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)
Once I remove the comments, it doesn't parse because the path doesn't contain pythonX.Y, so it can't determine the python version, so we need to do something there anyway. -- 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-681320463___ 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)
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. -- 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-680652546___ 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)
Any updates on the test suite investigation? Is there anything I can do to help? -- 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-670355509___ 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)
As a further note about the packaging dependency -- pkg_resources has been using it since I think at least version 32 (I'd have to go back and check), and you've not noticed because setuptools vendors 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#issuecomment-662881474___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] [rpm-software-management/rpm] pythondistdeps: Switch to importlib.metadata (#1317)
Stop using the heavy-weight pkg_resources and switch to a subclass of importlib.metadata. This does add a dependency on packaging as well for requirement and version parsing. I have also removed the comments about the attempted rewrite, since I have addressed many of the concerns. You can view, comment on, or merge this pull request online at: https://github.com/rpm-software-management/rpm/pull/1317 -- Commit Summary -- * pythondistdeps: Switch to importlib.metadata -- File Changes -- M scripts/pythondistdeps.py (192) -- Patch Links -- https://github.com/rpm-software-management/rpm/pull/1317.patch https://github.com/rpm-software-management/rpm/pull/1317.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/1317 ___ 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)
Yes, flake8 warns on needing two blank lines, not one. https://www.flake8rules.com/rules/E302.html -- 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-614517974___ 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)
Would you like me to revert that part of the change? They are fairly self-contained, so easy enough to pull out in a separate change. -- 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-614494499___ 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)
@s-t-e-v-e-n-k pushed 1 commit. 0afbfbb67b8b5d8346271a7d8d2351ad4ed0f411 scripts/pythondistdeps: Switch to argparse -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1181/files/0ea13f93d5b179c416042274d80203c8ca6ae24f..0afbfbb67b8b5d8346271a7d8d2351ad4ed0f411 ___ 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)
@s-t-e-v-e-n-k commented on this pull request.
> +parser.add_argument('-P', '--provides', action='store_true', help='Print
> Provides')
+parser.add_argument('-R', '--requires', action='store_true', help='Print
Requires')
+parser.add_argument('-r', '--recommends', action='store_true', help='Print
Recommends')
+parser.add_argument('-C', '--conflicts', action='store_true', help='Print
Conflicts')
+parser.add_argument('-E', '--extras', action='store_true', help='Print Extras')
Ahh, now getopt didn't do that! :-P But sure, let me look at doing 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/1181#discussion_r409302119___
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: Switch to argparse (#1181)
Stop using getopt, and switch to a modern solution for parsing arguments. Drive-by fixing some flake8 warnings, mainly whitespace. You can view, comment on, or merge this pull request online at: https://github.com/rpm-software-management/rpm/pull/1181 -- Commit Summary -- * scripts/pythondistdeps: Switch to argparse -- File Changes -- M scripts/pythondistdeps.py (125) -- Patch Links -- https://github.com/rpm-software-management/rpm/pull/1181.patch https://github.com/rpm-software-management/rpm/pull/1181.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/1181 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
