-
-
Notifications
You must be signed in to change notification settings - Fork 269
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
quick proof of concept of down
#3802
base: master
Are you sure you want to change the base?
Conversation
Also cc @ChrisRackauckas who has thought about these stuff. |
Hm, shouldn't there be a way to work around that though? Like if we flip the situation and look at I might be missing something, but I think it should be possible for |
Yes, but as I wrote in #1062 (comment), the whole reason to do this is to exactly test that your code works with the given lower bounds. So if you aren't hitting them exactly you are just testing some semi-random version configuration that have low versions "on average". Like in this case, if we install Scratch to 1.1, is the Scratch 1.0 lower bounds correct? You cannot really say anything about that but that is the actual question we wanted to answer by running our tests with I guess you could say that you could in theory minimize the version for one dependency at a time and run the tests for that but that would:
|
Ah okay, sorry I missed that comment, so this is a (nicer implementation of) the idea in https://github.com/julia-actions/julia-downgrade-compat I'll say that I didn't really find downgrade-compat very useful for packages with many deps because
|
Okay, I think this can be extended to do that. I will try... Edit: Thinking about this some more, I think it requires basically a full resolver since there is for example not a unique answer to this question of the "oldest set of compatible packages". |
Yeah, I was imagining the exact same behaviour as |
Afterall, we also think it's okay that when I |
Again, I am not sure that is useful in answering the question of if our lower bounds are correct. If you run the tests with this version of [compat]
A = "0.5.1"
B = "0.7.1" and you get when testing |
I think this is different because when we are normally testing our package we are not explicitly testing that our upper bounds are correct because these are talking about an unknown future which can only be discussed from a "contract" p.o.v like semver. That's why I think that this is not symmetric. For normal testing we are saying:
For
|
That's fair. I guess though I feel that having a If we do go with something as strict and fussy as pinning all the lower bounds, then I think nobody is going to use it unless we get much more informative and actionable error messages than we currently have, but making those error messages informative and actionable seems like a very difficult task. I don't know enough about this to have a strong opinion though. |
I'm not sure if the error messages will be so bad. They will basically always be of the form that one of the direct dependencies limits another of the direct dependencies to something higher than the lower bounds that the package itself sets on it. For example in the Plots example the relevant stuff are:
|
That said, maybe with this implementation it's not so critical that the error messages are good since the feedback loop is so much faster than with the The process here would be fundamentally the same, but in practice not so painful because I just need the
In my experience, I couldn't figure out what was going on. Sure, it's not so bad when direct dependancies start getting angry at eachother, but once you have nested indirect dependancies fighting, I get thoroughly confused. But as said above, maybe it's not so bad with this implementation. |
In https://github.com/julia-actions/julia-downgrade-compat, why does it set
What if you require a bugfix in Foo 1.2.4 (so the |
You can set which mode it uses with a command line arg: https://github.com/julia-actions/julia-downgrade-compat/blob/main/downgrade.jl#L41-L47, but yeah I can't comment on the specific choices made there. |
Just my 2c: I would definitely expect More strict behavior could be enabled by a switch similar to the existing |
Ok you expect this but why would it be useful? |
I guess that the name |
That can't be true in general (unless I'm misunderstanding precisely what you mean). E.g.,
Means I'm compatible with |
Since this is intended to be used only for testing, why not make that explicit in the name? E.g., |
Having a less fussy I definitely see the value of an |
Where is the asymmetry you're pointing to? There's only a finite number of versions of any package in the general registry, so you can't end up with Kristoffer is right in that there's an asymmetry induced by the fact that new features can only appear in one direction, so it's good to have a function that demands the absolute oldest version set, whereas it makes less sense to demand the absolute newest version set, but even then I have some doubts. You could say that it's important that you get the absolute newest version set if you want to test for potentially breaking changes in your dependencies. But we're okay with testing on just a reasonably new set of compatible packages, so I'd also be okay with testing on a reasonably old set. |
I disagree, you would have failed to verify your lower bounds so even if your tests pass your lower bounds can still be buggy and give runtime errors for users (which is what you wanted to avoid in the first place). |
Yes, but perfect can be the enemy of good. There are also other positions in the compat interval not tested. Testing (potential one of multiple) lowest possible version that can be achieved in practice -- that a user might actually see, |
I understand the argument that ideally, we should test every point of the "Pareto front" of resolvable lower bounds. But realistically speaking, this front can be very large and very expensive to test. As far as I can tell, From the perspective of a package developer with multiple dependencies (and possibly hundreds of indirect dependencies), this means that I therefore have to agree with @oxinabox on "perfect can be the enemy of good".
|
That is not needed, you just want to test your lower bounds here and with the assumption that packages follow semver you are good to go (unless you try to support multiple breaking releases).
It works, you just have to bump the lower bounds so they agree with your dependencies so you can actually run your tests with the lower bounds you claim to support. |
In theory you are right, but in practice, this manual bisection can take hours – I've tried and given up before. |
It could maybe be automated somehow? |
Not just that: a new version of a dependency can restrict versions of its dependencies further. So, a test run that relies on the lowest version from direct compats being instantiable can become failing at any time whenever a dependency is updated. |
Very quick implementation of #1062 (comment), (with some debug logging in it to show what happens), specifically:
An example of its usage:
Trying this on Plots.jl we get resolver errors (as @oxinabox predicted) due to Scratch.jl being lower bounded to 1.0.0 while one of our dependencies (RelocatableFolders has a lower bound of 1.1):
cc @carlobaldassi, @oxinabox