-
Notifications
You must be signed in to change notification settings - Fork 34
Modifications to support non-Linux OSes (tested on FreeBSD 14) #32
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
Conversation
Do you plan to finish this? (Note argp-standalone is now supported in master) |
I have added support for macOS to my fork; please feel free to modify it as needed. I have also added support for refreshing the status page. If it's something we can move into the main project, let me know. TIA. |
Sorry yes I will look at it! |
So I modified CMakeLists.txt based on your recommendations. It almost works on FreeBSD, except for the fact that the calls to check_library_exists for both ARGP and EPOLL fail unless I replace the third argument by "/usr/local/lib", since this is where non-system libraries are installed on FreeBS. Do you have a suggestion on how to deal with this? |
Sounds like a CMake bug? https://cmake.org/cmake/help/latest/module/CheckLibraryExists.html says the empty location should search default paths, which CMake devs say should include /usr/local on FreeBSD: https://gitlab.kitware.com/cmake/cmake/-/issues/21117 |
Yes I guess it is a bug on the CMake side, and can probably be ignored for here |
Should be good now. |
Do you want to add instructions to README and/or a CI task? |
I modified the README and started working on the CI task (in progress). I also modified CMakeLists.txt to add a workaround for the missing /usr/local/lib FreeBSD system directory in cmake, since it is also affecting the CI task. I will let you know once the CI task is finished. Currently it does builds datum and run it in the FreeBSD VM, so there is not much left to do. |
f2a6c70
to
a7a9971
Compare
Ok the CI task should be working fine now |
CMakeLists.txt
Outdated
if(CMAKE_SYSTEM_NAME STREQUAL "FreeBSD") | ||
check_library_exists(argp argp_parse "/usr/local/lib" ARGP) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This workaround will probably break normal usage once FreeBSD/CMake is fixed. :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why so? The directory for FreeBSD will not change so /usr/local/lib will remain valid even once the cmake port/package for FreeBSD fix the value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed the CI issue related to gcc. The lack of bash in the FreeBSD VM was causing it, so I changed the condition to be compatible with sh...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For example, if argp is moved to main, it would then be under /usr, not /usr/local. Or if someone forks FreeBSD to do that (or perhaps something more complicated).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All non-system libraries for FreeBSD are in /usr/local . It would only be in /usr/lib if argp was integrated into FreeBSD itself and the argp package disappeared, in which case the Datum README and the CI would need to be modified along with CMakeLists.txt? I just pushed a change that now looks in /usr/lib in addition to /usr/local/lib if this ever becomes the case. This will also look into subdirectories...
e2c3f05
to
f08ddf8
Compare
Sure this can also work, thanks. I committed it to the PR. |
This is required for compilation on FreeBSD 14
…location on FreeBSD Co-authored-by: Luke Dashjr <[email protected]>
Rebased this to the 0.2.x branch and cleaned up the commit history. Can you confirm the new branch works fine on FreeBSD? |
Tested on Free-BSD - DATUM is unable to connect to OCEAN's prime servers. |
Ok. I have only been solo mining with my setup, but I guess this is a good excuse to try this as well... |
Do you get a seg fault and the following error? |
No, this is what I get:
|
Ok thanks. My environment is quite locked down. I might have to fix a few things first. |
Ok I now get this in DEBUG mode, which seems to correspond to the issue you also have: |
Ok, so I did some tests, and the issue is in datum_protocol_client (from datum_protocol.c). In the first iteration of the main while loop (when i is incremented to 1), epoll_wait times out while connect() is still in progress. By the time the second iteration in the main while loop occurs (when i is incremented to 2), server_out_buf is non-zero, so send() is called, but as connect() is still in progress, send() returns the error ENOTCONN. Modifying the epoll_wait line by the following seems to fix the timing issue for me: |
@BitcoinMechanic So I created the following branch in my fork with modified code that fixes the issue: https://github.com/21M4TW/datum_gateway/tree/fix_client_connect_in_progress . I tested it and it works for me. Would you like to test it as well before I submit a PR? Thanks |
Probably makes sense to just add it to this PR? (Assuming it works) |
I can certainly test - gimme a bit |
How about this? --- a/src/datum_protocol.c
+++ b/src/datum_protocol.c
@@ -1629,7 +1629,7 @@ void *datum_protocol_client(void *args) {
server_out_buf = 0;
}
} else {
- if (!(errno == EAGAIN || errno == EWOULDBLOCK)) {
+ if (!(errno == EAGAIN || errno == EWOULDBLOCK || errno == ENOTCONN)) {
pthread_mutex_unlock(&datum_protocol_send_buffer_lock);
break;
} |
This didn't work for me - same error as before. |
this works |
…n ENOTCONN error when the connect() command was still in progress. This change prevents the main loop from exiting (it keeps trying to send the data) when such a situation occurs.
This worked for me as well. I added the change to this PR, as suggested. |
-Modifying CMakeLists.txt to support non-Linux OSes that do not include epoll and argp. Using epoll-shim and the standalone argp libraries. Tested on FreeBSD 14
-Including <netiniet/in.h> in src/datum_protocol.c. This is required for compilation on FreeBSD 14