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

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

Merged
merged 3 commits into from
Feb 24, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions L/LibCURL/CURL/build_tarballs.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
include("../common.jl")

build_libcurl(ARGS, "CURL")
3 changes: 3 additions & 0 deletions L/LibCURL/LibCURL/build_tarballs.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
include("../common.jl")

build_libcurl(ARGS, "LibCURL")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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].

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

83 changes: 0 additions & 83 deletions L/LibCURL/build_tarballs.jl

This file was deleted.

111 changes: 111 additions & 0 deletions L/LibCURL/common.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
# Note that this script can accept some limited command-line arguments, run
# `julia build_tarballs.jl --help` to see a usage message.
using BinaryBuilder, Pkg

function build_libcurl(ARGS, name::String)
version = v"7.81.0"
hash = "ac8e1087711084548d788ef18b9b732c8de887457b81f616fc681d1044b32f98"

if name == "CURL"
this_is_curl_jll = true
elseif name == "LibCURL"
this_is_curl_jll = false
else
msg = "Not a valid name: $(name). Valid names are: LibCURL, CURL"
throw(ArgumentError(msg))
end

# Collection of sources required to build LibCURL
sources = [
ArchiveSource("https://curl.se/download/curl-$(version).tar.gz", hash),
]

# Bash recipe for building across all platforms
script = "THIS_IS_CURL=$(this_is_curl_jll)\n" * raw"""
cd $WORKSPACE/srcdir/curl-*

# Holy crow we really configure the bitlets out of this thing
FLAGS=(
# Disable....almost everything
--without-ssl --without-gnutls --without-gssapi
--without-libidn --without-libidn2 --without-librtmp
--without-nss --without-polarssl
--without-spnego --without-libpsl --disable-ares --disable-manual
--disable-ldap --disable-ldaps --without-zsh-functions-dir
--disable-static --without-libgsasl

# A few things we actually enable
--with-libssh2=${prefix} --with-zlib=${prefix} --with-nghttp2=${prefix}
--enable-versioned-symbols
)


if [[ ${target} == *mingw* ]]; then
# We need to tell it where to find libssh2 on windows
FLAGS+=(LDFLAGS="${LDFLAGS} -L${prefix}/bin")

# We also need to tell it to link against schannel (native TLS library)
FLAGS+=(--with-schannel)
elif [[ ${target} == *darwin* ]]; then
# On Darwin, we need to use SecureTransport (native TLS library)
FLAGS+=(--with-secure-transport)

# We need to explicitly request a higher `-mmacosx-version-min` here, so that it doesn't
# complain about: `Symbol not found: ___isOSVersionAtLeast`
if [[ "${target}" == *x86_64* ]]; then
export CFLAGS=-mmacosx-version-min=10.11
fi
else
# On all other systems, we use MbedTLS
FLAGS+=(--with-mbedtls=${prefix})
fi

./configure --prefix=$prefix --host=$target --build=${MACHTYPE} "${FLAGS[@]}"
make -j${nproc}
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
Comment on lines +65 to +73
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I followed what I did in

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

install_license COPYING
"""

# These are the platforms we will build for by default, unless further
# platforms are passed in on the command line
platforms = supported_platforms()

# The products that we will ensure are always built
if this_is_curl_jll
# CURL_jll only provides the executable
products = [
ExecutableProduct("curl", :curl),
]
else
# LibCURL only provides the library
products = [
LibraryProduct("libcurl", :libcurl),
]
end

# Dependencies that must be installed before this package can be built
dependencies = [
Dependency("LibSSH2_jll"),
Dependency("Zlib_jll"),
Dependency("nghttp2_jll"),
# Note that while we unconditionally list MbedTLS as a dependency,
# we default to schannel/SecureTransport on Windows/MacOS.
Dependency("MbedTLS_jll"; compat="~2.28.0", platforms=filter(p->Sys.islinux(p) || Sys.isfreebsd(p), platforms)),
]

if this_is_curl_jll
# Curl_jll depends on LibCURL_jll
push!(dependencies, Dependency("LibCURL_jll"; compat="$(version)"))
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")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 1.8 only?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@ericphanson ericphanson Feb 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Still, thanks for your help with this!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@giordano @staticfloat What if we:

  1. Don't make CURL_jll depend on LibCURL_jll
  2. 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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

# 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.

end