-
Notifications
You must be signed in to change notification settings - Fork 24
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
Build / install issue #1
Comments
Hi, For v2, please do instead:
I have no removed the git submodule update, since it did not work in v1. |
Ok v1 is still broken, it seems, that npm strips the whole .git stuff before installing, in this way also the submodules are gone..., this need some additional work. |
Ok regarding v1, there seems to be a problem in npm since version 7 (npm/cli#2774) that breaks submodule support in npm. Maybe you can revert back to npm 6, or pursue v2 further. |
hi again, v2 seems to work a little further .... but the main problem is the code relies on the epoll implementation in quiche which is not supported in OSX :( "Working" steps:
It is possible to comment the simple_epoll_server out of the CMakeList.txt file. But the http3Server seems to be deeply rooted in EPoll. Note1I had to use clang v13 to avoid some compilation errors with quiche (I used the v13.0.1 binary distribution for OSX on llvm website). The flag CXXFLAGS="--std=c++17" fixed some issues but others remained. Note2I havent seen the off64_t error again but will look into your comments if this happens again. Thanks again for your help ! |
Thanks this is good to know. |
Hi
Ok, and epoll is not available on OS X? Then there is probably not an easy way to fix this, of course, one could try to write and replacement (kqueue) , if a similar mechanism exists, so something that wakes up, if some thing happens to a fd. But for me without a mac, it has zero priority. And it probably needs some time.
Yes the whole architecture of libquiche seems to be event driven by the epollserver, but of course it should be possible to be replaced.
that means
|
Hi, Understood :) I forked the project and will try to adapt the server to libuv, libev or kQueue. Thanks again ! |
Hmm it could have been that I ran sequential compiles and the first errors were skipped, only to come back again at the "end". I would have to look more into this.... I could be wrong. |
I do not think that you have to do so much changes. Please look into: |
Note: I understood the reason for the build.ninja not found error: when the configure script errors before starting to compile the files, on the next run, it will try to go to the next step and not find the build.ninja file. You have to remove the build directory (after solving the original issue) so that it builds properly. |
So it seems the flag was not taken into account on Mac. I had to add this line in CMakeList.txt:
|
You can just run |
Compiles also fine under linux, I have commited it. |
Great. Glad I could help. |
Ok, I have added this as well. |
Hi! I tried to find all changes, I hope I did not miss any. |
Hello. CMake:
One thing though: it would be nice to add the libuv submodule. I have now installed libuv on my mac but I know it was not there before, so it would be nice to add this submodule as well, dont you think? This would avoid an extra step of installation for people checking out the repository. Is it always installed on Linux? I will give a fresh install a try shortly and let you know. |
Hi,
But anyway, if it is needed, hard coding the path is not an option, since it may change. So if it is missing, a CMake variable or mechnaism, that will determine the path should be used, otherwise it will break if the path changes or is different for different versions.
Here it is the same, we have to use a mechanism, which adds the missing part of the object automatically during configure, similar to the stuff for example for protobuf.
No it is not always there for linux. But it is an integral part of node itself (and is also used by nan internally) and it is important to use the same lib in this case (for the other libs in the submodules it is important, that we use a different http3 capable version), so that not problems with mixing two libs occurs.
so that all parts used for nan (and libuv is one of them) are already present when building lib quiche. |
Nice! 👯♂️ |
Hello! This fixed the problem, around line 807 in CMakeLists.txt:
While investigating I found out that the -isysroot option added by CMake on OSX may disable the lookout for this directory: here Other than that, the test ran without any issue once this modification was brought to CMakeLists :) |
Actually, I realize CMakeLists.txt has a section to find the Protobuf install. It should be possible to re-use This seems to work:
~line 795:
EDIT: still seems to be hanging sometimes. |
This becomes unrelated to this thread but, are you seeing some of the following issues?
|
I did not see this, or I did not pay attention. I will have a lock at the weekend.
I have seen this as well, but not after I made some additional changes/fixes, however I forget to push. I will look in the evening or tomorrow morning to push it.
Yes, I found this and fixed it, but I forgot to push. (Hopefully this is the reason...) |
Now, the forgotten fixes are commited and pushed. Hopefully this fixes the segfault. But anyway there might be still issues related to garbage collection and object deletion. So if anything odd happens please post it. |
I have added these chnages as well. Static linking is fine, in this way binaries will also work on systems with out protobuf installed, which may be beneficial. I think no we got the mac os x case. |
Hi |
This seems to match this
|
Maybe there has been a confusion between the |
Okay so: on the So we only have to resolve the ICU library runtime issue and this solves it all? Then - hopefully - the server will run fine and I can look into the Reader runtime error... |
find_package/FindICU is not finding ICU on my machine, which is normal. |
Ok, yes the windows branch is completely based on the libevent branch, so testing the libevent branch makes only sense, if something is broken on the windows branch and not on the libevent branch. The icu library is used by googleurl. However the stuff needed by googleurl is never called by the stuff we need. |
I think the variable that's searched is U_DEF_ICUDATA_ENTRY_POINT whose name is pre-compiled line 172 of utypes.h |
Yes I think that is the variable. |
And udata.cc is needed by other files... at least on the windows build |
The server started when commenting out udata.cpp ! :) |
Perfect, but may be we can also just leave icu on mac out.... |
Right. This was working just fine before :) |
Yes you remove it, and if it works, it will only be conditional included in windows. |
A quick incremental build seems to have done the job: the library is working without ICU. Will try the syntax change. If I don't use await, you mean switching to a promise |
Yes, I think the problem is the mixing of the two types so either catch and then or await with a try catch block. |
Or your catch can return {done: true} |
Oh right. That could totally be the reason. I did change the syntax few months ago but maybe didnt have errors there for some time. Will report once I printout info |
Well actually 0.0.1 did not raise some error, when it should have risen the errors.... |
I have some "[warn] kevent: Invalid argument" at startup as well. But QUIC is running fine. Testing now the "syntax" error... Got caught up. |
No idea, what the "kevent Invalid argument" means, anyway this must be a problem in quiche itself. |
The connection behaves normally after that so no worries for now indeed. I think the ping performance is better though, if it even makes sense? |
I have no clue, I do not think another event loop should affect the ping performance. But it is a newer version of the quic library and may have some improvements in the mean time. |
Error is definitively at the app level and caused by the mix of syntax. You can disregard this one. And thanks for helping :) |
Ok, thanks, I have just commit something to include icu only on windows. |
I think so: on the |
So Windows is almost done and now on the development branch, |
I found the issue leading to the failure of the mac builds... |
Hello Marten. Sorry I was not on my computer. |
No, thanks, finally the unit test on macOS passed. I think it should work now..... |
okay. great to hear. I ran a rebuild-debug without issue :) |
Well github provides the github actions, you have just to write a yml, which the stuff you want to do. In this case I check on all three platforms, if it builds and if the test are working. |
Hello,
Thanks for making this repository!
I'm having trouble installing/running the repository.
When following the install directions (v1), it seems git is not finding a valid git repository.
When trying to install after downloading the repository (v2), the submodules are properly installed but the build is failing because build.ninja is missing ... ?
I know I do not have perl6 installed yet but it doesn't seem to be related to this quite yet and I wanted to try with the default perl for now...
OS: Mac OSX 10.13.6 (High Sierra)
Node: Node v16.14.0
NPM: v8.3.1
protoc: libprotoc 3.19.4
ninja: 1.10.2
Go: go1.17.8 darwin/amd64
cmake-js: 6.3.0
cmake: 3.23.0-rc2
clang: Apple LLVM version 10.0.0 (clang-1000.11.45.2)
perl: v5.18.2
STEPS v1:
ERROR v1:
STEPS v2:
ERROR v2:
The text was updated successfully, but these errors were encountered: