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

[NetCDF] Reorganise to support multiple Julia versions #2402

Merged
merged 6 commits into from
Jan 26, 2021
Merged

[NetCDF] Reorganise to support multiple Julia versions #2402

merged 6 commits into from
Jan 26, 2021

Conversation

Alexander-Barth
Copy link
Contributor

@Alexander-Barth Alexander-Barth commented Jan 17, 2021

This PR aims to address #2401. However, curl (a standard library in julia 1.6) cannot be found by the build process.
Any ideas are most welcome :-)

@Alexander-Barth
Copy link
Contributor Author

Good idea @visr , here is the PR: #2402

@Alexander-Barth
Copy link
Contributor Author

The PR fails with:

   Resolving package versions...
ERROR: LoadError: Unsatisfiable requirements detected for package LibCURL_jll [deac9b47]:
 LibCURL_jll [deac9b47] log:
 ├─possible versions are: 7.71.1 or uninstalled
 └─restricted to versions 7.73.0 by an explicit requirement — no versions left

7.73 is the version number that I see in share/julia/stdlib/v1.6/LibCURL_jll/StdlibArtifacts.toml in my julia 1.6.0-beta1 installation.

@giordano
Copy link
Member

giordano commented Jan 17, 2021

If you want to support both Julia 1.5- and 1.6+, you have to do something more maintainable, similar to, for example, libpolymake_julia, where you have a common script and then multiple different implementations for the number of different version you want to support (I think for the moment only julia 1.3 and 1.6 are necessary?). Note that you need to figure out a new versioning scheme, multiplying each part of the version numbers by 100 + something else is a good idea.

Regarding the trouble with installing the dependencies, if you don't specify a number you should automatically get the right versions for Julia v1.6. Hopefully we'll have soon something more user-friendly

@giordano
Copy link
Member

giordano commented Jan 17, 2021

To be clear, this needs to:

  1. have a different version number, otherwise you'll break the package for Julia 1.5-
  2. add the keyword argument julia_compat="1.6" to build_tarballs()

@Alexander-Barth
Copy link
Contributor Author

Alexander-Barth commented Jan 17, 2021

If you want to support both Julia 1.5- and 1.6+,

From my perspective, it is OK if a new version of NetCDF_jll (and NCDatasets) only supports julia 1.6. But NetCDF_jll is also used in NetCDF.jl developed by @meggart and @visr.

I am hesitant to disconnect the version number of NetCDF_jll from the upstream version as it might confuse some users (but of course breaking packages julia 1.5- would be very bad).

I am wondering if julia_compat="1.6" would not be sufficient to prevent the installation of a new NetCDF_jll on julia 1.5-.

But if having a version 400.700.400 is the only option, then that's fine for me.

By the way, Debian version numbers uses an epoch number:
https://www.debian.org/doc/debian-policy/ch-controlfields.html#version
Maybe this would be useful to consider.

@giordano
Copy link
Member

I am wondering if julia_compat="1.6" would not be sufficient to prevent the installation of a new NetCDF_jll on julia 1.5-.

If we continue with the current version scheme, merging this PR as it is would create the version 4.7.4+2, which will overshadow all other 4.7.4+* releases, which happen to be all of the existing releases for NetCDF_jll, and make the package completely unusable in Julia v1.5- for all users.

The matter of fact is that Pkg and the registry cannot handle build versions nicely, we can only meaningfully use the three numbers X.Y.Z. This isn't something that can be fixed for old versions, so it's something we have to live with, period. We've been luckily so far to not have had huge issues, but we're reaching the point where always using the upstream version simply can't possibly work.

I am hesitant to disconnect the version number of NetCDF_jll from the upstream version as it might confuse some users

I also love using the upstream version number, but at some point we must face reality and admit that we don't have enough version numbers to keep going the upstream version and our multiple rebuilds. Also, end users shouldn't care too much about JLL packages most of the time. I don't like having a lookup table to find the upstream version, but the "100x trick" kind of give you that information. If you have other ideas I'd be very happy to listen to them, but the version number must change

@visr
Copy link
Contributor

visr commented Jan 17, 2021

Just got sniped by @giordano, but posting anyway :)

From my perspective, it is OK if a new version of NetCDF_jll (and NCDatasets) only supports julia 1.6.

This is fine by me as well, although a bit unfortunate. For GDAL & Co I decided not to do this, because it would cause a bunch of packages to require 1.6 if they'd want the latest features, and it isn't even out yet. And for 1.7 it may be the same story.

I am hesitant to disconnect the version number of NetCDF_jll from the upstream version
I am wondering if julia_compat="1.6" would not be sufficient to prevent the installation of a new NetCDF_jll on julia 1.5-.

Indeed it would prevent installation, but it is incompatible with keeping the same version number unfortunately. It would need at least a minor version bump if I understand correctly, so they would disconnect anyway.

I was also hestitant about multiplying the version numbers by 100, as I just mentioned here. Though it is a system that at least makes it easy to recognize the upstream version, and provides a way around the issues we are dealing with.

@giordano
Copy link
Member

To be clear, I'm not married to the 100x versioning scheme, but it does have a few advantages. I'm eager to listen to alternative versioning schemes, but in any case the version number must change, unfortunately

@Alexander-Barth
Copy link
Contributor Author

Thank you very much for your explanations. To me it seems that we are getting hit by the inadequacy of semantic versioning for binary package.
I revised the PR by multiplying version numbers by 100. I don't see a better alternative in the current system.

@visr
Copy link
Contributor

visr commented Jan 23, 2021

If possible I'd like to have a setup such that we can keep bringing out new builds for julia 1.3+ without too much effort. Because looking at #2421 it seems we'd face the same issue at the next point release. Though I don't want to hold this up either. @Alexander-Barth, is there a reason that it still says [WIP] in the title?

If you want to support both Julia 1.5- and 1.6+, you have to do something more maintainable, similar to, for example, libpolymake_julia

@giordano based on this example I don't quite see how it works. We don't need to take a build dependency on libjulia_jll like libpolymake, right? And looking at the polymake_jll release, I only see a single release that requires julia = 1.0 in the compat.

If we have 3 build_tarballs.jl, do they all get built and tagged at the same time? Say if for NetCDF_jll we would want to simultaneously maintain builds for different minor julia releases

upstream_version = v"4.7.4"
version = v"400.700.400"
julia_compat="1.0"  # really, 1.3, but 1.0 to make them installable
Dependency("LibCURL_jll", v"7.71.1")
Dependency("LibSSH2_jll", v"1.9.0")
Dependency("MbedTLS_jll", v"2.16.8")
Dependency("nghttp2_jll", v"1.40.0")

upstream_version = v"4.7.4"
version = v"400.701.400"  # higher minor version, for higher minor julia compat
julia_compat="1.6"
Dependency("LibCURL_jll")
Dependency("LibSSH2_jll")
Dependency("MbedTLS_jll")
Dependency("nghttp2_jll")

Is this about what you had in mind?

Since the minor version of the JLL is higher for the 1.6 release, it will pick that one. In downstream packages, we'd put the compat on v"400.700", and if we'd wrap a future NetCDF C API of 4.8, we'd bump it to v"400.800". It seems for many packages in Yggdrasil, like XZ, there is simply one build_tarballs.jl with julia_compat="1.6", and the latest releases are being built only for the latest julia. That is the same approach used here now, and one that currently seems simpler. But if I build a new minor release, and want to directly wrap the new functions in this version in the downstream julia package, I have to require julia 1.6 there, and for relatively foundational packages like NCDatasets/NetCDF/Proj4/GDAL, I'd like to avoid that if possible.

@giordano
Copy link
Member

giordano commented Jan 25, 2021

Does this sound good? We have v400.701.400 for Julia v1.3-5, and v400.702.400 for Julia v1.6+. There is a single common build script, to reduce maintenance burden, when you want a new build you have to do the changes you want in the common script and bump the offset number in the two build_tarballs.jl files

@visr
Copy link
Contributor

visr commented Jan 25, 2021

Awesome, that looks pretty good, and answers my questions, thanks. It looks like the old julia versions are having a party together ~1.0, ~1.1, ~1.2, ~1.3, ~1.4, ~1.5 🎉

@Alexander-Barth
Copy link
Contributor Author

This is great! Thank you very much.

@Alexander-Barth Alexander-Barth changed the title [WIP] Update of NetCDF_jll to use stdlib libcurl and MbedTLS in julia 1.6 Update of NetCDF_jll to use stdlib libcurl and MbedTLS in julia 1.6 Jan 25, 2021
@giordano
Copy link
Member

Ok, I'm going to merge this, but will pause registration in the general registry to allow you doing some tests, ok?

@giordano
Copy link
Member

Actually, I think I'll remove the recipe for Julia v1.6 from this PR and will add in a separate PR (also to avoid conflicts in the PR to General, which are a mess to handle manually)

@giordano giordano changed the title Update of NetCDF_jll to use stdlib libcurl and MbedTLS in julia 1.6 [NetCDF] Reorganise to support multiple Julia versions Jan 26, 2021
@giordano giordano merged commit fd2dfbb into JuliaPackaging:master Jan 26, 2021
@giordano
Copy link
Member

@Alexander-Barth @visr remember that now in your packages you need to allow the new versions 400.701.400 and 400.702.400 of NetCDF_jll

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.

3 participants