Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 332639 - toolchain-binutils.eclass: toolchain-binutils_pkg_postinst doesn't recover from corrupt install
Summary: toolchain-binutils.eclass: toolchain-binutils_pkg_postinst doesn't recover fr...
Status: RESOLVED NEEDINFO
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Eclasses (show other bugs)
Hardware: All Linux
: High normal (vote)
Assignee: Gentoo Toolchain Maintainers
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-13 16:56 UTC by Fabio Erculiani (RETIRED)
Modified: 2010-08-18 18:55 UTC (History)
0 users

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


Attachments
log file off "equo install sys-devel/binutils --debug" (binpkg.log,20.27 KB, text/plain)
2010-08-13 17:05 UTC, Fabio Erculiani (RETIRED)
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Fabio Erculiani (RETIRED) gentoo-dev 2010-08-13 16:56:57 UTC
It looks like toolchain-binutils_pkg_postinst fails to update binutils profile correctly due to the below condition returning true. I didn't have time to understand why pkg_postrm doesn't remove the old config file (the code itself looks error-prone, too). The result is broken links in /usr/${CTARGET}/bin.

[[ -e ${ROOT}/etc/env.d/binutils/config-${CTARGET} ]] && return 0

I'm actually having this problem in Entropy, too, which does strictly follow portage install phases order.

Let me know if I can provide more specific info regarding the issue.

Reproducible: Always

Steps to Reproduce:
1. install older binutils via emerge.
2. make a binpkg of a newer binutils.
3. emerge with -K.
Comment 1 Fabio Erculiani (RETIRED) gentoo-dev 2010-08-13 17:05:17 UTC
Created attachment 242805 [details]
log file off "equo install sys-devel/binutils --debug"

This is the output generated by running portage phases in debug mode off Entropy. It's a reinstall (over a broken binutils-config profile), actually.
Comment 2 Fabio Erculiani (RETIRED) gentoo-dev 2010-08-13 17:06:56 UTC
Maybe this:
 elif [[ $(CHOST=${CTARGET} binutils-config -c) == ${CTARGET}-${BVER} ]] ; then
should be rather:
 elif [[ $(CHOST=${CTARGET} binutils-config -c) != ${CTARGET}-${BVER} ]] ; then
Comment 3 SpanKY gentoo-dev 2010-08-13 19:21:34 UTC
your PM is broken.  the older package's pre/post rm is run before the newer package's postinst and so any files from the old package are gone.  dont waste my time until you've validated things with a real PM like portage.
Comment 4 SpanKY gentoo-dev 2010-08-13 20:13:03 UTC
USE=-multitarget:

vapier / # emerge =binutils-2.19* -Kq
>>> Emerging binary (1 of 1) sys-devel/binutils-2.19.1-r1
>>> Installing (1 of 1) sys-devel/binutils-2.19.1-r1

vapier / # ld --version | head -n 1
GNU ld (GNU Binutils) 2.19.1

vapier / # emerge =binutils-2.20* -Kq
>>> Emerging binary (1 of 1) sys-devel/binutils-2.20.1-r1
>>> Installing (1 of 1) sys-devel/binutils-2.20.1-r1

