Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

macOS CMake Updates #1077

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open

macOS CMake Updates #1077

wants to merge 24 commits into from

Conversation

nameloCmaS
Copy link
Contributor

Description

Library file extensions

MacOS extensions for shared libraries are .dylib except for the Python extension loaded by Python (_dlite.so) which must follow the Unix naming of .so (not 100% obvious in the docs).

Changes this one target to be .so.

Remove unused variable pyext_ext with the target extensions.

Note: This should probably be covered by the swig_add_library function call as it deals with the .pyd extension on Windows.

macOS symbols and types

Updated various locations where symbols are defined in alternate headers on macOS.

Changes to types also done as explained in the note in the commit (to do with multiple architectures possible in macOS binaries meaning the type was set as 0 - the note in the section 'Generate integers.h' of ./src/utils/CMakeLists.txt explains the CMake nuance where multiple architectures are possible). See docs.

LD_LIBRARY_PATH changes

Changed to add LD_LIBRARY_PATH to UNIX only, so NOT WIN32 becomes UNIX AND NOT APPLE.

LD_LIBRARY_PATH is not so useful for macOS due to the System Integrity Protect (SIP) which filters this environment variable (except in particular circumstances).

One section in the root ./CMakeLists.txt file collects these paths and assigns to PYTHONPATH on macOS only.

dlite_PATH changes

Changed dlite_PATH to dlite_PATH_NATIVE in same blocks as it caused a couple of issues.

configure_file config.h.in

Within ./src/utils/CMakeLists.txt moved configure_file(config.h.in config.h) to the end of the file because it uses variables assigned beneath the original function call position.

The issue was this only worked properly on the second run of the cmake configure.

Debug parameters for GCC compatible compilers

Added debug parameters for GCC compatible compilers including Clang and AppleClang and removed case of depreciated CMAKE_COMPILER_IS_GNUCC.

Added CMAKE_MACOSX_RPATH

Added CMAKE_MACOSX_RPATH as macOS uses shared library search paths slightly differently to Unix.

Fixed dlite_PYTHONPATH string backslash

This changes the substitution from a string REPLACE (single path) to a list (multiple paths) TRANSFORM.

rpl_snprintf missing on macOS

Changed rpl_snprintf to snprintf in ./src/utils/tests/test_snprintf.c.

snprintf is defined in compat.h as

#define snprintf rpl_snprintf

where available, and where not available:

#define snprintf _snprintf

Type of change

  • Bug fix & code cleanup
  • New feature
  • Documentation update
  • Test update

Checklist for the reviewer

This checklist should be used as a help for the reviewer.

  • Is the change limited to one issue?
  • Does this PR close the issue?
  • Is the code easy to read and understand?
  • Do all new feature have an accompanying new test?
  • Has the documentation been updated as necessary?

nameloCmaS and others added 18 commits January 9, 2025 20:30
…Pre-processor updates, lib changes, and switch changes for code handling files.
…n extension loaded by Python which follow Unix naming of .so

Change this one target to be .so

Remove unused variable with target extensions.
… `UNIX AND NOT APPLE`.

`LD_LIBRARY_PATH` is not so useful for MacOS due to System Integrity Protect (SIP) filtering this environment variable, except in particular circumstances.

Changed `dlite_PATH` to `dlite_PATH_NATIVE` in same blocks.
…it uses variables made beneath the original position and thus it only worked properly on the second run of the cmake configure.
…nd AppleClang and removed case of depreciated `CMAKE_COMPILER_IS_GNUCC`.
…intf rpl_snprintf` or `... _snprintf` thus ok.
@nameloCmaS nameloCmaS mentioned this pull request Jan 22, 2025
7 tasks
@nameloCmaS nameloCmaS changed the title macOS Make Updates macOS CMake Updates Jan 22, 2025
@jesper-friis
Copy link
Collaborator

Note: This should probably be covered by the swig_add_library function call as it deals with the .pyd extension on Windows.

Good idea.

We have earlier been hold back on what features in the UseSwig CMake module we could use, because systems with very old versions of swig. However, now when CMake is pip-installable, we can use the latest version of CMake.

@jesper-friis
Copy link
Collaborator

jesper-friis commented Jan 24, 2025

There is another new issue. For some reasons HAVE_PathFileExistsW is not correctly set on Windows. Results in the following error:

D:\a\dlite\dlite\src\utils\fileutils.c(708,1): fatal error C1189: #error:  "Windows have no GetFullPathNameW()" [D:\a\dlite\dlite\python\build\temp.win-amd64-cpython-38\Release\src\utils\dlite-utils.vcxproj]

@nameloCmaS
Copy link
Contributor Author

Note: This should probably be covered by the swig_add_library function call as it deals with the .pyd extension on Windows.

Good idea.

We have earlier been hold back on what features in the UseSwig CMake module we could use, because systems with very old versions of swig. However, now when CMake is pip-installable, we can use the latest version of CMake.

Apologies I should have been more specific. It seems to be missing in the CMake swig_add_library function itself, so we need to add it. The function deals with setting the extension to .pyd on Windows but not .so on macOS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants