Don't make PRs to packages that have a dependency that upper bounds the package CompatHelper bumps about compathelper.jl HOT 27 CLOSED

Here's a screenshot of my inbox:

😆

from compathelper.jl.

So it seems like there are two orthogonal issues here:

1. The original issue: "Don't make PRs to packages that have a dependency that upper bounds the package CompatHelper bumps". Correct me if I'm wrong @DilumAluthge, but CompatHelper doesn't use the resolver when updating compat entries and instead pulls the latest versions directly from the registries. Because of this, the new compat entry may be invalid in that the resolver would never let you instantiate an environment with the new upper bound. The approach I took here solves this issue.
2. Making sure the tests are run with the updated compat entries as @DilumAluthge explained above. It seems like this issue stems from the fact that there is a "test-time" environment which is a result of resolving a package's dependencies and test-specific dependencies. While this could be handled in CompatHelper (possibly using one of the approaches in #160 (comment)), I think this is probably best handled by Pkg. Given the discussion in JuliaLang/Pkg.jl#1233, it sounds like this may be a non-issue in the future if one uses the new test/{Project,Manifest}.toml scheme since it would remove any test-time resolving. I can confirm that using Run.jl to emulate this behavior works well (Run.test() is basically just Pkg.activate("test"); Pkg.instantiate(); include("test/runtests.jl")).

Does that sound correct?

from compathelper.jl.

This is now fixed by the combination of the following:

1. JuliaLang/Pkg.jl#2439
2. julia-actions/julia-runtest#20
3. https://github.com/julia-actions/julia-runtest/releases/tag/v1.6.0

If you use GitHub Actions for CI, and you use the julia-runtest action, and you use v1 of the julia-runtest action, you will get this fix automatically on CI jobs that run on Julia nightly.

For example, if your GitHub Actions workflow file uses this line to run the tests...

- uses: julia-actions/julia-runtest@v1

...then you will get this fix automatically on CI jobs that run on Julia nightly.

If you want to double-check that you are getting the fix, you can check the logs. If you have the fix, you will see the following message in the logs:

[ Info: This is a Dependabot/CompatHelper job, so force_latest_compatible_version has been set to true

If you use a different CI provider, or if you use GitHub Actions but you do not use the julia-runtest action, then you will not get this fix automatically.

from compathelper.jl.

What about:

1. Create a temp copy of the project.
2. Delete all [compat] entries except for julia
3. Pkg.update()
4. Read back the versions in the updated manifest.

That should get you the latest possible versions of all packages while respecting the upper bounds of dependencies, right?

from compathelper.jl.

I'm happy with exploring different non-default modes for the resolver. The logic would be around here:

https://github.com/JuliaLang/Pkg.jl/blob/a9d85e7b8a22b1f0fe8cdbdfa4fe9d5f60ee5ea0/src/Operations.jl#L425-L431

from compathelper.jl.

See my example above. Even if the resolver gives you 0.8 of a package, if your compat entry is this:

[compat]
Foo = "0.7, 0.8"

then your test suite might actually use 0.7 instead of 0.8.

So even if your test suite passes, it doesn't mean that your package is compatible with 0.8.

from compathelper.jl.

I agree. I’m just not sure how to implement this.

from compathelper.jl.

It's like what @colinxs already said but, instead of adding this feature in CompatHelper.jl, can we have a mode in Pkg.add (and Pkg.test) to ignore indirect compat bounds if they prevent updating direct dependencies?

from compathelper.jl.

It is worth figuring out how to reduce these notifications.

Certainly one solution would be to have a single pull request for all suggested changes, instead of a different pull request for each suggested change: #126

from compathelper.jl.

That would be a great change. But waiting to submit the PR until the resolver will actually use the version whose bounds are loosening is also useful. The main reason these are hanging around in my inbox is that these are the ones I couldn't deal with immediately because I needed to work my way up the hierarchy of dependencies.

from compathelper.jl.

I wanted to share a port of CompatHelper that I've been using internally which addresses this issue. Rather than deleting the existing compat entries like I described above, it replaces them with inequality specifiers. So a compat entry that looked like:

Foo = "0.2, 0.5, 0.6"

would get replaced with:

Foo = ">= 0.2"

before calling Pkg.API.up to find an updated set of packages. A single PR is then made with the updated compat entries, which addresses @timholy's concern above.

Some other differences that I can think of:

1. PR's are formatted with Markdown because why not. See here for an example.
2. A single compat branch is used, with new changes overwriting an existing PR.
3. A Personal Access Token is used rather than GITHUB_TOKEN so that GitHub Actions workflows are triggered.

I originally intended on split this up into PRs to CompatHelper, but it doesn't share much code since I'm relying on Pkg to do all the heavy lifting. If folks find this approach useful perhaps we can find a way to incorporate the changes?

from compathelper.jl.

I don’t think that is sufficient.

In your example, your bot makes a PR that replaces this line:

Distributions = "0.22"

With this line:

Distributions = "0.22, 0.23"

There is no guarantee that the test suite will use Distributions 0.23. This is the same problem that CompatHelper has, so I’m not sure how this approach solves it.

(Keep in mind that at the beginning of the test suite, when the test dependencies are resolved, the resolver is allowed to also change the versions of the direct dependencies.)

The core issue here is that when you run the test suite, you need to enforce that Distributions 0.23 is used.

from compathelper.jl.

Just to elaborate: even if Pkg.up gives you Distributions 0.23, that doesn’t mean that the test suite will give you Distributions 0.23. When you run Pkg.test, the resolver is allowed to downgrade you to Distributions 0.22 if it wants to.

from compathelper.jl.

I don’t mean to be negative! It’s a good idea, I just don’t think it solves the core problem.

I really like the markdown output, the single branch for all changes, and the option to use the PAT. It would be great to have those features in CompatHelper, and I’d be happy to accept PRs for those features.

from compathelper.jl.

You're not being negative :)

Perhaps I'm misinterpreting, but from the Pkg.test docs:

The tests are run by generating a temporary environment with only pkg and its (recursive) dependencies in it. If a manifest exists, the versions in that manifest are used...

So after the call to Pkg.up we get Distributions 0.23 from the current Context, and if you're checking in the Manifest.toml corresponding to the Context then the tests will use Distributions 0.23.

Of course if you don't check in a Manifest.toml, then all bets are off.

Based on JuliaLang/Pkg.jl#1233 and other discussions, it sounds like the goal is to have Pkg.test() be equivalent to Pkg.activate("test"); Pkg.instantiate(); include("test/runtests.jl"), which would address this. I've been using @tkf 's Run.jl until this happens.

from compathelper.jl.

So, there are two problems here.

The first problem is that many people (myself included) do not check in the /Manifest.toml file into source control. There are many reasons for this. For example, one reason is that manifest files are not necessary compatible across different versions of Julia.

Therefore, we cannot have a solution that requires people to check /Manifest.toml into source control.

The second problem is that those docs are out-of-date and are no longer correct.

I have set up an example package to demonstrate:

Look at the /Project.toml file:

name = "Foo"
uuid = "8f191a96-946f-40fe-b50f-e281de84ebcd"
authors = ["Dilum Aluthge <[email protected]>"]
version = "0.1.0"

[deps]
StatsBase = "2913bbd2-ae8a-5f71-8c99-4fb6c76f3a91"

[compat]
DataFrames = "0.18"
StatsBase = "0.32, 0.33"

[extras]
DataFrames = "a93c6f00-e57d-5684-b7b6-d8193f3e46c0"
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"

[targets]
test = ["DataFrames", "Test"]

If you clone this package, enter it with julia --project, and run import Pkg; Pkg.update(), you will see that it gives you StatsBase v0.33.0.

And if you look at the /Manifest.toml file, you will see that indeed, it has StatsBase v0.33.0. Here is the relevant portion of /Manifest.toml:

[[StatsBase]]
deps = ["DataAPI", "DataStructures", "LinearAlgebra", "Missings", "Printf", "Random", "SortingAlgorithms", "SparseArrays", "Statistics"]
git-tree-sha1 = "a6102b1f364befdb05746f386b67c6b7e3262c45"
uuid = "2913bbd2-ae8a-5f71-8c99-4fb6c76f3a91"
version = "0.33.0"

Now, open a new Bash session and run the following lines:

export JULIA_DEPOT_PATH="$HOME/.julia.temp"
rm -rf $HOME/.julia.temp
export JULIA_PROJECT="@."
rm -rf Foo.jl
git clone https://github.com/aluthge-test/Foo.jl.git Foo.jl
cd Foo.jl
julia

You are now in a Julia REPL. Run the following lines in Julia:

import Pkg
Pkg.build(; verbose = true)
Pkg.status(; mode = Pkg.PKGMODE_PROJECT)
Pkg.status(; mode = Pkg.PKGMODE_MANIFEST)
Pkg.test()

This Bash script and Julia script replicate exactly what the default Travis script for Julia does.

Here are the results:

$ export JULIA_DEPOT_PATH="$HOME/.julia.temp"
$ rm -rf $HOME/.julia.temp
$ export JULIA_PROJECT="@."
$ rm -rf Foo.jl
$ git clone https://github.com/aluthge-test/Foo.jl.git Foo.jl
Cloning into 'Foo.jl'...
remote: Enumerating objects: 8, done.
remote: Counting objects: 100% (8/8), done.
remote: Compressing objects: 100% (6/6), done.
remote: Total 8 (delta 0), reused 8 (delta 0), pack-reused 0
Unpacking objects: 100% (8/8), 2.49 KiB | 425.00 KiB/s, done.
$ cd Foo.jl
$ julia
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.5.0-DEV.573 (2020-04-06)
 _/ |\__'_|_|_|\__'_|  |  Commit e25b3574d6 (3 days old master)
|__/                   |

julia> import Pkg

julia> Pkg.build(; verbose = true)
    Cloning default registries into `~/.julia.temp`
######################################################################## 100.0%
      Added registry `General` to `~/.julia.temp/registries/General`
  Installed SortingAlgorithms ── v0.3.1
  Installed DataAPI ──────────── v1.1.0
  Installed Missings ─────────── v0.4.3
  Installed OrderedCollections ─ v1.1.0
  Installed DataStructures ───── v0.17.11
  Installed StatsBase ────────── v0.33.0

julia> Pkg.status(; mode = Pkg.PKGMODE_PROJECT)
Project Foo v0.1.0
     Status `~/Downloads/Foo.jl/Project.toml`
   2913bbd2 StatsBase v0.33.0

julia> Pkg.status(; mode = Pkg.PKGMODE_MANIFEST)
Project Foo v0.1.0
     Status `~/Downloads/Foo.jl/Manifest.toml`
   9a962f9c DataAPI v1.1.0
   864edb3b DataStructures v0.17.11
   e1d29d7a Missings v0.4.3
   bac558e1 OrderedCollections v1.1.0
   a2af1166 SortingAlgorithms v0.3.1
   2913bbd2 StatsBase v0.33.0
   2a0f44e3 Base64
   8ba89e20 Distributed
   b77e0a4c InteractiveUtils
   8f399da3 Libdl
   37e2e46d LinearAlgebra
   56ddb016 Logging
   d6f4376e Markdown
   de0858da Printf
   9a3f8284 Random
   9e88b42a Serialization
   6462fe0b Sockets
   2f01184e SparseArrays
   10745b16 Statistics
   8dfed614 Test
   4ec0a83e Unicode

julia> Pkg.test()
    Testing Foo
┌ Error: Pkg.Resolve.ResolverError("Unsatisfiable requirements detected for package StatsBase [2913bbd2]:\n StatsBase [2913bbd2] log:\n ├─possible versions are: [0.24.0, 0.25.0, 0.26.0, 0.27.0, 0.28.0-0.28.1, 0.29.0, 0.30.0, 0.31.0, 0.32.0-0.32.2, 0.33.0] or uninstalled\n ├─restricted to versions 0.32-0.33 by Foo [8f191a96], leaving only versions [0.32.0-0.32.2, 0.33.0]\n │ └─Foo [8f191a96] log:\n │   ├─possible versions are: 0.1.0 or uninstalled\n │   └─Foo [8f191a96] is fixed to version 0.1.0\n ├─restricted to versions 0.33.0 by an explicit requirement, leaving only versions 0.33.0\n └─restricted by compatibility requirements with DataFrames [a93c6f00] to versions: [0.24.0, 0.25.0, 0.26.0, 0.27.0, 0.28.0-0.28.1, 0.29.0, 0.30.0, 0.31.0, 0.32.0-0.32.2] — no versions left\n   └─DataFrames [a93c6f00] log:\n     ├─possible versions are: [0.11.7, 0.12.0, 0.13.0-0.13.1, 0.14.0-0.14.1, 0.15.0-0.15.2, 0.16.0, 0.17.0-0.17.1, 0.18.0-0.18.4, 0.19.0-0.19.4, 0.20.0-0.20.2] or uninstalled\n     └─restricted to versions 0.18 by an explicit requirement, leaving only versions 0.18.0-0.18.4", nothing)
└ @ Pkg.Operations ~/dev/repos/aluthge-forks/julia/usr/share/julia/stdlib/v1.5/Pkg/src/Operations.jl:1385
     Status `/private/var/folders/jy/7hh5zyw95cg2lh66lfpthqq80000gn/T/jl_05x15G/Project.toml`
   a93c6f00 DataFrames v0.18.4
   8f191a96 Foo v0.1.0 `~/Downloads/Foo.jl`
   2913bbd2 StatsBase v0.32.2
   8dfed614 Test
     Status `/private/var/folders/jy/7hh5zyw95cg2lh66lfpthqq80000gn/T/jl_05x15G/Manifest.toml`
   324d7699 CategoricalArrays v0.7.7
   34da2185 Compat v2.2.0
   9a962f9c DataAPI v1.1.0
   a93c6f00 DataFrames v0.18.4
   864edb3b DataStructures v0.17.11
   e2d170a0 DataValueInterfaces v1.0.0
   8f191a96 Foo v0.1.0 `~/Downloads/Foo.jl`
   82899510 IteratorInterfaceExtensions v1.0.0
   682c06a0 JSON v0.21.0
   e1d29d7a Missings v0.4.3
   bac558e1 OrderedCollections v1.1.0
   69de0a69 Parsers v1.0.1
   2dfb63ee PooledArrays v0.5.3
   189a3867 Reexport v0.2.0
   a2af1166 SortingAlgorithms v0.3.1
   2913bbd2 StatsBase v0.32.2
   3783bdb8 TableTraits v1.0.0
   bd369af6 Tables v0.2.11
   2a0f44e3 Base64
   ade2ca70 Dates
   8bb1440f DelimitedFiles
   8ba89e20 Distributed
   9fa8497b Future
   b77e0a4c InteractiveUtils
   76f85450 LibGit2
   8f399da3 Libdl
   37e2e46d LinearAlgebra
   56ddb016 Logging
   d6f4376e Markdown
   a63ad114 Mmap
   44cfe95a Pkg
   de0858da Printf
   3fa0cd96 REPL
   9a3f8284 Random
   ea8e919c SHA
   9e88b42a Serialization
   1a1011a3 SharedArrays
   6462fe0b Sockets
   2f01184e SparseArrays
   10745b16 Statistics
   8dfed614 Test
   cf7118a7 UUIDs
   4ec0a83e Unicode
    Testing Foo tests passed

As you can see, Pkg.test attempted to resolve the test dependencies, but failed to do so with the original manifest. Therefore, Pkg.test re-resolved everything. As you can see, the test suite ended up using StatsBase v0.32.2.

So, even though the /Manifest.toml file had StatsBase v0.33.0, the test suite ended up using StatsBase v0.32.2.

So, even though your /Project.toml file had this [compat] entry:

StatsBase = "0.32, 0.33"

The test suite ended up using StatsBase v0.32.2. This is the core issue, and it cannot be solved by checking a /Manifest.toml file into source control that has StatsBase v0.33.0.

from compathelper.jl.

I think that you may need to be on a relatively recent Julia master in order to reproduce these results.

Edit: Actually, you should be able to see these results on Julia 1.4.0.

from compathelper.jl.

Thank you for the super detailed example! Indeed, this happens on 1.4.0. I also agree that checking in a Manifest.toml doesn't make sense for everyone, which doesn't handle this anyways.

I'm sure the Pkg devs had a reason for it, but perhaps Pkg.test should fail if it cannot resolve the test dependencies with /Manifest.toml?

Other alternatives I can think of are:

1. Generate new compat entries using the same, merged environment that Pkg.test would see so that the unsatisfied requirement is never generated.
2. Make /test/Project.toml a superset of /Project.toml and use /test/Manifest.toml to generate new compat entries.

Either way, it looks like the problem is more challenging than I originally suspected.

from compathelper.jl.

I don't know Pkg internals very well, but isn't the right algorithm something like:

• parse the Project.toml file
• check the registry for newer versions of each dependency
• call the resolver with the "loosened" bounds and see what version it chooses
• submit PR only if the version it uses is newer than the bound in the Project.toml file, submitting the version that the resolver chose. For example, if the package is bounded at 0.7, and both 0.8 and 0.9 are available but the resolver chose 0.8.1, submit a new bound of 0.8 rather than 0.9.

from compathelper.jl.

Yes, it seem pretty important to be able to replicate the environment without having to run the tests.

from compathelper.jl.

As a temporary stopgap, I hacked together a script that (I believe) solves points (1) and (2) above for those using the Run.jl workflow (i.e. /Project.toml is dev'ed relative to /test/Project.toml). It takes a similar approach as above:

1. Replace existing compat entries with >= specifiers in both /Project.toml and /test/Project.toml/.
2. Run Pkg.up on /test/Manifest.toml.
3. Use the versions in /test/Manifest.toml to generate compat entries for both /Project.toml and /test/Project.toml.
4. Optionally, Pkg.up on /Manifest.toml with the new compat entries.

The end result is that the direct dependencies shared between /Project.toml and /test/Project.toml have the same compat entries and can be tested (since we generated them from /test/Manifest.toml). Here's an example PR and implementation.

Again, it's a ugly hack, but appears to be working.

from compathelper.jl.

See my example above. Even if the resolver gives you 0.8 of a package, if your compat entry is this:

[compat]
Foo = "0.7, 0.8"

then your test suite might actually use 0.7 instead of 0.8.

Wouldn't obtaining the actually used versions be quite easy? For example, calling Pkg.status(; io) after Pkg.instantiate() reports them. As a demo, I have a Project.toml containing DataFrames = "0.21, 0.22". On the generated website, the output shows DataFrames v0.21.8.

from compathelper.jl.

When Pkg.test runs, it is allowed to re-resolve all of the versions.

from compathelper.jl.

In the demo above, all the versions have just freshly been obtained in a CI job. You mean that Pkg.test would still lead to other versions?

from compathelper.jl.

Yeah, it's possible. Because you often have test-only dependencies, which add new constraints to the resolver.

The best long-term solution to this issue is JuliaLang/Pkg.jl#2176

from compathelper.jl.

Fair enough. Thanks

from compathelper.jl.

See #298 to track the deployment of this fix across the most commonly used CI providers.

from compathelper.jl.