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

Consider readopting Boost to mitigate unmaintained dependencies #492

Closed
1 of 5 tasks
ranisalt opened this issue Mar 27, 2023 · 9 comments
Closed
1 of 5 tasks

Consider readopting Boost to mitigate unmaintained dependencies #492

ranisalt opened this issue Mar 27, 2023 · 9 comments
Labels
Stale Type: Enhancement New feature or request

Comments

@ranisalt
Copy link
Contributor

ranisalt commented Mar 27, 2023

Priority

Enhancement

Area

  • Data
  • Source
  • Docker
  • Other

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!):

  • Using Boost.UUID over stduuid: they provide the same functionality, but the last significant update in stduuid was in April 2022 (and it was only released 8 months later!); it is not packaged Linux distros, which makes using it difficult; importing it is unreliable, as vcpkg changed install location, and it's not possible to build without source changes

  • Using Boost.PropertyTree over interning TinyXML: Boost.PropertyTree uses RapidXML under the hood, which is significantly faster than TinyXML; it would also remove a large chunk of code that should not be in this repo.

  • Using boost::lexical_cast instead of stdext/cast.h: again, less code to maintain; and cast.h is as slow as it possibly can (writing to a stringstream, then reading from it), so there's no benefit in it.

  • Since the codebase uses C++20, it is possible to use feature testing to selectively use Boost or C++ STL depending on compiler support - use STL if available, use Boost if not. It's the best of both worlds!

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

  • I agree to follow this project's Code of Conduct
@github-actions github-actions bot added Priority: High Represent a high impact in key areas of the base/user experience Status: Pending Test This PR or Issue requires more testing Type: Enhancement New feature or request labels Mar 27, 2023
@dudantas
Copy link
Collaborator

dudantas commented Mar 27, 2023

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.

@dudantas dudantas removed Priority: High Represent a high impact in key areas of the base/user experience Status: Pending Test This PR or Issue requires more testing labels Mar 27, 2023
@BenDol
Copy link
Collaborator

BenDol commented Mar 27, 2023

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

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

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

dudantas commented Mar 27, 2023

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

dudantas commented Mar 27, 2023

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

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

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

@github-actions github-actions bot added the Stale label Jun 28, 2023
@mehah
Copy link
Owner

mehah commented Mar 22, 2024

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

@mehah mehah closed this as completed Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Stale Type: Enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants