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

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

Open
janrysavy opened this issue Dec 9, 2023 · 21 comments
Open

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

janrysavy opened this issue Dec 9, 2023 · 21 comments

Comments

@janrysavy
Copy link
Contributor

I am considering how to handle third-party libraries in Salamander. Currently, this issue arises with #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
Copy link

amay5267 commented Dec 9, 2023

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

janrysavy commented Dec 9, 2023

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

lejcik commented Dec 10, 2023

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

janrysavy commented Dec 11, 2023

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

lejcik commented Dec 11, 2023

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

lejcik commented Dec 16, 2023

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

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

@janrysavy
Copy link
Contributor Author

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

lejcik commented Dec 16, 2023

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

WaldemarH commented Dec 18, 2023

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

janrysavy commented Dec 19, 2023

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

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

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

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

@WaldemarH
Copy link

WaldemarH commented Dec 21, 2023

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

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

lejcik commented Dec 30, 2023

I've updated the pull request #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
Copy link

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....

@lejcik
Copy link

lejcik commented Feb 1, 2025

it's time to revive the discussion, as I recently prepared update for PictView plugin (see #61), people want to try it out and they are asking me to give them binary files (not everybody is skilled enough to compile the project), so that I prepared an automated build for OpenSalamander, it is now available in my fork: lejcik#2

I prepared a github action script, it builds a zip package with compiled salamander binaries, see this sample run and mind the attached build artifacts: https://github.com/lejcik/salamander/actions/runs/13088683936

this is the continuous integration script: https://github.com/lejcik/salamander/blob/github-ci/.github/workflows/ci.yml

it'd need a bit more tuning, but for now it shows that it's possible to compile salamander on github,

NOTES:

  • the latest Windows SDK introduced an issue and that exif library failed to build, here is a temporary patch: lejcik@97546e7 (it will be removed once the issue will be fixed in upstream),
  • I tried out to integrate vcpkg to the build,
  • the UnRAR plugin uses unrar.dll which was just taken from winrar web, now vcpkg has a recipe for it, and plugin was updated to build it as its dependency,
  • moreover, I saw that many plugins load additional DLLs by LoadLibrary() and GetProcAddress() calls, this approach is not so comfortable, I tried to change this behavior for UnRAR plugin (it directly links to unrar.dll), it required a minor update in salamander core, see this commit lejcik@342e695 and changes in src/plugins1.cpp file,
  • simply, before the plugin is loaded, temporarily I add its path with SetDllDirectory() to the DLL search paths list, and after load I reset the paths back to the previous state, the added path is valid only for the short moment the plugin is loading,

what do you think about this? similarly I'd update also the PictView and TAR plugins...

@melloware
Copy link

@lejcik i support this 1000% as I am one of those users that wants to test out beta builds but doesn't want to have to install all the tools and compile the app myself. Do you have a current build available somewhere to test with your new PictView plugin?

as a note ( I am a Java Developer) and I use the JReleaser GitHub action which builds my CLI Executable for Linux, WIndows, and MacOs and publishes it right under the releases section so any user can just download it.

Image

@ags1234
Copy link

ags1234 commented Feb 1, 2025

I also offer to test out beta builds of Open Salamander or its plugins.
Are there any builds now?

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

7 participants