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

build: update OSX builds to llvm clang #5822

Merged
merged 8 commits into from
Dec 19, 2024

Conversation

FriedLongJohns
Copy link
Contributor

Checklist

Required

Optional

  • This is a PR that modifies build process or code organization.
    • If documentation for this feature or process does not exist, please write it.

Purpose of change

From what I can understand, OSX builds weren't building due to apple clang being different from normal clang.

Describe the solution

I've updated the workflow files to use llvm clang from brew (COMPILER=$(brew --prefix llvm)/bin/clang++) and install the dependencies required for building (on my Mac, at least): llvm astyle from brew and polib luaparser from pip.

A line of code in src/sol/sol.hpp around line 6759 had to be changed to make the builds work, but I won't admit to knowing what I did to fix it, because chatgpt was a very poor explainer.

Also adds osx.yml for building OSX in the same way that there exists android.yml and msys-cmake.yml.

Describe alternatives you've considered

  1. Fixing the part of the code that broke OSX builds in the first place

It felt like a more complete solution to me to completely decouple builds from an osx-specific clang so that maintenance would be easier later down the line.

  1. Accepting that software development does not usually happen for OSX and likely will never.

Testing

I built the game with the same commands on my computer, opened a new world, played a random character, and killed a zed with a pool cue. Worked.

Should see if the tests work OK on this PR, and if it fixes builds.
Might cause a memory leak since I've messed with c++ without knowing c++... unsure how to test for that.

give it a section to show how to install and explicitly build with brew's clang instead of apple clang.
@github-actions github-actions bot added docs PRs releated to docs page src changes related to source code. labels Dec 19, 2024
Copy link
Contributor

autofix-ci bot commented Dec 19, 2024

Autofix has formatted code style violation in this PR.

I edit commits locally (e.g: git, github desktop) and want to keep autofix
  1. Run git pull. this will merge the automated commit into your local copy of the PR branch.
  2. Continue working.
I do not want the automated commit
  1. Format your code locally, then commit it.
  2. Run git push --force to force push your branch. This will overwrite the automated commit on remote with your local one.
  3. Continue working.

If you don't do this, your following commits will be based on the old commit, and cause MERGE CONFLICT.

src/sol/sol.hpp Show resolved Hide resolved
Copy link
Member

@scarf005 scarf005 left a comment

Choose a reason for hiding this comment

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

LGTM, please ignore the clang-tidy errors, sol.hpp should've been excluded anyways because it's meant to be included from other header

@scarf005 scarf005 merged commit 8fdbce2 into cataclysmbnteam:main Dec 19, 2024
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs PRs releated to docs page src changes related to source code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OSX builds broken after PR #5685
2 participants