[CMake] fix runpath for ELF platforms

[finagolfin]

Remove the absolute path to the host toolchain's stdlib from libXCTest.so and add $ORIGIN.

Otherwise, you see the following in the current official release for linux:

readelf -d swift-5.2.3-RELEASE-ubuntu18.04/usr/lib/swift/linux/libXCTest.so | ag runpath
0x000000000000001d (RUNPATH)            Library runpath: [/home/buildnode/jenkins/workspace/oss-swift-5.2-package-linux-ubuntu-18_04/build/buildbot_linux/swift-linux-x86_64/lib/swift/linux]

[finagolfin]

@ddunbar, I notice you had a similar commit before, eb46e08, let me know what you think.

[finagolfin]

@briancroom, is there someone who could review this? I've had similar pulls merged recently in other Swift repos, swiftlang/swift-llbuild#670 and swiftlang/swift-corelibs-libdispatch#541.

[briancroom]

Hi @buttaface, @compnerd is responsible for most of the CMake in this repo, so he seems like the most likely person to be able to effectively review the change, but if he doesn’t have feedback, I think we can go ahead and merge this assuming CI results come back green!

[compnerd]

The RPATH should be controlled by the CMake variables for it. That will ensure that the build is done with the absolute parts which of absolutely desired and essential (avoids the wrong library used during testing).

For the origin, that’s required door repackaging. I think that you should pad that flag view CMAKE_Swift_FLAGS rather than forcefully set that.

[finagolfin]

The RPATH should be controlled by the CMake variables for it. That will ensure that the build is done with the absolute parts which of absolutely desired and essential (avoids the wrong library used during testing).

As you're well aware, CMake can't remove the default runtime library path added by the Swift compiler, only the -no-toolchain-stdlib-rpath flag I added can do that. $ORIGIN is added by a CMake flag, as you want.

I think that you should pad that flag view CMAKE_Swift_FLAGS rather than forcefully set that.

I don't see why, for example, Foundation does it this way too, with INSTALL_RPATH.

[finagolfin]

@briancroom, in the meantime, could you run the linux CI?

[spevans]

@swift-ci please test

[finagolfin]

No response forthcoming, @spevans, can you merge?

[compnerd]

I dont think that the issue has been resolved, the previous comments still hold. Having the conversation over 4 different PRs is not helpful. I recommend that all of them are closed off, a proper solution is discussed on the forums and then applied uniformly across all the repositories.

[finagolfin]

the previous comments still hold

Which would that be? I pointed out that you used INSTALL_RPATH yourself to set $ORIGIN in an earlier pull for Foundation that's still used, yet here you are telling me not to do that. You didn't respond for the last week.

I think it's pretty obvious your comments are ridiculous and should just be ignored, that's what I'm asking @spevans to do.

[finagolfin]

Getting back to this again, removed the $ORIGIN portion that @compnerd asked for. This patch was already merged to the 5.3 branch a couple weeks ago in #308, just getting trunk synced here then will get this in 5.4 too.

[finagolfin]

Ping, this was already merged in the 5.3 release branch. @spevans, mind rerunning the CI and merging?

[spevans]

@swift-ci test

[finagolfin]

@compnerd, presumably you're okay with this pull now that it's just removing the host toolchain's absolute rpath?

[finagolfin]

@spevans, not sure why we haven't heard from Saleem, but he already OKed this patch for the 5.3 branch in #308 and as noted there, this fixes point 2. in SR-1650, so I think this should be merged.

[tomerd]

cc @compnerd

[compnerd]

Seems reasonable to me.

[finagolfin]

@briancroom, anything holding this back?