search

BabylonJS / BabylonNative

Build cross-platform native applications with the power of the Babylon.js JavaScript framework
MIT License
757 stars 131 forks source link

babylonjs\bgfx.cmake should have build ci support #563

Closed chrisfromwork closed 1 year ago

chrisfromwork commented 3 years ago

I checked in a bad commit/build break recently for bx dependencies in bgfx.cmake. We should look into adding build ci for the bgfx.cmake repo to make sure this doesn't happen again

bwrsandman commented 2 years ago

You should be using https://github.com/bkaradzic/bgfx.cmake it's the official cmake scripts for bgfx and it's actively maintained. Also we have automated ci on it.

bghgary commented 2 years ago

We are updated with the official one you linked to. We can't use the official one because we have custom changes to bgfx that we haven't contributed back yet.

bghgary commented 2 years ago

I suppose we can reconfigure the GitHub fork to actually be parented to the official one now. We will have to delete the repo and fork a new one. https://stackoverflow.com/questions/5749246/how-do-i-change-which-github-project-i-forked-from

bwrsandman commented 2 years ago

Which custom changes have you made? Is there a list somewhere? I'm a maintainer of bgfx.cmake so I can facilitate that process. If you're using a forked version of bgfx we can work out a way to use a different version or path.

bghgary commented 2 years ago

We have a forked version of bgfx. I don't think we have changes to bgfx.cmake itself at the moment.

If you're using a forked version of bgfx we can work out a way to use a different version or path.

How does this work?

bwrsandman commented 2 years ago

You would configure bgfx.cmake with -DBGFX_DIR to the path of your fork

bghgary commented 2 years ago

Sorry, I don't get it. What happens to the submodules under bgfx.cmake?

bwrsandman commented 2 years ago

From what you've said it seems like you're using your own fork of bgfx.cmake because you're using a forked version of bgfx and bx. Aside from that there are no changes to bgfx.cmake.

If that's the case you can use upstream bgfx.cmake in the following way:

Submodules:

The submodules under bgfx.cmake wouldn't be replaced with yours so the recursive git submodule init would still get them.

It might not be ideal but it's one way to reduce the number of forks and use bgfx.cmake from upstream until you get your changes to bx and bgfx in. I don't know of a way to override the submodules in git.

bghgary commented 2 years ago

I'm not sure this is a good solution. We will have two submodules for bx, bgfx, and possibly bimg. That seems a bit crazy. Forking bgfx.cmake is much cleaner and I don't see a downside. Why is this a worse solution?

It seems like if we want bgfx.cmake to be more flexible, it should not have submodules and consumers can choose to add submodules if they want.

bghgary commented 1 year ago

Closing as we don't seem to have an issue with the current solution.