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

MinGW: Remove manual setting of library prefix/suffix #277

Merged
merged 3 commits into from Feb 13, 2024

Conversation

qmfrederik
Copy link
Collaborator

@qmfrederik qmfrederik commented Feb 12, 2024

We no longer need to manually set these suffixes/prefixes as CMake in MSYS2 now knows about Objective C.

See msys2/MINGW-packages#20028
See https://gitlab.kitware.com/cmake/cmake/-/merge_requests/9244
See msys2/MINGW-packages#20024 (comment)

/cc @MehdiChinoune

- Use "lib{library}.dll.a" for import libraries
- Use "{library.dll}" for shared libraries

See msys2/MINGW-packages#20024 (comment)
@MehdiChinoune
Copy link

I don't where does the issue come from, those are set by default by CMake, they shouldn't be added at all.

@qmfrederik
Copy link
Collaborator Author

My best guess is that this is because the CMake project declares itself as a OBJC/OBJCXX project, and CMake may not have sensible defaults for Objective-C on MSYS2/MINGW and friends.

@MehdiChinoune
Copy link

My best guess is that this is because the CMake project declares itself as a OBJC/OBJCXX project, and CMake may not have sensible defaults for Objective-C on MSYS2/MINGW and friends.

We should add support for MinGW for OBJC/OBJCXX to CMake.
I will try to patch CMake and upstream.

@MehdiChinoune
Copy link

@qmfrederik Do you know which clang library has objc_alloc, objc_alloc_init and objc_allocWithZone symbols?

@davidchisnall
Copy link
Member

Do you know which clang library has objc_alloc, objc_alloc_init and objc_allocWithZone symbols?

These are not provided by clang, they are provided by libobjc2.

@MehdiChinoune
Copy link

Do you know which clang library has objc_alloc, objc_alloc_init and objc_allocWithZone symbols?

These are not provided by clang, they are provided by libobjc2.

I am trying to support OBJ/OBJCXX on CMake, but It fails with undefined reference to those symbols!
Looks like they weren't exported.

@qmfrederik
Copy link
Collaborator Author

@MehdiChinoune Have you got a small repo? libobjc2 does export those symbols. You also need to make sure you use the right clang parameters -- e.g. -fobjc-runtime=gnustep-2.0 if you you use the libobjc2.0 runtime.

@MehdiChinoune
Copy link

MehdiChinoune commented Feb 12, 2024

@MehdiChinoune Have you got a small repo? libobjc2 does export those symbols. You also need to make sure you use the right clang parameters -- e.g. -fobjc-runtime=gnustep-2.0 if you you use the libobjc2.0 runtime.

It's about this repo, I am testing your PR in MinGW-package

@davidchisnall
Copy link
Member

Those symbols won’t be exported from anywhere until libobjc2 is built…

@MehdiChinoune
Copy link

Those symbols won’t be exported from anywhere until libobjc2 is built…

The issue appears when building tests, but that occurs with LLVM/Clang 18.1.0-rc2.
It builds successfully with 17.0.6

@qmfrederik
Copy link
Collaborator Author

@MehdiChinoune Thanks for your patches for cmake (msys2/MINGW-packages#20028). I've removed the the hard-coded values for CMAKE_IMPORT_LIBRARY_SUFFIX, CMAKE_LINK_LIBRARY_SUFFIX, and CMAKE_SHARED_LIBRARY_PREFIX and the the shared library and linker library now get installed as bin/libobjc.dll and lib/libobjc.dll.a.

Can you confirm this is what you had in mind?

@MehdiChinoune
Copy link

Yes, fixing shared/import libraries suffix/prefix was the intention.
before you merge this PR, could you try setting LINKER_LANGUAGE (https://cmake.org/cmake/help/latest/prop_tgt/LINKER_LANGUAGE.html) to CXX and see if it fixes the required LDFLAGS.

@qmfrederik
Copy link
Collaborator Author

qmfrederik commented Feb 13, 2024

@MehdiChinoune I tried:

  • Setting _DLINKER_LANGUAGE=CXX but that variable is not used: "CMake Warning: Manually-specified variables were not used by the project: LINKER_LANGUAGE"
  • I thought I could perhaps pull in stdc++ and gcc_s into ObjC projects by setting CMAKE_OBJC_IMPLICIT_LINK_LIBRARIES to CMAKE_CXX_IMPLICIT_LINK_LIBRARIES. That causes the code to link with a bunch of other libraries, too (including mingwex and mingw32) which causes the exception handling tests to fail

Long story short, I would leave it at this for now -- we can always revisit later and try to optimize things.

@qmfrederik qmfrederik changed the title MinGW: Update library prefix/suffix MinGW: Remove workaround to manually set library prefix/suffix Feb 13, 2024
@qmfrederik qmfrederik changed the title MinGW: Remove workaround to manually set library prefix/suffix MinGW: Remove manual setting of library prefix/suffix Feb 13, 2024
@qmfrederik
Copy link
Collaborator Author

@davidchisnall I think this is ready now -- long story short: there was a workaround to manually set the library prefix/suffix, but this is no longer needed as CMake in MSYS2 now knows about ObjC. Do you agree?

Copy link
Member

@davidchisnall davidchisnall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the caveat that I don't have a Windows machine to try this on, it looks fine.

@davidchisnall
Copy link
Member

Build looks green for MinGW CI, and I'm in favour of removing complexity from the build system if it is not essential.

@qmfrederik qmfrederik merged commit f983cdb into gnustep:master Feb 13, 2024
59 checks passed
@qmfrederik qmfrederik deleted the mingw-naming branch February 13, 2024 10:43
@MehdiChinoune
Copy link

@MehdiChinoune I tried:

  • Setting _DLINKER_LANGUAGE=CXX but that variable is not used: "CMake Warning: Manually-specified variables were not used by the project: LINKER_LANGUAGE"

LINKER_LANGUAGE is a target property, It couldn't be passed by command.

@qmfrederik
Copy link
Collaborator Author

@MehdiChinoune Do you know which clang library has objc_alloc, objc_alloc_init and objc_allocWithZone symbols?

You were right, they weren't being exported on Windows. I've included a fix for that in #280.

qmfrederik added a commit that referenced this pull request Mar 21, 2024
We no longer need to manually set these suffixes/prefixes as CMake in MSYS2 now knows about Objective C.

See msys2/MINGW-packages#20028
See https://gitlab.kitware.com/cmake/cmake/-/merge_requests/9244
See msys2/MINGW-packages#20024 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants