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

Devendoring libraries #15

Open
McSinyx opened this issue Apr 22, 2022 · 33 comments
Open

Devendoring libraries #15

McSinyx opened this issue Apr 22, 2022 · 33 comments

Comments

@McSinyx
Copy link

McSinyx commented Apr 22, 2022

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.

@McSinyx McSinyx changed the title Devendoring libraries. Devendoring libraries Apr 22, 2022
@EmpSurak
Copy link
Contributor

Conan might be also a possible cross-platform alternative.

@autious
Copy link
Collaborator

autious commented Apr 22, 2022

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
Copy link
Collaborator

autious commented Apr 22, 2022

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
Copy link
Author

McSinyx commented Apr 22, 2022 via email

@autious
Copy link
Collaborator

autious commented Apr 22, 2022

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
Copy link
Collaborator

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
Copy link

YouRik commented Apr 25, 2022

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
Copy link
Collaborator

autious commented Apr 25, 2022

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
Copy link

YouRik commented Apr 25, 2022

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
Copy link
Collaborator

autious commented Apr 25, 2022

@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
Copy link
Collaborator

@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
Copy link
Contributor

feliwir commented Apr 25, 2022

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
Copy link
Collaborator

autious commented Apr 25, 2022

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
Copy link
Collaborator

autious commented Apr 25, 2022

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
Copy link
Collaborator

Conan-Kudo commented Apr 25, 2022

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

@feliwir
Copy link
Contributor

feliwir commented Apr 25, 2022

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
Copy link
Collaborator

autious commented Apr 25, 2022

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

@feliwir
Copy link
Contributor

feliwir commented Apr 25, 2022

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

@autious
Copy link
Collaborator

autious commented Apr 25, 2022

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
Copy link
Collaborator

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

@feliwir
Copy link
Contributor

feliwir commented Apr 25, 2022

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

  • the current submodules are private - so everyone who uses git clone --recursive to get all other submodules will trigger an error / encounter an error about missing permissions.
  • Using FetchContent makes CMake a hard requirement - other buildsystems won't work anymore
  • Both solutions will improve the repository structure a lot

@McSinyx
Copy link
Author

McSinyx commented Apr 25, 2022 via email

@feliwir
Copy link
Contributor

feliwir commented Apr 25, 2022

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
Copy link
Collaborator

autious commented Apr 25, 2022

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
Copy link
Author

McSinyx commented Apr 25, 2022

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 (61fcc67) 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
Copy link
Collaborator

autious commented Apr 25, 2022

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

@Conan-Kudo
Copy link
Collaborator

Conan-Kudo commented Apr 25, 2022

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
Copy link
Collaborator

autious commented Apr 25, 2022

That could work!

@turol
Copy link

turol commented Apr 25, 2022

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

@feliwir
Copy link
Contributor

feliwir commented Apr 25, 2022

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

@shinymerlyn
Copy link
Contributor

shinymerlyn commented Apr 29, 2022

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? #15 (comment)

I also would want to make sure that download zip -> cmake -> build continues to be well supported. As mentioned here: #20 (comment)

@q4a
Copy link

q4a commented Aug 23, 2022

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
Copy link

Related: #130

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

No branches or pull requests

10 participants