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.

Sign up for GitHub

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

Jump to bottom

[jlqml] Bump version to v0.3 #4264

Merged
merged 1 commit into from
Jan 30, 2022
Merged

[jlqml] Bump version to v0.3 #4264

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 15 additions & 7 deletions J/jlqml/build_tarballs.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,19 @@
# `julia build_tarballs.jl --help` to see a usage message.
using BinaryBuilder, Pkg

julia_version = v"1.6.0"
# See https://github.com/JuliaLang/Pkg.jl/issues/2942
# Once this Pkg issue is resolved, this must be removed
uuid = Base.UUID("a83860b7-747b-57cf-bf1f-3e79990d037f")
delete!(Pkg.Types.get_last_stdlibs(v"1.6.3"), uuid)
Comment on lines +7 to +8
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this actually break everything when we try to target Julia v1.6? I don't see LibOSXUnwind_jll in https://dev.azure.com/JuliaPackaging/Yggdrasil/_build/results?buildId=16459&view=logs&jobId=3e3b6bc7-f97f-5652-f6ca-c52d13eda498&j=a151bd49-d959-5dd9-e79c-4da26ad24f9f&t=fa03432c-08c9-5f99-931b-dc1b3c71632b, which looks wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I understand, LibOSXUnwind_jll is a dependency of libjulia_jll and removing it from the stdlibs makes Pkg choose the one in the registry. It doesn't appear to cause harm, at least on Linux I get a usable JLL with binaries for Julia 1.6, 1.7 and 1.8: https://github.com/barche/jlqml_jll.jl

Copy link
Member

Choose a reason for hiding this comment

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

I still don't see LibOSXUnwind_jll in https://dev.azure.com/JuliaPackaging/Yggdrasil/_build/results?buildId=16556&view=logs&j=11cc9737-c3b2-5b2e-49c1-3ecd4dd3473d&t=6864626c-b206-5daa-5d51-831805962223. @fingolfin maybe it wasn't necessary after all? If so, I think the best thing to do would be to have a libjulia_jll for v1.6 which doesn't depend at runtime on LibOSXUnwind_jll, fix this in the registry so libjulia_jll never tries to pull in LibOSXUnwind_jll. That'd solve all of our problems without hacks like this.

Should we keep this hack (I hope we don't), at very least please add a comment, I'm sure someone will be very confused by reading these lines without context in a few months.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That seems like the way to go, but how is this dependency triggered, exactly? I don't see a LibOSXUnwind_jll in the Project.toml of libjulia_jll v1.7.0+8? Is it about removing these:

https://github.com/JuliaBinaryWrappers/libjulia_jll.jl/blob/2e64b99b0e204bf25308727beb971269affd3c4b/src/wrappers/x86_64-apple-darwin-julia_version%2B1.6.3.jl#L6

Copy link
Member

Choose a reason for hiding this comment

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

It's only in v1.6

Copy link
Member

Choose a reason for hiding this comment

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

It was never a question about linking to libOSXUnwind.dylib. The only concern is compiling against julia.h -- some of the headers include libunwind.h. If one includes one of these headers (possibly indirectly) this can result in a compile time error. If it doesn't, then that means it is picking up a stray libunwind.h from somewhere else, which could be bad -- though I guess my patch shows that it doesn't matter in practice....

Anyway, if this builds fine with your hack, then that's great. But I don't like that now all packages linking against libjulia_jll need such a hack. I think it's much cleaner and easier if we update libjulia_jll, dropping the dep on LibOSXUnwind_jll in there. That's what #4320 is about (though it may indeed be possible to simplify the patch, and instead e.g. do touch $includedir/libunwind.h)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, so I guess the possibility of corrupt binaries is excluded, at worst we should get a linking error. I have added a comment, I'd propose to merge this and then investigate JuliaLang/Pkg.jl#2942 further, libjulia_jll also doesn't build because of that right now.

Copy link
Member

Choose a reason for hiding this comment

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

I agree, let's test this, if it works, this is a good short term solution that we can use for the time being also elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, from JuliaLang/Pkg.jl#2963 I also conclude that this hack will in this case actually have the same effect as the (possible) fix. LibOSXUnwind_jll is here only encountered in the historical dependencies and then triggers a bug in the dependency resolution. So, @giordano this is now good to merge I think :)

Copy link
Member

Choose a reason for hiding this comment

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

Ok, let's do it


julia_versions = [v"1.6.3", v"1.7.0", v"1.8.0"]

name = "jlqml"
version = v"0.2"
version = v"0.3"

# Collection of sources required to complete build
sources = [
GitSource("https://github.com/barche/jlqml.git", "4bc5d39bdc261eff12e3b7d1cd8b28ff0519e316"),
GitSource("https://github.com/barche/jlqml.git", "4d21e2fcf0bcae29c45e76c8ac9d1d45893a77e6"),
]

# Bash recipe for building across all platforms
Expand Down Expand Up @@ -44,10 +49,13 @@ install_license $WORKSPACE/srcdir/jlqml*/LICENSE.md
# These are the platforms we will build for by default, unless further
# platforms are passed in on the command line
include("../../L/libjulia/common.jl")
platforms = libjulia_platforms(julia_version)
platforms = vcat(libjulia_platforms.(julia_versions)...)
platforms = expand_cxxstring_abis(platforms)

filter!(p -> libc(p) != "musl", platforms) # Qt_jll is currently not available for muslc
# Qt5Declarative_jll is not available for these architectures:
filter!(p -> arch(p) != "armv6l", platforms)
filter!(p -> !(arch(p) == "aarch64" && Sys.isapple(p)), platforms)

# The products that we will ensure are always built
products = [
Expand All @@ -60,10 +68,10 @@ dependencies = [
Dependency("Qt5Declarative_jll"),
Dependency("Qt5Svg_jll"),
BuildDependency("Libglvnd_jll"),
BuildDependency(PackageSpec(name="libjulia_jll", version=julia_version)),
BuildDependency("libjulia_jll"),
]

# Build the tarballs, and possibly a `build.jl` as well.
build_tarballs(ARGS, name, version, sources, script, platforms, products, dependencies;
preferred_gcc_version=v"9",
julia_compat = "$(julia_version.major).$(julia_version.minor)")
preferred_gcc_version = v"9",
julia_compat = "1.6")