Re: [gentoo-portage-dev] Refactoring ebuild.sh

2006-08-28 Thread Simon Stelling
Brian Harring wrote:
>>> Further, all of these are still overridable by *bashrc and env.
>> How?
> readonly -f .
> bashrc, env, etc, all can force different definitions of functions.  
> 
> This is why ebd sets it's required functions and marks them readonly 
> *before* it even goes near ebuild data; do it the other way around, 
> you're not guranteeing anything.

Right. Actually the next change I wanted to propose was to move all of
these functions above all sourcings, that's why I didn't even think
about that way. I decided it would be better to split up the patch into
the individual steps as it would be easier to adjust that way, but
obviously I split it in the wrong place ;) Either way, going to drop the
readonly -f stuff for this round.

-- 
Kind Regards,

Simon Stelling
Gentoo/AMD64 developer
-- 
gentoo-portage-dev@gentoo.org mailing list



Re: [gentoo-portage-dev] Refactoring ebuild.sh

2006-08-27 Thread Brian Harring
On Sun, Aug 27, 2006 at 10:26:28AM +0200, Simon Stelling wrote:
> Brian Harring wrote:
> >>diefunc()
> >>dump_trace()
> > 
> > these are general utility, not debugging.
> 
> Where would you stick them? 'die' to 'ebuild helpers' and 'dump_trace'
> to 'internals'?

util, same for hasq.

> > If you're going to try breaking the beast up, suggest you go the step 
> > further and break up what must be supplied from the ebuild.sh 
> > implementation, and what is bound to the ebuild's environment.
> > 
> > 
> >> Internal eclass functions:
> >>newdepend()
> >>newrdepend()
> >>newpdepend()
> >>do_newdepend()
> > 
> > These aren't eclass functions- usable without involving eclasses.
> > inherit ought to be in internal eclass functions also (yes it's user 
> > visible, but it's also ebuild.sh implementation specific).
> 
> Where to put them, then?

...they're ebuild functions.  stick them with the other ebuild 
functions (use_with fex).

Personally, I think trying to seperate it up at this level is probably 
not a great idea; splitting the EAPI specific bits into seperate 
modules, yes, but when/if EAPI=1 arrives changing chunks, it's going 
to be a mess going through each module and ifdef'ing functions.


> >> I moved them into own files within a subdir in bin/, called modules, and
> >> source these files at the right time, so that nothing changes
> >> functionality wise. The global scope code I left untouched, except for a
> >> bunch of exports that set sane defaults for the do*/new* helpers.
> >>
> >> This cuts down ebuild.sh to 403 lines, which makes it far more readable,
> >> IMO.
> 
> > Except there are now magic vars hidden away in submodules.  If trying 
> > to seperate it into seperate blocks, document 'em.
> 
> Uhm, how would that documentation part work? What exactly do you want
> documented? (What do "it" and "'em" refer to?)
> 
> About the magic vars.. I wasn't sure either whether putting the defaults
> into the modules was a good idea. I thought it would be good because
> they are not used anywhere else (didn't check), but I can just as well
> stick them back into ebuild.sh.

Sticking magic vars (globals mostly, eclass_depth for example) in 
locations seperate from the functionality that uses it needs to be 
documented; moving all of the globals up to ebuild.sh means that for 
any code to reuse those modules without using ebuild.sh, it has to 
maintain it's own globals list.

So... magic vars need to stay in the module that uses them, and need 
documentating if another module relies on it; with ebuild.sh, this 
isn't an issue, everything is guranteed loaded, with submodules it's a 
bit more of an issue.


> >> I also set all the functions that aren't meant to be overridden by saved
> >> env, profile bashrc, portage bashrc or ebuilds/eclasses readonly.
> > 
> > Don't think much thought was put into this tbh- use* and has* (all but 
> > useq and hasq basically) are bound to the ebuild environment.
> 
> This is actually true.
> 
> > Why?  Because they got mangled transitioning from 2.0.50 2.0.51, for 
> > the ebuild to function properly it needs to use the original func from 
> > the environment, not what the ebuild.sh implementation tries to force.
> > 
> > Identified all of them that are affected by this?
> 
> Well, as it stands now, all variables except the sandbox related ones
> are defined after env & bashrc are sourced and before the ebuild is
> sourced. That means that the env can't override them, which means that
> they are considered implementation specific in the current
> implementation. I agree that consideration is nonsense, though ;)
>
> So, to address the problem: What functions are bound to the ebuild
> rather than ebuild.sh?
pkg_*, src_*, *opts, *into, *depend .

Basically, think of which are implementation specific (portageq) vs 
which aren't directly reliant on implementation details.  If it's not 
implementation specific, it's likely bound to the ebuild env.

> Beside {use,has}{,v} I don't know any.
> 
> > Further, all of these are still overridable by *bashrc and env.
> 
> How?
readonly -f .

bashrc, env, etc, all can force different definitions of functions.  

This is why ebd sets it's required functions and marks them readonly 
*before* it even goes near ebuild data; do it the other way around, 
you're not guranteeing anything.


