julia: cleanup/reorganisation #117881
julia: cleanup/reorganisation #117881
Conversation
|
Ah, forgot to mention that we are now a few people over on the Julia slack in |
ofborg
bot
added
8.has: clean-up
8.has: package (new)
ofborg
bot
added
11.by: package-maintainer
|
I think a part of that explixit numbering was that Julia has been enough of a moving target that sometimes moving from the freshly-unsupported version to the just-packaged-but-not-yet-fully version was quite a bit of a tradeoff, and depending on the use case different moments were optimal migration points for different users. Do you expect Julia packaging to be more stable going forward? (expression code looks fine to me) |
|
@7c6f434c That is a very fair point and one I had not immediately considered, I guess it is all the more relevant on NixOS as we can not as easily just grab the binary release. Happy to rework this part as long as we can agree on some sort of policy going forward that I can document in the expressions. How about we continue using numbered releases (but do offer the aliases) and keep around releases not supported upstream for say six months? Is there a good way to signal an upcoming deprecation to the Nixpkg end-user? I feel it is not really responsible on our side not to let them know that security patches etc. will not be coming their way. |
|
***@***.*** That is a very fair point and one I had not immediately considered, I guess it is all the more relevant on NixOS as we can not as easily just grab the binary release.
It might be not that bad with some amount of `patchelf`, but apparently the issues one would find have some similarities to the issues one needs to figure out to update a source build. Probably simpler/fewer, though.
Happy to rework this part as long as we can agree on some sort of policy going forward that I can document in the expressions. How about we continue using numbered releases (but do offer the aliases) and keep around releases not supported upstream for say six months?
I am in favour of making -stable/-lts the default/recommended/documented/release-notes-mentioned way of doing things, pointing to the best approximation we have (I guess if latest is packaged in a half-broken way, this will continue pointing to the previous version). I, personally, is not even against immediate removal a short period after both conditions «upstream dropped support» and «things work as well as they worked for the previous release» hold. But preferably not before…
Is there a good way to signal an upcoming deprecation to the Nixpkg end-user? I feel it is not really responsible on our side not to let them know that security patches etc. will not be coming their way.
Good question. I have a feeling that when the latest version is not packaged or not fully usable, the general policy would be against printing any warnings (both in terms of a top-level attribute not evaluating without warnings, which is a mandatory check for Nixpkgs, and in general terms of user having no way to get rid of the warning). We might even have a scheme where picking a numbered version requires going into something more complicated (possibly also not Hydra-built? not sure), so there partial support could even be a part of the attribute path; depends on what is the timeframe we are talking about,
|
|
Generally speaking I am also in favor of making the naming convention of "lts/stable" the new default. Not sure how much need there is to keep numbered versions (internally). Even if we don't keep numbered versions around it should be relatively straightforward for a user to use the stable version from a previous NixOS release, no? |
|
Mixing glibc versions gets annoying annoyingly quickly, though (and Julia has some great FFI-based packages…) |
|
On Sun 28 Mar 2021, Michael Raskin wrote:
> That is a very fair point and one I had not immediately considered, I guess it is all the more relevant on NixOS as we can not as easily just grab the binary release.
It might be not that bad with some amount of `patchelf`, but apparently the issues one would find have some similarities to the issues one needs to figure out to update a source build. Probably simpler/fewer, though.
Yes and no. While `patchelf` on the official binaries does work, you end up with some fairly nasty errors [1] when interacting with the package system for which the only solution really is that we have some moderately complex patch we apply to Julia at build time (off topic, but the reason why you see this PR is actually my desire to build that specific patch '^^).
[1]: https://builds.sr.ht/~ninjin/job/445206#task-test-314
>Happy to rework this part as long as we can agree on some sort of policy going forward that I can document in the expressions. How about we continue using numbered releases (but do offer the aliases) and keep around releases not supported upstream for say six months?
I am in favour of making -stable/-lts the default/recommended/documented/release-notes-mentioned way of doing things, pointing to the best approximation we have (I guess if latest is packaged in a half-broken way, this will continue pointing to the previous version). I, personally, is not even against immediate removal a short period after both conditions «upstream dropped support» and «things work as well as they worked for the previous release» hold. But preferably not before…
>Is there a good way to signal an upcoming deprecation to the Nixpkg end-user? I feel it is not really responsible on our side not to let them know that security patches etc. will not be coming their way.
Good question. I have a feeling that when the latest version is not packaged or not fully usable, the general policy would be against printing any warnings (both in terms of a top-level attribute not evaluating without warnings, which is a mandatory check for Nixpkgs, and in general terms of user having no way to get rid of the warning). We might even have a scheme where picking a numbered version requires going into something more complicated (possibly also not Hydra-built? not sure), so there partial support could even be a part of the attribute path; depends on what is the timeframe we are talking about,
Thank you, plenty of what you wrote there gave me further insight into how things are handled by the Nixpkgs community (I am still fairly new as a contributor even if I have used NixOS as a daily driver since 2018). I think it mostly boils down to: “Do not break userland”. This I can always get behind. ;) You have convinced me that we should keep the numbered versions around though, here is a proposal:
1. Each release branch gets its own expression as we already do: `x.y.nix` for `julia_xy`.
2. In our documentation we refer to `julia`, `julia-stable`, and `julia-lts` as the canonical names, but really they are aliases defined in `pkgs/top-level/aliases.nix` and we have a somewhat defined process as to when we point them to newer versions. This allows our users to “stay behind” by using the numbered versions if they so desire, even if we gradually retire them as the community “moves on”.
3. As soon as there is an RC, we create a new expression `x.y.nix` so that we can easily track the impact on the Julia/Nix ecosystem. If at the time of release of that stable branch things are looking fine, we change our aliases, if not, we hold off (I do note here that there is no RC process for the LTS, rather a stable becomes an LTS if it has been deemed stable by the Julia devs at the time it is superseded by another stable release. I believe we can trust them here as they have been very careful post-1.0 never to break an LTS by testing the whole package system for each backported patch).
4. I document this process in a `README.md` in `pkgs/development/compilers/julia` (along with links to relevant Julia documentation, etc.).
In regards to 3, the actual reason why I went down this rewrite rabbit hole was that I was working on building a CI system for monitoring the Julia package ecosystem on Nix to determine exactly what issues currently exist (PkgEvalNix [2]: It mostly a mock up, but you should get the general idea). Once finished, we could use it to track the impact throughout the RC process.
[2]: https://ninjin.srht.site/pkgevalnix
Thoughts?
Unrelated, I am fairly tired of rebuilding LLVM a gorillion times just by virtue of minute changes to the expression – it greatly slows down the process… Would it be frowned upon if I broke out an expression to build LLVM out of the main Julia expression using the same source? Defined in the same file as it will only ever be used by that specific Julia expression. I hear that Guix rips out the patches (I can not confirm as I stay clear from anything GPL) and patch their “vanilla” LLVM expression, but this is far too fragile for my taste as we have to keep the build process in sync with upstream Julia or risk potentially passing on truly nasty errors to our users (I mean, our expressions already deviated greatly from upstream Julia and we needed a rewrite). Is there precedence for this that I could look at or strong opinions against it?
|
Note that there is no consensus on basically anything (but top-level warning prohibition works as a piece of code, so that's definitely there). A lot of people want to minimise the number of extra versions around, but for large packages especially where «working» is a complicated notion having multiple version happens.
I think it should be fine. |
|
LTS releases are not interchangeable, and the release process leaves open the possibility that there might be multiple LTS versions worth using at a given time. With regard to linux kernel packages, I am glad to hear people are interested in maintaining julia/nix; thank you too for adding yourself to the maintainers in this PR. |
Yes yes yes to this! It is such a huge PITA to work on the julia nix derivations in some tiny way and then wait years for it to finish rebuilding all of LLVM... |
|
On Sun 04 Apr 2021, Samuel Ainsworth wrote:
> @@ -88,11 +88,6 @@ stdenv.mkDerivation rec {
;
patches = [
- # Julia recompiles a precompiled file if the mtime stored *in* the
- # .ji file differs from the mtime of the .ji file. This
- # doesn't work in Nix because Nix changes the mtime of files in
- # the Nix store to 1. So patch Julia to accept mtimes of 1.
- ./allow_nix_mtime.patch
Has something changed in Julia such that precompilation semantics have shifted? I mean it seems that this patch was necessary at some point in time. I just don't understand what has changed such that it would no longer be necessary now?
I am actually not 100% sure, what I do remember is how the coverage of the system image has expanded over time to improve startup speed. However, if you want some evidence that this is superfluous you can inspect the result of the v1.0.5 derivation and see that there are no `.ji` files present apart from three that come from unit tests. The same is true for later versions, but for those there is precisely zero `.ji` files present in the Nix store.
As a general comment, the way I arrived at this “clean” expression was to test my assumptions with every line and try to remove everything that had seemingly no effect after reading the Nixpkgs manual, Julia documentation, and Julia source code; then move to verify it by running the unit tests. It took a lot of time (looking at you LLVM!), but I am reasonably confident that I got it right, although time (and review!) will tell.
|
|
On Mon 29 Mar 2021, Jim Garrison wrote:
LTS releases are not interchangeable, and the [release process](https://julialang.org/blog/2019/08/release-process/#long_term_support) leaves open the possibility that there might be multiple LTS versions worth using at a given time. With regard to linux kernel packages, `lts` is not even part of the name -- instead, the current supported versions are given (e.g. `linux_4_19`). Why do something different for julia?
The stable/LTS terminology is what Julia officially uses and will therefore in my mind also be what our users would expect (see for example the downloads page [1]). Given my connection within the Julia community I can say almost for certain that `release-1.0` will be dropped as an LTS branch the moment a new one is designated given the amount of energy it is taking to support a branch that is about three years old at this point. I am also not sure where in the release process blog post there is an indication of multiple parallel LTS branches, but it could be that my head is a bit too heavy after midnight or that I am reading it wrong.
[1]: https://julialang.org/downloads
I am glad to hear people are interested in maintaining julia/nix; thank you too for adding yourself to the maintainers in this PR.
Happy to do my part for the community. I have to admit though that Julia is a one heck of a step “up” from something as small and compartmentalised as fdm that was the only thing I have maintained up until now.
I have pushed changes restoring the “numbered” aliases and also an expression for `julia_16` with the test suite passing. Lastly, I took the liberty of dropping `julia_15` given how slow I have been to get this through the door I really did not think it was sane to try to “backport” what I did to get `julia_16` working to unbreak `julia_15`.
@samuela and @cstich: With your #118000 and #118483 I suspect you are among a selected few that have taken a stab at packaging v1.6.0 and I would deeply appreciate if you have the time to look at my attempt here. General comments would also be welcome and I will go in and highlight some parts which I am unsure if they are “best practices”. Lastly, I am mildly terrified over what will come out of the CI for `julia_16` on macOS.
On a mildly related note, something else to come out of this is a set of patches to build Julia with Binary Builder if you want to hack on Julia itself under NixOS. Have a look at the `master` branch for my fork below:
https://sr.ht/~ninjin/julia-nix
|
...evelopment/compilers/julia/patches/1.6/0005-nix-Enable-parallel-unit-tests-for-sandbox.patch
Outdated
Show resolved
Hide resolved
|
Thanks @ninjin ! This is a heroic effort! I'll try to review and test it out in the next few days. |
...ment/compilers/julia/patches/1.6/0001-nix-Remove-library-file-version-numbers-for-JLLs.patch
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
overall lgtm as long as all the tests pass.
could we keep the update script though? the following should do (untested):
#!/usr/bin/env nix-shell
#!nix-shell -i python3 -p python3 python3Packages.requests
import os
import re
import requests
import subprocess
latest = requests.get("https://api.github.com/repos/JuliaLang/julia/releases/latest").json()["tag_name"]
assert latest[0] == "v"
major, minor, patch = latest[1:].split(".")
assert major == "1"
# When a new minor version comes out we'll have to refactor/copy this update script.
assert minor == "6"
sha256 = subprocess.check_output(["nix-prefetch-url", "--unpack", f"https://github.com/JuliaLang/julia/releases/download/v{major}.{minor}.{patch}/julia-{major}.{minor}.{patch}-full.tar.gz"], text=True).strip()
nix_path = os.path.join(os.path.dirname(__file__), "1.6.nix")
nix0 = open(nix_path, "r").read()
nix1 = re.sub("version = \".*\";", f"version = \"{latest[1:]}\";", nix0)
nix2 = re.sub("src_sha256 = \".*\";", f"src_sha256 = \"{sha256}\";", nix1)
open(nix_path, "w").write(nix2)with the src_sha256 refactored as suggested in the review.
...elopment/compilers/julia/patches/1.6/0004-nix-Skip-maximum-number-of-BLAS-threads-test.patch
Outdated
Show resolved
Hide resolved
...evelopment/compilers/julia/patches/1.6/0005-nix-Enable-parallel-unit-tests-for-sandbox.patch
Outdated
Show resolved
Hide resolved
I really would prefer to drop it as I think it offers very little improvement in convenience at the cost of additional maintenance and fragility. We do have update scripts elsewhere in the Nixpkgs source tree, but most of them are for more complex projects. What I mean by this is that for Julia the process is actually fairly simple and standard in relationship to what one would have to do for any package: 1.) Go to download page for the URL 2.) Run |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/get-going-with-julia-on-nixos-in-under-an-hour/12530/4 |
According to the Julia release process [1] there will only ever be two supported release branches at any given time: stable and LTS. Once a new version of either is released, support for the previous release will be dropped upstream. Introducing aliases for these branches allows our users to easily track either depending on their need for stability. [1]: https://julialang.org/blog/2019/08/release-process/#long_term_support
This is potentially controversial, but it is unclear to me whether julia_1 should refer to either the latest stable or LTS release of 1.x. We may want to revisit this when/if we get 2.x, but as it stands the Julia release process [1] makes it clear that there will only ever be a single supported stable and LTS release at any given time which should speak in favour of using the julia-stable and julia-lts aliases instead. [1]: https://julialang.org/blog/2019/08/release-process/#long_term_support
Erroneously disabled by 3ae5e6c as it mistook Julia for using CMake (it is used by some of the vendored dependencies).
Reverts the non-syntax part of 1e4c335.
As far as I can tell this patch is redundant as all pre-compiled code generated at build time is baked into the Julia system image and will thus never get invalidated: Note that for both julia_10 and julia_15 there are no `.ji` files produced in the derivations.
Fix merged upstream and backported to Julia 1.0.5:
JuliaLang/julia#31443
Makes the top-level directory organisation easier with an increasing number of patches.
Provides very little comfort compared what is outlined in the manual [1], only supports a single version, and would probably be better to implement as a general Nixpkg tool. [1]: https://nixos.org/manual/nixpkgs/stable/#sec-source-hashes
No change in semantics, but makes it consistent with the expressions. This is a fairly pedantic thing to do to be honest.
Provides a few hopefully helpful pointers that would not work well as inline comments in the expressions themselves. Most likely the README will need to be expanded upon over time to cover how we handle the Julia release process, but I hope this is a good starting point.
|
I think a patched upstream binary release and non-perfect source build can coexist and both add value. Do I understand correctly that LLVM is not broken out right now? |
|
On Wed 28 Apr 2021, Michael Raskin wrote:
I think a patched upstream binary release and non-perfect source build can coexist and both add value.
Super! I will look into cleaning up the expressions – possibly tomorrow (Thursday) – to file a separate PR.
Do I understand correctly that LLVM is not broken out right now?
That is correct, both for v1.0.x and v1.5.x.
|
|
I wonder if it is a good idea to have a summary in the description about the build status of the latest tried iteration… I think searching for |
|
I've just been experimenting with https://git.sr.ht/~ninjin/nixpkgs/log/julia-bin for Julia 1.6 and must say that it is a massive improvement over a
So, I'm definitely a proponent of adding the bin derivation for Julia 1.6 to nixpkgs. Also because Julia developers are advised to only support the latest Julia release which is 1.6 and missing from nixpkgs currently. EDIT: Nevermind. I already found an issue on Linux. With Gadfly: |
|
I tried running the test suite on julia_15 on AWS, and it has the same BLAS errors as always. But that's no fault of this PR and I think we've made @ninjin suffer enough here already. I propose we
|
|
Indeed, the current state seems to be mostly a cleanup more than too much reorganisation. @samuela thanks for the summary, I was not sure I am not missing something among all these test reports and wanted someone to confirm that there is nothing obvious hanging. |
|
@ninjin Any progress on the 1.6 binary version? The branch-off for NixOS 21.05 is happening May 21st, so it would be great to have the new 1.6 merged in by then! |
|
On Sun 09 May 2021, Samuel Ainsworth wrote:
@ninjin Any progress on the 1.6 binary version? The branch-off for NixOS 21.05 is happening May 21st, so it would be great to have the new 1.6 merged in by then!
Oof… No pressure, eh? '^^ Thank you for the poke, I will increase the priority over the next few days as it really is not that much work.
|
Maybe you could ask for access to https://github.com/nix-community/aarch64-build-box to debug aarch64 builds. |
Sorry, only meant for it to be a friendly ping! I think your 1.6 solution is exciting, but no pressure intended. We're all volunteers here. |
ghost
deleted the
|
Firstly, thank you for the merge. I was too bogged down with work at the time and did not find the time to say thank you, only now did I remember to do so.
On Mon 10 May 2021, Samuel Ainsworth wrote:
> Oof… No pressure, eh? '^^ Thank you for the poke, I will increase the priority over the next few days as it really is not that much work.
Sorry, only meant for it to be a friendly ping! I think your 1.6 solution is exciting, but no pressure intended. We're all volunteers here.
@samuela: Haha, no worries. I took it as nothing but a friendly thing. =)
@symphorien: Thank you for the pointer, I had no clue that it was even an option and I have applied for access.
As for the build status, test failures, etc. I am tempted to create an overreaching organising issue to try to summarise and organise our efforts. @7c6f434c: My guess is that you are the most experienced among us in this discussion, do you think that opening such an issue would be okay? Otherwise I guess I will try to open several smaller issues for each failure and feature.
|
|
@ninjin I think project-tracking issues are common in nixpkgs. |
|
It is probably even better to have a single issue «not all Julia versions can be built with all tests passing everywhere» so that if that matches an unrelated search it can be skipped as a single entry… (oh wait GitHub search is effectively broken for Nixpkgs, but if it ever gets fixed) |
Motivation for this change
The Julia ecosystem needed – in my opinion – a bit of work in that expressions had large redundant chunks, v1.0.x was broken due to CVEs, etc.
Things done
I am not sure how the Nixpkgs community feels about very large PRs, but it was all interconnected and I have done my utmost to break everything down into small incremental commits. Comments are more than welcome and here is a high-level summary reflecting the individual commits:
julia_13as it is not supported upstreamjulia_1as it was unclear if it referred to v1.0.x or v1.y.z.julia-ltsandjulia-stableto replace the “numbered” aliases as these are the only two releases that will ever be supported upstream. This saves us from deprecating multiple aliases each year and simplifies our expressions somewhat.Comments are more than welcome. I had also intended to include an expression for v1.6.0, but the one I have needs more work before I consider it PR-worthy. If it takes some time to get this PR merged I may add it as I will be working on it in the meantime.
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)