Zandronum Chat @ irc.zandronum.com
#zandronum
Get the latest version: 3.0
Source Code

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0003239Doomseeker[All Projects] Bugpublic2017-09-01 15:012017-10-07 11:54
ReporterWubTheCaptain 
Assigned ToZalewa 
PrioritynormalSeveritymajorReproducibilityhave not tried
StatusresolvedResolutionfixed 
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 Informationhttps://wiki.debian.org/RpathIssue [^]

https://lintian.debian.org/tags/binary-or-shlib-defines-rpath.html [^]
Attached Files

- Relationships
related to 0003256new Failure to reproduce builds with variations in build path 
child of 0003246assignedWubTheCaptain Debian packaging. 

-  Notes
User avatar (0018210)
WubTheCaptain (developer)
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.
User avatar (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.
User avatar (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.
User avatar (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?
User avatar (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.
User avatar (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.
User avatar (0018317)
WubTheCaptain (developer)
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.
User avatar (0018318)
WubTheCaptain (developer)
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.
User avatar (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.
User avatar (0018337)
WubTheCaptain (developer)
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.

User avatar (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 [^]
User avatar (0018463)
WubTheCaptain (developer)
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.

User avatar (0018464)
WubTheCaptain (developer)
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?
User avatar (0018465)
Zalewa (developer)
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.
User avatar (0018466)
WubTheCaptain (developer)
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.

User avatar (0018467)
WubTheCaptain (developer)
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



Issue Community Support
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






Questions or other issues? Contact Us.

Links


Copyright © 2000 - 2017 MantisBT Team
Powered by Mantis Bugtracker