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-09-19 07:13
ReporterWubTheCaptain 
Assigned ToZalewa 
PrioritynormalSeveritymajorReproducibilityhave not tried
StatusassignedResolutionopen 
Platformx86_64OSDebian GNU/LinuxOS Versionbuster/sid
Product Version1.1 
Target VersionFixed in Version 
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 0003246new Debian packaging. 

-  Notes
User avatar (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.
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 (reporter)
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 (reporter)
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 (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.


Issue Community Support
Only registered users can voice their support. Click here to register, or here to log in.
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






Questions or other issues? Contact Us.

Links


Copyright © 2000 - 2017 MantisBT Team
Powered by Mantis Bugtracker