birdofpreyru / react-native-static-server

Embedded HTTP server for React Native
https://dr.pogodin.studio/docs/react-native-static-server
Other
142 stars 22 forks source link

Revise lighttpd build targets #28

Closed birdofpreyru closed 1 year ago

birdofpreyru commented 1 year ago

Currently the targets during lighttpd core build include mod_indexfile, and mod_staticfile; however as of lighttpd v1.4.69 the sources of those modules are built-in into the main lighttpd target, so no need to build those modules separately.

gstrauss commented 1 year ago

Please consider watching https://redmine.lighttpd.net/projects/lighttpd/news and reviewing lighttpd release notes, which try to announce planned behavior changed. You might alternatively subscribe to the lighttpd-announce mailing list.

birdofpreyru commented 1 year ago

@gstrauss yeah, I am watching release news on https://www.lighttpd.net, I guess they are the same.

Actually, I had a question about built-in modules: when I do a build with shared libraries (for Android), it is all cool. However, when I do a static build (for iOS), I tried to cherry-pick https://github.com/lighttpd/lighttpd1.4/commit/949da5e94e3b3bdb7c49637c6a16b5b9e9df1f30, to get rid of those targets for built-ins, and it failed for me to load those modules during the runtime. However, right as I was writing this comment, and looking for a commit to refer, I now noticed this commit https://github.com/lighttpd/lighttpd1.4/commit/2925a095bdd9059f2c92ce5c6fafbd30c35e04d9 in your repo, is it fixing exactly that problem I bumped into?

Yeah, https://github.com/lighttpd/lighttpd1.4/commit/2925a095bdd9059f2c92ce5c6fafbd30c35e04d9 probably not fixing it for CMake build...

gstrauss commented 1 year ago

However, when I do a static build (for iOS), I tried to cherry-pick https://github.com/lighttpd/lighttpd1.4/commit/949da5e94e3b3bdb7c49637c6a16b5b9e9df1f30, to get rid of those targets for built-ins,

Why are you cherry-picking? Until I merge the _WIN32 code, you should first try to keep pace with lighttpd git master branch. If you are unable to do that, please explain why. After I merge the _WIN32 code, you should try to keep pace with lighttpd releases.

You should attempt to minimize the patch set you are maintaining against lighttpd.

gstrauss commented 1 year ago

and it failed for me to load those modules during the runtime.

That is lacking details.

lighttpd is modular. Building lighttpd statically involves creating "plugin-static.h". This is (sparsely) mentioned in lighttpd INSTALL file under sections "static build using SCons" and "static build using make". In the code, see lighttpd source src/plugin.c and LIGHTTPD_STATIC define. Historically, building lighttpd fully-static was supported only with SCons. I extended it to building with autoconf and autotools, with a couple manual steps. The same probably applies to building lighttpd statically using CMake or meson.

birdofpreyru commented 1 year ago

That is lacking details.

Yeah...

lighttpd is modular. Building lighttpd statically involves creating "plugin-static.h".

Yeah... so, my Android / iOS variants are on par with the latest Lighttpd v1.4.69; I only used mod_indexfile, mod_dirlisting, mod_staticfile, and it all works fine, both with shared and static builds. Now, mod_indexfile and mod_dirlisting are built-in modules, so my expectation is that I can drop them from my build targets, and in case of static build for iOS, also drop them from plugin-static.h, and remove all separate targets for built-in modules in CMakeLists, as you did in https://github.com/lighttpd/lighttpd1.4/commit/949da5e94e3b3bdb7c49637c6a16b5b9e9df1f30. As I said, this however, does not work; perhaps because in "shared mode" you do some special checks dedicated for built-in modules (https://github.com/lighttpd/lighttpd1.4/blob/fdb7ffed88b9dfe09a51e7fb58e5ddfe938c1ec9/src/plugin.c#L172-L175), but I don't think you do something similar in static version of plugins_load() (https://github.com/lighttpd/lighttpd1.4/blob/fdb7ffed88b9dfe09a51e7fb58e5ddfe938c1ec9/src/plugin.c#L125-L157). Also, probably, I just miss something, I liked rapidly tried it, failed to make it work in short time, and just decided to look into it some time later.

Why are you cherry-picking?

That commit is after v1.4.69; and there is no 1.4.70 yet, thus I don't wanna risk to merge in everything latest in master prior to you releasing it as the official v1.4.70. That particular commit was just tempting to merge to drop those built-in modules from explicit build targets, and also it felt safe to do, as those removals from CMakeLists I totally understand what they do.

@gstrauss

gstrauss commented 1 year ago

Now, mod_indexfile and mod_dirlisting are built-in modules,

No. mod_dirlisting is not a default built-in module. Where did you get that impression? It is most certainly not listed in the lighttpd release notes for lighttpd 1.4.69 where, under "Future Scheduled Behavior Changes", there is an explicit list of default built-in modules.

However, if you attempt a full-static build of lighttpd, mod_dirlisting is included along with all other lighttpd modules, and maybe you misread that part of CMakeLists.txt.

birdofpreyru commented 1 year ago

Sorry, I mistaked when writing it now, I meant mod_indexfile, and mod_staticfile, I get it that built-in modules are those listed here: https://github.com/lighttpd/lighttpd1.4/blob/fdb7ffed88b9dfe09a51e7fb58e5ddfe938c1ec9/src/CMakeLists.txt#L844-L857

gstrauss commented 1 year ago

That is lacking details.

Yeah...

...and you still haven't provided the actual error trace issued by lighttpd.

gstrauss commented 1 year ago

As I said, this however, does not work; perhaps because in "shared mode" you do some special checks dedicated for built-in modules (https://github.com/lighttpd/lighttpd1.4/blob/fdb7ffed88b9dfe09a51e7fb58e5ddfe938c1ec9/src/plugin.c#L172-L175), but I don't think you do something similar in static version of plugins_load() (https://github.com/lighttpd/lighttpd1.4/blob/fdb7ffed88b9dfe09a51e7fb58e5ddfe938c1ec9/src/plugin.c#L125-L157).

In a static build, all the modules are expected to be built-in to the static executable, and the desired modules are expected to be present in "plugin-static.h". The static build does not require linking with -ldl, which might not be available, and does not require dlsym(), which might not be available.

birdofpreyru commented 1 year ago

Ok, @gstrauss , you are right, I am wrong, let's pretend I've never wrote my previous messages in this thread :sweat_smile:

I looked into the matter again, and sure it turns out that mentions of module targets in my build script for iOS just hang there since the time I was exploring and switching between shared and static build, and they do nothing for my static build now. And my mistake was that I got a thought at some moment that built-in modules do not have to be mentioned in plugin-static.h in case of static build, and tried to remove them from there; but they should be listed there, same as other modules, so what I tried had no sense.

gstrauss commented 1 year ago

Yes, for static build, modules must be built into the binary AND must be listed in plugin-static.h

Any reason this issue is still "open"?

birdofpreyru commented 1 year ago

Any reason this issue is still "open"?

Nope, I just keep it open till I clean-up related mistakes from my code, and release it. Will close soon.

birdofpreyru commented 1 year ago

Hey @gstrauss , I actually might have found an issue with loading static modules in _WIN32 code version.

~Don't throw tomatoes into me if I got something wrong again 🤣~

So... in this case I build it on Windows with MinGW (MSYS2 UCRT64), as shared library (and when troubleshooting I built it as executable with DLL modules), using CMake. I figured out, when compiler arrives to the following block:

https://github.com/gstrauss/lighttpd1.4/blob/728f038681c8135036c7c8ef7e66e6ec9aed99ad/src/first.h#L30-L45

_MSC_VER is undefined for me, thus __declspec_dllexport__ remains undefined, thus mod_XXX_plugin_init() functions of built-in modules aren't exported from compiled exe (or dll), and thus here the code does not find build-in plugins, tries to load them as external DLL modules, and fails if those DLLs aren't there.

I have verified, if I just unconditionally force #define __declspec_dllexport__ __declspec(dllexport) infirst.h it all works as a charm: the mod_XXX_plugin_init() functions are exported from exe / dll, and the plugin loader finds built-in modules. So, I'd say #ifdef _MSC_VER should be either removed, or modified to also declare __declspec_dllexport__ for some compilers that do not define that (and going into my lack of knowledge / best guessing territory, _MSC_VER is defined by MSVS, and MSYS2 UCRT64 probably builds that with GCC).

birdofpreyru commented 1 year ago

Though, then I found that forcing #define __declspec_dllexport__ __declspec(dllexport) breaks me the linking of additional DLL modules against the main lighttpd dll (when I build it as a library), unless I prefix a bunch of functions that go into the main DLL with __declspec_dllexport__.

gstrauss commented 1 year ago

lighttpd (executable) works for me when using cygwin mingw and autoconf, and also when using VisualStudio (using CMake).

I think you might want to look at lighttpd src/Makefile.am which uses --export-all-symbols for liblightcomp.dll, or look at lighttpd src/CMakeLists.txt

SET_TARGET_PROPERTIES(lighttpd PROPERTIES ENABLE_EXPORTS ON)
SET_TARGET_PROPERTIES(lighttpd PROPERTIES WINDOWS_EXPORT_ALL_SYMBOLS TRUE)

Both are attempts to workaround the obtuseness that is Microsoft, which appears to require special-casing when building items that define a global symbol versus use a global symbol. More intelligent systems do the right thing with extern and additionally have the ability to control symbol visibility with __attribute__((__visibility__("default"))) or "hidden" or "internal"

You probably need the workaround to set SET_TARGET_PROPERTIES(... PROPERTIES WINDOWS_EXPORT_ALL_SYMBOLS TRUE) on your custom target for building lighttpd into a library.

lighttpd is not a large executable (or library), so I do not plan to spend any time optimizing symbol visibility for Windows, as Windows is not an optimized platform when compared to many other systems. For example, the native _WIN32 lighttpd.exe is 1/4 performance for serving static files (!!!) than a comparable lighttpd executable running on the same exact hardware and running under Linux. lighttpd on Linux is 4x (400%) FASTER. The comparison is not even close.

birdofpreyru commented 1 year ago

I don't know, I have both WINDOWS_EXPORT_ALL_SYMBOLS and ENABLE_EXPORTS enabled, both for library and exe builds; and once I've built a binary, I check its exports with dumpbin /exports lighttpd.exe, and it shows no exports unless I have __declspec(dllexport) added explicitly. The runtime GetProcAddress() call does not find me plugin init functions either. If I add explicit __declspec(dllexport), then both dumpbin shows me exports, and GetProcAddress() finds plugin init functions.

When I build DLL, even without __declspec(dllexport) added dumpbin /exports lighttpd.dll shows me lots of exports, init functions of built-in plugins among them, but the runtime GetProcAddress() does not find them. And if I add explicit __declspec(dllexport) then anything without explicit __declspec(dllexport) is not exported, not visible to dumpbin, and this way breaks my overall build.

Might be the causes of what I see are different for EXE and DLL. I am wondering now, if dumpbin tells these are exported from DLL, probably (int(WINAPI *)(plugin *))(intptr_t) GetProcAddress(GetModuleHandle(NULL), tb->ptr); does not work as intended when placed inside a DLL... I tried GetModuleHandle(L"lighttpd.dll") as my first guess that I should look into a different module in this case, but that returned me NULL module handler... Will keep continue looking into this tomorrow ¯\_(ツ)_/¯

gstrauss commented 1 year ago

You seem to try too many things at once and to confuse yourself. Try one change at a time.

If you are building lighttpd statically (with lighttpd CMakeLists.txt option BUILDSTATIC defined), and have configured plugin-static.h properly, then lighttpd is building those symbols into the lighttpd executable (or your lighttpd dll) and the dllexport should not been needed, since the symbols are used internally to the executable (or dll). If you are building lighttpd statically and using plugin-static.h, then if it compiles and links, the intended symbols should be present for mod*_plugin_init.

gstrauss commented 1 year ago

Though, then I found that forcing #define __declspec_dllexport__ __declspec(dllexport) breaks me the linking of additional DLL modules against the main lighttpd dll (when I build it as a library), unless I prefix a bunch of functions that go into the main DLL with __declspec_dllexport__.

If you compile lighttpd into a separate dll from the rest of your code, then, yes, you will need to export the symbols that you want to call from a different dll.

birdofpreyru commented 1 year ago

You seem to try too many things at once and to confuse yourself. Try one change at a time.

I am forced to by restrictions imposed by different platforms. The BUILD_STATIC is my choice for Android & iOS, but if I recall it correctly I wasn't able to build it statically for Windows because it attempted to bundle-in some mingw and system DLLs which broke the build. Probably, I did something wrong back then, but opting for shared build of Lighttpd was the option which worked out way easier for me & Windows, so I went with it.

And it seems I have figured out my problem here. It was indeed GetModuleHandle(NULL) call in the build-in module detection code — with NULL it returned me the handle of my test program EXE, which is different from the main Lighttpd DLL I am using in this case, which contains all built-in modules. Then, because my EXE is always UWP, it only can package and load DLLs with LoadPackagedLibrary(L"ReactNativeStaticServer\\lighttpd.dll", 0), and it seems no matter what argument I pass in into GetModuleHandle() it is not able to look up that DLL; however, if I just call LoadPackagedLibrary(L"ReactNativeStaticServer\\lighttpd.dll", 0) instead GetModuleHandle(NULL), even inside the lighttpd.dll itself it returns me the correct module handle, and the built-in modules are found correctly for me.

I know, it is all irrelevant for the main Lighttpd project, I am just sharing the end of this story in case you are curious about it :) As for me, just replacing the call in my fork of Lighttpd will do fine for now. I'll close this issue as soon as I clean-up and retest my library on all systems.

gstrauss commented 1 year ago

And it seems I have figured out my problem here. It was indeed GetModuleHandle(NULL) call in the build-in module detection code — with NULL it returned me the handle of my test program EXE, which is different from the main Lighttpd DLL I am using in this case, which contains all built-in modules.

The lighttpd code in src/plugin.c contains a comment merely 6 lines above the call to GetModuleHandle(NULL). The comment says /* check if module is built-in to main executable */.

I am glad that you have found a solution for your use case in a Windows UWP app, using LoadPackagedLibrary() instead of GetModuleHandle(NULL). For the moment, that seems to me like a simple patch that should hopefully be fairly stable, and simple to maintain for your project.