> > Drop the readonly crap; it's not actually blocking anything, just 
> > attempting to catch non real issues; in the process, it actually is a 
> > step in the wrong direction.
> 
> I will drop it for now, but I would like to know the answers to above
> questions nevertheless.
> 
> > Reordering is a bit arbitrary, but fine.  Attempted env changes, would 
> > avoid those.
> 
> My primary goal is to cut down ebuild.sh to a managable size. I don't
> see how that can be solved with a different env handling, tbh.

Env handling affects the flow of the code; never said it reduces the 
total size of code though (just said don't go screwing 

Re: [gentoo-portage-dev] Refactoring ebuild.sh

2006-08-27 Thread Simon Stelling
Brian Harring wrote:
> categorization is a bit whonky in a few spots-

I won't fight that. It was pretty hard to categorize functions that I've
never really noticed before ;)

>> Debugging:
>>  register_die_hook()
> 
> ebuild specific func, while debugging, it's not ebuild.sh debugging.
> 
>>  diefunc()
>>  dump_trace()
> 
> these are general utility, not debugging.

Where would you stick them? 'die' to 'ebuild helpers' and 'dump_trace'
to 'internals'?

>> Action functions:
>>  dyn_setup()
>>  dyn_unpack()
>>  dyn_clean()
>>  dyn_compile()
>>  dyn_test()
>>  dyn_install()
>>  dyn_preinst()
>>  dyn_help()
>>
>>  Abort handlers:
>>  abort_handler()
>>  abort_compile()
>>  abort_unpack()
>>  abort_test()
>>  abort_install()
>>  killparent()
>
> internal is a better label.

Well, that stuff under 'Abort handlers:' which I accidentally indented
was intended to go in an own file. However, seeing the list this way, it
might be better to stick them in the same one, which the label internal.
Could also put the 'Internal helpers' there as they are really short anyway.

> has is missing also...

Well, has* is already in isolated-functions.sh. I've thought about
killing that and moving the functions to the appropriate sections, but I
was not sure whether isolated-functions.sh was sourced from different
places too so that I simply put it off for the moment. If it's not
sourced otherwise, I'd certainly move the functions there too.

> If you're going to try breaking the beast up, suggest you go the step 
> further and break up what must be supplied from the ebuild.sh 
> implementation, and what is bound to the ebuild's environment.
> 
> 
>> Internal eclass functions:
>>  newdepend()
>>  newrdepend()
>>  newpdepend()
>>  do_newdepend()
> 
> These aren't eclass functions- usable without involving eclasses.
> inherit ought to be in internal eclass functions also (yes it's user 
> visible, but it's also ebuild.sh implementation specific).

Where to put them, then?

>> I moved them into own files within a subdir in bin/, called modules, and
>> source these files at the right time, so that nothing changes
>> functionality wise. The global scope code I left untouched, except for a
>> bunch of exports that set sane defaults for the do*/new* helpers.
>>
>> This cuts down ebuild.sh to 403 lines, which makes it far more readable,
>> IMO.

> Except there are now magic vars hidden away in submodules.  If trying 
> to seperate it into seperate blocks, document 'em.

Uhm, how would that documentation part work? What exactly do you want
documented? (What do "it" and "'em" refer to?)

About the magic vars.. I wasn't sure either whether putting the defaults
into the modules was a good idea. I thought it would be good because
they are not used anywhere else (didn't check), but I can just as well
stick them back into ebuild.sh.

>> I also set all the functions that aren't meant to be overridden by saved
>> env, profile bashrc, portage bashrc or ebuilds/eclasses readonly.
> 
> Don't think much thought was put into this tbh- use* and has* (all but 
> useq and hasq basically) are bound to the ebuild environment.

This is actually true.

> Why?  Because they got mangled transitioning from 2.0.50 2.0.51, for 
> the ebuild to function properly it needs to use the original func from 
> the environment, not what the ebuild.sh implementation tries to force.
> 
> Identified all of them that are affected by this?

Well, as it stands now, all variables except the sandbox related ones
are defined after env & bashrc are sourced and before the ebuild is
sourced. That means that the env can't override them, which means that
they are considered implementation specific in the current
implementation. I agree that consideration is nonsense, though ;)

So, to address the problem: What functions are bound to the ebuild
rather than ebuild.sh?

Beside {use,has}{,v} I don't know any.

> Further, all of these are still overridable by *bashrc and env.

How?

> Actually it doesn't fail sourcing.

Yeah, my bad. But you'd get a nice error message at least ;)

> Drop the readonly crap; it's not actually blocking anything, just 
> attempting to catch non real issues; in the process, it actually is a 
> step in the wrong direction.

I will drop it for now, but I would like to know the answers to above
questions nevertheless.

> Reordering is a bit arbitrary, but fine.  Attempted env changes, would 
> avoid those.

My primary goal is to cut down ebuild.sh to a managable size. I don't
see how that can be solved with a different env handling, tbh.

--
Kind Regards,

Simon Stelling
Gentoo/AMD64 developer
-- 
gentoo-portage-dev@gentoo.org mailing list



Re: [gentoo-portage-dev] Refactoring ebuild.sh

2006-08-26 Thread Brian Harring
On Sat, Aug 26, 2006 at 07:54:41PM +0200, Simon Stelling wrote:
> Hi all,
> 
> ebuild.sh is a mess. There are a lot of functions scattered to the four
> winds. Searching for a function in ebuild.sh takes a lot of time, and it
> is very tiring to scroll down huge chunks of totally unrelated functions
> just to find the next one which one is after. The current layout also
> makes it very hard to find the spots where new functions/variables are
> loaded from other files, what gets overwritten when profiles/eclasses
> are sourced etc.
> 
> I would like to change that, so I tried to categorize all the functions
> we have in ebuild.sh. I ended up with this setup:

categorization is a bit whonky in a few spots-

> Sandbox:
>   addread()
>   addwrite()
>   adddeny()
>   addpredict()
>   lchown()
>   lchgrp()
> 
> Unknown?:
>   esyslog()
> 
> Debugging:
>   register_die_hook()

ebuild specific func, while debugging, it's not ebuild.sh debugging.

>   diefunc()
>   dump_trace()

these are general utility, not debugging.

>   debug-print()
>   debug-print-function()
>   debug-print-section()
> 
> Internal helpers:
>   strip_duplicate_slashes()
>   remove_path_entry()
> 
> Action functions:
>   dyn_setup()
>   dyn_unpack()
>   dyn_clean()
>   dyn_compile()
>   dyn_test()
>   dyn_install()
>   dyn_preinst()
>   dyn_help()
> 
>   Abort handlers:
>   abort_handler()
>   abort_compile()
>   abort_unpack()
>   abort_test()
>   abort_install()
>   killparent()

internal is a better label.


> Default functions for ebuilds:
>   pkg_setup()
>   pkg_nofetch()
>   src_unpack()
>   src_compile()
>   src_test()
>   src_install()
>   pkg_preinst()
>   pkg_postinst()
>   pkg_prerm()
>   pkg_postrm()
>   pkg_config()
> 
> Ebuild helpers:
>   into()
>   insinto()
>   exeinto()
>   docinto()
>   insopts()
>   diropts()
>   exeopts()
>   libopts()
>   keepdir()
>   unpack()
>   econf()
>   einstall()
>   use()
>   usev()
>   useq()
>   has_version()
>   portageq()
>   best_version()
>   use_with()
>   use_enable()
>   gen_wrapper()
>   check_KV()
>   inherit()
>   EXPORT_FUNCTIONS()

Most of these are mislabed; gen_wrapper is an internal function that 
ebuilds shouldn't know about, nor ever access.  It's existance was 
strictly to cover gcc's ass; considering it's usage was strictly 
implementation side (rather then ebuild side), probably can be 
dropped.

Either way, ain't a helper.  has is missing also...

If you're going to try breaking the beast up, suggest you go the step 
further and break up what must be supplied from the ebuild.sh 
implementation, and what is bound to the ebuild's environment.


> Internal eclass functions:
>   newdepend()
>   newrdepend()
>   newpdepend()
>   do_newdepend()

These aren't eclass functions- usable without involving eclasses.
inherit ought to be in internal eclass functions also (yes it's user 
visible, but it's also ebuild.sh implementation specific).


> I moved them into own files within a subdir in bin/, called modules, and
> source these files at the right time, so that nothing changes
> functionality wise. The global scope code I left untouched, except for a
> bunch of exports that set sane defaults for the do*/new* helpers.
> 
> This cuts down ebuild.sh to 403 lines, which makes it far more readable,
> IMO.

Except there are now magic vars hidden away in submodules.  If trying 
to seperate it into seperate blocks, document 'em.


> I also set all the functions that aren't meant to be overridden by saved
> env, profile bashrc, portage bashrc or ebuilds/eclasses readonly.

Don't think much thought was put into this tbh- use* and has* (all but 
useq and hasq basically) are bound to the ebuild environment.

Why?  Because they got mangled transitioning from 2.0.50 2.0.51, for 
the ebuild to function properly it needs to use the original func from 
the environment, not what the ebuild.sh implementation tries to force.

Identified all of them that are affected by this?

Further, all of these are still overridable by *bashrc and env.


> If an ebuild would ever try to re-define a function which is 
> internally used by portage, it would fail sourcing, which is a good 
> thing, IMO.

Actually it doesn't fail sourcing.

echo 'f() { echo "grande tiza"; }; :;' > t;
. t
readonly -f f
. t && echo "it relies on the last exit code, which was $?"


> So, if noone objects, I would like to push the attached patch into SVN.

Drop the readonly crap; it's not actually blocking anything, just 
attempting to catch non real issues; in the process, it actually is a 
step in the wrong direction.

If a func is going to marked as immutable, it's implicitly 
implementation specific- as such it has no business winding up in the 
environment file.

R