Re: [gentoo-portage-dev] Refactoring ebuild.sh
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
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
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
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