On Mon, Mar 26, 2012 at 4:12 PM, Gregory M. Turner <[email protected]> wrote:
> Hello, I would appreciate if those of you with portage development experience
> and a moment to spare could please take a look at:
>
> https://github.com/gmt/gmt-cygwin-overlay/blob/master/sys-apps/portage/files/portage-2.2.01.20271-cygdll_protect.patch
>
> Not gunning for upstream or anything, but I would eventually like to start
> moving towards "not a complete pile of crap" status :)
>
> Notes:
Consistency in the style would be nice.
For instance in cygdll-update:
# Here you have spaces after the $( and before the )
extra_protection="$( portageq cygdll_protect_list ${EROOT} )"
# Later on you don't put spaces...
eval $(portageq envvar -v CYGDLL_PROTECT PORTAGE_HOSTNAME \
+ PORTAGE_CONFIGROOT PORTAGE_INST_GID PORTAGE_INST_UID \
+ PORTAGE_TMPDIR EROOT USERLAND)
Assuming $PORTAGE_BASH is always bash, you should use [[ consistently
in your bash.
IMHO it is a common mistake to write your own argument processing, is
there a reason getops is not appropriate (portability?)
Onto the python:
if os.environ.__contains__("PORTAGE_PYTHONPATH"):
Any reason why
if "PORTAGE_PYTHONPATH" not in os.environ:
was not used?
Python functions are not perl, don't pass 'argv' to the function.
If the function takes to arguments, it should be:
def func1(arg1, arg2)
If some of the args have defaults:
def func1(arg1, arg2=5)
>
> o based against a-few-weeks-old rsync prefix-portage
>
> o may contain a very few hunks that should arguably go
> upstream--I will make bugs for them and they are not
> what (I feel) needs eyeballs.
>
> o trying to actually run this may be difficult. let me
> know where you're stuck and I'll try to help.
>
> o I must admit, I pretty much learned python as I went,
> so... if you are thinking "that looks like a total
> rookie mistake" it probably is, and I'd probably
> appreciate your constructive critique.
I've never seen anyone use <> before; I didn't even know it existed.
Most folks use !=.
I can't really imagine why people do stuff like share /var/lib/portage
across machines, so adding a bunch of complexity just to make it work
seems nuts to me. That being said I have not worked on portage in
years and Zac seems happier to support weird use cases.
>
> o I'll probably keep hacking on this so... moving target.
> But it's mostly working, I think.
>
> o This doesn't make any big changes to upstream code. So,
> If it looks like it might be a big indentation change,
> it probably is.
>
> o this is to solve the problem that, in cygwin, a running python
> crashes as soon as you change any filesystem .dll's on which
> it depends and fork() -- this is semi-orthogonal to the
> typical cygwin "rebasing" problem and distinct from it.
> The problem has nothing to do with locked files (except
> that my solution does use some standard portage file-
> locking features).
>
> -gmt
>