Use a common build script for both CURL and LibCURL #4472

DilumAluthge · 2022-02-21T02:14:34Z

Fixes #2720
Closes #4153

DilumAluthge · 2022-02-21T02:15:06Z

cc: @ericphanson @giordano

L/LibCURL/CURL/build_tarballs.jl

L/LibCURL/LibCURL/build_tarballs.jl

L/LibCURL/common.jl

giordano · 2022-02-21T11:56:57Z

L/LibCURL/LibCURL/build_tarballs.jl

+include("../common.jl")
+
+build_libcurl(ARGS, "LibCURL")


I don't want to rebuild libcurl, nothing changed. This file should be removed from this PR and added in a follow-up PR with [skip ci].

Is it cool if I leave the file in until the PR is ready to merge, just to make sure that CI passes (and that I don't have any bugs for the libcurl code path)? And then, once the PR is ready to merge, I'll remove the file from this PR, and then you can merge immediately afterwards?

L/LibCURL/common.jl

giordano · 2022-02-21T13:00:53Z

~~One thing which you may want to consider is to do a static build of curl which doesn't depend on libcurl at all, which is annoying because that's tied to a julia version.~~

Edit: on a second thought, you'd need also static libraries of libmedtls and libssh2, which we don't have, so that's not going to fully disentangle curl from julia's libraries anyway.

Co-authored-by: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>

DilumAluthge · 2022-02-24T17:40:05Z

@giordano @staticfloat Apart from deleting the L/LibCURL/LibCURL/build_tarballs.jl file, is there anything else that needs to be done for this PR?

giordano · 2022-02-24T22:50:19Z

This still creates libcurl in CURL_jll and curl in LibCURL_jll. Isn't there a way to avoid this duplication? This also kind of defeats the point of having CURL_jll depending on LibCURL_jll.

DilumAluthge · 2022-02-24T22:52:12Z

I have no idea. @staticfloat?

giordano · 2022-02-24T22:53:53Z

Or, put in another way: what do Linux distributions do? They also split libcurl and curl into different packages.

staticfloat · 2022-02-24T23:02:02Z

Just delete libcurl from the CURL_jll package. Once we have the ability to split a single build out into multiple JLLs we can avoid rebuilding the products twice.

giordano · 2022-02-24T23:32:10Z

L/LibCURL/common.jl

+    if [[ "${THIS_IS_CURL}" == true ]]; then
+        # Manually install only `curl`
+        install -Dm 755 "src/.libs/curl${exeext}" "${bindir}/curl${exeext}"
+    else
+        # Install everything...
+        make install
+        # ...but remove `curl`
+        rm "${bindir}/curl${exeext}"
+    fi


I followed what I did in

Yggdrasil/H/HarfBuzz/common.jl

Lines 38 to 45 in 2fa71a4

if [[ "${ICU}" == true ]]; then

# Manually install only ICU-related files

cp src/libharfbuzz-icu*${dlext}* ${libdir}/.

cp meson-private/harfbuzz-icu.pc ${prefix}/lib/pkgconfig/.

cp ../src/hb-icu.h ${includedir}/harfbuzz/.

else

ninja install

fi

which is what I've been telling to look at

ericphanson · 2022-02-25T00:14:24Z

L/LibCURL/common.jl

+    end
+
+    # Build the tarballs, and possibly a `build.jl` as well.
+    build_tarballs(ARGS, name, version, sources, script, platforms, products, dependencies; julia_compat="1.8")


Why 1.8 only?

https://github.com/JuliaLang/julia/blob/release-1.7/stdlib/LibCURL_jll/Project.toml#L3
https://github.com/JuliaLang/julia/blob/release-1.8/stdlib/LibCURL_jll/Project.toml#L3

Oh… I guess that’s why we’d want a fully static thing like you mentioned. Annoying.

Still, thanks for your help with this!

@giordano @staticfloat What if we:

Don't make CURL_jll depend on LibCURL_jll

When building CURL_jll, don't delete libcurl. So then CURL_jll will supply both libcurl and curl.

Would that allow us to use the same version of CURL_jll on multiple versions of Julia?

#4472 (comment)

I see, so we'd need static libraries for all of the dependencies.

So this just means that we need to remember to make a new release of CURL_jll every time there's a new minor release of Julia, right?

No, just always build CURL_jll together with (well, right after) a new LibCURL_jll.

No, just always build CURL_jll together with (well, right after) a new LibCURL_jll.

It would be nice if we could automate that process.

JuliaPackaging/BinaryBuilder.jl#778. This is what #4472 (comment) and

Yggdrasil/H/HarfBuzz/common.jl

Lines 17 to 19 in 2fa71a4

# Bash recipe for building across all platforms

# Side note: this is a great use-case for

# https://github.com/JuliaPackaging/BinaryBuilder.jl/issues/778

were referring to.

DilumAluthge · 2022-02-25T15:22:52Z

@giordano @staticfloat @ericphanson Thanks so much for your help with this!

giordano · 2022-02-25T15:25:56Z

For the record, showing this is working:

julia> using CURL_jll

julia> run(`$(curl()) --version`);
curl 7.81.0 (x86_64-pc-linux-gnu) libcurl/7.81.0 mbedTLS/2.28.0 zlib/1.2.11 libssh2/1.10.0 nghttp2/1.41.0
Release-Date: 2022-01-05
Protocols: dict file ftp ftps gopher gophers http https imap imaps mqtt pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp 
Features: alt-svc AsynchDNS HSTS HTTP2 IPv6 Largefile libz NTLM NTLM_WB SSL UnixSockets

Yggdrasil pull request JuliaPackaging/Yggdrasil#4472 deleted the file `L/LibCURL/build_tarballs.jl`, which made the test checking that the file exists fail. Arguably, we should make the check more robust, but it'd be considerably more work for relatively little gain (vast majority of packages do follow the predictable naming scheme).

DilumAluthge requested a review from giordano February 21, 2022 02:14

DilumAluthge mentioned this pull request Feb 21, 2022

add CURL recipe from LibCURL #4153

Closed

ericphanson reviewed Feb 21, 2022

View reviewed changes

L/LibCURL/CURL/build_tarballs.jl Outdated Show resolved Hide resolved

ericphanson reviewed Feb 21, 2022

View reviewed changes

L/LibCURL/LibCURL/build_tarballs.jl Outdated Show resolved Hide resolved

DilumAluthge force-pushed the dpa/curl-executables branch from 2333d10 to 66b9472 Compare February 21, 2022 03:18

giordano reviewed Feb 21, 2022

View reviewed changes

DilumAluthge force-pushed the dpa/curl-executables branch from 14e32be to 7455cc6 Compare February 21, 2022 12:37

Use a common build script for both CURL and LibCURL

fe8851b

Co-authored-by: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>

DilumAluthge force-pushed the dpa/curl-executables branch from 7455cc6 to fe8851b Compare February 24, 2022 16:04

Install only curl in CURL_jll, and remove curl from LibCURL_jll

cc6286d

giordano reviewed Feb 24, 2022

View reviewed changes

Delete build_tarballs.jl

81815de

giordano merged commit 7cfae68 into master Feb 24, 2022

giordano deleted the dpa/curl-executables branch February 24, 2022 23:41

ericphanson reviewed Feb 25, 2022

View reviewed changes

DilumAluthge mentioned this pull request Feb 25, 2022

Add the L/LibCURL/LibCURL/build_tarballs.jl file #4504

Merged

giordano mentioned this pull request Feb 28, 2022

Fix hanging tests JuliaPackaging/BinaryBuilder.jl#1175

Merged

giordano mentioned this pull request Oct 17, 2023

add a patched libcurl v7 build for use with Julia 1.6 #7549

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use a common build script for both CURL and LibCURL #4472

Use a common build script for both CURL and LibCURL #4472

DilumAluthge commented Feb 21, 2022 •

edited

Loading

DilumAluthge commented Feb 21, 2022

giordano Feb 21, 2022

DilumAluthge Feb 21, 2022

giordano commented Feb 21, 2022 •

edited

Loading

DilumAluthge commented Feb 24, 2022 •

edited

Loading

giordano commented Feb 24, 2022

DilumAluthge commented Feb 24, 2022

giordano commented Feb 24, 2022

staticfloat commented Feb 24, 2022

giordano Feb 24, 2022

ericphanson Feb 25, 2022

giordano Feb 25, 2022

ericphanson Feb 25, 2022 •

edited

Loading

DilumAluthge Feb 25, 2022

giordano Feb 25, 2022

DilumAluthge Feb 25, 2022

DilumAluthge Feb 25, 2022

giordano Feb 25, 2022

DilumAluthge Feb 25, 2022

giordano Feb 25, 2022

DilumAluthge commented Feb 25, 2022

giordano commented Feb 25, 2022

	if [[ "${ICU}" == true ]]; then
	# Manually install only ICU-related files
	cp src/libharfbuzz-icu${dlext} ${libdir}/.
	cp meson-private/harfbuzz-icu.pc ${prefix}/lib/pkgconfig/.
	cp ../src/hb-icu.h ${includedir}/harfbuzz/.
	else
	ninja install
	fi

	# Bash recipe for building across all platforms
	# Side note: this is a great use-case for
	# https://github.com/JuliaPackaging/BinaryBuilder.jl/issues/778

		include("../common.jl")

		build_libcurl(ARGS, "LibCURL")

Use a common build script for both CURL and LibCURL #4472

Use a common build script for both CURL and LibCURL #4472

Conversation

DilumAluthge commented Feb 21, 2022 • edited Loading

DilumAluthge commented Feb 21, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

giordano commented Feb 21, 2022 • edited Loading

DilumAluthge commented Feb 24, 2022 • edited Loading

giordano commented Feb 24, 2022

DilumAluthge commented Feb 24, 2022

giordano commented Feb 24, 2022

staticfloat commented Feb 24, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericphanson Feb 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DilumAluthge commented Feb 25, 2022

giordano commented Feb 25, 2022

DilumAluthge commented Feb 21, 2022 •

edited

Loading

giordano commented Feb 21, 2022 •

edited

Loading

DilumAluthge commented Feb 24, 2022 •

edited

Loading

ericphanson Feb 25, 2022 •

edited

Loading