-
Notifications
You must be signed in to change notification settings - Fork 80
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
Suggestion: Include compiler ranges in package metadata #255
Comments
I think we considered it in the past - the big issue with allowing users to specify compatibility ranges is that they are often incorrect in two ways:
To sum up: we wouldn't be able to trust such bounds, which might be worse than having them in the first place! I think a more useful solution for this issue is something like the Hackage Matrix Builder, where all the packages that are uploaded to Hackage are built with various versions of the compiler to figure out the compatible versions (here's e.g. the build matrix for the |
I agree that compatibility ranges are likely to be incorrect for the reasons you've specified, and I also think they're still useful to include somewhere that tools like Pursuit would be able to access them. So long as it's possible for Pursuit to access the I agree that something like the Hackage Matrix Builder is a good way to determine actual compatibility bounds for a package upon publication. Would a process like this make sense? Package Update When a package is added or updated, we test it across compiler versions:
Compiler Update When a new compiler version is released, we walk through all packages and check if they compile with the new version. (We could narrow this by only choosing packages that already compiled with at least one version in the compiler series, ie. when 0.14.3 comes out we only re-check packages that compiled with at least 0.14.0, 0.14.1, or 0.14.2).
|
I've updated the name of this issue to reflect storing this information in the package metadata instead of in the purs.json file. |
This assumes that a library which compiles on something like 0.14.4 will also compile on 0.14.0, which isn't always the case. For example, nameless instances were introduced in a "patch" release, though since it's still 0.x versioning, it's dubious to call it a patch release. |
Alternately, we could compile with all compiler versions from a certain start version up to now; it’s a lot of compilation, but over not an enormous number of packages and so perhaps it’s ok. |
I'd like to propose a specific solution for the build matrix. This assumes we have access to all compilers from the earliest library releases that have been registered (say, Package Build Matrix Process If we're doing this in bulk, then first topologically sort all packages by their dependencies such that no package is processed until its dependencies have been processed, then do the rest of this one-by-one. Try to compile the package with all compilers according to the following rules.:
Then, write the range to the New Compiler Added When a new compiler version is added to the build matrix, all packages that could have admitted that version should be retried. For example, if In the case a new major compiler version comes out we should do the same thing. We would retry all packages with the upper bound If the package fails with the new compiler then its bounds are unchanged. Issues1. Overly-restrictive bounds Is it reasonable for a solver to take a compiler version into account, too, so that when solving 2. Delayed metadata updates |
We can implement it as a fake package dependency that represents the compiler version bounds. I don't think think there'd any additional benefit to special casing it in the solver, since a fake dep would be used to prune possibilities the same, and this would mean we get to see it in the errors for free. |
In #639 I added a script which computes the supported compiler versions (from 0.13.0 onward) for every I'd like to discuss the results of that script run, and discuss what our next steps could be. ResultsYou can view the script I used to derive these results here. Sanity CheckMy first check was to glance at how many manifests we were able to verify a compiler version for: Total Manifests: 10863 At first I was a bit concerned by this result - only 4358? I thought there were 10863 manifests currently in the registry. Well, you're right that there are 10863 in the registry - however - then I realized that there have only been 3242 manifests published since the date 0.13.0 was released. With that in mind, this seems pretty good! CorrectnessMy second check was for correctness. I went through every package set from 0.13.0 to now, and checked if we had recorded that package set's compiler version for each package in the set. Total unique package releases in package sets: 2003 Click here to view all package releases that were in a package set, but we didn't compute that package sets compiler version for that release
I took a glance through a few of the entries in the following list, and in each case I could easily identify an overly restrictive dependency bound on a core library as the reason we wouldn't have been able to compute the "correct" supported compiler version. I am pretty happy with that! Complete list of post-0.13.0 packages without supported compiler versionsClick here to view.arraybuffer: 11.0.0 In particular, I can't find any core or contrib packages in this list - which I take as a good sign. Next StepsI am pretty confident that the results of the script are correct in the sense that we've compute the proper supported compiler versions for each manifest while respecting its dependency bounds. I'd like to move forward with the results of this script and actually extend
|
I'd go for a clean slate and drop whatever is necessary to do that. |
In #577 we agreed to drop pre-0.13 packages (though I’m surprised at so many versions affected!) Personally I don’t think we should consider inclusion in a package set as evidence a package version compiled with the set’s compiler version, if the set breaks the package version’s bounds. The registry guarantees all package versions in the registry solve. With compiler versions in metadata, we also guarantee each package version compiles with at least one compiler. I think this is where we should stop; no need to go hunting through package sets to try and find more compiler versions that work (or, on publication a new package set, go hunting for packages that may now work with the given compiler version). |
Agreed with Thomas - I'd also like to remark that adding compiler versions retroactively is a "nice-to-have": it's not critical to have this information, so no need to drop packages where it's not straightforward to figure it out. |
I don’t think we should keep any package versions for which we couldn’t determine any viable compiler version. That means that the version either doesn’t solve, or that the way it solves is not usable with any compiler (you’ll download dependencies at versions that themselves can’t be compiled). We didn’t historically check if packages solved or compiled, we just imported them wholesale from bower and GitHub tags. Later we determined we would require packages to be solvable and any new packages must compile. We’ve always had gaps and inconsistencies, though, because the legacy importer disables some checks and allows package versions into the registry we wouldn’t otherwise let in. And it runs every day! The work done here simply finishes the job by extending the solve & compile rule to mean every package version must solve & compile with at least one compiler version supported by the registry. In turn, that means we can make the legacy importer work the same as the normal publish pipeline and get rid of the “Legacy” vs “Current” distinction. Why would we want to include package versions that don’t solve and compile with any supported compiler? The only reason I can think of is because they could technically compile in a package set where you’re explicitly ignoring their bounds. But I feel like this is a weak motivation; the package version still ought to be solvable & buildable with at least a single compiler version. It can also be resolved for any existing package versions that would be dropped here. We could follow Colin’s guidance and grandfather in existing package versions by adjusting their bounds based on their package set inclusion as of the day we do the big update, hence keeping packages that would otherwise be removed. But future packages would have to work with at least one compiler. @f-f I may be missing your point somewhat; are you seeing cases that make this solve/compile requirement untenable? For example, we’re generating (quite lenient) bounds for folks who don’t have bowerfiles or spago.yaml files so there’s a chance our solutions are wrong. Still, if our solutions are wrong, then it seems we should treat that as ‘unsolvable’. |
I'm ok with making this a strict check (i.e. dropping packages for which we don't find at least one compiler version), if the amount of post-0.13 packages that we'd drop is very low (ideally zero). I can't figure out what this number is in Colin's post above, but looking at the list it seems like it's about ~100 versions? We should go through these and make sure that we're not missing anything straighforward that would make these solve. |
That sounds like a good idea. I know there were 56 package versions that existed in package sets but had no workable compiler version. We reviewed a bunch of these and it was always some overly-restrictive dependency. So we can fix those. We’ll try to get as many as possible to work, and once we’re at a number we all think is reasonable we can move forwards |
Thomas is right that there were 56 package versions that existed in a package set, but we didn't compute that package set's compiler version in the packages supported compilers array - however, that doesn't mean that we didn't find any supported compilers for the package version. The amount of package versions that were in a package set but for which we computed no supported compilers is lower at 42. Do we want to update the compiler versions for just these 42 packages, or update all 56 so that we report that we support each compiler version we have evidence for via inclusion in a package set? We should be able to easily add compiler versions for the ones we have evidence worked via their inclusion in a package set (it should be pretty easy to just bump the dependencies based on the others in the package set). In total there were 246 manifests published after 0.13.0 for which we didn't compute any supported compilers. I can look through the 246 to see if there's something obvious missing, but that's a lot to go through, frankly. (In particular, at a glance, it is mostly old versions of things). Are you all proposing literally updating the manifests to get things to solve so that we can admit a larger set of things to the registry? |
I'm not proposing any manual process. But we do have some automated possibilities:
The second point there's a bit iffy and probably not worth the effort. But we could at least go with the first option. I agree that having to go look through 246 manifests and figure out how to fix them all is a bit much. That said, if something jumps out as an obvious reason why many fail, but could have been admitted, it would be good to fix it in an automated fashion if possible. |
I haven’t gotten around to discovering all packages that would have to be reimported (as well as their dependents), but I was thinking on it and — we already plan to reimport the whole registry once this build matrix change is in, so why not just have this be part of the wholesale reimport of the registry? I quite frankly don’t have much time and just bundling this in to the existing full reimport plan would be a better use of it. cc: @f-f |
@thomashoneyman agreed - I think "trying to compile a package to figure out if it can be compiled with that compiler version" is a good plan in general, and maybe we should just rely on that since the script approach still gives us over 200 packages without a compatible compiler, and the other bounds that it comes up with might still be overly restrictive. |
A PureScript package normally is compatible with some, but not all, compiler versions. I suggest that we allow package authors to specify this information in their package manifest the same way that dependencies are specified (as described in #43). In other words, a
purs.json
file would include:Field names could be
purs
,purs-version
,compiler
,compiler-version
, or something like that.Why?
A manifest including supported compiler versions opens up a few new possibilities:
The text was updated successfully, but these errors were encountered: