Created attachment 118417 [details] [review] Patch to not hardcode libdir but instead use from environment We would like to use polkit on systems that support Application Binary Interface (http://www.x86-64.org/documentation/abi.pdf) and File Hierarchy Standard (http://www.pathname.com/fhs/) where on 64 bit systems the library directory is /usr/lib64. By not hardcoding the library directory and instead using libdir from the environment we can support these environments.
Comment on attachment 118417 [details] [review] Patch to not hardcode libdir but instead use from environment Review of attachment 118417 [details] [review]: ----------------------------------------------------------------- Looks OK to me.
Except: src/polkitagent/polkitagentsession.c: helper_argv[0] = PACKAGE_PREFIX "/lib/polkit-1/polkit-agent-helper-1"; You need to fix this too.
Um, how exactly is placing a helper program into /usr/lib an x86_64 ABI or FHS violation? The ABI only talks about _libraries_; polkit-agent-helper-1 and polkitd are not libraries. FHS 2.3 and 3.0 are AFAICS somewhat ambiguous on whether /usr/lib<qual> should contain only libraries or also helper binaries, but explicitly allow helper binaries in /usr/lib. Apart from strict formalism, I don’t think there is any benefit to making either of those programs multilib/parallel-installable; systems which ship both 32-bit and 64-bit versions of libpolkit* still only need one instance of these binaries (as long as the kernel can run them of course, and as long as the versions of libpolkitagent are roughly in sync). So, it is actually more useful for the path not to change between architectures, and for all versions of the arch-dependent libpolkitagent to use the same hard-coded path: this makes it easier to package polkit without patching it, and get multilib versions of the libraries and no duplicated binaries. The path can be /usr/lib/*, AFAICS in compliance with FHS, or the GNU and Fedora convention of /usr/libexec/*, now codified in FHS 3.0. (This is dangerously close to bikeshedding. Colin, feel free to just commit the patch anyway.)
I agree that the current /usr/lib is fine; I'm curious whether there's an actual problem here or just a blind "64 bit binaries in /usr/lib" rpmlint type warning in some downstream build system. I don't know offhand whether the Debian multiarch spec has something to say about this, but if it does that could be a reason to change.
Actually, this change would effectively require every multilib distribution to either locally revert this patch or change packaging to ship two instances of polkit-agent-helper-1. This would not be a disaster but it is negative enough that I’d like to know about a clear offsetting benefit, and comment#1 is not that to me. What exactly is it that you are trying to do and in what way does placing a binary in /usr/lib not “support” “these environments” (which ones?). There well may be alternative ways to achieve your goal.
(In reply to Colin Walters from comment #5) > I don't know offhand whether the Debian multiarch spec has something to say > about this, but if it does that could be a reason to change. Good point. https://www.debian.org/doc/debian-policy/ch-opersys.html#s9.1 seems to allow, but not require, usage of /usr/lib/$arch-subdirectory. Also https://wiki.debian.org/Multiarch/TheCaseForMultiarch#Proposed_Solution: > Non-target-specific libraries and header files remain in prefix/lib and prefix/include So using /usr/lib seems to be fine with Debian. OTOH the policy manual (first link above) explicitly says FHS 2.3, so no /usr/libexec.
(In reply to Miloslav Trmac from comment #6) > Actually, this change would effectively require every multilib > distribution to either locally revert this patch or change packaging > to ship two instances of polkit-agent-helper-1. I do not understand why a revert of this would be required. The distro would select the libdir and this patch would result in that be respected. > This would not be a disaster but it is negative enough that I’d like to > know about a clear offsetting benefit, and comment#1 is not that to me. > What exactly is it that you are trying to do and in what way does placing a > binary in /usr/lib not “support” “these environments” (which ones?). There > well may be alternative ways to achieve your goal. The goal is indeed to support multilib environments. This patch originates from the OpenEmbedded project (http://www.openembedded.org/wiki/Main_Page) where it is currently applied to polkit to support multilib environments. You can view the patch in the OpenEmbedded code at http://cgit.openembedded.org/meta-openembedded/tree/meta-oe/recipes-extended/polkit/polkit/0001-do-not-hardcoded-libdir.patch - the current version on master contains two of the hunks from the patch I submitted here, I still need to update that patch to include the change in data/Makefile.am so that systemd service file points to the right place. It would be great to see this change in polkit self so that it is not necessary to carry additional patches for polkit to support multilib.
(In reply to Reinette Chatre from comment #8) > (In reply to Miloslav Trmac from comment #6) > > > Actually, this change would effectively require every multilib > > distribution to either locally revert this patch or change packaging > > to ship two instances of polkit-agent-helper-1. > > I do not understand why a revert of this would be required. The distro would > select the libdir and this patch would result in that be respected. libdir also specifies where libpolkit*.so are placed, so it needs to be the usual/default value used in that distribution for shared libraries. Having both multilib variants specify e.g. /usr/lib so that only one copy of the helper executables is used would also cause one of the shared libraries to be overwritten. > > This would not be a disaster but it is negative enough that I’d like to > > know about a clear offsetting benefit, and comment#1 is not that to me. > > What exactly is it that you are trying to do and in what way does placing a > > binary in /usr/lib not “support” “these environments” (which ones?). There > > well may be alternative ways to achieve your goal. > > The goal is indeed to support multilib environments. This patch originates > from the OpenEmbedded project (http://www.openembedded.org/wiki/Main_Page) > where it is currently applied to polkit to support multilib environments. > You can view the patch in the OpenEmbedded code at > http://cgit.openembedded.org/meta-openembedded/tree/meta-oe/recipes-extended/ > polkit/polkit/0001-do-not-hardcoded-libdir.patch - the current version on > master contains two of the hunks from the patch I submitted here, That does not contain any rationale either; why is this patch “necessary to support multilib”? Look at Fedora/RHEL, it is supporting multilib with the current system just fine. (Perhaps we differ in the definition of “multilib”.) In fact, because that patch is also missing the helper_argv hunk, it is about equivalent to (rm /usr/lib*/polkit-1/polkit-agent-helper-1). If “the helper can be found and works” is not among your criteria for “support multilib”, what is the thing that motivated writing that patch?
(In reply to Colin Walters from comment #3) > Except: > > src/polkitagent/polkitagentsession.c: helper_argv[0] = PACKAGE_PREFIX > "/lib/polkit-1/polkit-agent-helper-1"; > > You need to fix this too. Indeed ... that should be changed to PACKAGE_LIB_DIR. Thank you for catching this. I will resubmit.
(In reply to Miloslav Trmac from comment #9) > (In reply to Reinette Chatre from comment #8) > > (In reply to Miloslav Trmac from comment #6) > > > > > Actually, this change would effectively require every multilib > > > distribution to either locally revert this patch or change packaging > > > to ship two instances of polkit-agent-helper-1. > > > > I do not understand why a revert of this would be required. The distro would > > select the libdir and this patch would result in that be respected. > > libdir also specifies where libpolkit*.so are placed, so it needs to be the > usual/default value used in that distribution for shared libraries. Having > both multilib variants specify e.g. /usr/lib so that only one copy of the > helper executables is used would also cause one of the shared libraries to > be overwritten. This is exactly what this patch is trying to prevent by not hardcoding libdir.
(In reply to Reinette Chatre from comment #11) > This is exactly what this patch is trying to prevent by not hardcoding > libdir. No, this is what the patch introduces. Without the patch: > i386 uses --libdir=/usr/lib; this results in /usr/lib/libpolkit-agent-1.so* and /usr/lib/polkit-1/polkit-agent-helper-1 > x86_64 uses --libdir=/usr/lib64; this results in /usr/lib64/libpolkit-agent-1.so* and /usr/lib/polkit-1/polkit-agent-helper-1 Either can be installed and works stand-alone; or they can be both installed at the same time and will work fine (whichever version of polkit-agent-helper-1 is installed, as long as the kernel can run it). (This may need some support from the packaging system, e.g. the RPM “color” mechanism, or the multilib packaging may need to split the libraries and the helper binaries into multilib-specific and multilib-independent parts. But this is nothing new and the system needs to deal with exactly the same issue for /bin/bash already.) With the patch, either > i386 uses --libdir=/usr/lib; this results in /usr/lib/libpolkit-agent-1.so* and /usr/lib/polkit-1/polkit-agent-helper-1 > x86_64 uses --libdir=/usr/lib64; this results in /usr/lib64/libpolkit-agent-1.so* and /usr/lib64/polkit-1/polkit-agent-helper-1 and there are two unnecessary instances of the helper when one would do, or > i386 uses --libdir=/usr/lib; this results in /usr/lib/libpolkit-agent-1.so* and /usr/lib/polkit-1/polkit-agent-helper-1 > x86_64 uses --libdir=/usr/lib; this results in /usr/lib/libpolkit-agent-1.so* and /usr/lib/polkit-1/polkit-agent-helper-1 which has only one copy of the helper, but also makes the x86_64 .so unusable because it is in the wrong place, and if it overwrote the i386 version of the library in /usr/lib it would break i386 callers as well.
(In reply to Miloslav Trmac from comment #12) > > i386 uses --libdir=/usr/lib; this results in /usr/lib/libpolkit-agent-1.so* and /usr/lib/polkit-1/polkit-agent-helper-1 I suspect modern OpenEmbedded might be following in the Debian footsteps of "multiarch" - distinct from the "multilib" that ended up in Fedora and OpenSUSE derivatives. https://wiki.debian.org/Multiarch I don't think multiarch is useful for the polkit helpers even in Debian, but I can understand wanting the binaries to land in their arch libdir.
(In reply to Colin Walters from comment #13) > (In reply to Miloslav Trmac from comment #12) > > > > i386 uses --libdir=/usr/lib; this results in /usr/lib/libpolkit-agent-1.so* and /usr/lib/polkit-1/polkit-agent-helper-1 > > I suspect modern OpenEmbedded might be following in the Debian footsteps of > "multiarch" - distinct from the "multilib" that ended up in Fedora and > OpenSUSE derivatives. > > https://wiki.debian.org/Multiarch Yeah, but the issue is exactly the same. > I don't think multiarch is useful for the polkit helpers even in Debian, but > I can understand wanting the binaries to land in their arch libdir. As linked above, the Debian design explicitly expects for non-arch-specific content in /usr/lib (not /usr/lib/$triplet). I guess moving to /usr/libexec, and having separate --libdir and --libexecdir (and telling Debian to set --libexecdir=/usr/lib) might make everyone here happy enough, but it is really impossible to tell without knowing what the OpenEmbedded definition of “supports multilib” is.
(In reply to Miloslav Trmac from comment #14) > I guess moving to /usr/libexec, and having separate --libdir and > --libexecdir (and telling Debian to set --libexecdir=/usr/lib) might make > everyone here happy enough, but it is really impossible to tell without > knowing what the OpenEmbedded definition of “supports multilib” is. The documentation I found about their multilib usage is at http://www.yoctoproject.org/docs/latest/mega-manual/mega-manual.html#combining-multiple-versions-library-files-into-one-image In response to your other comments: Thank you very much for the detailed explanation of the polkit environment. At this time polkit fails for me in the OpenEmbedded created images when using multilib and systemd. I understand now your organization and will pursue this with the openembedded folks where I hope to be able to bring your goals in line with their images. Likely in that series of conversations I will learn the details of the multilib goals.
(In reply to Reinette Chatre from comment #15) > (In reply to Miloslav Trmac from comment #14) > > I guess moving to /usr/libexec, and having separate --libdir and > > --libexecdir (and telling Debian to set --libexecdir=/usr/lib) might make > > everyone here happy enough, but it is really impossible to tell without > > knowing what the OpenEmbedded definition of “supports multilib” is. > > The documentation I found about their multilib usage is at > http://www.yoctoproject.org/docs/latest/mega-manual/mega-manual. > html#combining-multiple-versions-library-files-into-one-image This is all talking about the recipe system specific concepts; I can’t easily decipher the intended end result on the file system. Anyway the wider context of the 5.5 section seems to suggest this is only discussing libraries (*.so, *.a), not other helper files.
(In reply to Miloslav Trmac from comment #16) > This is all talking about the recipe system specific concepts; I can’t > easily decipher the intended end result on the file system. Anyway the wider > context of the 5.5 section seems to suggest this is only discussing > libraries (*.so, *.a), not other helper files. Indeed. I submitted a patch to openembedded that reverts the original patch that my patch submitted here was based on. If accepted then openembedded images will stop making changes to where the polkit binaries are installed. http://article.gmane.org/gmane.comp.handhelds.openembedded/71058 http://patches.openembedded.org/patch/104129/
Patch was accepted into meta-openembedded
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.