mehah / otclient

An alternative tibia client for otserv written in C++20 and Lua, made with a modular system that uses lua scripts for ingame interface and functionality, making otclient flexible and easy to customize
Other
263 stars 206 forks source link

Consider readopting Boost to mitigate unmaintained dependencies #492

Closed ranisalt closed 7 months ago

ranisalt commented 1 year ago

Priority

Enhancement

Area

What is missing?

Hello! While I understand the motivation behind #252 and coming from a Javascript background I know how devs tend to over-rely in polyfilling libraries such as jQuery and Boost, I feel like this project still has a lot to take from Boost. At least, Boost is way more mature than some dependencies being used.

For context to new devs, Boost is a useful C++ library that provides a range of features and tools for building high-performance software applications. Boost offers a set of well-maintained and well-tested libraries that can implement missing or planned features of the C++ standard library, while reducing the need of using outdated or unmaintained third-party libraries. Additionally, Boost has a large and active user community that can provide support and guidance on using its libraries in production environments.

There are a few situations where using Boost would improve the project quality (this is list is definitely non-exhaustive!):

Therefore, considering how beneficial it can be, and how unintrusive we can make it, I would like Boost to be re-added as a dependency and relied upon for these missing features of the standard library.

Thanks!

Code of Conduct

dudantas commented 1 year ago

I believe that neither otclient nor any other open tibia project requires boost resources. For example, the pull request mentioned involved only a few modifications. Removing boost libraries is not merely a trendy decision, but rather one that simplifies development. Including boost means adding over 70 libraries to the project (today is 16), which can be an overwhelming task for those who frequently compile C++. However, this may not be a significant issue for those who only use the Lua language.

Overall, I believe that the benefits of not using boost outweigh the drawbacks. I see adding them back as a step backward, rather than a step forward.

Even more, considering that the features of c++ 20 and 23 are very advanced and more and more features are being added that replace boosts.

BenDol commented 1 year ago

I vote to avoid monolithic libraries where possible. I'm not a fan of depending on bloated libraries for very little gain personally, granted it can seem like a great idea, I think in practice it doesn't really buy us much other than monolithic headaches.

EDIT: It's been a while since I used boost, from what I remember it would pull in a ton of header files. Internal dependencies, etc. I mean if the boost module can be added as a tiny one header file module as we would expect from an in house solution then I have no issues with it.

ranisalt commented 1 year ago

Including boost means adding over 70 libraries to the project (today is 16), which can be an overwhelming task for those who frequently compile C++.

Not true. Boost is modular, and you do not have to install & include everything in order to use it. Besides, removing Boost only replaced Boost for something else, so you are just compiling a different source code, not compiling less code.

Also, as I mentioned in the OP but I think you missed it, it is way easier to install Boost than some shady dependency, I don't see how installing a package readily available through any package manager can be more overwhelming than having to download from a third-party source or compiling by hand - stduuid, for example, is not available in any Linux distro, nor is it available in Homebrew for MacOS, and must be manually downloaded, built and installed.

Even more, considering that the features of c++ 20 and 23 are very advanced and more and more features are being added that replace boosts.

If you want to use these features now, you need to use Boost. Support for C++23 has just started being added to gcc and clang, and support for C++20 is still incomplete, and that's in the latest releases of said compilers.

I vote to avoid monolithic libraries where possible

Same as above. You can use only the libraries you want from Boost. I'm not sure I understand your concept of monolithic here.

dudantas commented 1 year ago

I vote to avoid monolithic libraries where possible. I'm not a fan of depending on bloated libraries for very little gain personally, granted it can seem like a great idea, I think in practice it doesn't really buy us much other than monolithic headaches.

EDIT: It's been a while since I used boost, from what I remember it would pull in a ton of header files. Internal dependencies, etc. I mean if the boost module can be added as a tiny one header file module as we would expect from an in house solution then I have no issues with it.

Most of the boosts install other dependencies, there are few that are simple headers. An example of a boost that installs a lot of dependencies is the lexical cast: https://github.com/microsoft/vcpkg/blob/master/ports/boost-lexical-cast/vcpkg.json

Look at the amount of dependencies it has, and each of these boosts has other dependencies... And so it goes.

dudantas commented 1 year ago

Not true. Boost is modular, and you do not have to install & include everything in order to use it. Besides, removing Boost only replaced Boost for something else, so you are just compiling a different source code, not compiling less code.

