WolfireGames / overgrowth

Open Source codebase of the game Overgrowth by Wolfire Games LLC
Apache License 2.0
2.51k stars 260 forks source link

Devendoring libraries #15

Open McSinyx opened 2 years ago

McSinyx commented 2 years ago

bullet and angelscript are vendored in Projects. Both are packaged on most distros. Ideally, downstream version should be preferred so that library updates across packages can be preformed uniformly.

It also seems that a fair amount of in Libraries are vendored. I suppose due to the lack of a proper package manager it's a must for Windows and macOS but for others it should be minimized.

EmpSurak commented 2 years ago

Conan might be also a possible cross-platform alternative.

autious commented 2 years ago

I don't mind an option for enabling local-library use. But vendoring has been intentional, to make sure that all targets are using the same version and have the same behaviour.

autious commented 2 years ago

We've also applied some local patches to some of these libraries. Meaning there's no guarantee there's a functional 1:1 in all of them, of course this has been minimal, restricted to things like fixing bugs and making the code compile on our targets.

McSinyx commented 2 years ago

Bug fixes should be upstreamed and compatibility should be ensured for a relatively recent range of dependencies' version, unless Overgrowth maintainers plan to also maintain these libraries indefinitely.

Of course this is not necessary (nor possible) to happen overnight, but I'd like you guys to consider accepting patches devendoring those libraries for a more sustainable future of the engine.

autious commented 2 years ago

You do make a good point, and i agree, it should be the long term plan. The less code we maintain responsbility for in this project, the higher the chances are that the game will still be possible to compile and run in 2 decades.

Conan-Kudo commented 2 years ago

Ideally, we should split the vendored libraries out into another repository and have control switches for using them vs system copies across all platforms. That way it's easy to maintain instead of having a complex set of conditionals.

YouRik commented 2 years ago

Is the concept of "system libraries" on Windows and macOS similar to the one on Linux OSs? CMake on Linux systems, usually without any major issues, finds them in one of the various candidate locations and links them correctly. As Windows and macOS don't have the same support for package managers (I know there is winget on Windows and homebrew on macOS but not everyone uses them), what's the de-facto standard developer experience for handling libraries on these platforms? What would be the best option in that regard for this project?

autious commented 2 years ago

On windows it's a wild west. There is conan, that some people use, but it's not a generally standardized approach, and has its issues. Bundling the .dll files into the repo isn't at all uncommon, or requiring that people manually install them into a bunch of local folder by downloading and setting it up, which takes a lot of time and effort, and is pretty error prone.

"Vendoring" the software has so far been the easiest way for us to maintain the development environment for all developers and their respective operating systems and maintaining forwards compatibility with new compilers.

At one point we would bundle vs compile .dll files into the repo, for windows users, but as soon as a new version of visual studio came out there would be issues with incompatibility.

There's a concept of "DLL Hell" on windows. And all applications tend to bundle their own versions of different dependencies. Common system library re-use is generally limited to rendering API's and low level os features.

I think it's a little better on MacOSX, using something called homebrew/brew. But it's not part of the OS itself. Applications on mac also tends to bundle libraries into the .App folders that is used for deployment.

YouRik commented 2 years ago

Thanks. I joined this discussion because I was trying to compile on my M1 Mac and realized that some of the vendored libraries are pre-compiled and not compatible with ARM64. From what I've checked so far, it will be possible for me to add M1 compiled ones (e.g. from Homebrew) or make some additional changes (which I will make a PR for if all goes well).

Basically, this is a case in point for what @McSinyx suggested and I'd be happy to help with this.

@autious, were the library patches documented?

autious commented 2 years ago

@YouRik No, they weren't, not explicitally. They are logged in the original git repo as individual commits, so i can go back and see what changes did occur, if requested.

Conan-Kudo commented 2 years ago

@autious We should split them out into a separate repo before adding any more of these. The Git repo is slow(ish) right now and we should not make it worse.

feliwir commented 2 years ago

Hey - sorry for missing this is a similar discussion to #19 I agree with @autious that the library versions should be fixed / keeping them vendored to achieve a consistent behaviour across platforms.

However looking at the current project structure the buildsystems / folder structures of some libraries seem to be customized, making it hard to upgrade some dependencies. I think updating a dependencies shouldn't be much more effort than changing a number.

Most dependencies (in their current form) have a CMakeLists.txt in their root folder allowing to just call add_subdirectory on them. A somewhat similar route would be to use CMake's FetchContent which combines downloading sources & adding them to the buildtree: https://cmake.org/cmake/help/latest/module/FetchContent.html

This was added in CMake 3.11 and thus would require the project to make this the minimum requirement.

autious commented 2 years ago

I believe we can't upgrade to Cmake 3.11, last i checked the steamos runtime uses an older version of cmake, so it would result in the steam builds breaking.

This is the runtime i'm referring to. When building on linux for steam, we use a chroot constructed by valve. https://github.com/ValveSoftware/steam-runtime

autious commented 2 years ago

I decided to check, and thankfully i appear to be wrong! It has cmake 3.13.2

This target is usually the one with most restrictions regarding C++ language features and tooling. So i use it as the minimum requirement target.

image

Conan-Kudo commented 2 years ago

CMake 3.11 is a good floor, since that's supported across all currently supported Enterprise Linux and long-term stale distributions.

feliwir commented 2 years ago

So would it be a viable route to replace the in-repo dependencies/ libraries with FetchContent (one by one - each one in a seperate PR)?

autious commented 2 years ago

I believe so, does FetchContent support fetching a git repo?

feliwir commented 2 years ago

