Add curl 7.69.1

[Vertexwahn]

Add curl 7.69.1

@lalten @aiuto

[fmeum]

Are these comments leftovers?

[fmeum]

This looks like it won't work with Bzlmod. You may need to get the repository name from a Label defined in a .bzl file.

[fmeum]

This select on the Windows platform should probably be a select on the MSVC compiler instead (see @rules_cc//cc/compiler)

[fmeum]

Could these be local_defines?

[fmeum]

This could be private.

[fmeum]

The CA cert logic is problematic since it inspects the host in a genrule, which will not work correctly with remote execution. We could define a Starlark string_flag and require Linux users to set it explicitly.

[aiuto]

My first reaction is that we don't want to push a curl 7.x. There are enough security fixes so that we should be encouraging 8.x. I can look at the actual content of the PR later this week.

…

On Sun, Feb 4, 2024 at 2:31 AM Fabian Meumertzheim ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In modules/curl/7.69.1/patches/add_build_file.patch <#1429 (comment)> : > ++ "-Wl,Security", ++ ], ++ ":windows": [ ++ "-DEFAULTLIB:ws2_32.lib", ++ "-DEFAULTLIB:advapi32.lib", ++ "-DEFAULTLIB:crypt32.lib", ++ "-DEFAULTLIB:Normaliz.lib", ++ ], ++ "//conditions:default": [ ++ "-lrt", ++ ], ++ }), ++ visibility = ["//visibility:public"], ++ deps = [ ++ # Use the same version of zlib and c-ares that gRPC does. ++ #"//external:madler_zlib", Are these comments leftovers? ------------------------------ In modules/curl/7.69.1/patches/add_build_file.patch <#1429 (comment)> : > ++ }), ++) ++ ++# Finally, on Windows and macOS we do not need to define CURL_CA_BUNDLE at all, ++# so on those platforms we skip the branch of the dependencies altogether. ++cc_library( ++ name = "define-ca-bundle-location", ++ deps = select({ ++ ":windows": [], ++ ":macos": [], ++ "//conditions:default": [":define-ca-bundle-location-linux"], ++ }), ++) ++ ++CURL_WIN_COPTS = [ ++ "/Iexternal/com_github_curl_curl/lib", This looks like it won't work with Bzlmod. You may need to get the repository name from a Label defined in a .bzl file. ------------------------------ In modules/curl/7.69.1/patches/add_build_file.patch <#1429 (comment)> : > ++ "lib/vtls/openssl.c", ++ ], ++ }), ++ hdrs = [ ++ "include/curl/curl.h", ++ "include/curl/curlver.h", ++ "include/curl/easy.h", ++ "include/curl/mprintf.h", ++ "include/curl/multi.h", ++ "include/curl/stdcheaders.h", ++ "include/curl/system.h", ++ "include/curl/typecheck-gcc.h", ++ "include/curl/urlapi.h", ++ ], ++ copts = select({ ++ ":windows": CURL_WIN_COPTS, This select on the Windows platform should probably be a select on the MSVC compiler instead (see @rules_cc//cc/compiler) ------------------------------ In modules/curl/7.69.1/patches/add_build_file.patch <#1429 (comment)> : > ++ "-D_GNU_SOURCE", ++ "-DBUILDING_LIBCURL", ++ "-DHAVE_CONFIG_H", ++ "-DCURL_DISABLE_FTP", ++ "-DCURL_DISABLE_NTLM", # turning it off in configure is not enough ++ "-DHAVE_LIBZ", ++ "-DHAVE_ZLIB_H", ++ "-Wno-string-plus-int", ++ ], ++ }) + select({ ++ ":macos": [ ++ "-fno-constant-cfstrings", ++ ], ++ "//conditions:default": [], ++ }), ++ defines = ["CURL_STATICLIB"] + select({ Could these be local_defines? ------------------------------ In modules/curl/7.69.1/patches/add_build_file.patch <#1429 (comment)> : > ++# limitations under the License. ++ ++# Description: ++# curl is a tool for talking to web servers. ++ ++licenses(["notice"]) # MIT/X derivative license ++ ++exports_files(["COPYING"]) ++ ++config_setting( ++ name = "windows", ++ constraint_values = [ ++ ***@***.***//os:windows", ++ ***@***.***//cpu:x86_64", ++ ], ++ visibility = ["//visibility:public"], This could be private. ------------------------------ In modules/curl/7.69.1/patches/add_build_file.patch <#1429 (comment)> : > ++ constraint_values = [ ++ ***@***.***//os:windows", ++ ***@***.***//cpu:x86_64", ++ ], ++ visibility = ["//visibility:public"], ++) ++ ++config_setting( ++ name = "macos", ++ constraint_values = [ ++ ***@***.***//os:macos", ++ ], ++ visibility = ["//visibility:public"], ++) ++ ++# On Linux, libcurl needs to know, at compile time, the location for the The CA cert logic is problematic since it inspects the host in a genrule, which will not work correctly with remote execution. We could define a Starlark string_flag and require Linux users to set it explicitly. — Reply to this email directly, view it on GitHub <#1429 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXHHHHCJ7LWSYGZTSH2J5DYR42LTAVCNFSM6AAAAABCYLCZEOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQNRRGI4DMMRUHE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[lalten]

Note that there is also https://github.com/hedronvision/bazel-make-cc-https-easy/blob/main/curl.BUILD which works with curl 8.6 and seems to be OS aware
Cc @cpsauer

[Wyverald]

Hello! I'm doing some routine cleanup of stale PRs. If you're still working on this and planning to make progress soon, please let me know and we can re-open it.

[keith]

i opened a 8.x curl PR based on a different original BUILD file, not sure how easy it will be to get working #1666

[aiuto]

It is extremely difficult to write a good and mostly portable BUILD file for curl. I know from experience. There are a lot of assumptions you have to make which any given user might want to change: - are you building libcurl, the curl binary, or both? - are you cross compiling for a different architecture? (the configure setup makes that extremely difficult) - do you depend on a built openssl or the system one. Warning: This choice needs to be made by the top level binary using curl. Not the curl BUILD file itself - similar concern for libnghttp2, brotli, zlib - are you going to support windows and macos? - what protocols do you want in it? curl can be built with or without support for virtually every protocol. Adding ones you do not need is both code bloat and a security vulnerability. To blindly build everything is a mistake. The best path forward might be a minimal BUILD file with a very explicit list of the assumptions made posted at the top. It must also include instructions about how one would use their own BUILD file to override those choices.

…

On Thu, Mar 21, 2024 at 8:03 PM Keith Smiley ***@***.***> wrote: i opened a 8.x curl PR based on a different original BUILD file, not sure how easy it will be to get working #1666 <#1666> — Reply to this email directly, view it on GitHub <#1429 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXHHHCPW4WP7JRGOON66CTYZNYNHAVCNFSM6AAAAABCYLCZEOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJUGA3TEMRQGU> . You are receiving this because you were mentioned.Message ID: ***@***.***>