-
-
Notifications
You must be signed in to change notification settings - Fork 285
Add the compat_modifier kwarg to Pkg.test and remove the force_latest_compatible_version and allow_earlier_backwards_compatible_versions kwargs
#2498
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
|
@KristofferC @00vareladavid I had to add back the code for converting
The problem is this: I'm happy to remove the "convert a |
9726339 to
b94d1e3
Compare
|
@KristofferC I have removed the |
b27b501 to
dbcdb26
Compare
|
In the real world o don't think this is a practical solution to testing oldest versions nor to testing random ones. Getting either of those out of that would in practice be very hard AFAICT Getting the oldest version of the packages is not quite the same as setting the version spec for each package to be only the oldest permitted. More generally would want to test some point(/s) on the Pareto frontier of oldness. Similarity you can't get random compatible versions by independently randomly restricting version specs. But maybe I misunderstand. It seems like one can't do this without running the resolver with an alternative objective. |
|
Instead of passing the versions one at a time to |
If we pass a vector (one element for each direct dependency) to the |
|
Yes, and to do so the user would have to reimplement the resolver. And if they are doing that,then we might want to step back and ask if this is the API we want. |
|
Fair enough. In that case, I guess the main benefit of this PR is the CompatHelper use case, i.e. force the latest compatible versions of all direct dependencies. So, I'll keep the current setup of passing one dependency at a time to However, I'll add |
8155e27 to
80410e3
Compare
…atest_compatible_version` and `allow_earlier_backwards_compatible_versions` kwargs
80410e3 to
9bf730c
Compare
|
Okay, now we also pass
Some examples:
|
I think this is an important point made by @oxinabox in that the desired end result (affect the resolver output in various ways) and the implementation (restricting compat entries) are not really in "harmony". The way you would get the lowest compatible version of everything is to tell the resolver to "like" low versions over high versions which is quite different from changing compat. Because changing compat is very hard and you might very well restrict into something that has no solution.
This just looks like a FWIW, I personally think this exposes too much API for the demand there is on the functionality it provides and to use it properly you need to use a lot of Pkg internals. If there is a need to override compat for CompatHelper then I think it is ok to put something very specific (and non-public) that handles that case but starting to generalize that too much might not be a good idea. |
Not quite. If I understand correctly, you can only |
That's fair. We already have the following kwargs for the CompatHelper case:
So we can just keep those two kwargs, and close this PR. |
I have implemented this suggestion in #2541. |
|
Closing in favor of #2541 |
Summary
This pull request:
compat_modifier::Union{Function, Nothing}keyword argument to thePkg.testfunctionforce_latest_compatible_versionkeyword argument from thePkg.testfunctionallow_earlier_backwards_compatible_versionskeyword argument from thePkg.testfunctionThe default value of
compat_modifieriscompat_modifier = nothing.If
compat_modifieris aFunction, it is called with exactly one positional argumentxwhich has the following properties:x.nameis the name of the dependencyx.uuidis the UUID of the dependencyx.has_compat::Boolis:[compat]entry:true[compat]entry:falsex.versions::Set{VersionNumber}is[compat]entry: the set of all registered versions of the dependency that are compatible with the original[compat]entry for the dependency[compat]entry: the set of all registered versions of the dependencyIf
compat_modifieris aFunction, it must return exactly one return value, which must be aUnion{Pkg.Versions.VersionSpec, Nothing}.See also: #2439
Description
If
compat_modifierisnothing, then we do:Pkg.testsets up the temporary sandbox project.Pkg.testruns the resolver inside the temporary sandbox project.Pkg.testruns thetests/runtests.jlfile inside the temporary sandbox project.If
compat_modifieris aFunction, then we do:Pkg.testsets up the temporary sandbox project.DepName, we do the following:has_compattotrueif the project has a[compat]entry forDepName, andfalseotherwise.DepName.has_compat:has_compatistrue: We get the original[compat]entry forDepName, and then for each registered version number ofDepName, we determine whether or not the version number is inside the original[compat]entry forDepName. At the end of this step, we have theSet{VersionNumber}containing all of the registered version numbers ofDepNamethat are compatible with the original[compat]entry forDepName. We setversionsto be this set.has_compatisfalse: We setversionsto be theSet{VersionNumber}containing all registered version numbers ofDepName.compat_modifierfunction as follows:spec = compat_modifier((; name, uuid, has_compat, versions))specis aUnion{Pkg.Versions.VersionSpec, Nothing}and throw an error if it is not.spec:specis aNothing, we do nothing.specis aPkg.Versions.VersionSpec, we do the following, based on the value ofhas_compat:has_compatistrue: We delete the original[compat]entry for DepName and set the new[compat]entry forDepNameto be the intersection ofspecand the original[compat]entry forDepName.has_compatisfalse: We delete the original[compat]entry for DepName and set the new[compat]entry forDepNameto bespec.Pkg.testruns the resolver inside the temporary sandbox project.Pkg.testruns thetests/runtests.jlfile inside the temporary sandbox project.Example Usage
Suppose that your package has a
[compat]entry that looks like this:And suppose that the
Foo.jlfunction has the following versions in the registry:0.1.00.1.10.2.00.2.10.3.00.3.10.4.00.4.10.5.00.5.1Observe that only the following versions of
Foo.jlare compatible with your package's[compat]entry forFoo:0.2.00.2.10.3.00.3.10.4.00.4.1You can control which version of
Foo.jlis used in your package's test suite by passing a function to thecompat_modifierkeyword argument. The function will be applied to all direct dependencies. (It will not be applied to any indirect dependencies.) Here are some examples:compat_modifier0.4.1x -> Pkg.Versions.semver_spec("=$(maximum(x.versions))")0.4.xx -> Pkg.Versions.semver_spec("^$(Pkg.Operations.earliest_backwards_compatible(maximum(x.versions)))")0.2.0x -> Pkg.Versions.semver_spec("=$(minimum(x.versions))")0.2.xx -> Pkg.Versions.semver_spec("^$(minimum(x.versions))")x -> Pkg.Versions.semver_spec("=$(StatsBase.sample(collect(x.versions)))")If you only want to apply this to direct dependencies that have
[compat]entries (i.e. you want to skip any direct dependency that does not have a[compat]entry), you can do the following instead:compat_modifier0.4.1Pkg.Operations.force_latest_compatible_exact0.4.xPkg.Operations.force_latest_compatible_family0.2.0x -> x.has_compat ? Pkg.Versions.semver_spec("=$(minimum(x.versions))") : nothing0.2.xx -> x.has_compat ? Pkg.Versions.semver_spec("^$(minimum(x.versions))") : nothing)x -> x.has_compat ? Pkg.Versions.semver_spec("=$(StatsBase.sample(collect(x.versions)))") : nothingFor CompatHelper/Dependabot PRs, we would use the bolded row, i.e. the second row of the second table above.
Motivation
In #2439, we added the ability to do the following when running your package tests:
The motivation for #2439 was to meet the CompatHelper/Dependabot use case.
However, users may have other restrictions that they want to place on their direct dependencies when running their package tests. Some examples include:
Initially, I considered adding this functionality directly to Pkg. However, this would require adding a lot of extra keyword arguments to the
Pkg.testfunction.Instead, I figured it would be better to have only a single
compat_modifierkeyword argument to thePkg.testfunction. Then, users can write a function that does exactly what they want, and pass this function to thecompat_modifierkwarg.