Yes, it does (and that's the primary use of it)

autious commented 2 years ago

Question is if this is a clean solution, considering we already have a submodule set up for some other dependencies. So we'd be mixing.

Conan-Kudo commented 2 years ago

We can use a submodule instead for consistency if you want. But that makes triggering automatic upgrades on builds harder.

feliwir commented 2 years ago

Well, i'm open for both solutions. Here my personal opinions:

McSinyx commented 2 years ago

I don't think downloading during build time is good idea though, all distributions explicitly forbid packaging recipes doing that. If used it should be overridable somehow (usually via a CMake flag) instead of requiring downstream to patch every time.

I still don't understand the benefit of doing this vs using submodules (which also needs to be overridable FWIW) in case building manually by an user. Submodules would allow

  1. Downloading everything at fetch time, so one can clone and build at a different time. Fetching via CMake might be be equivalent in theory, but it is up to the engine's maintainer to make sure there is no check done before all fetches.
  2. Tracking changes in the same manner via git pull --recurse-submodules.

Regarding the proprietary submodules, I do not think they should be included in this libre repo. If it is somehow a concern, it is possible to have their inclusion as a patch which is unlikely to meet any conflict. I do not think that it is useful to be on this repo, but over Wolfire I suppose you can keep a branch and rebase the commit on each update.

feliwir commented 2 years ago

I agree that git submodules are probably more "established" but having the proprietary submodules makes that route a bit more complicated. If it's an option to remove these 2 submodules (which indeed don't have any use for the opensource community) i'd glady use git submodules aswell.

However linux packaging shouldn't be our concern, since there's no point in packaging overgrowth binaries without any assets (which still need to be bought).

autious commented 2 years ago

It would simplify my life is there was some way to maintain having the proprietary dependencies accessible for the build system even on master. So it's easy to maintain parity on the steam build and the officially supported oned. That way the broader audience can access the open source improvements as they get added and motivate engagement.

That doesn't mean that it has to be via submodules, we could do it some other way.

McSinyx commented 2 years ago

It would simplify my life is there was some way [...] to maintain parity on the steam build and the officially supported one

As I mentioned before, this can be done by keeping these submodules as a patch (61fcc6713854bf39dee0d7dac90d9fce8e3dfba7) on top of main.

git switch main
git revert 61fcc6713854bf39dee0d7dac90d9fce8e3dfba7
git switch -c proprietary
git cherry-pick 61fcc6713854bf39dee0d7dac90d9fce8e3dfba7

Now every time main is updated,

git switch main
git pull
git switch proprietary
git rebase main
autious commented 2 years ago

That means i'll have to manually perform these steps for each commit though. Unless you suggest automizing these steps?

Conan-Kudo commented 2 years ago

We can do it simpler than that if we want to eliminate the submodules for consistency. We can configure CMake with a flag to switch over to "Wolfire Build" mode that grabs the extra repos and reconstructs the whole tree for that purpose. That includes the vendored libraries, the proprietary bits, and so on.

autious commented 2 years ago

That could work!

turol commented 2 years ago

There's also a copy of some version of PCG in ./Source/Utility/pcg_basic.cpp which should be moved under Libraries/

feliwir commented 2 years ago

I started doing some PRs and will continue doing so. Some which need more testing will be marked as drafts

shinymerlyn commented 2 years ago

If and when devendoring, probably best not to sync to main for any dependencies. Should at least lock to a specific version. At least until this repo has enough maintainers to have fixes within hours and not weeks.

I personally think vendoring libraries poses the least risk and most stability for this repo, but I can see why others might want something else. Eliminating any local patches is certainly a must. The rest of the benefits I see as just a tradeoff of reliability (all source is local, upgrade is delete a single directory and extract a new tarball) vs slight convenience of an automated upgrade (switch fetched version/submodule in cmake/git) - but maybe I'm missing something.

I think that wouldn't be acceptable for distro packaging, but as said above I'm not sure that should necessarily be a goal? https://github.com/WolfireGames/overgrowth/issues/15#issuecomment-1108592891

I also would want to make sure that download zip -> cmake -> build continues to be well supported. As mentioned here: https://github.com/WolfireGames/overgrowth/pull/20#issuecomment-1111409948

q4a commented 2 years ago

Hi. Several months have passed. Are there any updates?

We can configure CMake with a flag to switch over to "Wolfire Build" mode that grabs the extra repos and
reconstructs the whole tree for that purpose. That includes the vendored libraries, the proprietary bits,
and so on.

I'm not sure, how did you plan to do it, but I know about CMake ExternalProject_Add. So all open source parts become modules, and proprietary (closed) parts can be added like (this example if you need to build something in proprietary parts):

include(ExternalProject)
ExternalProject_Add(overgrowth_proprietary
    GIT_REPOSITORY    https://github.com/WolfireGames/overgrowth_proprietary
    GIT_TAG           10ad009d6aad8181c48d27e64ec7ac343caf2600
    GIT_SHALLOW       ON
    BUILD_ALWAYS      OFF
    CONFIGURE_HANDLED_BY_BUILD ON
    CONFIGURE_COMMAND cmake ..
    BUILD_COMMAND     make -j4
    INSTALL_COMMAND   ""
)
ExternalProject_Get_property(overgrowth_proprietary SOURCE_DIR BINARY_DIR)
set(OVERGROWTH_PROPRIETARY_INCLUDE_DIR "${SOURCE_DIR}/your_include_filder")
set(OVERGROWTH_PROPRIETARY_LIB ${BINARY_DIR}/your_so_or_lib_files)
include_directories("${OVERGROWTH_PROPRIETARY_INCLUDE_DIRS}")
ADD_CUSTOM_TARGET(dependencies ALL DEPENDS overgrowth_proprietary)

PS I'm interested to build Overgrowth on 32 bit arm (armhf), but it fails because bad/old angelscript version.

Apteryks commented 7 months ago

Related: https://github.com/WolfireGames/overgrowth/pull/130