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

Enable cross-compiling of HogMaker #465

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

Conversation

tophyr
Copy link
Contributor

@tophyr tophyr commented Jun 24, 2024

Pull Request Type

  • GitHub Workflow changes
  • Documentation or Wiki changes
  • Build and Dependency changes
  • Runtime changes
    • Render changes
    • Audio changes
    • Input changes
    • Network changes
    • Other changes

Description

HogMaker is a build tool that must run on the host. It must not, then, be compiled as part of the overall target build. For builds that must cross-compile, like Android (and eventually iOS/visionOS), we need to be able to reference an already-built HogMaker project.

Checklist

  • I have tested my changes locally and verified that they work as intended.
  • I have documented any new or modified functionality.
  • I have reviewed the changes to ensure they do not introduce any unnecessary complexity or duplicate code.
  • I understand that by submitting this pull request, I am agreeing to license my contributions under the project's license.

Copy link
Member

@Lgt2x Lgt2x left a comment

Choose a reason for hiding this comment

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

I understand the motivation behind this PR: HogMaker's target arch is not necessarily the same as the game's target arch. With CMake, you can only ever load a single compiler toolchain at a time in a project, so you've decided to create a separate project for HogMaker that uses a separate toolchain, not cross-compiling this time.

Why not make HogMaker a separate project for all archs, and find it in the top-level CMakeLists? This way, the target is available to all subdirectories, without find_package in each one, and without conditionals for Android.

tools/CMakeLists.txt Outdated Show resolved Hide resolved
tools/CMakeLists.txt Outdated Show resolved Hide resolved
@Lgt2x Lgt2x mentioned this pull request Jun 24, 2024
13 tasks
@tophyr
Copy link
Contributor Author

tophyr commented Jun 25, 2024

@Lgt2x ah, from your comments it's clear I needed to add some documentation about the process. When cross-compiling, there have to be two CMake invocations: the first builds HogMaker using the host toolchain (and exports the HogMakerConfig.cmake file) and then the second, using the target toolchain, gets pointed to the first build.

# creates HogMakerConfig.cmake
cmake -B builds/host
# creates the host HogMaker binary
cmake --build builds/host --target HogMaker

# now, do the actual android (or target) build
cmake -B builds/target -DCMAKE_TOOLCHAIN_FILE=/path/to/android/toolchain.cmake -DHogMaker_DIR=$(pwd)/builds/host
cmake --build builds/target

So -

HogMakerConfig.cmake was not included in this PR.
Where is the HogMaker package created in case we're building for Android

That file gets created by the first CMake invocation

in this else(), we're not targeting Android, which means that HogMaker will not be looked up using find_package. So why exporting the target?

correct. we have to export it from this one, so that the android build can subsequently reference it.

@tophyr tophyr requested a review from Lgt2x June 25, 2024 04:31
@tophyr
Copy link
Contributor Author

tophyr commented Jun 26, 2024

Conditionals improved and README updated.

@tophyr tophyr force-pushed the cross-compile-hogmaker branch 2 times, most recently from 0da903d to 90224b4 Compare June 30, 2024 02:41
@tophyr
Copy link
Contributor Author

tophyr commented Jun 30, 2024

ping @Lgt2x for re-review

@tophyr tophyr force-pushed the cross-compile-hogmaker branch 2 times, most recently from e4b6113 to 715a4e9 Compare July 19, 2024 04:56
@Lgt2x
Copy link
Member

Lgt2x commented Jul 21, 2024

On a second thought, this solution of running cmake twice with different options to load the native toolchain to build HogMaker, and then running the cross-compilation toolchain for the game adds complexity to our build system. I experimented with a solution using ExternalProject cmake logic, which allows running two different toolchains in one build for my WASM build, and considers HogMaker a different project. The change to cross compile HogMaker looks like this: Lgt2x@db3a886

Do you think this could work for Android too?

@tophyr
Copy link
Contributor Author

tophyr commented Aug 4, 2024

Reworked PR to use @Lgt2x's idea of ExternalProject_Add instead of manually prebuilding and then using find_package.

@tophyr
Copy link
Contributor Author

tophyr commented Aug 20, 2024

@Lgt2x @winterheart how's this one looking review-wise? I'll get the merge conflicts updated tonight, but are there any other structural/functional/stylistic changes you'd like to see?

@Lgt2x
Copy link
Member

Lgt2x commented Aug 20, 2024

Well I'm fine with the change as I mostly wrote the patch. Is it fine for you @winterheart ?

@winterheart
Copy link
Collaborator

Look good for me, but I'm not tested it.

@winterheart
Copy link
Collaborator

winterheart commented Sep 1, 2024

Can we try this approach for cross-compile purposes?
This snippet before add_custom_command should do trick:

if(CMAKE_CROSSCOMPILING)
   find_package(HogMaker)
else()
   # Build HogMaker executable
   add_executable(HogMaker ...)
   # Export executable as target
   export(TARGETS HogMaker FILE
          "${CMAKE_BINARY_DIR}/HogMakerConfig.cmake")
endif()
# Call add_custom_command with HogMaker involved
...

And then first build native tools, after that configure cross-compiled environment:

# Build native tools
mkdir build-native; cd build-native
cmake ..
make HogMaker
# Cross-platform build
cd ..
mkdir build-cross; cd build-cross
cmake -DCMAKE_TOOLCHAIN_FILE=MyToolchain.cmake \
      -DHogMaker_DIR=~/src/build-native/ ..
make

With this approach you'll need to first build host tools and then reuse them on cross-compile environment.

@tophyr
Copy link
Contributor Author

tophyr commented Sep 2, 2024

With this approach you'll need to first build host tools and then reuse them on cross-compile environment.

This was the original approach with this PR, a few months ago. See above: #465 (comment)

It worked alright, but @Lgt2x noted:

this solution of running cmake twice with different options to load the native toolchain to build HogMaker, and then running the cross-compilation toolchain for the game adds complexity to our build system.

The currently-proposed solution adds no complexity to the build process.

@tophyr
Copy link
Contributor Author

tophyr commented Sep 2, 2024

@Lgt2x this is blocked on your changes requested btw; seems like you're ok with it so just flagging that

@Lgt2x
Copy link
Member

Lgt2x commented Sep 2, 2024

sorry, all good :)

@winterheart winterheart mentioned this pull request Oct 21, 2024
13 tasks
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