vapier / # ld --version | head -n 1
GNU ld (GNU Binutils) 2.20.1.20100303
Comment 5 Fabio Erculiani (RETIRED) gentoo-dev 2010-08-13 20:24:55 UTC
(In reply to comment #3)
> your PM is broken.  the older package's pre/post rm is run before the newer
> package's postinst and so any files from the old package are gone.  dont waste
> my time until you've validated things with a real PM like portage.
> 

Whatever will be the bug in the end, you've proven once again to be an unpleasant arrogant person. I'm sorry to say that I'm going to report your repeated attitude to devrel.
Comment 6 Fabio Erculiani (RETIRED) gentoo-dev 2010-08-13 21:23:00 UTC
(In reply to comment #3)
> your PM is broken.  the older package's pre/post rm is run before the newer
> package's postinst and so any files from the old package are gone.  dont waste
> my time until you've validated things with a real PM like portage.
> 

You probably spent 5 seconds reading the bug and its rationale and even less trying to understand if there can be a corner case bug your code doesn't handle.
If you did dare to ask, you could have realized that Entropy phases are exactly what you say here: http://gitweb.sabayon.org/?p=entropy.git;a=blob;f=libraries/entropy/client/interfaces/package.py;h=d804971b5db7d16227fcb64548c0853fc5c56943;hb=HEAD#l3282
Comment 7 Fabio Erculiani (RETIRED) gentoo-dev 2010-08-13 21:54:19 UTC
Issue 1:
if binutils-config -c output reports a broken (not available) profile:
elif [[ $(CHOST=${CTARGET} binutils-config -c) == ${CTARGET}-${BVER} ]] ; then
^^^ this is evaluated false and the profile and links are kept broken

Issue 2:
if [[ ! -e ${BINPATH}/ld ]] && [[ ${current_profile} == ${CTARGET}-${BVER} ]] ; then
BINPATH is always valid because it points to ld just installed, so the whole if statement evaluates false unless ld has been just removed, which is not the case.

So, you are not even validating the output of "binutils-config -c" blindly trusting it. Moreover, you're not even checking its exit status, another bad thing.
Comment 8 SpanKY gentoo-dev 2010-08-13 22:17:52 UTC
i'm not going to spend time debugging your PM.  you choose to write your own which means you choose to debug it.  show me portage failing and i'll gladly look at the eclass.

your analysis of the pkg_* funcs is also incorrect.  did you read the comment right above that code ?
Comment 9 Fabio Erculiani (RETIRED) gentoo-dev 2010-08-13 23:31:36 UTC
Yes I did and I found several bad coding practices, like trusting file content without validation. It's easy to break the logic that you implemented, just write CURRENT=<some-invalid-ver> inside /etc/env.d/binutils/config-x86_64-pc-linux-gnu and emerge -K <your-previously-quickpkg'd-binutils> and boom.
Everybody makes mistakes, you don't need to behave like someone that never fails. If you had written a more robust, less error-prone code, you would have handled unexpected conditions just fine.
Comment 10 SpanKY gentoo-dev 2010-08-14 00:03:51 UTC
if by "bad coding practices" you actually mean that you dont understand all the pieces and exactly how they work together, then i agree.  the point of that statement is not to handle invalid profiles and if the change you propose were added, then USE=multislot behavior (the entire point of the check) would be broken.

yes, a check to see if the current profile is corrupt could be added, but that still doesnt explain why your setup is broken.  installing/removing binary packages works just fine if the files are correct which they should be all the time in a sane system.

i'm not behaving like i dont make mistakes, i'm behaving like you're trying to pawn a broken system of your own onto someone else.  show me a realistic bug (you havent yet -- corruption doesnt count), and i'll look.
Comment 11 Fabio Erculiani (RETIRED) gentoo-dev 2010-08-14 00:31:29 UTC
I've found a small bug (now fixed) in Entropy phases handling that caused toolchain-binutils.eclass to not update binutils config file correctly, but the main issue, the one that was driving me nuts and brought me here, is that toolchain-binutils_pkg_postrm and toolchain-binutils_pkg_postinst cannot recover from invalid data in ${ROOT}/etc/env.d/binutils/config-${CTARGET} once the file is corrupted. Especially toolchain-binutils_pkg_postinst, that just does a sloppy check against the config file, without even caring about its data validity.
Comment 12 Fabio Erculiani (RETIRED) gentoo-dev 2010-08-14 00:33:14 UTC
This doesn't justify your usual and well-known among us, childish attitude against anybody that tries to discuss about a possible bug in a civil manner.
Comment 13 SpanKY gentoo-dev 2010-08-17 02:03:41 UTC
so, to recap because ive forgotten about this bug, your problem was:
 - your PM was buggy and corrupted binutils installs
 - toolchain-binutils.eclass does not account for corruption and so does not recover from PM induced corruption

none of the existing code in the eclass is buggy.  in fact, the comments in there and my comments in this bug explain exactly why each piece exists and why each of your proposed changes are wrong.