Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 327999 - >=sys-libs/glibc-2.9 has pkg_ functions missing from DEFINED_PHASES in metadata cache
Summary: >=sys-libs/glibc-2.9 has pkg_ functions missing from DEFINED_PHASES in metada...
Status: RESOLVED OBSOLETE
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: [OLD] Core system (show other bugs)
Hardware: All Linux
: High normal (vote)
Assignee: PMS/EAPI
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 273627
  Show dependency tree
 
Reported: 2010-07-12 21:16 UTC by David Leverton
Modified: 2017-06-07 15:10 UTC (History)
2 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Leverton 2010-07-12 21:16:24 UTC
The DEFINED_PHASES pseudo-variable (line 17) in the metadata cache for glibc versions later than 2.9 does not contain list the pkg_setup, pkg_preinst and pkg_postinst functions, even though they are used.  This appears to be because they are conditionally defined in the ebuild:

# FILESDIR might not be available during binpkg install
for x in setup {pre,post}inst ; do
	e="${FILESDIR}/eblits/pkg_${x}.eblit"
	if [[ -e ${e} ]] ; then
		. "${e}"
		eval "pkg_${x}() { eblit-run pkg_${x} ; }"
	fi
done

but ${FILESDIR} is not set during metadata generation as of Portage revision 297d759 (29th of January this year), for bug 300378.  The simplest workaround is probably to move the eval call outside the if block, although I haven't tested this.
Comment 1 SpanKY gentoo-dev 2010-07-13 21:40:25 UTC
base-system has nothing to do with toolchain packages.  please refrain from direct assignment of bugs as the wranglers will do it correctly for you.
Comment 2 SpanKY gentoo-dev 2010-07-13 21:41:35 UTC
it emerges fine for me, and if FILESDIR doesnt exist, then the pkg_* funcs cant be set.  so i dont know what exactly you expect to change.
Comment 3 David Leverton 2010-07-14 12:28:18 UTC
(In reply to comment #1)
> base-system has nothing to do with toolchain packages.  please refrain from
> direct assignment of bugs as the wranglers will do it correctly for you.
> 

https://bugs.gentoo.org/show_activity.cgi?id=327999

(In reply to comment #2)
> it emerges fine for me, and if FILESDIR doesnt exist, then the pkg_* funcs cant
> be set.  so i dont know what exactly you expect to change.
> 

It causes the cache to inaccurately reflect what the ebuild does, which confuses tools that use that information.  As the comment says, FILESDIR should only be missing for binary packages, but in that case the eblit code will have been loaded when the binary was created and stored in the environment, so it's safe to call it unconditionally (maybe zmedico can confirm the behaviour here?)  In fact, I'd say it's better to die in pkg_setup if the eblit function isn't already present and can't be loaded, rather than just ignoring it.

(For reference, regarding DEFINED_PHASES:
* proposal - http://thread.gmane.org/gmane.linux.gentoo.devel/58906
* Council discussion/approval - http://www.gentoo.org/proj/en/council/meeting-logs/20081211.txt
* PMS (a bit out of date, but no relevant changes recently) - http://dev.gentoo.org/~tanderson/pms/head/html/pms.html#x1-740008.4
)
Comment 4 SpanKY gentoo-dev 2010-07-18 19:51:11 UTC
still dont see anything that needs fixing here
Comment 5 Ciaran McCreesh 2010-07-18 20:00:50 UTC
Well, either this needs to be changed or we need to cancel the whole DEFINED_PHASES thing. The whole point of DEFINED_PHASES is to allow the package manager to skip executing phases that don't exist, and going by DEFINED_PHASES here, your pkg_ functions don't exist and thus don't need to be run.

QA: is this something you're prepared to step in and get fixed, or does DEFINED_PHASES have to be scrapped?
Comment 6 SpanKY gentoo-dev 2010-07-18 20:07:36 UTC
feel free to re-open once there is speced functionality available to actual do per-package eclass type of thing.  until there is, i'm not wasting time to "fix" a minor issue that doesnt actually cause problems.
Comment 7 Brian Harring (RETIRED) gentoo-dev 2010-07-18 20:30:17 UTC
(In reply to comment #6)
> feel free to re-open once there is speced functionality available to actual do
> per-package eclass type of thing.  until there is, i'm not wasting time to
> "fix" a minor issue that doesnt actually cause problems.

Causes an issue if the manager blindly trusts DEFINED_PHASES.  The reason (I assume) ciaran is pushing hard on that var is the speed implications of pkg_pretend w/out it...
Comment 8 Fabio Erculiani (RETIRED) gentoo-dev 2010-07-18 20:35:59 UTC
It breaks binary packages support in Entropy. This is definitely an issue and using eval() is not really the best thing on Earth.
Comment 9 SpanKY gentoo-dev 2010-07-18 22:23:06 UTC
show *actual* examples of binary packages breaking.  the current code will result in the files being in the environment which is saved into the binpkg and restored at the emerge time.
Comment 10 Brian Harring (RETIRED) gentoo-dev 2010-07-18 22:33:47 UTC
(In reply to comment #8)
> It breaks binary packages support in Entropy. This is definitely an issue and
> using eval() is not really the best thing on Earth.

You're confusing two different things here

Binpkgs shouldn't be running from a raw sourcing of the ebuild (again)... they should be running strictly from their env dump.  The breakage (if any) isn't from the technique, it's from the manager (portage) using source ebuilds instead of the env bundled with the binpkg.
Comment 11 Zac Medico gentoo-dev 2010-07-19 04:05:05 UTC
(In reply to comment #10)
> Binpkgs shouldn't be running from a raw sourcing of the ebuild (again)... they
> should be running strictly from their env dump.  The breakage (if any) isn't
> from the technique, it's from the manager (portage) using source ebuilds
> instead of the env bundled with the binpkg.

I'm pretty sure you've missed the point. The point was that entropy relies on /var/db/pkg/*/*/DEFINED_PHASES, which in turn comes from the 'depend' phase alone. Package managers shouldn't need to re-generate DEFINED_PHASES from environment.bz2, because if it gives a different result from the 'depend' phase then we may as well scrap DEFINED_PHASES as Ciaran said.
Comment 12 Brian Harring (RETIRED) gentoo-dev 2010-07-19 04:59:21 UTC
(In reply to comment #11)
> (In reply to comment #10)
> > Binpkgs shouldn't be running from a raw sourcing of the ebuild (again)... they
> > should be running strictly from their env dump.  The breakage (if any) isn't
> > from the technique, it's from the manager (portage) using source ebuilds
> > instead of the env bundled with the binpkg.
> 
> I'm pretty sure you've missed the point. The point was that entropy relies on
> /var/db/pkg/*/*/DEFINED_PHASES, which in turn comes from the 'depend' phase
> alone. Package managers shouldn't need to re-generate DEFINED_PHASES from
> environment.bz2, because if it gives a different result from the 'depend' phase
> then we may as well scrap DEFINED_PHASES as Ciaran said.

I presumed this was in reference to portage trying to redo the env via source if it can find it actually- specifically inability to find FILESDIR during initial resourcing, rather then entropy taking DEFINED_PHASES at face value.. if that's the case, yeah, problematic.

One thing to keep in mind... DEFINED_PHASES is optional in all used EAPIs (4 is the sole exception, which is also not going anywhere right now nor in use).  Y'all are relying on something that has zero guarantee of being reliable in this context and getting cranky it's not behaving as you want.

Fact of the matter is, the eblits crap while annoying is valid from a spec standpoint.  Further fact, DEFINED_PHASE's basically castrates the intentions of elibs/eblits, mainly as a way for the PM to know if a pkg_pretend is defined- it's a speed hack for a questionable feature.

Either way, a compromise would be having the PM honor ebuild tell the manager to leave DEFINED_PHASES as empty (thus requiring the manager(s) to run all phases instead of relying on that list).
Comment 13 Ciaran McCreesh 2010-07-19 07:37:23 UTC
DEFINED_PHASES *is* reliable, in that the spec is carefully worded to say that if it's set in the metadata cache at all then it's set to a legal value.

Paludis will, where legal according to PMS, skip executing phases that aren't in DEFINED_PHASES. This causes breakage here.

So, either something has to be done about the ebuilds, or we have to go back to the Council to get them to remove DEFINED_PHASES from PMS, revert all the Portage code etc.
Comment 14 Tomáš Chvátal (RETIRED) gentoo-dev 2010-07-19 16:25:46 UTC
Just 2 things:
1) please stop opening and closing it all the time, leave it as is now until we find some solution if it is really bug or not.

2) What could zac (portage devs) tell us on this issue?

FTR i have not much experinece on the issue itself so thats why i would like to hear why is one opinion right :]
Comment 15 David Leverton 2010-07-19 17:31:14 UTC
(In reply to comment #12)
> Further fact, DEFINED_PHASE's basically castrates the intentions
> of elibs/eblits, mainly as a way for the PM to know if a pkg_pretend is
> defined- it's a speed hack for a questionable feature.

GLEP 33 explicitly bans elibs from modifying phase functions - "Additionally, they cannot modify any ebuild template functions- src_compile, src_unpack." so unless I'm missing something there's no conflict with DEFINED_PHASES.  I can't speak for Mike's intentions when he introduced eblits, but I don't see any indication that this bug points to a fundamental incompatibility, just an implementation hiccup.
Comment 16 Zac Medico gentoo-dev 2010-07-19 18:00:50 UTC
(In reply to comment #14)
> 2) What could zac (portage devs) tell us on this issue?

There are already some parts of portage, such as in emerge --info when the pkg_info phase is executed, which rely on DEFINED_PHASES having a correct value. If we can't rely on it having a correct value, then we should scrap it. Note that scrapping DEFINED_PHASES will present a considerable performance disadvantage for pkg_pretend, which is slated for inclusion in the next EAPI.
Comment 17 Ciaran McCreesh 2010-07-19 20:27:50 UTC
Adding QA back: are you prepared to step in here, or do we have to go to the Council and ask them to remove DEFINED_PHASES from the spec and have support removed from Portage?
Comment 18 Fabio Erculiani (RETIRED) gentoo-dev 2010-07-19 21:04:47 UTC
Scrapping DEFINED_PHASES is just silly. The performance improvements it brings is considerable and eventually somebody outside the openrc project is considering performance a matter that deserves proper attention.

Just fix what is broken and keep DEFINED_PHASES.

Besides, I tried to build a binpkg out of sys-libs/glibc-2.11.2 and DEFINED_PHASES looks fine: "compile install postinst preinst setup test unpack". Am I missing something?
Comment 19 SpanKY gentoo-dev 2010-07-19 21:06:47 UTC
lets see ... you start with a package that is valid according to the current spec and does some funky stuff to workaround limitations in the existing spec to make maintenance of large packages a lot easier.  then you slowly change the spec over time until it breaks a valid package without ever going back and addressing the underlying issue.  then that package is flagged as being in violation.  even though it is still written using the original EAPI=0 which in theory should never be changing.  nice.

design something that people have been asking for for literally years (these eblits have been in use for 3 years now, but the idea is older) and there will be no problem converting glibc and other eblit consumers to it.  but dont expect a rosy reception when you screw people over without giving them any viable alternatives.
Comment 20 David Leverton 2010-07-19 21:14:48 UTC
I don't think anyone's suggesting killing off eblits here.  (I just realised that the title might give that impression, but that's not what was intended, apologies.)  I think what I suggested originally should be fine, just needs confirmation that it will work properly with binary packages and things:

(In reply to comment #0)
> The simplest workaround is probably to move the eval call outside the if
> block, although I haven't tested this.
>