Anonymous | Login | Signup for a new account | 2024-03-19 09:47 UTC |
My View | View Issues | Change Log | Roadmap | Doomseeker Issue Support Ranking | Rules | My Account |
View Issue Details [ Jump to Notes ] | [ Issue History ] [ Print ] | ||||||||
ID | Project | Category | View Status | Date Submitted | Last Update | ||||
0003239 | Doomseeker | [All Projects] Bug | public | 2017-09-01 15:01 | 2018-10-27 22:55 | ||||
Reporter | WubTheCaptain | ||||||||
Assigned To | Zalewa | ||||||||
Priority | normal | Severity | major | Reproducibility | have not tried | ||||
Status | closed | Resolution | fixed | ||||||
Platform | x86_64 | OS | Debian GNU/Linux | OS Version | buster/sid | ||||
Product Version | 1.1 | ||||||||
Target Version | 1.2 | Fixed in Version | 1.2 | ||||||
Summary | 0003239: engines/libzandronum.so: Defines RPATH from somewhere | ||||||||
Description | Zandronum engine in Doomseeker defines RPATH from somewhere. This may create issues with Debian's local policy or cause issues for multilib, among other issues.$ objdump -x /usr/share/doomseeker/engines/libzandronum.so | grep RPATH RPATH $ORIGIN Lintian tag (error): binary-or-shlib-defines-rpath (severity serious, certainty possible) | ||||||||
Steps To Reproduce | Found in both deb.drdteam.org binary builds and debuild source builds. | ||||||||
Additional Information | 'https://wiki.debian.org/RpathIssue [^]' 'https://lintian.debian.org/tags/binary-or-shlib-defines-rpath.html [^]' | ||||||||
Attached Files | |||||||||
Relationships | |||||||||||
|
Notes | |
(0018210) WubTheCaptain (reporter) 2017-09-01 15:02 |
I suspect this is related to zlib. zlib itself doesn't have this error in Debian's distribution. Could also be related to issue 0003232, having the engines in /usr/lib/doomseeker/ may fix the issue. |
(0018301) Zalewa (developer) 2017-09-17 15:55 |
I finally got around to investigating why this happens and found out that it is caused by line 89 in Zandronum's CMakeLists.txt. target_link_libraries there makes the Zandronum plugin be directly dependant on Wadseeker library. Why there's such dependency in the first place? Because Zandronum offers to install testing builds that are downloaded as archives and Wadseeker already has all the necessary code to extract those archives. Here, at line 316. On Linux I can remove this dependency and the code will still compile and with RPATH gone. I'm not sure why this happens, though - does it somehow resolve the necessary code against Doomseeker? Unfortunately, removing this line on Windows breaks compilation. Getting to the point, we can do: 1. I think the most proper solution would be to remove the direct dependency of that plugin on Wadseeker. Instead, UnArchive functionality should be provided by Doomseeker itself. Of course, Doomseeker will only provide a shallow proxy (unbeknownst to plugins) and still forward all calls to Wadseeker. 2. Dirty solution is to add 'if' check to Zandronum's CMakeLists.txt and link with Wadseeker on Windows only. 3. Another dirty solution would be to run chrpath on the built plugin. |
(0018306) Blzut3 (administrator) 2017-09-17 21:22 |
Yes. On I believe all OSs besides Windows, symbols are loaded into a single pool so as long as something provides the symbol then it will resolve. Now I don't know if I'd say one way is better than the other, but it is what it is. There is an INSTALL_RPATH property which I was going to check to make sure it has a sane value. Rereading the ticket now though I realize WubTheCaptain may have been wanting no RPATH to be defined, which can be had by adding "-DCMAKE_SKIP_BUILD_RPATH=TRUE" to the CMake command line. |
(0018308) Zalewa (developer) 2017-09-18 07:33 |
There's also CMAKE_SKIP_RPATH and CMAKE_SKIP_INSTALL_RPATH. I tested this in Docker and CMAKE_SKIP_BUILD_RPATH also affects the result of 'make install' so I have no idea why CMAKE_SKIP_RPATH exists. The doc doesn't make it clear either: 'https://cmake.org/cmake/help/v3.9/variable/CMAKE_SKIP_BUILD_RPATH.html [^]' 'https://cmake.org/cmake/help/v3.9/variable/CMAKE_SKIP_RPATH.html [^]' 'https://cmake.org/Wiki/CMake_RPATH_handling#No_RPATH_at_all [^]' Maybe it makes difference on Mac? |
(0018312) Blzut3 (administrator) 2017-09-18 07:55 |
I generally don't care for using make install so I'm not sure what the defaults come out to on the Mac. But the INSTALL_RPATH property on the target would set the RPATH for the make install step. The default on Linux at least is to apparently not touch the RPATH. So the difference is CMAKE_SKIP_RPATH would actually fully disable RPATH whereas CMAKE_SKIP_BUILD_RPATH would still set the RPATH if one was specified in INSTALL_RPATH. I guess actually my suggestion in the last post should have been CMAKE_SKIP_RPATH. |
(0018316) Zalewa (developer) 2017-09-18 09:09 |
I can't decide on the right course of action here. While my gut tells me that, ideally, the only dependency of plugins should be Doomseeker itself, the fact is that they're already dependant on Qt and there's nothing we can do about that. If Wadseeker was code that wasn't in the same CMake project, or something that we didn't control, or an optional dependency of Doomseeker, then 1. would be undoubtedly the correct course of action here. But that's not the case, and adding another MAIN_EXPORT API in Doomseeker that would mirror WADSEEKER_API 1-to-1 means one more API/ABI to consider when making changes. I think that mentioning RPATH and how to get rid off it in COMPILE.txt will be sufficient for the time being. Maybe we can also consider turning it off by default for `make install` purposes. I also wonder how Wub managed to get $ORIGIN there, which seems to be a correct outcome. Builds in my Linux or in Docker result in an absolute path being put there. Doc's recommendations state that there is a CMAKE_INSTALL_RPATH variable, but that's unset by default and cmake-gui doesn't even display it on the list. |
(0018317) WubTheCaptain (reporter) 2017-09-18 09:09 |
Quote from Blzut3 Yup, but ideally the change should be upstreamed to the relevant binary. |
(0018318) WubTheCaptain (reporter) 2017-09-18 09:13 |
Quote from "Zalewa" I'd have to test this again to verify, but if I recall correctly /usr/share/doomseeker/engines/libzandronum.so was from debian.drdteam.org build and it's an absolute path before make install. |
(0018332) Blzut3 (administrator) 2017-09-19 00:14 |
Yes, the $ORIGIN rpath comes from me manually scrubbing the build rpath while packaging. I'm noticing now that it looks like by default CMake scrubs the rpath off of the Doomseeker binary during make install, so it's probably worth digging into why it doesn't think the same should be done for libzandronum.so. The build rpath is there for the same reason on both... While there does appear to be some work to be done here I do want to note that CMAKE_SKIP_RPATH and CMAKE_SKIP_INSTALL_RPATH are both user options exposed in the GUI/TUI. So while we should address the fact that a not so useful value sticks around after install, there are times where setting these at build time is the correct thing to do. For example the Mac release scripts use it since ancient versions of Mac OS X don't support having an rpath, but it is probably helpful to have it on modern macOS. |
(0018337) WubTheCaptain (reporter) 2017-09-19 07:11 edited on: 2017-09-19 07:13 |
Until you can find a real solution, there should be no issue with configuring the Debian source package to use -DCMAKE_SKIP_BUILD_RPATH=TRUE or overriding the warning. Thank you for all the information, by the way. |
(0018368) Zalewa (developer) 2017-09-24 16:56 |
Since linking the Zandronum plugin with Wadseeker is not needed on Linux, and this is the cause of it getting an RPATH, I just went ahead and wrapped the linking statement in a conditional check to disable linking on Linux: 1.'https://bitbucket.org/Doomseeker/doomseeker/commits/8d7667ab7ab1c6a68bdd73440edd6748a94ed4f3 [^]' I also made a subsequent commit when I felt that such strange action needs a comment: 2.'https://bitbucket.org/Doomseeker/doomseeker/commits/1d4d939a78e07993d3096cd2d0fb51bb0ec22cf1 [^]' |
(0018463) WubTheCaptain (reporter) 2017-10-07 11:14 edited on: 2017-10-07 11:17 |
RPATH/RUNPATH is gone post-install (installed .deb from debuild), tested with latest Mercurial source (1.2~beta-171002-2156M commit aaaf3f92ce67 revision 1506981393) where your commit is included. It is still present in pre-install after debuild, but the reason for that may be a bit more obvious. I did not manually insert -DCMAKE_SKIP_BUILD_RPATH=TRUE to anywhere in the build process, though I still probably could if I ever needed to. Lintian warning/error "binary-or-shlib-defines-rpath" is gone too. I suppose this is resolved, do you think? I suspect 0003256 will still remain unresolved however, because RUNPATH is still defined pre-install. |
(0018464) WubTheCaptain (reporter) 2017-10-07 11:19 |
Would it still be better to add SKIP_BUILD_RPATH at least conditionally on GNU/Linux systems, per the Debian wiki suggestion? |
(0018465) Zalewa (developer) 2017-10-07 11:19 |
Quote from "Wub" That should not be the case. I create makefile with all CMake settings set to default and both skip_rpaths are definitely off and yet "engines/libzandronum.so" is created without RPATH. |
(0018466) WubTheCaptain (reporter) 2017-10-07 11:28 edited on: 2017-10-07 11:29 |
Sorry, my bad. You're correct, I accidentally checked the Doomseeker executable instead.$ objdump -x obj-x86_64-linux-gnu/doomseeker | egrep "RPATH|RUNPATH" RUNPATH /tmp/doomseeker-1.2/obj-x86_64-linux-gnu: $ objdump -x obj-x86_64-linux-gnu/engines/libzandronum.so | egrep "RPATH|RUNPATH" $ objdump -x debian/doomseeker/usr/bin/doomseeker | egrep "RPATH|RUNPATH" $ objdump -x debian/doomseeker/usr/lib/doomseeker/engines/libzandronum.so | egrep "RPATH|RUNPATH" $ objdump -x /usr/bin/doomseeker | egrep "RPATH|RUNPATH" $ objdump -x /usr/lib/doomseeker/engines/libzandronum.so | egrep "RPATH|RUNPATH" $ Hopefully this answer is satisfactory. |
(0018467) WubTheCaptain (reporter) 2017-10-07 11:35 edited on: 2017-10-07 11:44 |
And just for good comparison, here's 1.1 .deb from source:$ objdump -x obj-x86_64-linux-gnu/doomseeker | egrep "RPATH|RUNPATH" RUNPATH /home/wub/.local/src/doomseeker/doomseeker-1.1/obj-x86_64-linux-gnu: $ objdump -x obj-x86_64-linux-gnu/engines/libzandronum.so | egrep "RPATH|RUNPATH" RUNPATH /home/wub/.local/src/doomseeker/doomseeker-1.1/obj-x86_64-linux-gnu: $ objdump -x debian/doomseeker/usr/bin/doomseeker | egrep "RPATH|RUNPATH" $ objdump -x debian/doomseeker/usr/lib/doomseeker/engines/libzandronum.so | egrep "RPATH|RUNPATH" objdump: 'debian/doomseeker/usr/lib/doomseeker/engines/libzandronum.so': No such file $ objdump -x debian/doomseeker/usr/share/doomseeker/engines/libzandronum.so | egrep "RPATH|RUNPATH" RUNPATH /home/wub/.local/src/doomseeker/doomseeker-1.1/obj-x86_64-linux-gnu $ objdump -x /usr/bin/doomseeker | egrep "RPATH|RUNPATH" $ objdump -x /usr/lib/doomseeker/engines/libzandronum.so | egrep "RPATH|RUNPATH" objdump: '/usr/lib/doomseeker/engines/libzandronum.so': No such file $ objdump -x /usr/share/doomseeker/engines/libzandronum.so | egrep "RPATH|RUNPATH" RUNPATH /home/wub/.local/src/doomseeker/doomseeker-1.1/obj-x86_64-linux-gnu |
This issue is already marked as resolved. If you feel that is not the case, please reopen it and explain why. |
|
Supporters: | No one explicitly supports this issue yet. |
Opponents: | No one explicitly opposes this issue yet. |
Issue History | |||
Date Modified | Username | Field | Change |
2017-09-01 15:01 | WubTheCaptain | New Issue | |
2017-09-01 15:02 | WubTheCaptain | Note Added: 0018210 | |
2017-09-01 16:30 | Zalewa | Relationship added | child of 0003246 |
2017-09-09 01:56 | Blzut3 | Relationship added | related to 0003256 |
2017-09-09 19:04 | Zalewa | Assigned To | => Zalewa |
2017-09-09 19:04 | Zalewa | Status | new => assigned |
2017-09-17 15:55 | Zalewa | Note Added: 0018301 | |
2017-09-17 21:22 | Blzut3 | Note Added: 0018306 | |
2017-09-18 07:33 | Zalewa | Note Added: 0018308 | |
2017-09-18 07:55 | Blzut3 | Note Added: 0018312 | |
2017-09-18 09:09 | Zalewa | Note Added: 0018316 | |
2017-09-18 09:09 | WubTheCaptain | Note Added: 0018317 | |
2017-09-18 09:13 | WubTheCaptain | Note Added: 0018318 | |
2017-09-19 00:14 | Blzut3 | Note Added: 0018332 | |
2017-09-19 07:11 | WubTheCaptain | Note Added: 0018337 | |
2017-09-19 07:13 | WubTheCaptain | Note Edited: 0018337 | View Revisions |
2017-09-19 07:13 | WubTheCaptain | Note Edited: 0018337 | View Revisions |
2017-09-24 16:56 | Zalewa | Note Added: 0018368 | |
2017-09-24 16:56 | Zalewa | Status | assigned => needs testing |
2017-10-07 11:14 | WubTheCaptain | Note Added: 0018463 | |
2017-10-07 11:17 | WubTheCaptain | Note Edited: 0018463 | View Revisions |
2017-10-07 11:17 | WubTheCaptain | Note Edited: 0018463 | View Revisions |
2017-10-07 11:19 | WubTheCaptain | Note Added: 0018464 | |
2017-10-07 11:19 | WubTheCaptain | Assigned To | Zalewa => |
2017-10-07 11:19 | WubTheCaptain | Status | needs testing => feedback |
2017-10-07 11:19 | WubTheCaptain | Assigned To | => Zalewa |
2017-10-07 11:19 | WubTheCaptain | Status | feedback => assigned |
2017-10-07 11:19 | Zalewa | Note Added: 0018465 | |
2017-10-07 11:19 | WubTheCaptain | Status | assigned => feedback |
2017-10-07 11:28 | WubTheCaptain | Note Added: 0018466 | |
2017-10-07 11:28 | WubTheCaptain | Status | feedback => assigned |
2017-10-07 11:29 | WubTheCaptain | Note Edited: 0018466 | View Revisions |
2017-10-07 11:35 | WubTheCaptain | Note Added: 0018467 | |
2017-10-07 11:35 | WubTheCaptain | Note Edited: 0018467 | View Revisions |
2017-10-07 11:37 | WubTheCaptain | Note Edited: 0018467 | View Revisions |
2017-10-07 11:40 | WubTheCaptain | Description Updated | View Revisions |
2017-10-07 11:40 | WubTheCaptain | Description Updated | View Revisions |
2017-10-07 11:43 | WubTheCaptain | Note Edited: 0018467 | View Revisions |
2017-10-07 11:44 | WubTheCaptain | Note Edited: 0018467 | View Revisions |
2017-10-07 11:54 | WubTheCaptain | Status | assigned => resolved |
2017-10-07 11:54 | WubTheCaptain | Resolution | open => fixed |
2017-10-07 11:54 | WubTheCaptain | Fixed in Version | => 1.2 |
2017-10-07 11:54 | WubTheCaptain | Target Version | => 1.2 |
2018-10-27 22:55 | WubTheCaptain | Status | resolved => closed |
Copyright © 2000 - 2024 MantisBT Team |