Also, as I mentioned in the OP but I think you missed it, it is way easier to install Boost than some shady dependency, I don't see how installing a package readily available through any package manager can be more overwhelming than having to download from a third-party source or compiling by hand - stduuid, for example, is not available in any Linux distro, nor is it available in Homebrew for MacOS, and must be manually downloaded, built and installed.

I'll give you a simple example of how I think you're wrong, which is the use of lexical cast (which you mentioned in the issue), it installs, unless I'm mistaken, more than 40 external dependencies, which is totally unfeasible.

If you want to use these features now, you need to use Boost. Support for C++23 has just started being added to gcc and clang, and support for C++20 is still incomplete, and that's in the latest releases of said compilers.

Not even! std is independent of any library, you just need to have the compiler updated to use it.

Edit

Note that just because they have a simple include doesn't mean they don't have external dependencies, look at the lib's portfile in vcpkg, and you will see that they have several external dependencies, that they have other dependencies and so on.

https://github.com/microsoft/vcpkg/blob/master/ports/boost-lexical-cast/vcpkg.json

dudantas commented 1 year ago

So, with a practical example, I don't see the point of having to install about 40 boosts to simply use the lexical cast, when, c++ which is a high level language allows us to implement a cast or safe conversion function, even having some things from c++ 17 and 20 that can also do this task, although in a more complex way and maybe with more lines, but that yes, can be done without having to install several dependencies of libs.

PS D:\OT\GitHub\vcpkg> .\vcpkg install boost-lexical-cast
warning: Starting with the September 2023 release, the default triplet for vcpkg libraries will change from x86-windows to the detected host triplet (x64-windows). To resolve this message, add --triplet x86-windows to keep the same behavior.
Computing installation plan...
Found a constraint violation:
    warning: dependency boost-build:x64-windows was expected to be at least version 1.81.0#3, but is currently 1.81.0#2.
Found a constraint violation:
    warning: dependency boost-modular-build-helper:x64-windows was expected to be at least version 1.81.0#4, but is currently 1.81.0#3.
Found a constraint violation:
    warning: dependency boost-build:x64-windows was expected to be at least version 1.81.0#3, but is currently 1.81.0#2.
Found a constraint violation:
    warning: dependency boost-modular-build-helper:x64-windows was expected to be at least version 1.81.0#4, but is currently 1.81.0#3.
The following packages will be built and installed:
  * boost-array[core]:x86-windows -> 1.81.0#2
  * boost-assert[core]:x86-windows -> 1.81.0#2
  * boost-bind[core]:x86-windows -> 1.81.0#2
  * boost-concept-check[core]:x86-windows -> 1.81.0#2
  * boost-config[core]:x86-windows -> 1.81.0#2
  * boost-container[core]:x86-windows -> 1.81.0#2
  * boost-container-hash[core]:x86-windows -> 1.81.0#2
  * boost-conversion[core]:x86-windows -> 1.81.0#2
  * boost-core[core]:x86-windows -> 1.81.0#2
  * boost-describe[core]:x86-windows -> 1.81.0#2
  * boost-detail[core]:x86-windows -> 1.81.0#2
  * boost-function[core]:x86-windows -> 1.81.0#2
  * boost-function-types[core]:x86-windows -> 1.81.0#2
  * boost-functional[core]:x86-windows -> 1.81.0#2
  * boost-fusion[core]:x86-windows -> 1.81.0#2
  * boost-integer[core]:x86-windows -> 1.81.0#2
  * boost-intrusive[core]:x86-windows -> 1.81.0#2
  * boost-io[core]:x86-windows -> 1.81.0#2
  * boost-iterator[core]:x86-windows -> 1.81.0#2
    boost-lexical-cast[core]:x86-windows -> 1.81.0#2
  * boost-move[core]:x86-windows -> 1.81.0#2
  * boost-mp11[core]:x86-windows -> 1.81.0#2
  * boost-mpl[core]:x86-windows -> 1.81.0#2
  * boost-numeric-conversion[core]:x86-windows -> 1.81.0#2
  * boost-optional[core]:x86-windows -> 1.81.0#2
  * boost-predef[core]:x86-windows -> 1.81.0#2
  * boost-preprocessor[core]:x86-windows -> 1.81.0#2
  * boost-range[core]:x86-windows -> 1.81.0#2
  * boost-regex[core]:x86-windows -> 1.81.0#2
  * boost-smart-ptr[core]:x86-windows -> 1.81.0#2
  * boost-static-assert[core]:x86-windows -> 1.81.0#2
  * boost-throw-exception[core]:x86-windows -> 1.81.0#2
  * boost-tuple[core]:x86-windows -> 1.81.0#2
  * boost-type-index[core]:x86-windows -> 1.81.0#2
  * boost-type-traits[core]:x86-windows -> 1.81.0#2
  * boost-typeof[core]:x86-windows -> 1.81.0#2
  * boost-uninstall[core]:x86-windows -> 1.81.0#2
  * boost-utility[core]:x86-windows -> 1.81.0#2
  * boost-vcpkg-helpers[core]:x86-windows -> 1.81.0#3
  * boost-winapi[core]:x86-windows -> 1.81.0#2
