def funcx(a,b='',c='',d='') means that args b,c, and d are *optional*, and not position dependant. So calling funcx like this funcx(1,2,3,4) is just *asking* for a pain in the ass bug. Stop doing it. Any changes made to funcx (hell just inadvertently changing the optional args ordering) will cause weirdo bugs to crop up. This bug is intended as a meta-bug, left open until portage cvs has been cleansed of these dumb calls. Offhand, they're litered throughout portage.py, and pretty prominant in portage_gpg.py . Reproducible: Always Steps to Reproduce:
Perhaps a list of functions with optional arguments that could be ticked off would be good? Need some way to gauge when this can be closed...
Created attachment 38373 [details] PERL script to check for correct python argument usage I created this script to help me find the problem areas in portage. It's a quick hack, and still probably has a few minor bugs in it (or incorrect assumptions as I'm still learning python and it's syntax), but it should still be useful.
Created attachment 38374 [details, diff] diff from 20040826 for arguments fix The first batch of fixes I did. This is against the CVS snapshot of 20040826. There are 33 corrections in this one. I want to make sure I am doing these correctly before I do more. Please let me know. Thanks.
A script is a good idea - but hard to get right. Still, if it doesn't pick up all occurences, it still saves a lot of work. :) I haven't gone through to verify that the arguments are correct for the patch at all. However, I did notice the following issue: return digestCheckFiles(myfiles, mydigests, basedir, note="src_uri", strict) "strict" in this case is being treated as a positional argument rathat than an optional. This will cause an error as python does not allow optionals after positionals. You could probably get around it in the script be checking for param= rather than param. I.e in the above it should be "strict=strict".
Created attachment 38402 [details] updated PERL script This fixes some regex bugs in the orignal script. It also removes some of the logic that was based on incorrect assumptions that I made about how python works with keyworded arguments. (I read the relevant chapters in Learning Python) Caveats: doesn't handle namespaces very well. But I'm not aiming for a complete python parser here, just something to help us find bad function calls. Which it does, it just reports extra errors if it can't find a definition for the function when the same name is used elsewhere (i.e. os.close vs close that defined in portage_db_anydbm.py - it reports that the os.close calls don't match the definition of close in portage_db_anydbm.py) Running this script on the portage CVS snapshot finds 285 bad calls (though some are not actually bad, just namespace issues which this script doesn't cope with) It checks for and reports: - keywords that are set but not defined - keywords set more then once - positional arguments set by both position and keyword (i.e. set twice) - positional arguments set after keyworded arguments - too many arguments - keyword arguments set by position - not all posiitonal arguments set Run it from the directory you want to check and it will check all .py files in all subdirectories (it uses find ./ -iname '*.py'). I will be updating my patch soon to fix my errors from my incorrect assumptions.
Created attachment 38649 [details, diff] updated fix against 20040826 Updated patch fixes 43 calls in portage.py. This should be the majority of them. There a select few cases that my lack of python experience got in my way of fixing so I left them for now: portage.py line 2801 if not digestcheck(checkme, mysettings, ("strict" in features)): should this be: if not digestcheck(checkme, mysettings, strict=("strict" in features)): or would that change the meaning of the call? There are still several in other files in portage, I didn't touch these as I don't know how permanent these files are, or whether they are even used in portage or were just created as test-cases of future functionality. (getbinpkg.py, cvstree.py, many others - my script lists them all) I also didn't touch any of the 52 calls to writemsg as I doubt this function will change and it would seem ugly to change the writemsg("text",1") to writemsg("text",noiselevel=1) everywhere.
-if not digestcheck(checkme, mysettings, ("strict" in features)): +if not digestcheck(checkme, mysettings, strict=("strict" in features)): E'yep, tiz valid. Thank you Michael- just slipped this into cvs, and will be uploading a new portage cvs tarball in a few minutes.
Regarding writemsg, at this point I could see doing ignoring that; writemsg won't change often, added, the function definition could be changed. def writemsg(text,*args): fex w/ args being allowed to contain *only* an int if specified. Although at the moment I could see just ignoring writemsg.
Created attachment 38939 [details, diff] Fixes against many files against 20040901 Layovers in random airports can be useful for portage hacking. Gotta love it when you miss your connection due to airline problems. This patch fixes the other files in the CVS snapshot.
Nuking this; cleansing occured in 2.1, and is occuring in 3x
