[gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete

[gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete

[Brian Dolbec]

   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>

Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete

[Brian Dolbec]

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>

Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete

[Brian Dolbec]

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>

Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete

[Brian Dolbec]

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>

Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete

[Alexander Berntsen]

-----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-----

Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete

[Brian Dolbec]

-----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-----

Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete

[Alexander Berntsen]

-----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-----

Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete

[Zac Medico]

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

Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete

[Zac Medico]

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

Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete

[Brian Dolbec]

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>

Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete

[Brian Dolbec]

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>

Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete

[Brian Dolbec]

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>

Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete

[Zac Medico]

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

Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete

[Brian Dolbec]

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>

Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete

[Zac Medico]

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

Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete

[Zac Medico]

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

Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete

[Brian Dolbec]

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>

Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete

[Zac Medico]

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

Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete

[Zac Medico]

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

Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete

[Zac Medico]

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

Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete

[Zac Medico]

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

Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete

[Zac Medico]

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

Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete

[Brian Dolbec]

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>

Re: [gentoo-portage-dev] [Patch] Repoman rewrite stage2 modularization conversion complete

[Zac Medico]

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