Additional packages (*) will be modified to complete this operation.
Detecting compiler hash for triplet x86-windows...
Restored 0 package(s) from C:\Users\eduar\AppData\Local\vcpkg\archives in 15.1326 ms. Use --debug to see more details.
Installing 1/40 boost-uninstall:x86-windows...
Building boost-uninstall[core]:x86-windows...
--
Please use the following command when you need to remove all boost ports/components:
    "./vcpkg remove boost-uninstall:x86-windows --recurse"

-- Performing post-build validation
Stored binary cache: "C:\Users\eduar\AppData\Local\vcpkg\archives\74\741ec8951a5112c37d34bbedaf1d60e2a8cd50c44c3692723f82077b8e84f0af.zip"
Elapsed time to handle boost-uninstall:x86-windows: 3.1112418 s
Installing 2/40 boost-vcpkg-helpers:x86-windows...

This is just an example, there are several other boost libraries that need a lot of dependencies, which ultimately ends up installing about 80/100 libs. It doesn't seem like a viable choice, as I said, this greatly reduces the ease of compiling, increases the complexity of the project, takes up many gigabytes, all this to use a few resources from the boost library, for example, assuming that we need to use the lexical cast, are you going to install 40 libs, to use it in a few places? When does c++ natively have multiple features that can do multiple types of conversion? Or that we can simply install a separate library that doesn't install other dependencies? That we just need to install itself. Your comparison with installing independent libraries with boost, which is a highly dependent library on other boosts, is not valid.

ranisalt commented 1 year ago

Most of the boosts install other dependencies, there are few that are simple headers. An example of a boost that installs a lot of dependencies is the lexical cast: https://github.com/microsoft/vcpkg/blob/master/ports/boost-lexical-cast/vcpkg.json

Look at the amount of dependencies it has, and each of these boosts has other dependencies... And so it goes.

This is great! At least it's packaged! It's already a huge improvement over stduuid for example, which is not packaged anywhere else - and it's incorrectly packaged in vcpkg, or embedding tinyxml instead of using a package. What exactly is so bad about having a higher number of dependencies installed that is worse than having fewer, lower quality ones?

But let's not cherry-pick on these single examples, you can ignore lexical_cast. Is there something about UUID or PropertyTree/XML parser, or something I did not mention?

Not even! std is independent of any library, you just need to have the compiler updated to use it.

Compiler packages usually lag far behind. I'm on Arch Linux, probably have one of the most up-to-date versions, and it still has half-baked C++23 support since it's on version 12.2. People using Ubuntu and other Debian-based distros have an even worse scenario.

Let's not pretend that implementing a feature in the compiler will make it available next week to users, no one is using trunk. These take months to be packaged, and half of the standard is still unimplemented.

This is just an example, there are several other boost libraries that need a lot of dependencies

Hmmm probably not, Boost is tightly integrated and has no external dependencies. After you have 1 package, adding another one will very likely not install anything else. As evidence, the boost-libs package in Arch Linux only depends on bzip2, zlib, icu and zstd - all part of the base system.

takes up many gigabytes

This is made up. Installing the whole of Boost, with both static and shared libraries and every single header amounts to less than 200 MB. We would not require and install most of it for a handful of usages.

With that said, this is because vcpkg is a fuck-up, as it's clear from the "boost-uninstall" thing they made up to remove it. Try any sane package manager and you'll see the process is simpler. I just have to install a single package - which was already installed, as Boost is required by gdb - to use it.

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 120 days with no activity.

mehah commented 7 months ago

I'm cleaning the issue list, if anyone wants to open a new one with the same subject, feel free.