-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add generator files. Wrap latest 1.9 release. #24
Conversation
Why is that bad? I understand we don't want a breaking release, but why block a minor release? |
I don't think we can assume semver on external C dependencies. Like we've seen with GDAL and PROJ, I rather be more strict in what we allow. |
Not in general no, but unnecessarily restricting to minor versions prevents upgrades from flowing through the ecosystem. Generally handling breaking versions is done at the Yggdrasil and General registry side, and the strategy there is to only restrict to minor versions if they break the ABI. For e.g. PROJ we have the x00.y00,z00 versioning scheme set up to put semver on top of the original version number. For LibSpatialIndex there has not yet been a reason to do so. If libspatialindex 1.10 comes out with this PR merged, it can still install that by installing the current 0.2.0 release of this package. So it would also have to be edited for past releases in the General registry. |
I'm aware, but ABI breakage is rare, while behaviour breakage is much more common, and you just don't know when updating a jll. Letting possible breakages flow automatically through the ecosystem seems like a bad move, as each it happens we break semver for the ecosystem. I think pinning strictly to a single version(s) might be even better, and make new releases only after reverse dependency checks. It's a balance between automatic updates (and developer ease) on the one hand, versus the user experience. It's an incredibly bad user experience to one day add an unrelated package, thereby update some packages automatically, and now your script errors. Since the jll is probably never a direct dependency, reverting this update is actually painful, as it involves understanding that a jll update caused it, including which jll caused it, and manually finding the version that doesn't break and now pinning it. Using a Manifest is a solution, but is not used extensively for the general environment.
That is a good point. Anyway, this PR doesn't seem the place to discuss this (libspatialindex is rarely updated and used less than say GDAL or PROJ), so I will revert the ~ here. |
This came up at the recent Munster workshop. We really cant mix non-semver versions into a semver ecosystem (and still say we have stick to semver in any packages with non-semver deps). I came out of that strongly in favour of locking down our binary deps like this wherever they hit a package with real semver. What we have with semver and hard upper bounds is truly amazing compared to Python/R ecosystems, and we should lean into it even more. Instead of reverting the ~, we can add it to all the previous versions in the registry? |
See also the Slack thread @evetion started in #pkg-dev. I agree of course the whole situation would be better if we had reverse CI in Yggdrasil to evaluate breaking changes before instead of sometimes after the fact. |
I've reverted the pin here, but made a new issue. This PR is about providing the code to autogenerate the C bindings. |
Pin the 1.9 jll release, so we don't automatically update to 1.10 (if that happens).see Pin jll version? #25