indilib / indi-3rdparty

INDI 3rd Party drivers repository
https://www.indilib.org/devices.html
GNU Lesser General Public License v2.1
124 stars 208 forks source link

FIX_MACOS_LIBRARIES not defined when compiling on OSX #831

Closed jodier closed 6 months ago

jodier commented 1 year ago

Describe the bug FIX_MACOS_LIBRARIES is an OSX macro defined in ./indi-3rdparty/CMakeLists.txt. This macro is never imported when compiling a library or a driver individually.

To Reproduce For example with libasi (but this is same as soon as a library or a driver needs to call FIX_MACOS_LIBRARIES, you can do a grep):

  1. git clone https://github.com/indilib/indi-3rdparty.git
  2. mkdir ./indi-3rdparty/libasi/build/
  3. cd ./indi-3rdparty/libasi/build/
  4. cmake ..

Expected behavior cmake must be able to see FIX_MACOS_LIBRARIES.

Suggestion Move the definition of FIX_MACOS_LIBRARIES from ./indi-3rdparty/CMakeLists.txt to ./indi-3rdparty/cmake_modules and import the dedicated module each time a library or a driver needs it.

Desktop

Log Files

-- The C compiler identification is AppleClang 14.0.3.14030022
-- The CXX compiler identification is AppleClang 14.0.3.14030022
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
CMake Error at CMakeLists.txt:45 (FIX_MACOS_LIBRARIES):
  Unknown CMake command "FIX_MACOS_LIBRARIES".
knro commented 7 months ago

This was implemented by @rlancaste in order to check ALL incompatible libraries under 3rdparty. I think it is supposed to be used by developers only to fix the MacOS library compatibility for binary-only drivers.

However, you are correct, it needs to be moved so that it doesn't cause errors for individual builds. @jodier Can you please submit a PR after you test it?

rlancaste commented 7 months ago

To give a little background, I did this work because we had a recurring issue with drivers on macOS where different developers would work on their drivers and libraries and would code with them in different folder structures on their computers. As a result the different libraries, binaries, and related files would get all sorts of different install names/ids. The issue is that other libraries, drivers, and other executable files in the 3rd party drivers and libraries get built based on those files and they need to work on anyone’s Mac computer using craft, homebrew, or just building based on the repo. While they would often build successfully, they would not link properly since they were using the erroneous install ids during the build and install steps using cmake and/or craft. We often were not finding out about the problem until drivers just mysteriously did not work or when in the final steps of a long build, it would just fail. After this happened a few times, I knew what to look for and how to fix it by changing the install ids in the files on the repo, but every time someone updated a binary file, library, or driver, it would break again and we wouldn’t find out right away. It was like a game of whack a mole.

This macro will both detect the problem and can also fix the problem if you set that option. Half the point of the macro is to flag the problem by causing a build error so that people know that someone needs to fix the install id. Using the macro, developers can fix the problem automatically, but it also makes it possible for end users who are building a driver for themselves to detect and fix the problem even if the developers have not gotten around to it yet. When you say Jasem that it needs to be moved so it doesn’t affect individual builds, I think I would disagree because whether you build all the drivers or just one driver, if it is not going to build, install and link properly so that it actually works, then that is a problem. The macro may need to be updated or changed if it is not working for all use cases, but I would not recommend disabling it or moving it so it doesn’t run when building just one driver.

does this help?

knro commented 7 months ago

@rlancaste I think the issue is that the macro works when you run it on indi-3rdparty, but not when you try to build individual drivers as reported by @jodier