Relocatable SWIG: allow locating SwigLib relative to executable in CMakeLists.txt #2509
Conversation
|
if |
fc5f8aa to
bb130ad
Compare
|
If this pull request is still needed, it'll need to pass the tests for serious consideration. They are currently failing due to the changes in here. |
bb130ad to
60f016a
Compare
Corrected a typo. The nuget build on GHA is passing now. Appveyor should report in 5 hours. And yes, I'd like to kindly ask for serious consideration here. |
| if (SWIG_LIB_RELATIVE_TO_EXE) | ||
| target_link_libraries(swig ${CMAKE_DL_LIBS}) | ||
| endif() | ||
| install (TARGETS swig DESTINATION bin) # TODO: seems like the executable in the zipped swigwin is actually installed at the root... |
There was a problem hiding this comment.
- TODO: what's the actual behavior that's wanted here? Install swig(.exe) in bin/ on all platforms? Or root on windows?
| # endif | ||
| static String *get_swig_lib_path(void) { | ||
| char buf[MAX_PATH]; | ||
| // TODO: if this is actually in install/bin/, need another call to strrchr |
There was a problem hiding this comment.
- TODO: based on the answer to whether swig.exe is in bin/ when insalled or not, 60f016a should be kept (if in bin/) or reverted (if at root)
| # ifndef SWIG_LIB_RELATIVE_PATH | ||
| # define SWIG_LIB_RELATIVE_PATH "Lib" | ||
| # endif |
There was a problem hiding this comment.
Fallback when autotools on windows.
|
What is the issue on I think a more long-term solution would be to define a |
As I noted in the OP:
Conan packages are moved around. Even if you build the package locally, locally you build in /path/to/build -> install in there -> move to /path/to/package Generally the package binaries are built on another machine completely and placed into your /path/to/package path. So you can't harcode the swig lib path. |
…the bin/ folder on windows, then figure out root path properly
68225e2 to
4817bce
Compare
|
Ok, but isn't the end-goal using conan to install to the production path after the dependency paths are pulled? I.e. for I plan to design the bundled |
|
This would be useful for any package manager with binary caching and variable installation locations. (Coming here from reviewing a vcpkg PR.) |
|
The autotools are the primary build system and CMake is a secondary build system. While I'm happy to accept CMake patches and do my best to review and test them, this option needs to be available using autotools too. The approach of using a location set at build time has worked on Unix systems since the nineties. Many Unix executables require some env variables or a configuration installed, usually into /etc. I don't know vcpkg nor conan, do they not work like all the other unix tools installed the OS package managers? If not what is the generally accepted approach? SWIG is not the first Unix executable to require a configuration of some sort, so this must be a solved problem. |
mv swig swig.exe
cat > swig <<EOF
#!/bin/sh
cd \`dirname \$0\`
export SWIG_LIB="\$PWD/../share/swig/4.1.1"
exec ./swig.exe "\$@"
chmod +x swig |
|
It seems there's a lack of progress here, and I see conan has dropped use of this (conan-io/conan-center-index#26734) so I wonder if this should just be closed. The patch also uses Using a wrapper script which sets |
|
No disagreement in over a month so closing. |
The idea is:
dladdrThis is motivated by being able to use swig via
conan, where the packages are by definition relocated. We had patched it on the conan recipe's side, but now that CMakeLists.txt has been added, I think there's a good case for cleaning and upstreaming the changes, so here it is. (I am terrible at autotools to be honest, so that was one of the roadblocks)Example:
SWIG_LIB_RELATIVE_TO_EXEis only needed here, but I'm also choosing to override it's install PATH via SWIG_LIB_RELATIVE_PATH which would default to<install_prefix>/LibotherwiseRelocate: