raduprv / Eternal-Lands

http://www.eternal-lands.com
Other
155 stars 56 forks source link

Add macOS build check #208

Closed lukehorvat closed 4 months ago

lukehorvat commented 5 months ago

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.

lukehorvat commented 5 months ago

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 commented 5 months ago

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

lukehorvat commented 5 months ago

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 commented 5 months ago

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

bendoughty commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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...

https://github.com/raduprv/Eternal-Lands/blob/bdb592bb27fd4b5ff0eac20a262c3554e1ef8cbd/macosx/Eternal%20Lands.xcodeproj/project.pbxproj#L1868-L1875

bendoughty commented 4 months ago

@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 commented 4 months ago

Merged. Thanks both.