OpenSalamander / salamander

Open Salamander
GNU General Public License v2.0
284 stars 40 forks source link

Third-party libraries for the Salamander core and plugins #8

Open janrysavy opened 8 months ago

janrysavy commented 8 months ago

I am considering how to handle third-party libraries in Salamander. Currently, this issue arises with https://github.com/OpenSalamander/salamander/pull/5 which includes lzma and zstd library port. At the same time, we need to address OpenSSL for the FTP plugin and UnRAR.dll for the UnRAR plugin.

One solution seems to be through git submodules. Allow plugins to include code from the \src\common\dep directory, where these third-party libraries would have their directories.

The second option would be vcpkg, but that means an additional build requirement, external source code. Git submodules seem more friendly to me in this regard. How do you see it?

amay5267 commented 8 months ago

Since you published the code on github i would like to see more github integration regarding github workflows for example to release binary and git submodules for plugins. I totally vote for git submodules.

janrysavy commented 8 months ago

Salamander was developed using RCS, subsequently CVS, and finally in Mercurial. The OpenSSL and UnRAR libraries were compiled externally.

Issue vs Discussion: This task looks to me like something we need to resolve as soon as possible for said PR instead of some open-ended discussion.

Edit: @neroux, please don't delete posts I respond to, thank you.

lejcik commented 8 months ago

Git submodules are fine until we will need to patch content of the submodule. For example, when I was porting lzma+zstd libraries, I had to modify (customize) the .vcxproj files. Moreover, see how many patches were added to vcpkg/ports to be able to compile such 3rd party libraries.

I'd rather vote for vcpkg. I use it for years for my projects. It has a good integration with MSVC, library ports are well tested and regularly updated, and MSVC can automatically install missing dependencies to vcpkg when it compiles a solution. For compiling on github (for the planned nightly builds), there are github actions available for vcpkg, such as: https://github.com/marketplace/actions/run-vcpkg (I haven’t tested it, but may try it out). I think that's all what we need for further development...

Here I mentioned how to compile certview plugin with vcpkg, and it's easy.

janrysavy commented 8 months ago

We can try vcpkg. I would like to use vcpkg in Manifest mode with pinned dependencies versions. Versioning is supported from 2021: https://github.com/microsoft/vcpkg/blob/master/docs/users/versioning.md

It can be useful for examining crash reports, where we want to debug source code exactly matching the PDB.

We should have one directory for these third-party dependencies, which will be used by both Salamander core and plugins. Some of the existing dependencies in the /src/common/dep directory can be moved to vcpkg.

lejcik commented 8 months ago

ok, I'll prepare a proof of concept, a demo project that uses vcpkg for handling external dependencies, just give me a few days,

hopefully, all the tools (vcpkg with versioning, github actions, etc.) are ready for integration, and there will be no bad surprises when I put everything together 🙂

lejcik commented 8 months ago

finally I have a demo project, that shows how to use vcpkg for installing 3rd party libraries, both static and dynamic ones, with fixed versions, see: https://github.com/lejcik/vcpkg-demo-project

vcpkg is now installed with Visual Studio, so there's no need to have it as a git submodule, also by default it's available in windows images for github actions, it just has to be initialized: https://github.com/lejcik/vcpkg-demo-project/blob/master/.github/workflows/msbuild.yml#L45

what do you think, is this setup suitable for the Salamander project? also, try to compile the demo project locally on your PC to see how comfortable solution this is...

janrysavy commented 8 months ago

Great, I'll play with it today and let you know. Thank you!

janrysavy commented 8 months ago

It looks really good, works fine!

Do I understand correctly that "builtin-baseline" ensures that even in a year or two vcpkg will download exactly the versions of packages corresponding to this baseline?

lejcik commented 8 months ago

yes, the builtin-baseline hash represents a commit in the vcpkg's git repository, so with a fixed hash we should have the same versions of the libraries forever,

I'd fix the baseline to a release tag (vcpkg's release cycle seems to be once per 1-2 months), such "snapshot" of the port files should be stable,

WaldemarH commented 8 months ago

There are some things I would really like to have:

  1. when you clone/fork a repo the repo shold have everything needed to build a project with a minimal external stuff to be defined (vcpkg fits nicelly into this) -> so it would be best if there would be a "paths" file which you would edit (or windows environment variables.. still prefer a project file to not spam systems with project stuff), define your environment paths and execute a batch file. Also when some processing step is defined please define with which version of the tool you were working.. I had too many experience where people say "it works on my end" and they are using a completelly different version of the tools then other people in the team.

