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 support #147

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

macOS support #147

wants to merge 5 commits into from

Conversation

chdominguez
Copy link

@chdominguez chdominguez commented Feb 13, 2025

Dear rDock members, we are developing an unofficial rDock plugin for Horus at the Barcelona Supercomputing Center. You can learn more about Horus in the official webpage.

Horus natively supports Linux and macOS and we wanted to distribute the plugin for both platforms. In order to do so, I had to compile rDock on macOS manually. For it to work I had to slightly modify some parts of the code and the makefiles.

During my tests, I did not see any change in the results outputs and the application is working flawlessly. I hope that this macOS support allows rDock to run on more platforms!

@ggutierrez-sunbright
Copy link
Contributor

Hi!
First of all, thanks for the contribution. osx support has been one of those things in my wish-list for a while : )

I have some questions (mostly out of curiosity, it's been a while since the last time I used a mac) and suggestions, I'll write them along the relevant lines

Thanks again and regards

Guillermo

Comment on lines +65 to +66
CXX_BASE_FLAGS += -I$(BREW_PREFIX)/include
LINK_FLAGS += -L$(BREW_PREFIX)/lib
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing this is mostly important for libpopt? or is it also necessary for c/c++ standard libraries?
I'm asking, because soon enough, I'll get rid of popt, and the project will have zero external dependencies (besides std lib)

Copy link
Author

Choose a reason for hiding this comment

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

It is needed only for libpopt. When the libpopt dependency is removed those lines can be safely erased.

README.md Outdated
Comment on lines 52 to 55
on `macOS`, you may need to run `export CC=gcc` before running make in order to use gcc instead of clang. Here is an example for gcc-14 installed with homebrew:

```
export CC=/opt/homebrew/Cellar/gcc/14.2.0_1/bin/gcc-14
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it necessary to export CC as well? I don't think it's being used in the makefile directly.
also, are there issues with clang? in linux it is supported as a compiler

Copy link
Author

Choose a reason for hiding this comment

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

You're right, only CXX is needed.

About the compiler: yes, because Apple's clang is more restrictive as there are some libraries that need to be imported differently (if I recall correctly it's about the import order). GCC does not have that problem and before changing the codebase I preferred to use a different compiler.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Comment on lines +9 to +14
#if defined(__APPLE__) || defined(__FreeBSD__)
#include <stdlib.h>
#else
#include <malloc.h>
#endif

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd say we can replace malloc with stdlib in the linux version as well

Suggested change
#if defined(__APPLE__) || defined(__FreeBSD__)
#include <stdlib.h>
#else
#include <malloc.h>
#endif
#include <stdlib.h>

@ggutierrez-sunbright ggutierrez-sunbright self-assigned this Feb 14, 2025
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