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

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

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

~harring