Versioning of the tools and libs is a big issue in real world when breaking changes are introduced.

  1. a similar thing to 1. wherever you use a path variable please make sure it can handle spaces. So always use "" if environment requires it (batch scripts,...)

  2. when a working build will be made, let us try to check which libraries are used and in what state they are and to try to update them to the latest versions.

  3. try to use as little external libs as possible (i.e. don't introduce a library just because it has one function that is needed where other 99% of the library is useles... have seen toooo many of those)

So whatever we do let us have a clean and simple environment where you don't spend 3 weeks just trying to setup everything, before you can work on OS. Well I like the pure win32 code of the OS without depending on too many libraries. :)

Anyway as always these are just wishes and nothing more. :)

Just a side notice... we have waited years for this gem, so let us do it properly and not rush it. :)

janrysavy commented 8 months ago

vcpkg looks like a good option for managing third-party libraries, we should give it a try...

Where do we place dynamically linked libraries? In the root directory to salamand.exe or in subdirectories using .manifest files?

One example of placing DLLs in subdirectories is Blender 4.0:

[Blender 4.0]
    [blender.crt]
        msvcp140.dll
        vcruntime140.dll
        blender.crt.manifest
    [blender.shared]
        SDL2.dll
        OpenAL32.dll
        blender.shared.manifest
    blender.exe

Some information on placing DLLs in subdirectories: https://stackoverflow.com/questions/3832290/altering-dll-search-path-for-static-linked-dll https://stackoverflow.com/questions/2100973/dll-redirection-using-manifests

WaldemarH commented 8 months ago

Hi Jan.

To me current AS configuration is ok... statically linked are in the same folder as exe, where all other stuff is in subfolders. This is how it was always in windows and why change if it works. :)

On the other hand if this is something that is bothering you this would be the best oportunity to do it. :)

michaelknigge commented 8 months ago

I'd like to see something like vcpkg for building OpenSalamander. This makes life easier and we can also getrid of "hidden stuff" in the code base of OpenSalamander if we use it everywhare (i. e., replace all the stuff under src\common\dep\pnglite with dependencies manages by vcpkg...

Sadly, I'm not familar with vcpkg ... how does it handle microsoft runtime dependencies? We should not come in a situation where OpenSalamander needs a specific msvcrtXXXX.dll and another dependency (i. E. OpenSSL or whatever) needs another one...

Does vcpkg rebuild the dependency if neccessary or is the usage of a static libraries the way to go?

janrysavy commented 8 months ago

Interesting blog about vcpkg versioning support and reproducible builds: https://devblogs.microsoft.com/cppblog/take-control-of-your-vcpkg-dependencies-with-versioning-support/

WaldemarH commented 8 months ago

@Michael microsoft runtime dependencies can be a real issue, but I assume that all the latest versions are build with the latest runtimes (hopefully).

Almost all projects that I worked on had versioning issues where different libraries require different versions for them to work. So setting all the versions properly normally takes time and what is at the end the biggest issue is that if you update one library everything collapses and you need to update everyting.. and you are then again at the task to find a combination where all the libraries fit together well.

There are then 2 solutions:

  1. if using packager managers (faster and less control) -> define exact version that needs to be used
  2. if not using -||- (slower with more control) -> have an sdk subfolder where we defined all the external libraries and we build them our selfs

Now I don't know how vcpkg works, but there are managers out there which gets the source code and then build them on your machine. So all the issues dissapear as it builds in the same environment as you.

I'll try vcpkg on some small 'hello world' project to see what kind of options it gives us... let me also read Jan's pasted interesting blog post.

janrysavy commented 8 months ago

I don't expect any problems with RTL, Salamander has its own copy and it works perfectly fine. I suppose we'll link everything else against this RTL.

lejcik commented 8 months ago

I've updated the pull request https://github.com/OpenSalamander/salamander/pull/5, it now uses vcpkg for handling external libraries,

for the tar plugin I'd prefer linking to static libraries, as we only use extracting algorithms from the libs, and that its binary file is smaller (currently tar plugin doesn't do archive compression, but this may change in the future, if there will be such requirement),

if this approach is ok, I may update other project files accordingly...

michaelknigge commented 7 months ago

Aaaaahhhh, got it... vcpkg downloads the source and builds it locally.... okay, then of course we should not come into issues with different runtimes....

I assumed vcpkg downloads pre-compiled binaries, like gradle or maven in the Java world...... Sorry, my fault....