Summary: | media-sound/apulse ABI_X86=32 doesn't install script that loads the 32bit libs | ||
---|---|---|---|
Product: | Gentoo Linux | Reporter: | Pablo Yanez Trujillo <shaoran> |
Component: | Current packages | Assignee: | zlg (RETIRED) <zlg> |
Status: | RESOLVED FIXED | ||
Severity: | minor | CC: | multilib+disabled |
Priority: | Normal | ||
Version: | unspecified | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Package list: | Runtime testing required: | --- |
Description
Pablo Yanez Trujillo
2015-04-23 20:17:00 UTC
I just realized that while assignments have changed on this bug, nobody's actually replied. This looks like a simple fix, so I'll see what I can do this week to get this in tree. I agree that a symlink would be a good idea to convey to users a 32-bit version is available. Just wanted to let you know this bug didn't go ignored! I don't like the idea of making ABI_X86=32 special by adding extra symlinks. Maybe it'd be better to make apulse script able to handle all ABIs? (In reply to Michał Górny from comment #2) > I don't like the idea of making ABI_X86=32 special by adding extra symlinks. > Maybe it'd be better to make apulse script able to handle all ABIs? That's a better idea, and would allow apulse's script to be at least somewhat 'future proof' should upstream expand apulse's functionality. I'll look into doing that. Pushed to git a few minutes ago. [0] /usr/bin/apulse is now a shell script that will automatically look at the application being launched and set the libdir correctly. Users who want to deliberately use one version or the other still can; they're found in /usr/bin/i686-pc-linux-gnu-apulse and /usr/bin/x86_84-pc-linux-gnu-apulse, just like before. The new script has been tested on a multilib ~amd64 machine with bash, targeting Skype as the primary use case. I'll go ahead and mark this resolved. Please reopen this bug if something was missed. Thanks for reporting! [0]: https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=a7d86faf81d3ea5b46abf91cd030ab9992928b78 You just did everything we told you not to do. Especially, you hardcoded random libdirs in incorrect locations, and your script is not going to work on any system except for dumb old amd64 profile. (In reply to Michał Górny from comment #5) > You just did everything we told you not to do. Especially, you hardcoded > random libdirs in incorrect locations, and your script is not going to work > on any system except for dumb old amd64 profile. I'm not sure I understand how I did what I was told not to do: what's hardcoded? The script checks `file -L` output for information on its header, which hints at its ABI. It then adds the correct apulse ABI version to LD_LIBRARY_PATH before the executable is ran. I lack the hardware to test other ABIs and profiles, but based on what I was able to learn about this situation and during my testing, it handles things correctly. What can I do to make this more correct? "Support all ABIs" is a bit vague in what's being asked for. apulse is only keyworded for x86 and amd64, and it's all I have access to, so the case block accounts for those two. It can easily be extended, if I know what the third field of `file -L` output says and the correct libdir for said ABI. Just to be clear, I didn't intentionally disregard anything. This script is my best try at supporting multiple ABIs. If I've missed something or there's a tool I don't know about, it'd be helpful to point me in the right direction so this mistake doesn't happen again. Thanks for the feedback. (In reply to Daniel Campbell from comment #6) > (In reply to Michał Górny from comment #5) > > You just did everything we told you not to do. Especially, you hardcoded > > random libdirs in incorrect locations, and your script is not going to work > > on any system except for dumb old amd64 profile. > > I'm not sure I understand how I did what I was told not to do: what's > hardcoded? The script checks `file -L` output for information on its header, > which hints at its ABI. It then adds the correct apulse ABI version to > LD_LIBRARY_PATH before the executable is ran. > > I lack the hardware to test other ABIs and profiles, but based on what I was > able to learn about this situation and during my testing, it handles things > correctly. What can I do to make this more correct? "Support all ABIs" is a > bit vague in what's being asked for. apulse is only keyworded for x86 and > amd64, and it's all I have access to, so the case block accounts for those > two. It can easily be extended, if I know what the third field of `file -L` > output says and the correct libdir for said ABI. > > Just to be clear, I didn't intentionally disregard anything. This script is > my best try at supporting multiple ABIs. If I've missed something or there's > a tool I don't know about, it'd be helpful to point me in the right > direction so this mistake doesn't happen again. > > Thanks for the feedback. Your code breaks when LIBDIR_x86=lib. Your code breaks on ARCH=x86. Your code breaks when the user calls a 32-bit application that then calls a 64-bit application. Your code breaks when the "application" called is a shell script that calls the real application. The easiest way to support all of these is to set the LD_LIBRARY_PATH variable to include the value of "/usr/$(get_abi_LIBDIR $ABI)/apulse" for *each* ABI installed, regardless of what command is called by the user (therefore, on my system, it would be "/usr/lib64/apulse:/usr/lib/apulse", as I have LIBDIR_x86=lib && SYMLINK_LIB=no). The correct libdir for a given ABI can be set by the user, and is not guaranteed to be the same on all systems. (In reply to Jonathan Callen from comment #7) > (In reply to Daniel Campbell from comment #6) > > (In reply to Michał Górny from comment #5) > > > You just did everything we told you not to do. Especially, you hardcoded > > > random libdirs in incorrect locations, and your script is not going to work > > > on any system except for dumb old amd64 profile. > > > > [snip] > > Your code breaks when LIBDIR_x86=lib. Your code breaks on ARCH=x86. Your > code breaks when the user calls a 32-bit application that then calls a > 64-bit application. Your code breaks when the "application" called is a > shell script that calls the real application. The easiest way to support > all of these is to set the LD_LIBRARY_PATH variable to include the value of > "/usr/$(get_abi_LIBDIR $ABI)/apulse" for *each* ABI installed, regardless of > what command is called by the user (therefore, on my system, it would be > "/usr/lib64/apulse:/usr/lib/apulse", as I have LIBDIR_x86=lib && > SYMLINK_LIB=no). The correct libdir for a given ABI can be set by the user, > and is not guaranteed to be the same on all systems. So if I understand correctly, this means I'll need to run `sed` multiple times in the ebuild itself to ensure that LD_LIBRARY_PATH has every ABI on the machine added? Something like: for abi in $ABI_VAR; do # add this line into the script with sed LD_LIBRARY_PATH="/usr/$(get_abi_LIBDIR $abi)/apulse:${LD_LIBRARY_PATH}" done ...? Thanks for providing more specific advice so I can get this fixed. (In reply to Daniel Campbell from comment #8) > (In reply to Jonathan Callen from comment #7) > > (In reply to Daniel Campbell from comment #6) > > > (In reply to Michał Górny from comment #5) > > > > You just did everything we told you not to do. Especially, you hardcoded > > > > random libdirs in incorrect locations, and your script is not going to work > > > > on any system except for dumb old amd64 profile. > > > > > > [snip] > > > > Your code breaks when LIBDIR_x86=lib. Your code breaks on ARCH=x86. Your > > code breaks when the user calls a 32-bit application that then calls a > > 64-bit application. Your code breaks when the "application" called is a > > shell script that calls the real application. The easiest way to support > > all of these is to set the LD_LIBRARY_PATH variable to include the value of > > "/usr/$(get_abi_LIBDIR $ABI)/apulse" for *each* ABI installed, regardless of > > what command is called by the user (therefore, on my system, it would be > > "/usr/lib64/apulse:/usr/lib/apulse", as I have LIBDIR_x86=lib && > > SYMLINK_LIB=no). The correct libdir for a given ABI can be set by the user, > > and is not guaranteed to be the same on all systems. > > So if I understand correctly, this means I'll need to run `sed` multiple > times in the ebuild itself to ensure that LD_LIBRARY_PATH has every ABI on > the machine added? Something like: > > for abi in $ABI_VAR; do > # add this line into the script with sed > LD_LIBRARY_PATH="/usr/$(get_abi_LIBDIR $abi)/apulse:${LD_LIBRARY_PATH}" > done > > ...? > > Thanks for providing more specific advice so I can get this fixed. You could do something like DIRS= _add_dir() { DIRS="${EPREFIX}/usr/$(get_libdir)/apulse${DIRS:+:${DIRS}}"; } multilib_foreach_abi _add_dir sed -i -e "s#@@DIRS@@#${DIRS}#g" apulse to get the list of directories into the wrapper. Commit 7345c7f2d1f42ec82330796f3b8cda8b6d558a49 includes the proposed changes. Thanks for that feedback and advice. Testing revealed all installed libdirs showed up for me. Please test it before I mark it resolved. Users should remerge the package using `emerge -a1 media-sound/apulse` to receive the updated script. Commit b252629653a11bdca61279c5de260d66e6ff8fbd places script editing in src_prepare phase where it belongs. Results are identical. Marking as RESOLVED unless there are further concerns. |