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

Add macOS build check #208

Merged
merged 3 commits into from
Feb 28, 2024
Merged

Add macOS build check #208

merged 3 commits into from
Feb 28, 2024

Conversation

lukehorvat
Copy link
Contributor

I saw the recent PRs from @Exzap adding build checks via GH Actions and thought I'd add a macOS one that follows the approach outlined by @bendoughty in el-build-methods.

I found it difficult to automate the installation of the dependencies. Most of them aren't available online in the "framework" format that this Xcode project is configured to expect, so you have to build them into frameworks yourself and I didn't have much success with automating that.

Fortunately, Ben provided me with prebuilt versions of the dependencies some time ago, so I've uploaded them here as Frameworks.zip. I propose that we go with this zip file approach for now and then maybe later someone (Ben?) could look at implementing a zipless approach.

If you approve of this PR, then before we merge I can make a PR that adds the zip file to the macOS folder in el-build-methods, then amend this PR to download it from there. At least that way that it won't need to live in this repo. I only included it here to prove that the CI builds successfully.

@@ -673,15 +679,15 @@
8D1107320486CEB800E47090 /* Eternal Lands.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = "Eternal Lands.app"; sourceTree = BUILT_PRODUCTS_DIR; };
D730263C244EE9FD00A6C002 /* data */ = {isa = PBXFileReference; lastKnownFileType = folder; name = data; path = "../../EL Development Assets/EL Data Files/data"; sourceTree = "<group>"; };
D7302641244F081F00A6C002 /* Eternal Lands.entitlements */ = {isa = PBXFileReference; lastKnownFileType = text.plist.entitlements; path = "Eternal Lands.entitlements"; sourceTree = "<group>"; };
D7302669244F639200A6C002 /* SDL2.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = SDL2.framework; path = ../../../Library/Frameworks/SDL2.framework; sourceTree = "<group>"; };
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I needed to update these framework references to use the USER_LIBRARY_DIR var instead of relative paths. The CI's workspace is at ~/work/Eternal-Lands/Eternal-Lands and the frameworks live in ~/Library/Frameworks, so it was a choice between USER_LIBRARY_DIR or ../../../../Library/Frameworks. I hope this is okay.

@lukehorvat
Copy link
Contributor Author

Just for posterity's sake, I did have success with a non-Frameworks.zip approach to installing the SDL deps, because they are available online as prebuilt frameworks. Here's how it looked:

- name: "Install dependencies"
  env:
    SDL_VERSION: "2.30.0"
    SDL_IMAGE_VERSION: "2.8.2"
    SDL_NET_VERSION: "2.2.0"
    SDL_TTF_VERSION: "2.22.0"
  run: |
    mkdir -p $HOME/Library/Frameworks

    curl -LO https://github.com/libsdl-org/SDL/releases/download/release-${SDL_VERSION}/SDL2-${SDL_VERSION}.dmg
    hdiutil attach SDL2-${SDL_VERSION}.dmg
    cp -r /Volumes/SDL2/SDL2.framework $HOME/Library/Frameworks
    hdiutil detach /Volumes/SDL2

    curl -LO https://github.com/libsdl-org/SDL_image/releases/download/release-${SDL_IMAGE_VERSION}/SDL2_image-${SDL_IMAGE_VERSION}.dmg
    hdiutil attach SDL2_image-${SDL_IMAGE_VERSION}.dmg
    cp -r /Volumes/SDL2_image/SDL2_image.framework $HOME/Library/Frameworks
    hdiutil detach /Volumes/SDL2_image

    curl -LO https://github.com/libsdl-org/SDL_net/releases/download/release-${SDL_NET_VERSION}/SDL2_net-${SDL_NET_VERSION}.dmg
    hdiutil attach SDL2_net-${SDL_NET_VERSION}.dmg
    cp -r /Volumes/SDL2_net/SDL2_net.framework $HOME/Library/Frameworks
    hdiutil detach /Volumes/SDL2_net

    curl -LO https://github.com/libsdl-org/SDL_ttf/releases/download/release-${SDL_TTF_VERSION}/SDL2_ttf-${SDL_TTF_VERSION}.dmg
    hdiutil attach SDL2_ttf-${SDL_TTF_VERSION}.dmg
    cp -r /Volumes/SDL2_ttf/SDL2_ttf.framework $HOME/Library/Frameworks
    hdiutil detach /Volumes/SDL2_ttf

I didn't include it in this PR since I figured it would be a bit messy if some deps lived in the zip file and some didn't.

@pjbroad
Copy link
Collaborator

pjbroad commented Feb 5, 2024

This is great. Is there a reason you can't use the zip file that is in the current release assets?

@lukehorvat
Copy link
Contributor Author

Is there a reason you can't use the zip file that is in the current release assets?

Yes, I forgot to mention. The framework zip files included on the release page don't include the headers needed for compilation. This one does:

@pjbroad
Copy link
Collaborator

pjbroad commented Feb 14, 2024

Ben's going to update the framework archive. We can update and merge this PR once that's done. Thanks again.

@bendoughty
Copy link
Contributor

Sincerest apologies for my delay in responding to this, gentlemen. The fun stuff always seems to happen when I am away!

@lukehorvat this is a really nice addition. I was planning on taking a look at this myself after seeing #206 a few weeks back, but you beat me to it!

I have just submitted a PR with an updated Xcode project file which includes the missing source files which have been added since the 1.9.6p1 release, among other things. Apologies again for the delay.

@pjbroad I'll send over that un-borked framework pack right now.

@pjbroad
Copy link
Collaborator

pjbroad commented Feb 21, 2024

OK. I've added the new framework file from @bendoughty to the latest release assets and removed the old file.

One question about having a macos build as part of the action. The project.pbxproj uses some random looking numbers for each module. How are those numbers generated? If a non-macos dev was to add another module to the code base and wanted to updated the mac build, how could we do that?

@bendoughty
Copy link
Contributor

One question about having a macos build as part of the action. The project.pbxproj uses some random looking numbers for each module. How are those numbers generated? If a non-macos dev was to add another module to the code base and wanted to updated the mac build, how could we do that?

This is a very good question! My initial response to this would be that modifying the project file externally is probably a bad idea and could very easily lead to it becoming damaged (a sentiment echoed by various folks on StackOverflow), however it certainly seems doable using the following method.

The random looking number is indeed just a random 96-bit UUID generated by Xcode when a new source file is added.

I have given this a try myself and it seems to work fine, but again, we need to be very careful if this is how we're going to handle it in future.

@lukehorvat
Copy link
Contributor Author

Thanks both! I rebased the branch on latest master and updated the workflow to download the new zip.

I guess we now just have the question of whether the USER_LIBRARY_DIR change is okay. One downside I noticed is that the frameworks appear red in Xcode, but it doesn't seem to prevent Xcode from building the client so I'm not sure how much it matters. 🤷‍♂️

For what it's worth, USER_LIBRARY_DIR was already referenced a few times in the project file without any issues...

HEADER_SEARCH_PATHS = (
"$(USER_LIBRARY_DIR)/Frameworks/SDL2.framework/Headers",
"$(USER_LIBRARY_DIR)/Frameworks/SDL2_net.framework/Headers",
"$(USER_LIBRARY_DIR)/Frameworks/SDL2_ttf.framework/Headers",
"$(USER_LIBRARY_DIR)/Frameworks/SDL2_image.framework/Headers",
"$(SRCROOT)/../nlohmann_json/single_include/",
"$(SRCROOT)/include/",
);

@bendoughty
Copy link
Contributor

bendoughty commented Feb 28, 2024

@lukehorvat changing to USER_LIBRARY_DIR is absolutely fine with me. In my experience Xcode has a tendency to do that red display thing with frameworks; I wouldn't be too concerned about it (it works fine for me too).

Thank you for working on this, it should make it a lot easier for folks developing on other platforms to make sure they aren't breaking stuff for the Mac cohort. I suspect there's more that can be done to help make adding new source files to the project file less of a headache for non-mac devs, but that may require a little research (and is beyond the scope of this PR anyway).

@pjbroad pjbroad merged commit 1c76b8b into raduprv:master Feb 28, 2024
5 checks passed
@pjbroad
Copy link
Collaborator

pjbroad commented Feb 28, 2024

Merged. Thanks both.

@lukehorvat lukehorvat deleted the macos-build branch March 1, 2024 18:12
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.

3 participants