Skip to content

Update README with macOS build instructions #1810

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.

Sign up for GitHub

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

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

Update README with macOS build instructions #1810

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2fc28ad
Update README with macOS build instructions
MacThings Oct 16, 2025
b7c1b57
Merge branch 'Return-To-The-Roots:master' into master
MacThings Nov 2, 2025
803d397
Update README.md
MacThings Nov 2, 2025
d72f10b
Update README.md
MacThings Nov 2, 2025
7c820bd
Merge branch 'Return-To-The-Roots:master' into master
MacThings Dec 5, 2025
b01e5aa
Update README.md
MacThings Dec 5, 2025
26af1f8
Update README.md
MacThings Dec 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,26 @@ make
Note: by using the `-G` option of `cmake` you can specify a generator, e.g. `cmake -G Xcode -DCMAKE_BUILD_TYPE=Release ..` will generate an Xcode project.
Please check `cmake --help` for more options.

### Especially macOS

```bash
git clone --recursive https://github.com/Return-To-The-Roots/s25client s25client
cd s25client
mkdir -p build && cd build
cmake .. \
-DCMAKE_BUILD_TYPE=Release \
-DCMAKE_OSX_ARCHITECTURES=arm64 \
Copy link
Copy Markdown
Member

@Flamefire Flamefire Oct 17, 2025

Choose a reason for hiding this comment

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

Why doesn't it work without that? We have code that should ensure it works out of the box

Copy link
Copy Markdown
Member

@Flow86 Flow86 Oct 28, 2025

Choose a reason for hiding this comment

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

CMAKE_OSX_ARCHITECTURES may not contain arm64 by default

Copy link
Copy Markdown
Member

@Flamefire Flamefire Oct 29, 2025

Choose a reason for hiding this comment

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

By default it is empty. In that case we detect the supported architectures including arm64: https://github.com/Return-To-The-Roots/libutil/blob/7198609c21eae58c31cee5e88c76dc3eddea603f/cmake/DetectOsXArchs.cmake#L61

We then use CMAKE_SYSTEM_PROCESSOR to select that or all available archs. That is set to uname -m by CMake.

So my question is what exactly fails in the current workflow such that we can improve it for everyone.

-DCMAKE_PREFIX_PATH="/opt/homebrew" \
-DENABLE_TESTS=OFF \
Copy link
Copy Markdown
Member

@Flamefire Flamefire Oct 17, 2025

Choose a reason for hiding this comment

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

This likely doesn't has any effect. Which issue does this solve?

-DCMAKE_CXX_FLAGS="-std=c++17 \
Copy link
Copy Markdown
Member

@Flamefire Flamefire Oct 17, 2025

Choose a reason for hiding this comment

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

That is better done by -DCMAKE_CXX_STANDARD=17, but why do it anyway? The CMake files already ensure this is done. Have you tried without that?

-Wno-error=missing-noreturn \
-Wno-error=deprecated-copy \
-Wno-error=unused-parameter \
-Wno-error=undef \
-Wno-error=cast-qual" ..
Copy link
Copy Markdown
Member

@Flamefire Flamefire Oct 17, 2025

Choose a reason for hiding this comment

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

Do you really need that? If so we should fix it in the code so they are not required.

Can you use -DRTTR_ENABLE_WERROR=OFF and post the list of those warnings please?

make -j$(sysctl -n hw.ncpu)
```

### On Windows

#### Prerequisites
Expand Down
Loading