| Summary: | Portage fails with AssertionError under MUSL when using EAPI=6. | ||
|---|---|---|---|
| Product: | Gentoo Linux | Reporter: | Aric Belsito <lluixhi> |
| Component: | Current packages | Assignee: | Gentoo musl team <musl> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | dev-portage, herrtimson, python, zmedico |
| Priority: | Normal | Keywords: | InVCS |
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | Linux | ||
| See Also: |
https://bugs.gentoo.org/show_bug.cgi?id=584328 https://github.com/gentoo/portage/pull/1290 |
||
| Whiteboard: | |||
| Package list: | Runtime testing required: | --- | |
| Bug Depends on: | 583412, 594744 | ||
| Bug Blocks: | 430702, 604854 | ||
| Attachments: |
Portage Traceback.
Workaround in portage C code for a python module to check locales for portage setup.py for check_locales.c module |
||
config.py uses check_locale(), which uses ctypes.util.find_libray("c").
The underlying cause seems to be that both approaches of python's
ctypes.util.find_library() to find the library name fail. One approach
depends on the non-existing ld.so cache. The other approach uses gcc to
successfully find /usr/lib/libc.so but then unsuccessfully looks for an
SONAME in the dynamic section. (musl libc does not have a soname.)
Created attachment 422474 [details, diff]
Workaround in portage
@Felix Janda
it uses ctypes.util.find_library("c")?
There is a patch for this that alpine and voidlinux are using in python:
https://bugs.alpinelinux.org/issues/4512
https://github.com/voidlinux/void-packages/commit/a99a70dcc8b21b0e7e12bbf25f9f6dad47e6f52c
We won't have to patch portage if that's the case -- I thought something else was broken.
The patch for python does not really look upstreamable. It makes the wrong assumption that all systems have libm, libcrypt, libpthread in libc like musl. Also it makes find_library also accept "libc.so" while according to the documentation it should only take "c". It seems that find_library is in portage only used in order to find and load the c library. It seems that the code in portage would be simplified if it would use a dedicated function for libc loading using CDLL instead. If musl doesn't have SONAME yet uses name matching it, it's a bug in musl. musl has no SONAME and uses the name libc.so. Its dlopen special cases "libc.*".
I guess that this behavior is not very portable and to find libc one
would need to check CDLL("libc.so"), CDLL("libc.dynlib") and probably
something completely different for cygwin. Maybe that's why
load_library(find_library("c")) is used.
@Michal They removed the SONAME in this commit: http://git.musl-libc.org/cgit/musl/commit/?id=dfdc337b3b276e6ea0e4786ede699f4d0d93dc40 I don't think they have any plans to bring it back. @Felix Yeah, in addition MUSL only has libc.so in /usr/lib, not libc.so.6 in /lib I just looked at this bug and its going to be a serious problem for us in musl. (In reply to Michał Górny from comment #5) > If musl doesn't have SONAME yet uses name matching it, it's a bug in musl. I know that glibc is full of versioning, but really a libc should just conform to standards. By enabling the correct defines (eg -D_GNU_SOURCE) you enable/disable what functions you want. Why would you need matching on an SONAME? (In reply to Aric Belsito from comment #7) > > @Felix > Yeah, in addition MUSL only has libc.so in /usr/lib, not libc.so.6 in /lib There is some magic in CDLL() that makes both CDLL('libc.so') and CDLL('libc.so.6') work on musl. You need 'libc.so.6' on glibc. The problem with Felix's patch is that it assumes the name libc.so.6 which is not the case in uclibc (its libc.so.0) or possibly other libcs. The original portage code makes no such assumption. The real fix is in the python which should not assume that there is either and ld.so.cache or that the libc has an SONAME. I see getting that fixed there an upstream battle. The alpine linux patch isn't going to work on many systems, so its not the way to go. I wonder if there's some other approach to running libc.toupper() and .tolower() which is why we're dlopening libc in the first place. By converting a list of upper to lower case letters and vice versa, we check the locales. Can't we do this without dlopening libc? No, we can't, because the point is to test the libc behavior and not random python hacks added on top of it. @Anthony: CDLL is almost the same as dlopen. musl's dynlinker does the magic.
I think CDLL("libc.so") only does not work on glibc and uclibc because of the ld script.
So try, CDLL("libc.so.6"), CDLL("libc.so.0"), CDLL("libc.so"), CDLL("libc.dylink")
and if nothing works throw an exception?
We could also try to come up with an upstreamable patch for python.
Indeed, musl's not having DT_SONAME is intentional. Programs that want to search for a library "by soname" should be treating a library that does not have a DT_SONAME as if its soname were the same as its filename. This is the existing convention. However I'm not clear why any search for the .so file is being made. The desired library name (e.g. "libc.so") should just be passed to dlopen which does the search, and in musl's case, this search is always resolved internally. Re-loading musl's libc.so via an absolute pathname is not safe and will result in a conflicting copy getting loaded; this is an actual bug we need to address on our side. (In reply to Felix Janda from comment #10) > @Anthony: CDLL is almost the same as dlopen. musl's dynlinker does the magic. > > I think CDLL("libc.so") only does not work on glibc and uclibc because of > the ld script. Nope. When the ld script is there, then the error message is that its not an ELF. Without the script it says file not found. > > So try, CDLL("libc.so.6"), CDLL("libc.so.0"), CDLL("libc.so"), > CDLL("libc.dylink") > and if nothing works throw an exception? Yeah this seems like the best solution. This is a heuristic search, so if some day uclibc switches ot libc.so.1 then it would break again, but it should be easy to find the breakage and it isn't likely to happen soon. @portage peeps, are you okay with is? > > We could also try to come up with an upstreamable patch for python. How that patch search library paths is a problem, but I'm not sure how to fix it in a way that'll work across system. For now, I think I'm just going to put a patch for portage on the musl overlay. I hope the portage team is willing to work with us on this one else we're going to be stuck in the mud. Alternatively, we could add a C extension that calls libc functions. (In reply to Michał Górny from comment #13) > Alternatively, we could add a C extension that calls libc functions. yeah that's a good idea. i can hack something out tomorrow. Created attachment 423214 [details]
C code for a python module to check locales for portage
Created attachment 423218 [details]
setup.py for check_locales.c module
(In reply to Anthony Basile from comment #15) > Created attachment 423214 [details] > C code for a python module to check locales for portage I've only been able to test this on systems where it works: Python 3.4.3 (default, Jan 5 2016, 03:19:14) [GCC 4.9.3] on linux Type "help", "copyright", "credits" or "license" for more information. >>> import check_locales >>> check_locales.portage_check_locales() 1 On a system where the toupper() and tolower() fail to give the correct values it will return 0. @mgorny, can you glue this into portage? (In reply to Anthony Basile from comment #17) > > @mgorny, can you glue this into portage? okay i'll see about producing a patch against portage, although i don't know portage internals that well. in the mean time, portage-2.2.26-r99 is on the musl overlay and it uses Felix's fix from above. (In reply to Anthony Basile from comment #18) > (In reply to Anthony Basile from comment #17) > > > > @mgorny, can you glue this into portage? > > okay i'll see about producing a patch against portage, although i don't know > portage internals that well. Since the portage codebase is entirely interpreted, I would like the check_locales module to be installed by a separate ebuild, and have portage fall back to ctypes if the check_locales module is not available. (In reply to Zac Medico from comment #19) > (In reply to Anthony Basile from comment #18) > > (In reply to Anthony Basile from comment #17) > > > > > > @mgorny, can you glue this into portage? > > > > okay i'll see about producing a patch against portage, although i don't know > > portage internals that well. > > Since the portage codebase is entirely interpreted, I would like the > check_locales module to be installed by a separate ebuild, and have portage > fall back to ctypes if the check_locales module is not available. okay, but before i throw a package together, are there some good tests i could throw into a test suite? (In reply to Anthony Basile from comment #20) > okay, but before i throw a package together, are there some good tests i > could throw into a test suite? The results of the check_locale function depend on these variables, where variables on the left are more significant than variables on the right: LC_ALL LC_CTYPE LANG The function should return False when the value of the most significant variable has a value of tr_TR, and True when the most significant variable has a value of C.UTF-8, en_US.UTF-8, en_GB.UTF-8, or C. (In reply to Zac Medico from comment #19) > (In reply to Anthony Basile from comment #18) > > (In reply to Anthony Basile from comment #17) > > > > > > @mgorny, can you glue this into portage? > > > > okay i'll see about producing a patch against portage, although i don't know > > portage internals that well. > > Since the portage codebase is entirely interpreted, I would like the > check_locales module to be installed by a separate ebuild, and have portage > fall back to ctypes if the check_locales module is not available. I agree. This will keep it easy to run portage from a git checkout, also helps with rescuing broken systems... If this is done similar to the xattr module that primarily uses the separately compiled and installed module, but falls back to some internal portage python code as backup. This will maintain that capability. (In reply to Brian Dolbec from comment #22) > (In reply to Zac Medico from comment #19) > > (In reply to Anthony Basile from comment #18) > > > (In reply to Anthony Basile from comment #17) > > > > > > > > @mgorny, can you glue this into portage? > > > > > > okay i'll see about producing a patch against portage, although i don't know > > > portage internals that well. > > > > Since the portage codebase is entirely interpreted, I would like the > > check_locales module to be installed by a separate ebuild, and have portage > > fall back to ctypes if the check_locales module is not available. > > I agree. This will keep it easy to run portage from a git checkout, also > helps with rescuing broken systems... > > If this is done similar to the xattr module that primarily uses the > separately compiled and installed module, but falls back to some internal > portage python code as backup. This will maintain that capability. I disagree. The separate code doesn't really have any other use, and you can keep all the extra capabilities while keeping the module code inside Portage repository. While modularity is good, please keep some common sense. There's really no point in inventing separate package for this -- yet another ebuild that is used only by portage. In fact, this will only make it harder to test Portage checkouts in their current state. Portage will either force the python version, ignoring the C extension, or use installed, possibly outdated extension. Forgive me if I'm wrong, it's been many, many years since I worked on compiled apps. Doesn't "C" code have to be compiled to run, which would imply it being installed. So, if portage relied strictly on this, then it could not reliably operate from a checkout. Also, just how often will this "C" code change? It seems to me, it is unlikely to change much if at all. Is this "C" code really only for musl? or is it going to be used for everything? It is not a part of the code I'm familiar with. If it is installed via portage ebuild as an extension, and you are testing a checkout, does the checkout code look for and find the installed C extension? Or do you have to build the checkout and run the built code? (In reply to Brian Dolbec from comment #24) > Forgive me if I'm wrong, it's been many, many years since I worked on > compiled apps. Doesn't "C" code have to be compiled to run, which would > imply it being installed. So, if portage relied strictly on this, then it > could not reliably operate from a checkout. If Portage relied strictly on this, it wouldn't run at all without having some random package installed. > Also, just how often will this "C" code change? It seems to me, it is > unlikely to change much if at all. Is this "C" code really only for musl? > or is it going to be used for everything? It is not a part of the code I'm > familiar with. That's not an argument. Random non-reusable code doesn't get split into random non-reusable packages just because it isn't supposed to change much. How about we split the rarely changing Python modules of Portage to an external package? This will reduce the size of release tarballs! > If it is installed via portage ebuild as an extension, and you are testing a > checkout, does the checkout code look for and find the installed C extension? > > Or do you have to build the checkout and run the built code? That's something you can control. I'd go for building and using local extension by default but supporting running without it. I wouldn't mix installed system extension with Portage much like we don't load random modules from installed Portage version. If the python code can run from a checkout using it's backup python code without the compiled extension. I'd be fine with it being built and installed with the portage setup.py and ebuild. Especially if this code is not useful to any other pkg. (In reply to Brian Dolbec from comment #26) > If the python code can run from a checkout using it's backup python code > without the compiled extension. I'd be fine with it being built and > installed with the portage setup.py and ebuild. Especially if this code is > not useful to any other pkg. I haven't dropped the ball on this, just been too busy. The code might be useful elsewhere, but I doubt it. So I'll include it in the portage code base. I'm thinking maybe I'll try to import the module and if it fails then fall back on the current python. From my standpoint the fundamental problem is that Python's ctypes module has a serious XY problem. The problem they want to solve is "given a bare library name A, load library A". But their find_library function is instead implementing "given a bare library name A, construct a full pathname for library A". At best what they're doing is redundant with the implementation's (libc's/libdl's/ldso's) implementation-defined library search; at worst, it's actually inconsistent and wrong. Rather than taking the string "c" and trying to transform it into an absolute pathname for libc.so to pass to dlopen, ctypes should just be transforming "c" to "libc.so" and passing the latter directly to dlopen. Then dlopen performs whatever implementation-defined search it performs and gives the right library. (In the case of musl, loading "libc.so" translates to returning a reference to the dynamic linker itself; if you actually forced it to load another libc with an absolute pathname and called functions in the resulting loaded library, you would quickly trash your process state.) Patches for this have been sent to gentoo-portage-dev@lists.gentoo.org (In reply to Anthony Basile from comment #29) > Patches for this have been sent to gentoo-portage-dev@lists.gentoo.org progress on this is help up at least for python2.7 because of bug #583412 blueness portage patches seem still not to be merged. Should they be resent? The patch from bug 584328 suppresses the issue. @zmedico Wait, what happened to the patches that blueness wrote over here? https://github.com/gentoo/musl/tree/master/sys-apps/portage/files (In reply to Zac Medico from comment #32) > The patch from bug 584328 suppresses the issue. There are two disadvantages with Arfrever's approach: 1) it suppresses the issue but it does not fix it by providing a working wrapper for libc's tolower() and toupper(). So you're really not doing a proper locale check on musl. 2) These patches were forward looking and supposed to make the addition of other wrappers for C functions easy. (In reply to Aric Belsito from comment #33) > @zmedico > > Wait, what happened to the patches that blueness wrote over here? > > https://github.com/gentoo/musl/tree/master/sys-apps/portage/files Those patches just got merged into master too. So, if there is something else needed for them both to play nice together, please submit them. Otherwise I'll be making a release on the weekend. (In reply to Brian Dolbec from comment #35) > (In reply to Aric Belsito from comment #33) > > @zmedico > > > > Wait, what happened to the patches that blueness wrote over here? > > > > https://github.com/gentoo/musl/tree/master/sys-apps/portage/files > > Those patches just got merged into master too. > > So, if there is something else needed for them both to play nice together, > please submit them. Otherwise I'll be making a release on the weekend. okay, i thought you guys were just going to opt with suppressing the issue. i'm okay with this. The ebuild now has a native-extensions USE flag to enable the libc bindings: https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=dbe0bd44461aff3097fde6c64147f1a61e30816d Fixed in portage-2.3.3. |
Created attachment 422436 [details] Portage Traceback. The traceback is attached.