Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: New test suite and various updates and fixes (#1195)
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)
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
Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: New test suite and various updates and fixes (#1195)
Don't bother. In the near future you can just use whatever you want for the tests: https://github.com/rpm-software-management/rpm/issues/1199#issuecomment-633979688 -- 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-633988092___ 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)
> 1. [...] That's not really different from when people work on their own > packages first and push it back upstream. Hmm, I thought this was in a separate repo actually, but I see it's just Fedora dist-git. Like noted in earlier comments, stuff developing their own test-suites is a certain step towards independence. > Having tests in autotest isn't a terrible problem, except for I don't really > if anyone really knows how to use autotest. I know it's the framework > autotools has for it, but how do we test Python code with it? 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 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. -- 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-633925317___ 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)
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)
At any rate, I'm totally fine with merging just the code right now and worry about the rest later. -- 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-633884333___ 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)
Maybe add a comment to the top of the script explaining the model: this is developed and tested at repository at `` and rpm only maintains a read-only copy of that, synced from time to time. At least that's how I perceive this thing. -- 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-633880805___ 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)
As a short term thing, we just need to merge in the code, so other changes don't diverge too much. Not having the tests here is something that I'd rather avoid -- how do we expect everybody to remember that the tests are somewhere else? -- 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-633852867___ 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)
Sorry for the holdup folks, I've been mulling over this quite a bit. The tests that is. The test-suite is a deal-breaker really. Rpm's test-suite is the autotest-based thing, and everything in rpm needs to use that for tests. We can't have individual bits and pieces bring in their own infra, dependencies and own make targets for integration. There are various possible paths here. One is adjusting the tests to use rpm's test-suite, but I sense there's a strong desire to have the tests runnable outside rpm. Which is perfectly understandable as the rest of the rpm tests are not relevant to this in the slightest. Quite clearly rpm is not the upstream here, it's more like downstream from where this is actually being developed. So we could also sync just the script itself, knowing that upstream has its own tests and we're just a distributor. Which opens the question of whether this is a meaningful arrangement again. I suppose the path of least resistance, at least short-term (think rpm 4.16), would be to acknowledge the actual development model for what it is and just pull the script changes into rpm without the 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/pull/1195#issuecomment-633836608___ 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)
I'd like to see this merged. -- 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-631572506___ 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)
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)
Since the high level discussion of whether this shall be split or not, can we please merge this for now to avoid the base diverging? -- 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-628550526___ 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)
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] scripts/pythondistdeps: New test suite and various updates and fixes (#1195)
This pull request **introduces 1 alert** when merging 90e68fa8f27a594067763476e69c723ab8beb971 into 022b48d21092f8a79103fa9318376fb26911e571 - [view on LGTM.com](https://lgtm.com/projects/g/rpm-software-management/rpm/rev/pr-6f63a05ddcb5d681a51a46456602e60a7916851b) **new alerts:** * 1 for Module is imported more than once -- 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-621355841___ 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)
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)
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] scripts/pythondistdeps: New test suite and various updates and fixes (#1195)
@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)
@hroncok 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. -- 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_r415943074___ 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)
@Conan-Kudo 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. LGTM -- 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_r415936789___ 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)
@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)
This pull request **introduces 1 alert** when merging e7cc87a083366439c0450c720ed36475b09d into 9a30bcdacca365a6d8c4078dc6b6f8014d31d66f - [view on LGTM.com](https://lgtm.com/projects/g/rpm-software-management/rpm/rev/pr-6a72d51c59c7dece06617335d467976326e344fd) **new alerts:** * 1 for Module is imported more than once -- 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-620054580___ 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)
> 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)
@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)
This pull request **introduces 1 alert** when merging 96be70abdb8f5b7633fb41829a8b5d496a466e03 into 9a30bcdacca365a6d8c4078dc6b6f8014d31d66f - [view on LGTM.com](https://lgtm.com/projects/g/rpm-software-management/rpm/rev/pr-37240692b220d858f7c84048312ded371ecb5b9f) **new alerts:** * 1 for Module is imported more than once -- 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-619945878___ 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)
@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)
@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)
@Conan-Kudo 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',
Hmm, okay
--
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_r415697446___
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)
@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)
> 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)
@Conan-Kudo 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. It's definitely the first. 藍 -- 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_r415688461___ 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)
@hroncok 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. Possible meanings of the emoji: - @Conan-Kudo is sad about the conclusion, but respects the way it was figured out. - The way this information is presented is making @Conan-Kudo sad. - @Conan-Kudo doesn't agree with the arguments presented in that commit message. - @Conan-Kudo is sad about current world situation and presented it here by accident. I hope it is the first listed. -- 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_r415687829___ 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)
@Conan-Kudo 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...
--
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_r415687029___
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)
@hroncok 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 agree that we don't *need* it. How big difference it is if we *support* 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/1195#discussion_r415685949___
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)
@Conan-Kudo 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',
Do we need comma-separated entries if we use `append`? Most people would call
it at most twice.
--
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_r415685129___
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)
@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)
@pmatilai I wish you were right, but in practice that is not what has happened. -- 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-619873262___ 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)
> Ultimately, the problem is that it not being in the rpm tarball means I have > no hammer to get people to discover it and use it. Unfortunately, the only > standardization point is in rpm itself. I would argue the opposite: people interested in Python are more likely to find and contribute to a python-specific project than this weird rpm thing with its archaic build- and test systems and all. -- 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-619872675___ 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)
@hroncok Ultimately, the problem is that it not being in the rpm tarball means I have no hammer to get people to discover it and use it. Unfortunately, the only standardization point is in rpm itself. I hesitate to split it out after spending so much work integrating it into rpm upstream to get everyone to start using 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/1195#issuecomment-619864395___ 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)
Having a separate repo here is actually not a compromise but IMHO the preferred option. rpm-extras was thought more as an interim solution for scripts that don't have a large enough contributor base to be projects on their own. Even better if people from multiple distributions can share the ownership of the repo. -- 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-619862405___ 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)
Totally fine by me. @torsava @Conan-Kudo @gordonmessmer @ignatenkobrain @s-t-e-v-e-n-k 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#issuecomment-619859881___ 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)
> Maybe there might be a compromise by having a separate repo in the > rpm-software-management namespace? That could be an option, and would be absolutely fine by me if it makes sense for the Python "SIG" around here. rpm-extras was supposed to be the place for scripts like these, but it's not catching on. And the way I see it, when a thing is mature enough to grow its own test-suite ecosystem, maybe its time for the parents to let go :sweat_smile: -- 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-619855467___ 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)
> We talk about upstreaming changes like this, but to me the situation looks > pretty much the opposite: this has an active upstream of its own and we're > merely syncing it from time to time, which is doing double work for ... I > don't know what. We are happy to maintain this ourselves. The idea was that if this lives here, other distros will use it (see the recent contribution by SUSE). Maybe there might be a compromise by having a separate repo in the rpm-software-management namespace? -- 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-619841494___ 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)
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. We talk about upstreaming changes like this, but to me the situation looks pretty much the opposite: this has an active upstream of its own and we're merely syncing it from time to time, which is doing double work for ... I don't know what. -- 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-619839716___ 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)
@Conan-Kudo requested changes 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',
Instead of making this comma separated, can we use `append` or `extend`
instead? It would make sense to store internally as a list. This would
eliminate the weird formatting requirements too...
> @@ -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.
--
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#pullrequestreview-400497608___
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)
@hroncok approved this pull request. FWIW I've reviewed this before the PR was opened. -- 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#pullrequestreview-400271090___ 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)
> **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
Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: New test suite and various updates and fixes (#1195)
This pull request **introduces 1 alert** when merging dbc365b59207f4b00624241ae25542dd3f0a83cd into ead227f2da98c223e4aa4a864f422722c1eb1e3c - [view on LGTM.com](https://lgtm.com/projects/g/rpm-software-management/rpm/rev/pr-2b24124a3278ffa623c1ef2c5b3fee7bf9b950c5) **new alerts:** * 1 for Module is imported more than once -- 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-618995335___ 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)
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
