MantisBT - Doomseeker
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0003239Doomseeker[All Projects] Bugpublic2017-09-01 15:012018-10-27 22:55
ReporterWubTheCaptain 
Assigned ToZalewa 
PrioritynormalSeveritymajorReproducibilityhave not tried
StatusclosedResolutionfixed 
Platformx86_64OSDebian GNU/LinuxOS Versionbuster/sid
Product Version1.1 
Target Version1.2Fixed in Version1.2 
Summary0003239: engines/libzandronum.so: Defines RPATH from somewhere
DescriptionZandronum 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 ReproduceFound 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 [^]'
TagsNo tags attached.
Relationships
related to 0003256feedback Pol M Failure to reproduce builds with variations in build path 
child of 0003246acknowledged  Debian packaging 
Attached Files
Issue History
Date ModifiedUsernameFieldChange
2017-09-01 15:01WubTheCaptainNew Issue
2017-09-01 15:02WubTheCaptainNote Added: 0018210
2017-09-01 16:30ZalewaRelationship addedchild of 0003246
2017-09-09 01:56Blzut3Relationship addedrelated to 0003256
2017-09-09 19:04ZalewaAssigned To => Zalewa
2017-09-09 19:04ZalewaStatusnew => assigned
2017-09-17 15:55ZalewaNote Added: 0018301
2017-09-17 21:22Blzut3Note Added: 0018306
2017-09-18 07:33ZalewaNote Added: 0018308
2017-09-18 07:55Blzut3Note Added: 0018312
2017-09-18 09:09ZalewaNote Added: 0018316
2017-09-18 09:09WubTheCaptainNote Added: 0018317
2017-09-18 09:13WubTheCaptainNote Added: 0018318
2017-09-19 00:14Blzut3Note Added: 0018332
2017-09-19 07:11WubTheCaptainNote Added: 0018337
2017-09-19 07:13WubTheCaptainNote Edited: 0018337bug_revision_view_page.php?bugnote_id=18337#r10966
2017-09-19 07:13WubTheCaptainNote Edited: 0018337bug_revision_view_page.php?bugnote_id=18337#r10967
2017-09-24 16:56ZalewaNote Added: 0018368
2017-09-24 16:56ZalewaStatusassigned => needs testing
2017-10-07 11:14WubTheCaptainNote Added: 0018463
2017-10-07 11:17WubTheCaptainNote Edited: 0018463bug_revision_view_page.php?bugnote_id=18463#r11066
2017-10-07 11:17WubTheCaptainNote Edited: 0018463bug_revision_view_page.php?bugnote_id=18463#r11067
2017-10-07 11:19WubTheCaptainNote Added: 0018464
2017-10-07 11:19WubTheCaptainAssigned ToZalewa =>
2017-10-07 11:19WubTheCaptainStatusneeds testing => feedback
2017-10-07 11:19WubTheCaptainAssigned To => Zalewa
2017-10-07 11:19WubTheCaptainStatusfeedback => assigned
2017-10-07 11:19ZalewaNote Added: 0018465
2017-10-07 11:19WubTheCaptainStatusassigned => feedback
2017-10-07 11:28WubTheCaptainNote Added: 0018466
2017-10-07 11:28WubTheCaptainStatusfeedback => assigned
2017-10-07 11:29WubTheCaptainNote Edited: 0018466bug_revision_view_page.php?bugnote_id=18466#r11069
2017-10-07 11:35WubTheCaptainNote Added: 0018467
2017-10-07 11:35WubTheCaptainNote Edited: 0018467bug_revision_view_page.php?bugnote_id=18467#r11071
2017-10-07 11:37WubTheCaptainNote Edited: 0018467bug_revision_view_page.php?bugnote_id=18467#r11072
2017-10-07 11:40WubTheCaptainDescription Updatedbug_revision_view_page.php?rev_id=11074#r11074
2017-10-07 11:40WubTheCaptainDescription Updatedbug_revision_view_page.php?rev_id=11075#r11075
2017-10-07 11:43WubTheCaptainNote Edited: 0018467bug_revision_view_page.php?bugnote_id=18467#r11076
2017-10-07 11:44WubTheCaptainNote Edited: 0018467bug_revision_view_page.php?bugnote_id=18467#r11077
2017-10-07 11:54WubTheCaptainStatusassigned => resolved
2017-10-07 11:54WubTheCaptainResolutionopen => fixed
2017-10-07 11:54WubTheCaptainFixed in Version => 1.2
2017-10-07 11:54WubTheCaptainTarget Version => 1.2
2018-10-27 22:55WubTheCaptainStatusresolved => closed

Notes
(0018210)
WubTheCaptain   
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   
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   
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   
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   
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   
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   
2017-09-18 09:09   
Quote from Blzut3
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.


Yup, but ideally the change should be upstreamed to the relevant binary.
(0018318)
WubTheCaptain   
2017-09-18 09:13   
Quote from "Zalewa"
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.


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   
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   
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   
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   
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   
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   
2017-10-07 11:19   
Quote from "Wub"
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.


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   
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   
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