Odin icon indicating copy to clipboard operation
Odin copied to clipboard

Fix vendor dynamic libraries not working on Linux

Open joakin opened this issue 1 year ago • 5 comments

Fixes https://github.com/odin-lang/Odin/issues/3368

Dynamic libraries like libraylib.so.5.0.0 in vendor aren't working on linux, because the name doesn't end in .so like the code expected.

joakin avatar Apr 03 '24 12:04 joakin

This actually doesn't fix the issue of the library loading, because of this line

https://github.com/joakin/Odin/blob/b632e90eb259e2eec07ffd5ddc4823daa167fddf/src/linker.cpp#L426

	lib_str = gb_string_append_fmt(lib_str, " -l:\"%s/%.*s\" ", cwd, LIT(lib));

Which is prepending the CWD to the lib path, resulting in a double path where the executable is, and where the vendor library is. In my case, something like:

/home/linuxbrew/.linuxbrew/bin/ld: cannot find -l:/home/joakin/dev/flee//home/joakin/dev/forks/Odin/vendor/raylib/linux/libraylib.so.5.0.0: No such file or directory

I believe the compiler assumes any dynamic library must be given just the name without the path and be present where the executable will be, but in the case of the vendor library it already has the full path in it.

@laytan What would be the next steps here? If we remove the CWD from it, then maybe it will break dynamic libraries that you place in your executable's path? I'm not sure.

joakin avatar Apr 03 '24 12:04 joakin

~~I guess you could add an if statement around it checking if the path is already absolute, if it then works? I know on Windows you need to put all shared libraries next to your executable, I guess this was added because that is also the case for Linux? But if it does work for you this way idk anymore, linking is so stupid.~~

laytan avatar Apr 03 '24 22:04 laytan

#2857 is a similar issue with shared libraries on Linux

laytan avatar Apr 04 '24 21:04 laytan

@joakin so I did some looking at this, and adding the cwd seems to be correct, if you ship a binary you also want it based on cwd so you can just ship a directory.

The reason why you have a cwd and then also the odin path is because you missed a spot where the same string contains check has to be done: https://github.com/odin-lang/Odin/blob/a61ae7c861fa301684ee1582507061317b11426b/src/parser.cpp#L5713

After this, it will kinda work, but you'd have to fiddle with your rpath on build through extra linker flags I think.

But it is a move forward, would you want to update the PR with this? At that point we can merge this and see how else to improve the story later.

laytan avatar Apr 16 '24 14:04 laytan

I've also added the same contains check to the parser line, I searched and ".so" is not used anywhere else, good catch.

With this it should work if copying the shared library file to the root of the exe.

joakin avatar Apr 19 '24 11:04 joakin