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

Lower the libcur dependency requirement from >=7.64.0 to >=7.55.0 #177

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Hacksign
Copy link

@Hacksign Hacksign commented Dec 3, 2024

Purpose

In some systems which is unable to update regularly, OpenWRT for example, current curl version requirement (>=7.64.0) is too high, this makes this project un-compatable to these old systems.

What has been done

I separated curl_url/curl_url_set/curl_url_get/curl_url_cleanup functions, which is used in src/main.c:44:hostname_from_url, from curl 8.11.1 to provide this project ability to compiled with curl >= 7.55.0.

Effects

After lower the dependency requirment, old systems shipped about 7 years ago, cloud compile this project.

When curl version is less than 7.62, curlite.c is compiled to provide missing functions in this project, when curl version is equal or above 7.62 the newest one will be used, curlite.c will not take effects.

Proves

I've tested the modified code on a 7x24 running router about 4 days, no problems has been found ;)

Here is a log that I compile the modified code with curl 7.55.0 (current minimum requirent, which is released about 7 years ago).

root@195d10693310:~/https_dns_proxy/build# cmake ../
CMake Deprecation Warning at CMakeLists.txt:1 (cmake_minimum_required):
  Compatibility with CMake < 3.10 will be removed from a future version of
  CMake.

  Update the VERSION argument <min> value.  Or, use the <min>...<max> syntax
  to tell CMake that the project requires at least <min> but has been updated
  to work with policies introduced by <max> or earlier.


-- The C compiler identification is GNU 6.3.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Setting build type to 'Release' as none was specified.
-- Could NOT find Git (missing: GIT_EXECUTABLE) 
CMake Warning at CMakeLists.txt:59 (message):
  Could not find git command!


CMake Warning at CMakeLists.txt:63 (message):
  Version unset, using hardcoded!


-- Using system libcurl
-- Curl version: 7.55.0-DEV
-- Detect old version of curl, use curlite.h/curlite.c
-- clang-tidy not found.
-- python3 not found, robot testing not possible
-- Configuring done (0.8s)
-- Generating done (0.0s)
-- Build files have been written to: /root/https_dns_proxy/build
root@195d10693310:~/https_dns_proxy/build# make
[ 11%] Building C object CMakeFiles/https_dns_proxy.dir/src/curlite.c.o
[ 22%] Building C object CMakeFiles/https_dns_proxy.dir/src/dns_poller.c.o
[ 33%] Building C object CMakeFiles/https_dns_proxy.dir/src/dns_server.c.o
[ 44%] Building C object CMakeFiles/https_dns_proxy.dir/src/https_client.c.o
/root/https_dns_proxy/src/https_client.c: In function 'write_buffer':
/root/https_dns_proxy/src/https_client.c:58:43: warning: ISO C99 requires at least one argument for the "..." in a variadic macro
     WLOG_REQ("Response size is too large!");
                                           ^
/root/https_dns_proxy/src/https_client.c:63:30: warning: ISO C99 requires at least one argument for the "..." in a variadic macro
     ELOG_REQ("Out of memory!");
                              ^
/root/https_dns_proxy/src/https_client.c: In function 'https_fetch_ctx_process_response':
/root/https_dns_proxy/src/https_client.c:348:40: warning: ISO C99 requires at least one argument for the "..." in a variadic macro
       DLOG_REQ("curl request succeeded");
                                        ^
/root/https_dns_proxy/src/https_client.c:352:96: warning: ISO C99 requires at least one argument for the "..." in a variadic macro
       WLOG_REQ("curl request failed with write error (probably response content was too large)");
                                                                                                ^
/root/https_dns_proxy/src/https_client.c:372:49: warning: ISO C99 requires at least one argument for the "..." in a variadic macro
                  "but no response was sent back");
                                                 ^
/root/https_dns_proxy/src/https_client.c:384:82: warning: ISO C99 requires at least one argument for the "..." in a variadic macro
         WLOG_REQ("No response (probably connection has been closed or timed out)");
                                                                                  ^
/root/https_dns_proxy/src/https_client.c:488:38: warning: ISO C99 requires at least one argument for the "..." in a variadic macro
       ELOG_REQ("Error getting timing");
                                      ^
/root/https_dns_proxy/src/https_client.c: In function 'https_fetch_ctx_cleanup':
/root/https_dns_proxy/src/https_client.c:509:36: warning: ISO C99 requires at least one argument for the "..." in a variadic macro
     WLOG_REQ("Request was aborted.");
                                    ^
/root/https_dns_proxy/src/https_client.c:512:56: warning: ISO C99 requires at least one argument for the "..." in a variadic macro
     ILOG_REQ("Response was faulty, skipping DNS reply.");
                                                        ^
[ 55%] Building C object CMakeFiles/https_dns_proxy.dir/src/logging.c.o
[ 66%] Building C object CMakeFiles/https_dns_proxy.dir/src/main.c.o
[ 77%] Building C object CMakeFiles/https_dns_proxy.dir/src/options.c.o
[ 88%] Building C object CMakeFiles/https_dns_proxy.dir/src/stat.c.o
[100%] Linking C executable https_dns_proxy
[100%] Built target https_dns_proxy

   in old version of libcurl.
2. UPDATE: `README.md` and `src/main.c`
@Hacksign Hacksign changed the title Lower the libcur dependency requirement from >=7.64.0 to >= 7.55.0 Lower the libcur dependency requirement from >=7.64.0 to >=7.55.0 Dec 3, 2024
@baranyaib90
Copy link
Contributor

baranyaib90 commented Dec 24, 2024

I understand the reasoning, but to me this is more like a hack instead of a nice solution.
I do not recommend to merge this to master.
It could be merged to a separate branch (with a large warning on top of README.md).
This project intends to support only the latest versons of dependent libraries.

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

Successfully merging this pull request may close these issues.

2 participants