-
Notifications
You must be signed in to change notification settings - Fork 488
Use RetroCap.jl to add monotonic "caps" (upper-bounded compat entries) to all packages #11114
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
Conversation
@maleadt Can you run PkgEval on the |
A bit too many just to be able to merge but not too crazy. Most of those resolver errors would be good to investigate. |
@DilumAluthge, does RetroCap.jl respect yanked releases (ignores them)? |
No. We should fix that, I.e. tell RetroCap to ignore yanked releases. |
Some of these failures are due to the fact that in 1.3.1, if something is "fixed" in the project manifest, then it doesn't get re-resolved when we resolve test dependencies. That is, we first resolve the project dependencies, and we fix those, and then we resolve test dependencies with no possibility of changing the project dependencies. I believe this has been fixed in either 1.4 or 1.5, in which we now have the ability to re-resolve project dependencies when we resolve test-dependencies. |
I think the tiered resolver will be available in 1.4. Let's put this on hold until 1.4 is released, and then we can return to it. |
So, I think most of the "unsatisfiable" errors should be fixed on 1.4 with the tiered resolver. As for the others, here is my stream of consciousness: One of these is really puzzling. The latest release of The master pkgeval job correctly tested Why did the two jobs test such drastically different versions of the same package? Actually, that's not the only package I saw this on. Take But retrocap branch ran it on 0.3.0: A whole bunch of these are a
Some of these seem to be network errors during the build step of a package. For example:
Some of these are artifact installation errors. For example:
It would be nice if we could automatically retry the ones that are artifact installation errors. There are some others that seem like compat problems on the master branch of General.
|
@maleadt Do you think you could do another PkgEval comparison, but this time, use the latest Julia-1.4 release candidate? |
9f34f61
to
905542c
Compare
|
Well, that’s quite nice! @KristofferC how do you feel about merging this once 1.4 is released and 1.4 binaries are available for download? |
If so, what do you guys say we merge this PR? |
I don't really understand how 1.4 could fix those cases like
Also, I am surprised not a single package started failing or passing. Usually, there are a few packages that are noisy and pass / fail a bit randomly. |
For example, I tried to add
Seems like that should have happened in PkgEval as well? Note that the reason for this failure is exactly due to the reason that we want to fix. On standard General it uses an old version of WorldOceanAtlasTools which does not have compat on Unitful. |
A lot of the failures are from RecipesBase that tagged a new breaking release because it caused the highly coupled Plots.jl to fail........ |
I was surprised as well, but made sure to compare this PR against two commits prior so that I could make sure one package got legitimately upgraded. Anyway, let me rerun just to be sure. @DilumAluthge, can you rebase once more to make sure it includes a fixed RecipesBase (or instead rebase right before that breaking release?). |
Yep I'll do it when I get back to a computer. |
905542c
to
4f1c5fc
Compare
Alright, I ran RetroCap on the latest General master. |
OK, so this was a false positive indeed, sorry for that. New results up here:
However, only 3 unsatisfiables. |
I think something weird is happening with the second run, almost all failures seem to be related do some download problems: Cloning failure:
Why is it cloning? Means the tarball download failed:
Other download failures:
etc. |
Maybe GitHub rate limited me? I can try rerunning. But aren't failures due to this change mostly going to manifest as failures to install |
401 does suggest rate limiting. The reason it doesn’t manifest at install time is that the build step doesn’t actually throw errors. So we don’t realize that the build failed until we try to import. |
Should I rerun RetroCap, and then we can do another PkgEval run? |
Just as an example, if I apply the following patch to the From 4c6496dcb96065fd26777b22ed484a7f27db3d42 Mon Sep 17 00:00:00 2001
From: Dilum Aluthge <[email protected]>
Date: Wed, 2 Sep 2020 06:27:23 -0400
Subject: [PATCH] Fix ODEInterfaceDiffEq
---
D/DiffEqBiological/Compat.toml | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/D/DiffEqBiological/Compat.toml b/D/DiffEqBiological/Compat.toml
index f8ee030c24..c0bc7e5b8b 100644
--- a/D/DiffEqBiological/Compat.toml
+++ b/D/DiffEqBiological/Compat.toml
@@ -34,7 +34,6 @@ Parameters = "0.10.3 - 0.12"
DiffEqBase = "5.1-5"
["3.8-3"]
-DataStructures = "0.0.0 - 0.17"
MacroTools = "0.0.0 - 0.5"
Parameters = "0.0.0 - 0.12"
Reexport = "0.0.0 - 0.2"
@@ -46,6 +45,9 @@ DiffEqBase = "5"
["3.8-4"]
DiffEqJump = "6"
+["3.8-4.0.1"]
+DataStructures = "0.0.0 - 0.17"
+
["3.8.1-3.9.0"]
Compat = "0.0.0 - 3"
@@ -62,7 +64,6 @@ Latexify = ["0.11", "2"]
["4.0"]
Compat = "0.0.0 - 3"
-DataStructures = "0.0.0 - 0.17"
DynamicPolynomials = "0.0.0 - 0.3"
HomotopyContinuation = "0.0.0 - 1"
MacroTools = "0.0.0 - 0.5"
@@ -73,6 +74,7 @@ SymEngine = "0.0.0 - 0.7"
julia = "1"
["4.0.2-4.0"]
+DataStructures = "0.0.0 - 0.18"
Latexify = "0.11.0 - 0.12"
["4.1"]
--
2.28.0 |
To explain a little bit on the strategy for fixing these: I look at the PkgEval log for the pre-cap registry (i.e. the |
@KristofferC you cool with me updating this branch and merging it? |
Should be ok, but it would indeed be good to follow up with the ones that started failing. |
…) to all packages
b5080d2
to
03ee42b
Compare
Yep. There are 12 unsatisfiables, which I will fix as soon as I merge this.
|
I just merged #21035, which fixes those 12 unsatisfiables. Hopefully there will only be at most a handful of additional error reports. |
xref JuliaRegistries/General#11114 By switching to depending on just ImageCore we make the dependencies easier to maintain (e.g., JuliaIO/QuartzImageIO.jl#66).
xref JuliaRegistries/General#11114 By switching to depending on just ImageCore we make the dependencies easier to maintain (e.g., JuliaIO/QuartzImageIO.jl#66).
xref JuliaRegistries/General#11114 By switching to depending on just ImageCore we make the dependencies easier to maintain (e.g., JuliaIO/QuartzImageIO.jl#66).
Thanks for doing this, @DilumAluthge! I do think this will make a lot of things more robust. We fixed a couple of compat issues for Julia 1.0 in JuliaIO/ImageMagick.jl#186. (Reference example: https://github.com/JuliaImages/ImageCore.jl/pull/140/checks?check_run_id=1090119276). Note that this was a conflict between the package (which loaded fine) and a test dependency (where version checks are performed after Julia commits to a particular set of versions for the package itself). |
Yep, on more recent versions of Julia, the tiered resolver can work around this, but obviously that's not available on Julia 1.0. Let me know if you also want to add those compat fixes into the General registry itself! |
Let's see how far we get with the one-package-at-a-time approach. Releasing an updated version of ImageMagick was enough to solve the issues for ImageCore---I suspect that most of the issues will arise from packages that jumped to a minimum of 1.3 to exploit the lovely artifacts system, and JuliaImages doesn't really have that many packages with binary dependencies. |
269: Do not automerge packages that have `[compat]` entries of the form `PkgA = "0"`, etc. r=DilumAluthge a=DilumAluthge I have realized that some people have `[compat]` entries of the form: ```toml [compat] PkgA = "0" ``` This `[compat]` entry includes an infinite number of breaking releases, which defeats the purpose of having upper-bounded `[compat]` entries. **The good news**: [RetroCap.jl](https://github.com/bcbi/RetroCap.jl) already recognizes that these `[compat]` entries are not true upper-bounded `[compat]` entries. Therefore, when we did the recent retrocapping in JuliaRegistries/General#11114, RetroCap.jl fixed these `[compat]` entries, replacing entries like this: ```toml [compat] PkgA = "0" ``` With something like this: ```toml [compat] PkgA = "0.0.0 - 0.3.7" ``` Where `0.3.7` would be the latest registered version of `PkgA`. **The bad news**: AutoMerge currently considers `PkgA = "0"` to be an upper-bounded `[compat]` entry. Therefore, AutoMerge is currently merging packages that have these `[compat]` entries, which is thus actively undoing the work we did in JuliaRegistries/General#11114. This pull request patches AutoMerge so that it no longer considers `PkgA = "0"` to be an upper-bounded `[compat]` entry. Co-authored-by: Dilum Aluthge <[email protected]>
This pull request uses RetroCap.jl to add monotonic upper-bounded compat entries to all packages.
cc: @KristofferC
The script to reproduce this PR: