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

Allow godot-cpp to be installable #9364

Open
ytnuf opened this issue Mar 23, 2024 · 10 comments · May be fixed by godotengine/godot-cpp#1418
Open

Allow godot-cpp to be installable #9364

ytnuf opened this issue Mar 23, 2024 · 10 comments · May be fixed by godotengine/godot-cpp#1418

Comments

@ytnuf
Copy link

ytnuf commented Mar 23, 2024

Describe the project you are working on

Currently making a GDExtension library for dammaku.

Describe the problem or limitation you are having in your project

Currently the only supported way to use godot-cpp is to use as a subproject of your project.
It kinda works but it's rather cumbersome because you need to rebuild godot-cpp when you build your project from scratch. This can be especially painful in CI.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

What this does is add an install command to the CMake script of godot-cpp.
A GDExtension project can then use find_package to use the built godot-cpp.
This makes it possible to share the compiled godot-cpp library among different project.

This also enables the usage with package-managers.

I have made a PR that would make godot-cpp installable.

godotengine/godot-cpp#1418

There are other PRs that also try to address this:

godotengine/godot-cpp#1309
godotengine/godot-cpp#1041

One of the authors would like to be able to build & install this in a docker image to save time. So this would have that benefit too.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

find_package("godot-cpp" 4.2.0 CONFIG REQUIRED)
target_link_library("my-gdextension-project" PRIVATE "godot::cpp")

If this enhancement will not be used often, can it be worked around with a few lines of script?

The work around is using godot-cpp as a subproject, but that can undesirable (unnecessary rebuilds).

Making the library installable, would mean it's a lot easier to port this to package managers.

Is there a reason why this should be core and not an add-on in the asset library?

This feature is for godot-cpp.

@sanderfoobar
Copy link

sanderfoobar commented Jun 29, 2024

This would be great, as it allows package maintainers of, for example, various Linux distributions to include godot-cpp as a package more easily. It's also nice when us developers are able to keep godot-cpp out of our project and have it just sit somewhere in /usr/local/, and locate it via find_package() and enforce a version.

Working with submodules is annoying, and imo is a relic of earlier times: "just vendor everything in your build tree as-is"

@dsnopek
Copy link

dsnopek commented Jul 1, 2024

Installing godot-cpp system-wide and dynamically linking it will create some issues with the binary compatibility model of godot-cpp.

Currently, you build your extension to statically include the version of godot-cpp and extension_api.json of the Godot version you are targeting, which will allow it to work with that version of Godot or later. Other extensions you use may be compiled against a different godot-cpp/extension_api.json, and you can use those together in the same project without problems.

If your extension were dynamically linked to a system-wide godot-cpp, and you updated the system-wide godot-cpp for a newer version of Godot, it could break existing extensions that were compiled against the older godot-cpp. So, you end up in a situation where you need to update and re-compile all your GDExtensions to work with the one godot-cpp.

@sanderfoobar
Copy link

@dsnopek CMake should generate library filenames that take into account the version, and a consumer only has to enforce the version requirement through find_package().

Not everything in e.g /usr/local/ is dynamically linked, and godot-cpp should be statically linked. By using CMake's project(godot-cpp VERSION x.x.x) and then install'ing it, CMake can write these files to the libdir in a manner that does not clash with others, as well as generate the necessary CMake config files to differentiate between the various installed versions.

In short, you'll be able to install multiple godot-cpp's to a libdir/includedir. There is nothing that says everything in /usr should be dynamically linked. See also find /usr/lib/x86_64-linux-gnu/ | grep 'a$'

@dsnopek
Copy link

dsnopek commented Jul 1, 2024

Ah, ok, thanks! If developers are still statically linking with godot-cpp, then there is no effect on binary compatibility.

@ytnuf
Copy link
Author

ytnuf commented Oct 13, 2024

Yep, this doesn't change how godot-cpp is built, so it will still be a static library.

@dsnopek
Copy link

dsnopek commented Oct 29, 2024

@enetheru Does adding this feature make sense to you?

@enetheru
Copy link

@dsnopek I think if it helps some of the users then I would be asking for reasons why not, rather than why. Then seeing if the negatives can be mitigated or catered to. For instance: feature parity with scons is mandatory, so these PR's only cater to cmake which is half the work needed.

I would think that in the beginning of a developers journey, reducing the barrier to entry, or steps and complexity required to get moving is a good thing. Pre-built libraries that are easy to consume satisfy that philosophy. When they eventually upgrade to custom builds of godot which require building godot-cpp from scratch they can cross the bridge of building it themselves.

Off the top of my head I think about cross compile toolchains, version naming, and the kinds of support requests that will come from users of pre-built packages, but I know all of these things have been solved many times over in other projects so they are just technical questions to be answered, and I believe a lot of that falls to package maintainers rather than godot, so long as the mechanisms for naming and install are there I think we'll be good.

My plan is to go over these, and other PR's after the modernise PR is done, approved, and merged. I'll work with willing developers to get their PR's merged, or at least brought upto date.

@ytnuf
Copy link
Author

ytnuf commented Nov 19, 2024

As for SCons feature parity, that can be done.

@dsnopek
Copy link

dsnopek commented Nov 19, 2024

I think if it helps some of the users then I would be asking for reasons why not, rather than why.

The usual "why not": it's more code that we'll have to maintain going forward.

As for SCons feature parity, that can be done.

Thanks for adding the SCons implementation to your PR! That makes it much more clear to me what you're trying to accomplish, since I don't know CMake very well.

It feels like a lot of code for a use-case that doesn't seem super common. But I'll spend some time thinking about it, and we should probably discuss this at a GDExtension meeting too.

@dsnopek
Copy link

dsnopek commented Nov 26, 2024

Discussed at the GDExtension meeting, and we don't think it makes sense to add support for this into godot-cpp's build system.

The use case described here ("including a pre-built godot-cpp in Docker images for CI") should already be possible without having to alter godot-cpp's build system, either by just building the godot-cpp directory in your project and including that in your Docker image, or by using custom scripts.

Overall, we don't want to give the impression that using godot-cpp as an installed library is a supported or recommended way to use it. godot-cpp is designed to be embedded within your GDExtension project, and this affects how many things are handled, for example, backwards compatibility and toolchain compatibility, as well as using a custom extension_api.json or build profile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants