* [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete @ 2016-01-10 21:40 Brian Dolbec 2016-01-10 22:28 ` Brian Dolbec 2016-01-11 15:39 ` Alexander Berntsen 0 siblings, 2 replies; 24+ messages in thread From: Brian Dolbec @ 2016-01-10 21:40 UTC (permalink / raw To: gentoo-portage-dev I have now pushed a fully rebased, properly sequenced commit history to the repoman branch. To be honest I think all the rebasing, re-testing took as much time as the initial conversion and testing. Anyway, I consider the branch now ready for final review. Since there are a total of 45 commits. I am not sending them to the list at this time. They are available for review via the repoman branch at https://gitweb.gentoo.org/proj/portage.git/log/?h=repoman https://github.com/gentoo/portage/commits/repoman or github pull request for commit order convienience https://github.com/gentoo/portage/pull/22 But please try to follow our standard rules and reply with any comments or code review to this email thread. I can also generate the email patches if necessary, but I fear they will just clog up your inbox. There are some patches that are quite large like the vcs plug-in conversion which could not be done piecemeal and still be able to run the code to test. Summary: This stage now has three plugin system loops to process all the checks and maintenance actions. The three loops are currently hard-coded and many of the checks must be run in a certain order as many of the checks add or modify the dynamic data required by later checks. I will endeavour to create a complete wiki page on the new system when approved, detailing how to create modules. I will also generate a table of dynamic data available and the modules responsible for it. In that way any modification to the modules run, added or removed can be determined. 1) Primary pkg level checks 2) Ebuild level checks 3) Final pkg level summary checks There are 2 primary plugin systems 1) The VCS plugin system 2) The Scan plugin system All use the existing portage plugin system used in emaint and the new sysnc systems. The scan plugin modules can have functions that run in any of the three loops. They can also have more that one function that runs in any one loop. All functions in the loop will be run in sequence in the order listed. The plugin system runs the functions via reference, not name, so the module lists the functions by reference. The runInPkgs, runInEbuild and runInFinal functions are designed in a way that allow for the most flexibility and control by the module itself. As such one loop level function can control the actions of the next levels. They are not forced to be hard-coded. Although, there are currently no modules that use or need that ability at this time. Still to do in further rewites: Create a file or files to store easily edited repository data that can be included in the repo metadata. In that way small changes can be done by maintainers without requiring a new repoman release because that data comes with the release. Make the plug-in checks loops configurable. Then the checks run can be configured for the repo being checked. All repos are not created equal and may have their own eclasses and eclass checks, etc.. Continued code optimizations. At this point many of the scan checks, etc. have not undergone extensive changes. Only what was necessary to fit the new structure. Many could probably be optimised for better configure-ability and execution. -- Brian Dolbec <dolsen> ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete 2016-01-10 21:40 [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete Brian Dolbec @ 2016-01-10 22:28 ` Brian Dolbec 2016-01-11 6:52 ` Brian Dolbec 2016-01-11 9:08 ` Brian Dolbec 2016-01-11 15:39 ` Alexander Berntsen 1 sibling, 2 replies; 24+ messages in thread From: Brian Dolbec @ 2016-01-10 22:28 UTC (permalink / raw To: gentoo-portage-dev On Sun, 10 Jan 2016 13:40:08 -0800 Brian Dolbec <dolsen@gentoo.org> wrote: > I have now pushed a fully rebased, properly sequenced commit > history to the repoman branch. To be honest I think all the rebasing, > re-testing took as much time as the initial conversion and testing. > Anyway, I consider the branch now ready for final review. Since there > are a total of 45 commits. I am not sending them to the list at this > time. They are available for review via the repoman branch at > > https://gitweb.gentoo.org/proj/portage.git/log/?h=repoman > > https://github.com/gentoo/portage/commits/repoman > > or github pull request for commit order convienience > > https://github.com/gentoo/portage/pull/22 > > But please try to follow our standard rules and reply with any > comments or code review to this email thread. OK, There are 2 test failures I need to fix. The patch to the plugin system seems to have broken emaint. Also there is one repoman test failure which is I think related to the metadata.dtd url change. -- Brian Dolbec <dolsen> ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete 2016-01-10 22:28 ` Brian Dolbec @ 2016-01-11 6:52 ` Brian Dolbec 2016-01-11 9:08 ` Brian Dolbec 1 sibling, 0 replies; 24+ messages in thread From: Brian Dolbec @ 2016-01-11 6:52 UTC (permalink / raw To: gentoo-portage-dev On Sun, 10 Jan 2016 14:28:51 -0800 Brian Dolbec <dolsen@gentoo.org> wrote: > On Sun, 10 Jan 2016 13:40:08 -0800 > Brian Dolbec <dolsen@gentoo.org> wrote: > > > I have now pushed a fully rebased, properly sequenced commit > > history to the repoman branch. To be honest I think all the > > rebasing, re-testing took as much time as the initial conversion > > and testing. Anyway, I consider the branch now ready for final > > review. Since there are a total of 45 commits. I am not sending > > them to the list at this time. They are available for review via > > the repoman branch at > > > > https://gitweb.gentoo.org/proj/portage.git/log/?h=repoman > > > > https://github.com/gentoo/portage/commits/repoman > > > > or github pull request for commit order convienience > > > > https://github.com/gentoo/portage/pull/22 > > > > But please try to follow our standard rules and reply with any > > comments or code review to this email thread. > > OK, There are 2 test failures I need to fix. The patch to the plugin > system seems to have broken emaint. Also there is one repoman test > failure which is I think related to the metadata.dtd url change. > I've fixed and rebased in the emaint breakage. As for the repoman test failure. The test data set up for test_simple has None as the vcs type. When I added code to get past the vcs_preserves_mtime check. It then fails for the ${VCS}changes file and class. So, it looks like I need to create a "None" vcs plugin. Then it will handle all info/functions just like any of the real vcs types. This is a side effect of spltting up the code from if cvs in ('foo', 'bar')... code into self directing code for the vcs type. -- Brian Dolbec <dolsen> ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete 2016-01-10 22:28 ` Brian Dolbec 2016-01-11 6:52 ` Brian Dolbec @ 2016-01-11 9:08 ` Brian Dolbec 1 sibling, 0 replies; 24+ messages in thread From: Brian Dolbec @ 2016-01-11 9:08 UTC (permalink / raw To: gentoo-portage-dev On Sun, 10 Jan 2016 14:28:51 -0800 Brian Dolbec <dolsen@gentoo.org> wrote: > On Sun, 10 Jan 2016 13:40:08 -0800 > Brian Dolbec <dolsen@gentoo.org> wrote: > > > I have now pushed a fully rebased, properly sequenced commit > > history to the repoman branch. To be honest I think all the > > rebasing, re-testing took as much time as the initial conversion > > and testing. Anyway, I consider the branch now ready for final > > review. Since there are a total of 45 commits. I am not sending > > them to the list at this time. They are available for review via > > the repoman branch at > > > > https://gitweb.gentoo.org/proj/portage.git/log/?h=repoman > > > > https://github.com/gentoo/portage/commits/repoman > > > > or github pull request for commit order convienience > > > > https://github.com/gentoo/portage/pull/22 > > > > But please try to follow our standard rules and reply with any > > comments or code review to this email thread. > > OK, There are 2 test failures I need to fix. The patch to the plugin > system seems to have broken emaint. Also there is one repoman test > failure which is I think related to the metadata.dtd url change. > Turned out to be more than just the None type vcs related failures. It also is failing due to me modifying the metadata.dtd url in repoman metadata.py updating it to the https://... url. But here is the strange part. I've traced the trouble to the resolverpalyground metadata.xml template still having it set to http://... BUT, it only correctly fails in python2.7 and pypy. Py3* it incorrectly passes tests. Both locally and Travis CI So, I believe that something must be broken if the tests pass in py3, but not py2/pypy for that type of static data. Since the data does not change, it should fail equally in all tested python versions. Zac, Arfrever, any ideas why? OK, after editing the template in tests/resolver/resolverplayground.py, I'm getting all passes locally. Not that it means much at this point. Summary: | Version | Status |-------------------- | 2.7 | PASS | 3.3 | PASS | 3.4 | PASS | pypy | PASS brian@professor-x ~/Dev/git/portage $ Here is the debug=True repoman/test_simple.py output: brian@professor-x ~/Dev/git/portage/pym/portage/tests $ python2.7 ./runTests.py repoman/test_simple.py testCopyrightUpdate (portage.tests.repoman.test_simple.SimpleRepomanTestCase) ... ok testSimple (portage.tests.repoman.test_simple.SimpleRepomanTestCase) ... >>> Creating Manifest for /tmp/tmpwfodHK/var/repositories/test_repo/dev-libs/A >>> Creating Manifest >>> for /tmp/tmpwfodHK/var/repositories/test_repo/dev-libs/A Creating >>> Manifest for /tmp/tmpwfodHK/var/repositories/test_repo/dev-libs/B >>> Creating Manifest >>> for /tmp/tmpwfodHK/var/repositories/test_repo/dev-libs/C RepoMan scours the neighborhood... ebuild.badheader 4 dev-libs/A/A-0.ebuild: Malformed Id header on line: 3 dev-libs/A/A-1.ebuild: Malformed Id header on line: 3 dev-libs/B/B-1.ebuild: Malformed Id header on line: 3 dev-libs/C/C-0.ebuild: Malformed Id header on line: 3 metadata.bad [fatal] 3 dev-libs/A/metadata.xml: DOCTYPE: SYSTEM should refer to 'https://www.gentoo.org/dtd/metadata.dtd', not 'http://www.gentoo.org/dtd/metadata.dtd' dev-libs/B/metadata.xml: DOCTYPE: SYSTEM should refer to 'https://www.gentoo.org/dtd/metadata.dtd', not 'http://www.gentoo.org/dtd/metadata.dtd' dev-libs/C/metadata.xml: DOCTYPE: SYSTEM should refer to 'https://www.gentoo.org/dtd/metadata.dtd', not 'http://www.gentoo.org/dtd/metadata.dtd' Note: use --include-dev (-d) to check dependencies for 'dev' profiles Please fix these important QA issues first. RepoMan sez: "Make your QA payment on time and you'll never see the likes of me." EROOT=/tmp/tmpwfodHK/ FAIL ====================================================================== FAIL: testSimple (portage.tests.repoman.test_simple.SimpleRepomanTestCase) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/brian/Dev/git/portage/pym/portage/tests/__init__.py", line 222, in run testMethod() File "/home/brian/Dev/git/portage/pym/portage/tests/repoman/test_simple.py", line 307, in testSimple "repoman failed in %s" % (cwd,)) AssertionError: repoman failed in ---------------------------------------------------------------------- Ran 2 tests in 0.621s FAILED (failures=1) -- Brian Dolbec <dolsen> ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete 2016-01-10 21:40 [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete Brian Dolbec 2016-01-10 22:28 ` Brian Dolbec @ 2016-01-11 15:39 ` Alexander Berntsen 2016-03-05 21:37 ` Brian Dolbec 1 sibling, 1 reply; 24+ messages in thread From: Alexander Berntsen @ 2016-01-11 15:39 UTC (permalink / raw To: gentoo-portage-dev -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 I read through these ages ago, and I think you've incorporated my feedback from back then. But like back then, I don't have too much feedback to offer as this is certainly not an area of Portage I'm nearly as familiar with as you are. It mostly just LGTM. There are some minor issues with commit messages. Like how they randomly don't have full stops in some sentences for some reason. "fetches.py: Create new isVcsDir() in the vcs plugins" says "teh" instead of "the". ThirdPartyMirrors is a bit low on documentation. I expect Zac will be of far more help than me. In any event, thanks a lot for working on this! You asked about a release on IRC -- did you want to get a release out before merging this? I would be fine with either way. Let me know what you need from me, and I'll get on it ASAP. - -- Alexander bernalex@gentoo.org https://secure.plaimi.net/~alexander -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCgAGBQJWk8yvAAoJENQqWdRUGk8BgVMQAJXFEpIHZ++Cp08rJ4RQhwJr MgAqqpfSO54KKuAQTQ0FZLpWI698YtrIRWMMo9cysJCW9usCgFp/TaB6k78ci/2/ NSu+LRhS4MjP+aZG/RiIcXThUQCTVWOFD3T5InWEek8i2Gasc8nZVNjZhHUn/G/R mRMEU0utsv8oueIX1XSYeQfW9jtD5HLJlpF4LEFSHufpzQA7ZJyy63URU/FmP3Tt n4Z6KRCzzxQEpv8UFG7bFjTS4rSUfZ1EXsmGuAbc33G+ZM7kQFsb86SKhLLDmAOv bOamyjhHWW5jRk7+R1O7lWNwZrisfTGLVWPRj8CjFko+Bl9bXjwKpQEzGPNTpOPe ocbjilxvhRoOoKJ945Q5ylsYfHGZD2uPyuSRM03KJHCptRsgM99zrrbCKakw8Aht DGfJlKcVrysrbR8NdmI/a5hBq6p7LqAYiPxXTfCwjexooEF6aMVxr1HGRabZMeeu aJdbmb6Ajk4i4b7xDzPpofivENdTTLwZrG+pi/0ZaEI0xvw1JiI1FGm7VLiHh2Oe GBJoKl/JQa63QJivJIL83OtUT4FYP3ZOnWSuhHj/p1LtzAyJU+FptjhCHlFdnl32 79ML0g/7RJAB8ackxCeruF8laclKIvKJNhwBYXbGzhOpmjU4Gp/fo6HieQBWvBlo CMfX/Ufv6M5gfgVgDJf1 =18rr -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete 2016-01-11 15:39 ` Alexander Berntsen @ 2016-03-05 21:37 ` Brian Dolbec 2016-03-07 12:21 ` Alexander Berntsen 2016-03-14 17:14 ` Zac Medico 0 siblings, 2 replies; 24+ messages in thread From: Brian Dolbec @ 2016-03-05 21:37 UTC (permalink / raw To: gentoo-portage-dev -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 On Mon, 11 Jan 2016 16:39:27 +0100 Alexander Berntsen <bernalex@gentoo.org> wrote: > I read through these ages ago, and I think you've incorporated my > feedback from back then. But like back then, I don't have too much > feedback to offer as this is certainly not an area of Portage I'm > nearly as familiar with as you are. It mostly just LGTM. > > There are some minor issues with commit messages. Like how they > randomly don't have full stops in some sentences for some reason. > "fetches.py: Create new isVcsDir() in the vcs plugins" says "teh" > instead of "the". > > ThirdPartyMirrors is a bit low on documentation. > > I expect Zac will be of far more help than me. In any event, thanks a > lot for working on this! > > > You asked about a release on IRC -- did you want to get a release out > before merging this? I would be fine with either way. Let me know what > you need from me, and I'll get on it ASAP. > - -- > Alexander > bernalex@gentoo.org > https://secure.plaimi.net/~alexander > Zac, I'm done with code changes in the rewrite. Ready for a last look before a merge. Can you have a look again? I did some changes/fixes and rebased them in. Floppym hasn't reported any more bugs, so I think it's ready for broader testing in a release. Then we can work on moving all the test data to a separate file in the tree or downloaded... I am just trying to get the docstrings done and then merge. - -- Brian Dolbec <dolsen> -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.1 iQJ8BAEBCgBmBQJW21GBXxSAAAAAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25zLm9w ZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXRBNUQ3Qzc0RTA4MUNDNzBEQjRBNEFBRjVG QkJEMDg3Mjc1ODIwRUQ4AAoJEPu9CHJ1gg7YUbEQAKCx30TOiuIwMiRU4Sp+8XqJ 05K20eRfidQYvnfLp/7jnpBcB1l2yjqdcC1EQCS9DeFuGsmQQd62Ggq14S2HvC2F jKF9DoQZaXOD/2l+CmE3xKkS88ksXsEzN/vm8DSAJsyc7cux/lvXUYS1fRd10DA6 skWTz985u72tAvAgZoAaUx1kx+6Ugy63aegjAf0Zitw16v72bDff+fDYjC8Ev0dk gCgzjJ8VI1DDxz8NWR2V6keRn27fHqBFWTRYlGfXErbMpbPhA/+iQwyphPZLkT6I q7wnFIc+qP4sJN3997c2wo523eRlYchWZdK5tDe8VXM9c0J+KB0GqEvE5Y5QS21E HmInRQJm5YjfwhNm7Lv5dvrQKA2SgMaMjPtPPpmXrTCosKFeJ5LBfsh5yvfwUXLZ N691rqqfro97vIGd7GKDbLIKqfjYf28FB9NCsPmh+Y+0BEb18bkoZwB1HkuWiqjQ kdI9IJqMuFd6rbIUU8ONdYyBdG1rv+G01zFZSXuTYacNVUC2W+Nc1LTbil1FW7S2 mzYplnVHfjjfKgCcRbxOzOFe4drv03Ek12DwT+Oj/r5l0Kz+V3tWtLLYbRiJCS7b YBZNijNLHAG/GOTCq5eNpnjc+n7R0zRWtsbxl861kpyQHvrYs+R8VSC25zooKPYr AXWn0HjAAYOkwDRjkBjl =LQn3 -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete 2016-03-05 21:37 ` Brian Dolbec @ 2016-03-07 12:21 ` Alexander Berntsen 2016-03-14 17:14 ` Zac Medico 1 sibling, 0 replies; 24+ messages in thread From: Alexander Berntsen @ 2016-03-07 12:21 UTC (permalink / raw To: gentoo-portage-dev -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 Great work, Brian! \o/ - -- Alexander bernalex@gentoo.org https://secure.plaimi.net/~alexander -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCgAGBQJW3XIxAAoJENQqWdRUGk8BmCQP/i5HwyFA/Ye+Vzd2D1jOrJrK U+E73kqnzb5AoEgRXThgn9OPI6piBnLudj57MvdhKjyhcZi/naO9YmAXX61VLbQI T/sqnuw433zASt9VEcVjHCV3UGvD/S8ulPEhfiwAYTInR8dWara5FwXuahwONO5g 9t1Cbe9+Z633ussSDKB7OJ44VtnN9D5pP5Kmsuru+N9di4dGKMqxl90LJ15Wh+3X /KxqDR21hcCl5aBGQo4YPKkEJFaO58tK67jbHd9R7i/Yu7Rx/oSPHNflym4lhzeV 28OLDbVrma3ZYVuGi+wUMtDBDywfehJqPDDiKPd0niJI4AU8ywfyJZbeP3dIVEsm sdpml0yKZvl/EXTBYERYdNLrJe32YXubtITr+F4x/KBoMCBEtIWPwd8XdPbyK6KV Qa9bF6Y6S2Z+4Z4hZLf5LCbpuEXonZL6NrkYoOU5xhb+5+rKZEtYKI7k9nvlV2DL YNf7skXN6xLRayY12tg1zPS9I3qH6+36tlqUODgJENyzmKGVUoJPriQUc0psEfJh WPZJktZExybINqDFJpn5BKOtf3+aMhmhjcnM7XM8rQJXtXoiBb9qp+9mtX5OqLL0 gMK+Rf4sp9V0zt73qraqwlx1JzH/YMkdGWoCv4fLTN5zTmZwC2pqQelSfZ8IjZ7s WPQ55hITaddbUx2sdpn2 =LZzu -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete 2016-03-05 21:37 ` Brian Dolbec 2016-03-07 12:21 ` Alexander Berntsen @ 2016-03-14 17:14 ` Zac Medico 2016-03-14 17:22 ` Zac Medico 2016-03-14 17:35 ` Brian Dolbec 1 sibling, 2 replies; 24+ messages in thread From: Zac Medico @ 2016-03-14 17:14 UTC (permalink / raw To: gentoo-portage-dev On 03/05/2016 01:37 PM, Brian Dolbec wrote: > Zac, I'm done with code changes in the rewrite. Ready for a last look > before a merge. Can you have a look again? I did some changes/fixes > and rebased them in. Floppym hasn't reported any more bugs, so I think > it's ready for broader testing in a release. Then we can work on > moving all the test data to a separate file in the tree or downloaded... The dynamic_data stuff in Scanner is a little hard to follow. Then it calls dynamic_data.update(rdata), is there any chance that the update operation might clobber something that shouldn't have been clobbered? Also, I see that the number of sys.exit calls in pym/repoman is up from 37 to 41. I would prefer that we raise exceptions for the caller to handle, rather than call sys.exit directly. I suppose we change that later though, after we merge the code. -- Thanks, Zac ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete 2016-03-14 17:14 ` Zac Medico @ 2016-03-14 17:22 ` Zac Medico 2016-03-14 17:52 ` Brian Dolbec 2016-03-14 17:35 ` Brian Dolbec 1 sibling, 1 reply; 24+ messages in thread From: Zac Medico @ 2016-03-14 17:22 UTC (permalink / raw To: gentoo-portage-dev On 03/14/2016 10:14 AM, Zac Medico wrote: > On 03/05/2016 01:37 PM, Brian Dolbec wrote: > >> Zac, I'm done with code changes in the rewrite. Ready for a last look >> before a merge. Can you have a look again? I did some changes/fixes >> and rebased them in. Floppym hasn't reported any more bugs, so I think >> it's ready for broader testing in a release. Then we can work on >> moving all the test data to a separate file in the tree or downloaded... > > The dynamic_data stuff in Scanner is a little hard to follow. Then it > calls dynamic_data.update(rdata), is there any chance that the update > operation might clobber something that shouldn't have been clobbered? To clarify my question, suppose that one function returns {'foo': True} and another one returns {'foo', False}, so now there first {'foo': True} setting is forgotten. Is that going to be a problem? -- Thanks, Zac ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete 2016-03-14 17:22 ` Zac Medico @ 2016-03-14 17:52 ` Brian Dolbec 2016-03-15 0:18 ` Zac Medico 0 siblings, 1 reply; 24+ messages in thread From: Brian Dolbec @ 2016-03-14 17:52 UTC (permalink / raw To: gentoo-portage-dev On Mon, 14 Mar 2016 10:22:11 -0700 Zac Medico <zmedico@gentoo.org> wrote: > On 03/14/2016 10:14 AM, Zac Medico wrote: > > On 03/05/2016 01:37 PM, Brian Dolbec wrote: > > > >> Zac, I'm done with code changes in the rewrite. Ready for a last > >> look before a merge. Can you have a look again? I did some > >> changes/fixes and rebased them in. Floppym hasn't reported any > >> more bugs, so I think it's ready for broader testing in a > >> release. Then we can work on moving all the test data to a > >> separate file in the tree or downloaded... > > > > The dynamic_data stuff in Scanner is a little hard to follow. Then > > it calls dynamic_data.update(rdata), is there any chance that the > > update operation might clobber something that shouldn't have been > > clobbered? > > To clarify my question, suppose that one function returns {'foo': > True} and another one returns {'foo', False}, so now there first > {'foo': True} setting is forgotten. Is that going to be a problem? No, as stated in my other reply. There are only a few things that are modified. Mostly as I made a new module, following the original order the checks were run. As data was discovered missing it was added to dynamic_data from the previous check that supplied it to the Scanner class. So, only data needed later was passed back to update the dynamic_data. Also all those checks originally ran in one huge 1k LOC loop with another slightly smaller ebuild loop nested inside it. So all those variables were subject to change already by previous code run. In the stage1 rewrite, I/we did the same thing in creating the separated checks classes. After the check was done, only the data required was brought back into the primary loop. -- Brian Dolbec <dolsen> ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete 2016-03-14 17:52 ` Brian Dolbec @ 2016-03-15 0:18 ` Zac Medico 2016-03-15 0:47 ` Brian Dolbec 0 siblings, 1 reply; 24+ messages in thread From: Zac Medico @ 2016-03-15 0:18 UTC (permalink / raw To: gentoo-portage-dev On 03/14/2016 10:52 AM, Brian Dolbec wrote: > On Mon, 14 Mar 2016 10:22:11 -0700 > Zac Medico <zmedico@gentoo.org> wrote: > >> On 03/14/2016 10:14 AM, Zac Medico wrote: >>> On 03/05/2016 01:37 PM, Brian Dolbec wrote: >>> >>>> Zac, I'm done with code changes in the rewrite. Ready for a last >>>> look before a merge. Can you have a look again? I did some >>>> changes/fixes and rebased them in. Floppym hasn't reported any >>>> more bugs, so I think it's ready for broader testing in a >>>> release. Then we can work on moving all the test data to a >>>> separate file in the tree or downloaded... >>> >>> The dynamic_data stuff in Scanner is a little hard to follow. Then >>> it calls dynamic_data.update(rdata), is there any chance that the >>> update operation might clobber something that shouldn't have been >>> clobbered? >> >> To clarify my question, suppose that one function returns {'foo': >> True} and another one returns {'foo', False}, so now there first >> {'foo': True} setting is forgotten. Is that going to be a problem? > > No, as stated in my other reply. There are only a few things that are > modified. Mostly as I made a new module, following the original > order the checks were run. As data was discovered missing it was added > to dynamic_data from the previous check that supplied it to the Scanner > class. So, only data needed later was passed back to update the > dynamic_data. > > Also all those checks originally ran in one huge 1k LOC loop with > another slightly smaller ebuild loop nested inside it. So all those > variables were subject to change already by previous code run. In the > stage1 rewrite, I/we did the same thing in creating the separated > checks classes. After the check was done, only the data required was > brought back into the primary loop. > I've found what may be a real instance of the kind of problem I was worried about. The 'allvalid' key is set in both pym/repoman/modules/scan/ebuild/isebuild.py and pym/repoman/modules/scan/ebuild/ebuild.py, and its non-trivial for me to determine whether this is a real problem or not. -- Thanks, Zac ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete 2016-03-15 0:18 ` Zac Medico @ 2016-03-15 0:47 ` Brian Dolbec 2016-03-15 1:04 ` Zac Medico 0 siblings, 1 reply; 24+ messages in thread From: Brian Dolbec @ 2016-03-15 0:47 UTC (permalink / raw To: gentoo-portage-dev On Mon, 14 Mar 2016 17:18:27 -0700 Zac Medico <zmedico@gentoo.org> wrote: > On 03/14/2016 10:52 AM, Brian Dolbec wrote: > > On Mon, 14 Mar 2016 10:22:11 -0700 > > Zac Medico <zmedico@gentoo.org> wrote: > > > >> On 03/14/2016 10:14 AM, Zac Medico wrote: > >>> On 03/05/2016 01:37 PM, Brian Dolbec wrote: > >>> > >>>> Zac, I'm done with code changes in the rewrite. Ready for a last > >>>> look before a merge. Can you have a look again? I did some > >>>> changes/fixes and rebased them in. Floppym hasn't reported any > >>>> more bugs, so I think it's ready for broader testing in a > >>>> release. Then we can work on moving all the test data to a > >>>> separate file in the tree or downloaded... > >>> > >>> The dynamic_data stuff in Scanner is a little hard to follow. Then > >>> it calls dynamic_data.update(rdata), is there any chance that the > >>> update operation might clobber something that shouldn't have been > >>> clobbered? > >> > >> To clarify my question, suppose that one function returns {'foo': > >> True} and another one returns {'foo', False}, so now there first > >> {'foo': True} setting is forgotten. Is that going to be a > >> problem? > > > > No, as stated in my other reply. There are only a few things that > > are modified. Mostly as I made a new module, following the original > > order the checks were run. As data was discovered missing it was > > added to dynamic_data from the previous check that supplied it to > > the Scanner class. So, only data needed later was passed back to > > update the dynamic_data. > > > > Also all those checks originally ran in one huge 1k LOC loop with > > another slightly smaller ebuild loop nested inside it. So all those > > variables were subject to change already by previous code run. In > > the stage1 rewrite, I/we did the same thing in creating the > > separated checks classes. After the check was done, only the data > > required was brought back into the primary loop. > > > > I've found what may be a real instance of the kind of problem I was > worried about. The 'allvalid' key is set in both > pym/repoman/modules/scan/ebuild/isebuild.py and > pym/repoman/modules/scan/ebuild/ebuild.py, and its non-trivial for me > to determine whether this is a real problem or not. It is not a problem. allvalid is initialized in the isEbuild class at the start of the pkg level checks to True, then there are several things that if found reset it to False... Then when it gets to the ebuild level checks the ebuild check there just sets it False if the pkg.invalid variable is True. In some cases it might be a double False, but never will it trounce the previous setting. The only consumer for that allvalid variable is the metadata UnusedCheck class. So the allvalid variable is True until found False by whichever checks along the way find it to be False. Like a fuse, it's good until it's blown, then it can never be good again. I don't think this particular variable justifies a special class that more fully mimics a fuse. Impossible to reset it like a breaker. To be honest I did not look into the pkg.invalid variable's need to be setting it False in the Ebuild class. It may in fact be a dupe of a setting in the isEbuild class or it might be moved there. That original spaghetti code could in fact have had duplicates since it was so hard to figure out where to embed something. -- Brian Dolbec <dolsen> ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete 2016-03-15 0:47 ` Brian Dolbec @ 2016-03-15 1:04 ` Zac Medico 2016-03-15 6:24 ` Zac Medico 2016-03-15 19:04 ` Brian Dolbec 0 siblings, 2 replies; 24+ messages in thread From: Zac Medico @ 2016-03-15 1:04 UTC (permalink / raw To: gentoo-portage-dev On 03/14/2016 05:47 PM, Brian Dolbec wrote: > On Mon, 14 Mar 2016 17:18:27 -0700 > Zac Medico <zmedico@gentoo.org> wrote: > >> On 03/14/2016 10:52 AM, Brian Dolbec wrote: >>> On Mon, 14 Mar 2016 10:22:11 -0700 >>> Zac Medico <zmedico@gentoo.org> wrote: >>> >>>> On 03/14/2016 10:14 AM, Zac Medico wrote: >>>>> On 03/05/2016 01:37 PM, Brian Dolbec wrote: >>>>> >>>>>> Zac, I'm done with code changes in the rewrite. Ready for a last >>>>>> look before a merge. Can you have a look again? I did some >>>>>> changes/fixes and rebased them in. Floppym hasn't reported any >>>>>> more bugs, so I think it's ready for broader testing in a >>>>>> release. Then we can work on moving all the test data to a >>>>>> separate file in the tree or downloaded... >>>>> >>>>> The dynamic_data stuff in Scanner is a little hard to follow. Then >>>>> it calls dynamic_data.update(rdata), is there any chance that the >>>>> update operation might clobber something that shouldn't have been >>>>> clobbered? >>>> >>>> To clarify my question, suppose that one function returns {'foo': >>>> True} and another one returns {'foo', False}, so now there first >>>> {'foo': True} setting is forgotten. Is that going to be a >>>> problem? >>> >>> No, as stated in my other reply. There are only a few things that >>> are modified. Mostly as I made a new module, following the original >>> order the checks were run. As data was discovered missing it was >>> added to dynamic_data from the previous check that supplied it to >>> the Scanner class. So, only data needed later was passed back to >>> update the dynamic_data. >>> >>> Also all those checks originally ran in one huge 1k LOC loop with >>> another slightly smaller ebuild loop nested inside it. So all those >>> variables were subject to change already by previous code run. In >>> the stage1 rewrite, I/we did the same thing in creating the >>> separated checks classes. After the check was done, only the data >>> required was brought back into the primary loop. >>> >> >> I've found what may be a real instance of the kind of problem I was >> worried about. The 'allvalid' key is set in both >> pym/repoman/modules/scan/ebuild/isebuild.py and >> pym/repoman/modules/scan/ebuild/ebuild.py, and its non-trivial for me >> to determine whether this is a real problem or not. > > It is not a problem. > > allvalid is initialized in the isEbuild class at the start of the pkg > level checks to True, then there are several things that if found reset > it to False... > > Then when it gets to the ebuild level checks the ebuild check there > just sets it False if the pkg.invalid variable is True. In some cases > it might be a double False, but never will it trounce the previous > setting. Makes sense, thanks for the explanation. > The only consumer for that allvalid variable is the metadata > UnusedCheck class. > > So the allvalid variable is True until found False > by whichever checks along the way find it to be False. Like a fuse, > it's good until it's blown, then it can never be good again. I don't > think this particular variable justifies a special class that more > fully mimics a fuse. Impossible to reset it like a breaker. Yeah, let's do it. It's a great opportunity to add clarity to the code, and prevent future goofs. > To be honest I did not look into the pkg.invalid variable's need to be > setting it False in the Ebuild class. It may in fact be a dupe of a > setting in the isEbuild class or it might be moved there. That original > spaghetti code could in fact have had duplicates since it was so hard > to figure out where to embed something. > It's not a dupe. The Package.invalid property is used to implement various checks that are not duplicated elsewhere. -- Thanks, Zac ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete 2016-03-15 1:04 ` Zac Medico @ 2016-03-15 6:24 ` Zac Medico 2016-03-15 19:04 ` Brian Dolbec 1 sibling, 0 replies; 24+ messages in thread From: Zac Medico @ 2016-03-15 6:24 UTC (permalink / raw To: gentoo-portage-dev, dolsen On 03/14/2016 06:04 PM, Zac Medico wrote: >> The only consumer for that allvalid variable is the metadata >> UnusedCheck class. >> >> So the allvalid variable is True until found False >> by whichever checks along the way find it to be False. Like a fuse, >> it's good until it's blown, then it can never be good again. I don't >> think this particular variable justifies a special class that more >> fully mimics a fuse. Impossible to reset it like a breaker. > > Yeah, let's do it. It's a great opportunity to add clarity to the code, > and prevent future goofs. We could have modules declare all of their input/output keys, filter input keys to only include those requested by the module, and trigger an error if a module's invocation returns unexpected key(s). That way, the interfaces will be clearly defined (we can even assert specific types if we want), and it will be much easier for a person to reason about the system. -- Thanks, Zac ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete 2016-03-15 1:04 ` Zac Medico 2016-03-15 6:24 ` Zac Medico @ 2016-03-15 19:04 ` Brian Dolbec 2016-03-15 19:38 ` Zac Medico 1 sibling, 1 reply; 24+ messages in thread From: Brian Dolbec @ 2016-03-15 19:04 UTC (permalink / raw To: gentoo-portage-dev On Mon, 14 Mar 2016 18:04:56 -0700 Zac Medico <zmedico@gentoo.org> wrote: > > The only consumer for that allvalid variable is the metadata > > UnusedCheck class. > > > > So the allvalid variable is True until found False > > by whichever checks along the way find it to be False. Like a fuse, > > it's good until it's blown, then it can never be good again. I > > don't think this particular variable justifies a special class that > > more fully mimics a fuse. Impossible to reset it like a breaker. > > Yeah, let's do it. It's a great opportunity to add clarity to the > code, and prevent future goofs. > Done, it is now dynamic_data['validity_fuse'] which is a Fuse instance. P.S. I tried, but I could only rebase one of your 2 patches to the current repoman code. I could not apply the unboundlocal error patch. It looks like it is not valid for the cvs and svn modules now that they are separate modules. -- Brian Dolbec <dolsen> ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete 2016-03-15 19:04 ` Brian Dolbec @ 2016-03-15 19:38 ` Zac Medico 2016-03-15 19:42 ` Zac Medico 0 siblings, 1 reply; 24+ messages in thread From: Zac Medico @ 2016-03-15 19:38 UTC (permalink / raw To: gentoo-portage-dev On 03/15/2016 12:04 PM, Brian Dolbec wrote: > On Mon, 14 Mar 2016 18:04:56 -0700 > Zac Medico <zmedico@gentoo.org> wrote: > > >>> The only consumer for that allvalid variable is the metadata >>> UnusedCheck class. >>> >>> So the allvalid variable is True until found False >>> by whichever checks along the way find it to be False. Like a fuse, >>> it's good until it's blown, then it can never be good again. I >>> don't think this particular variable justifies a special class that >>> more fully mimics a fuse. Impossible to reset it like a breaker. >> >> Yeah, let's do it. It's a great opportunity to add clarity to the >> code, and prevent future goofs. >> > > Done, it is now dynamic_data['validity_fuse'] which is a Fuse instance. Nice, thank you! We can also use Fuse for the 'can_force' boolean, right? -- Thanks, Zac ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete 2016-03-15 19:38 ` Zac Medico @ 2016-03-15 19:42 ` Zac Medico [not found] ` <20160315131731.2edf502d.dolsen@gentoo.org> 0 siblings, 1 reply; 24+ messages in thread From: Zac Medico @ 2016-03-15 19:42 UTC (permalink / raw To: gentoo-portage-dev, Brian Dolbec On 03/15/2016 12:38 PM, Zac Medico wrote: > On 03/15/2016 12:04 PM, Brian Dolbec wrote: >> On Mon, 14 Mar 2016 18:04:56 -0700 >> Zac Medico <zmedico@gentoo.org> wrote: >> >> >>>> The only consumer for that allvalid variable is the metadata >>>> UnusedCheck class. >>>> >>>> So the allvalid variable is True until found False >>>> by whichever checks along the way find it to be False. Like a fuse, >>>> it's good until it's blown, then it can never be good again. I >>>> don't think this particular variable justifies a special class that >>>> more fully mimics a fuse. Impossible to reset it like a breaker. >>> >>> Yeah, let's do it. It's a great opportunity to add clarity to the >>> code, and prevent future goofs. >>> >> >> Done, it is now dynamic_data['validity_fuse'] which is a Fuse instance. > > Nice, thank you! > > We can also use Fuse for the 'can_force' boolean, right? > For 'changed' as well. -- Thanks, Zac ^ permalink raw reply [flat|nested] 24+ messages in thread
[parent not found: <20160315131731.2edf502d.dolsen@gentoo.org>]
* Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete [not found] ` <20160315131731.2edf502d.dolsen@gentoo.org> @ 2016-03-15 20:25 ` Zac Medico 2016-03-15 20:31 ` Zac Medico 0 siblings, 1 reply; 24+ messages in thread From: Zac Medico @ 2016-03-15 20:25 UTC (permalink / raw To: Brian Dolbec, gentoo-portage-dev On 03/15/2016 01:17 PM, Brian Dolbec wrote: > On Tue, 15 Mar 2016 12:42:51 -0700 > Zac Medico <zmedico@gentoo.org> wrote: > >> On 03/15/2016 12:38 PM, Zac Medico wrote: >>> On 03/15/2016 12:04 PM, Brian Dolbec wrote: >>>> On Mon, 14 Mar 2016 18:04:56 -0700 >>>> Zac Medico <zmedico@gentoo.org> wrote: >>>> >>>> >>>>>> The only consumer for that allvalid variable is the metadata >>>>>> UnusedCheck class. >>>>>> >>>>>> So the allvalid variable is True until found False >>>>>> by whichever checks along the way find it to be False. Like a >>>>>> fuse, it's good until it's blown, then it can never be good >>>>>> again. I don't think this particular variable justifies a >>>>>> special class that more fully mimics a fuse. Impossible to >>>>>> reset it like a breaker. >>>>> >>>>> Yeah, let's do it. It's a great opportunity to add clarity to the >>>>> code, and prevent future goofs. >>>>> >>>> >>>> Done, it is now dynamic_data['validity_fuse'] which is a Fuse >>>> instance. >>> >>> Nice, thank you! >>> >>> We can also use Fuse for the 'can_force' boolean, right? >>> >> >> For 'changed' as well. > > can_force, is yes > > changed is a no. It is the VCS module Changes class instance. I see > now that I described it wrong in the docstrings. Maybe I should rename > it for better clarity to changes_inst or vcs_changes... ideas? Maybe 'changes'? Also, now that we are using Fuse, can't we stop returning things from these functions entirely, so that dynamic_data is only updated by side-effects? -- Thanks, Zac ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete 2016-03-15 20:25 ` Zac Medico @ 2016-03-15 20:31 ` Zac Medico [not found] ` <20160315135751.166ba608.dolsen@gentoo.org> 0 siblings, 1 reply; 24+ messages in thread From: Zac Medico @ 2016-03-15 20:31 UTC (permalink / raw To: Brian Dolbec, gentoo-portage-dev On 03/15/2016 01:25 PM, Zac Medico wrote: > On 03/15/2016 01:17 PM, Brian Dolbec wrote: >> On Tue, 15 Mar 2016 12:42:51 -0700 >> Zac Medico <zmedico@gentoo.org> wrote: >> >>> On 03/15/2016 12:38 PM, Zac Medico wrote: >>>> On 03/15/2016 12:04 PM, Brian Dolbec wrote: >>>>> On Mon, 14 Mar 2016 18:04:56 -0700 >>>>> Zac Medico <zmedico@gentoo.org> wrote: >>>>> >>>>> >>>>>>> The only consumer for that allvalid variable is the metadata >>>>>>> UnusedCheck class. >>>>>>> >>>>>>> So the allvalid variable is True until found False >>>>>>> by whichever checks along the way find it to be False. Like a >>>>>>> fuse, it's good until it's blown, then it can never be good >>>>>>> again. I don't think this particular variable justifies a >>>>>>> special class that more fully mimics a fuse. Impossible to >>>>>>> reset it like a breaker. >>>>>> >>>>>> Yeah, let's do it. It's a great opportunity to add clarity to the >>>>>> code, and prevent future goofs. >>>>>> >>>>> >>>>> Done, it is now dynamic_data['validity_fuse'] which is a Fuse >>>>> instance. >>>> >>>> Nice, thank you! >>>> >>>> We can also use Fuse for the 'can_force' boolean, right? >>>> >>> >>> For 'changed' as well. >> >> can_force, is yes >> >> changed is a no. It is the VCS module Changes class instance. I see >> now that I described it wrong in the docstrings. Maybe I should rename >> it for better clarity to changes_inst or vcs_changes... ideas? > > Maybe 'changes'? > > Also, now that we are using Fuse, can't we stop returning things from > these functions entirely, so that dynamic_data is only updated by > side-effects? > I think 'continue' is the only one left. We could just return a single boolean, or use an exception to do what 'continue' does. -- Thanks, Zac ^ permalink raw reply [flat|nested] 24+ messages in thread
[parent not found: <20160315135751.166ba608.dolsen@gentoo.org>]
* Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete [not found] ` <20160315135751.166ba608.dolsen@gentoo.org> @ 2016-03-15 21:03 ` Zac Medico 2016-03-15 21:19 ` Brian Dolbec 0 siblings, 1 reply; 24+ messages in thread From: Zac Medico @ 2016-03-15 21:03 UTC (permalink / raw To: Brian Dolbec, gentoo-portage-dev On 03/15/2016 01:57 PM, Brian Dolbec wrote: > On Tue, 15 Mar 2016 13:31:24 -0700 > Zac Medico <zmedico@gentoo.org> wrote: > >>> >>> Also, now that we are using Fuse, can't we stop returning things >>> from these functions entirely, so that dynamic_data is only updated >>> by side-effects? >>> >> >> I think 'continue' is the only one left. We could just return a single >> boolean, or use an exception to do what 'continue' does. > > NOPE :( not without a lot more work... > > arches.py: return {'continue': False, 'arches': arches} > depend.py: return {'continue': False, 'unknown_pkgs': unknown_pkgs, > 'type_list': type_list, 'badlicsyntax': badlicsyntax, 'baddepsyntax': > baddepsyntax} > ebuild.py: return {'continue': False, 'ebuild': self} > return {'continue': False, 'pkg': self.pkg} > isebuild.py: return {'continue': self.continue_, 'pkgs': pkgs, > 'can_force': not self.continue_} > live.py: return {'continue': False, > 'live_ebuild': LIVE_ECLASSES.intersection( > kwargs.get('ebuild').inherited)} > fetches.py: return {'continue': False, 'src_uri_error': > self._src_uri_error} > pkgmetadata.py: return {'continue': False, 'muselist': > frozenset(self.musedict)} > return {'continue': False, 'muselist': > frozenset(self.musedict)} > scan.py: return {'continue': False, 'eadded': > self.vcs_settings.status.eadded} > use_flags.py: return {'continue': False, 'ebuild_UsedUseFlags': > self.usedUseFlags, 'used_useflags': > used_useflags} > > > But we can define a generic dynamic_data class that holds the > data that can be modified in a similar way to the Fuse class. That way > we don't have to update it like we do a dictionary. > Can't we add all these things to the dynamic_data dict that's initialized in Scanner.scan_pkgs (along with Fuse instances), and just let them get passed in as arguments, so that the functions can act on them without having to return them? -- Thanks, Zac ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete 2016-03-15 21:03 ` Zac Medico @ 2016-03-15 21:19 ` Brian Dolbec 2016-03-15 22:37 ` Zac Medico 0 siblings, 1 reply; 24+ messages in thread From: Brian Dolbec @ 2016-03-15 21:19 UTC (permalink / raw To: gentoo-portage-dev On Tue, 15 Mar 2016 14:03:41 -0700 Zac Medico <zmedico@gentoo.org> wrote: > On 03/15/2016 01:57 PM, Brian Dolbec wrote: > > On Tue, 15 Mar 2016 13:31:24 -0700 > > Zac Medico <zmedico@gentoo.org> wrote: > > > >>> > >>> Also, now that we are using Fuse, can't we stop returning things > >>> from these functions entirely, so that dynamic_data is only > >>> updated by side-effects? > >>> > >> > >> I think 'continue' is the only one left. We could just return a > >> single boolean, or use an exception to do what 'continue' does. > > > > NOPE :( not without a lot more work... > > > > arches.py: return {'continue': False, 'arches': arches} > > depend.py: return {'continue': False, 'unknown_pkgs': unknown_pkgs, > > 'type_list': type_list, 'badlicsyntax': badlicsyntax, > > 'baddepsyntax': baddepsyntax} > > ebuild.py: return {'continue': False, 'ebuild': self} > > return {'continue': False, 'pkg': self.pkg} > > isebuild.py: return {'continue': self.continue_, 'pkgs': pkgs, > > 'can_force': not self.continue_} > > live.py: return {'continue': False, > > 'live_ebuild': LIVE_ECLASSES.intersection( > > kwargs.get('ebuild').inherited)} > > fetches.py: return {'continue': False, 'src_uri_error': > > self._src_uri_error} > > pkgmetadata.py: return {'continue': False, 'muselist': > > frozenset(self.musedict)} > > return {'continue': False, 'muselist': > > frozenset(self.musedict)} > > scan.py: return {'continue': False, 'eadded': > > self.vcs_settings.status.eadded} > > use_flags.py: return {'continue': False, 'ebuild_UsedUseFlags': > > self.usedUseFlags, 'used_useflags': > > used_useflags} > > > > > > But we can define a generic dynamic_data class that holds the > > data that can be modified in a similar way to the Fuse class. That > > way we don't have to update it like we do a dictionary. > > > > Can't we add all these things to the dynamic_data dict that's > initialized in Scanner.scan_pkgs (along with Fuse instances), and just > let them get passed in as arguments, so that the functions can act on > them without having to return them? Yeah, /me is being a dummy. The modules are currently being passed the dict as a **kwargs. So the individual attributes are accessible directly. We could just pass it as one arg instead then the modules can just modify the dictionary directly since it is passed in by pointer. Sorry, my head is bouncing back and forth between this and other work code... But I like how we're getting the code in better shape... something I had reserved for stage3 ;) The current method is the first thing that came to me when I discovered I needed to get new data back into the dynamic_data for trailing modules in the queue. -- Brian Dolbec <dolsen> ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete 2016-03-15 21:19 ` Brian Dolbec @ 2016-03-15 22:37 ` Zac Medico 0 siblings, 0 replies; 24+ messages in thread From: Zac Medico @ 2016-03-15 22:37 UTC (permalink / raw To: gentoo-portage-dev On 03/15/2016 02:19 PM, Brian Dolbec wrote: > On Tue, 15 Mar 2016 14:03:41 -0700 > Zac Medico <zmedico@gentoo.org> wrote: > >> On 03/15/2016 01:57 PM, Brian Dolbec wrote: >>> On Tue, 15 Mar 2016 13:31:24 -0700 >>> Zac Medico <zmedico@gentoo.org> wrote: >>> >>>>> >>>>> Also, now that we are using Fuse, can't we stop returning things >>>>> from these functions entirely, so that dynamic_data is only >>>>> updated by side-effects? >>>>> >>>> >>>> I think 'continue' is the only one left. We could just return a >>>> single boolean, or use an exception to do what 'continue' does. >>> >>> NOPE :( not without a lot more work... >>> >>> arches.py: return {'continue': False, 'arches': arches} >>> depend.py: return {'continue': False, 'unknown_pkgs': unknown_pkgs, >>> 'type_list': type_list, 'badlicsyntax': badlicsyntax, >>> 'baddepsyntax': baddepsyntax} >>> ebuild.py: return {'continue': False, 'ebuild': self} >>> return {'continue': False, 'pkg': self.pkg} >>> isebuild.py: return {'continue': self.continue_, 'pkgs': pkgs, >>> 'can_force': not self.continue_} >>> live.py: return {'continue': False, >>> 'live_ebuild': LIVE_ECLASSES.intersection( >>> kwargs.get('ebuild').inherited)} >>> fetches.py: return {'continue': False, 'src_uri_error': >>> self._src_uri_error} >>> pkgmetadata.py: return {'continue': False, 'muselist': >>> frozenset(self.musedict)} >>> return {'continue': False, 'muselist': >>> frozenset(self.musedict)} >>> scan.py: return {'continue': False, 'eadded': >>> self.vcs_settings.status.eadded} >>> use_flags.py: return {'continue': False, 'ebuild_UsedUseFlags': >>> self.usedUseFlags, 'used_useflags': >>> used_useflags} >>> >>> >>> But we can define a generic dynamic_data class that holds the >>> data that can be modified in a similar way to the Fuse class. That >>> way we don't have to update it like we do a dictionary. >>> >> >> Can't we add all these things to the dynamic_data dict that's >> initialized in Scanner.scan_pkgs (along with Fuse instances), and just >> let them get passed in as arguments, so that the functions can act on >> them without having to return them? > > Yeah, /me is being a dummy. The modules are currently being passed > the dict as a **kwargs. So the individual attributes are accessible > directly. We could just pass it as one arg instead then the modules can > just modify the dictionary directly since it is passed in by pointer. Multiple args work fine too, as long as those args refer to mutable instances that persist outside the function. > Sorry, my head is bouncing back and forth between this and other work > code... But I like how we're getting the code in better shape... > something I had reserved for stage3 ;) I just feel like each step should result in code that is more maintainable than the last, and in the current step we have gone somewhat backwards in maintainability in a specific area. > The current method is the first > thing that came to me when I discovered I needed to get new data back > into the dynamic_data for trailing modules in the queue. You can see how the new approaches we are coming up with are much more maintainable and easier for people to wrap their heads around. -- Thanks, Zac ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete 2016-03-14 17:14 ` Zac Medico 2016-03-14 17:22 ` Zac Medico @ 2016-03-14 17:35 ` Brian Dolbec 2016-03-14 19:09 ` Brian Dolbec 1 sibling, 1 reply; 24+ messages in thread From: Brian Dolbec @ 2016-03-14 17:35 UTC (permalink / raw To: gentoo-portage-dev On Mon, 14 Mar 2016 10:14:15 -0700 Zac Medico <zmedico@gentoo.org> wrote: > On 03/05/2016 01:37 PM, Brian Dolbec wrote: > > > Zac, I'm done with code changes in the rewrite. Ready for a last > > look before a merge. Can you have a look again? I did some > > changes/fixes and rebased them in. Floppym hasn't reported any > > more bugs, so I think it's ready for broader testing in a release. > > Then we can work on moving all the test data to a separate file in > > the tree or downloaded... > > The dynamic_data stuff in Scanner is a little hard to follow. Then it > calls dynamic_data.update(rdata), is there any chance that the update > operation might clobber something that shouldn't have been clobbered? > No, mostly it just adds new variables to the dynamic_data dictionary. In that way it is important the order the checks are run as many rely on data supplied by previous checks. As I recall there are only a few that are modified by the checks, but they need to be. I still need to establish a good way of tracking those. Maybe something new in the module_spec. But I don't know if that really needs an openrc like needs, before, after... But they do need to be listed to ease later additions/removals of modules. > Also, I see that the number of sys.exit calls in pym/repoman is up > from 37 to 41. I would prefer that we raise exceptions for the caller > to handle, rather than call sys.exit directly. I suppose we change > that later though, after we merge the code. > -- > Thanks, > Zac > hmm, I don't recall adding any, that doesn't sound like me. I did cringe a little every time I came across them. I do agree that they need a better system. But I was concentrating on the modularization. Rather than wrapping the modules in try: except: pairs, perhaps we could add another variable to the dynamic_data similar to "continue", only an abandon-ship one, maybe "abort" to take them out of the modules. Then the Scanner class could handle the exit in one consistent place. Try/except can work nicely, but I've had to disable far too many to troubleshoot code from their misuse and being nested 5 or more deep. -- Brian Dolbec <dolsen> ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete 2016-03-14 17:35 ` Brian Dolbec @ 2016-03-14 19:09 ` Brian Dolbec 0 siblings, 0 replies; 24+ messages in thread From: Brian Dolbec @ 2016-03-14 19:09 UTC (permalink / raw To: gentoo-portage-dev On Mon, 14 Mar 2016 10:35:26 -0700 Brian Dolbec <dolsen@gentoo.org> wrote: > On Mon, 14 Mar 2016 10:14:15 -0700 > Zac Medico <zmedico@gentoo.org> wrote: > > > On 03/05/2016 01:37 PM, Brian Dolbec wrote: > > > > Also, I see that the number of sys.exit calls in pym/repoman is up > > from 37 to 41. I would prefer that we raise exceptions for the > > caller to handle, rather than call sys.exit directly. I suppose we > > change that later though, after we merge the code. > > -- > > Thanks, > > Zac > > > > hmm, I don't recall adding any, that doesn't sound like me. I did > cringe a little every time I came across them. I do agree that they > need a better system. But I was concentrating on the modularization. > > Rather than wrapping the modules in try: except: pairs, perhaps we > could add another variable to the dynamic_data similar to "continue", > only an abandon-ship one, maybe "abort" to take them out of the > modules. Then the Scanner class could handle the exit in one > consistent place. Try/except can work nicely, but I've had to disable > far too many to troubleshoot code from their misuse and being nested 5 > or more deep. > Ok, I just had a look through to see about changing those. Many of those "New" sys.exit calls are due to code duplication moving from code blocks which handled multiple VCS types into the separate vcs modules, and at least 4 of those are in docstring comments. So, we should probably define an exception for those to raise. And hopefully find a nice central place to handle them correctly. We can do that in master before we make the .29 release I think. There are 3 in non-vcs code, 1 in the scan() in modules/scan/scan.py and 2 in modules, one of which is an import failure exit for the metadata.xml module. The other is in the manifest module code which looks like it could easily return {"continue": False, "abort": true} and handle the case in the Scanner class. That would also allow for future modules to make use of an "abort" if needed. As for the others, That is something we can work on in stage3 which was intended for fixing up the remaining (crappy) code and optimizing... -- Brian Dolbec <dolsen> ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2016-03-15 22:37 UTC | newest] Thread overview: 24+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-01-10 21:40 [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete Brian Dolbec 2016-01-10 22:28 ` Brian Dolbec 2016-01-11 6:52 ` Brian Dolbec 2016-01-11 9:08 ` Brian Dolbec 2016-01-11 15:39 ` Alexander Berntsen 2016-03-05 21:37 ` Brian Dolbec 2016-03-07 12:21 ` Alexander Berntsen 2016-03-14 17:14 ` Zac Medico 2016-03-14 17:22 ` Zac Medico 2016-03-14 17:52 ` Brian Dolbec 2016-03-15 0:18 ` Zac Medico 2016-03-15 0:47 ` Brian Dolbec 2016-03-15 1:04 ` Zac Medico 2016-03-15 6:24 ` Zac Medico 2016-03-15 19:04 ` Brian Dolbec 2016-03-15 19:38 ` Zac Medico 2016-03-15 19:42 ` Zac Medico [not found] ` <20160315131731.2edf502d.dolsen@gentoo.org> 2016-03-15 20:25 ` Zac Medico 2016-03-15 20:31 ` Zac Medico [not found] ` <20160315135751.166ba608.dolsen@gentoo.org> 2016-03-15 21:03 ` Zac Medico 2016-03-15 21:19 ` Brian Dolbec 2016-03-15 22:37 ` Zac Medico 2016-03-14 17:35 ` Brian Dolbec 2016-03-14 19:09 ` Brian Dolbec
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox