Skip to content
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 ez_opam_file.0.1.0 compatibility package for opam-file-format #17779

Merged
merged 1 commit into from
Dec 7, 2020

Conversation

lefessan
Copy link
Contributor

@lefessan lefessan commented Dec 6, 2020

opam-file-format 2.1.1 is incompatible with previous versions. This package provides a (minimal, for now) compatibility layer that allows drom and opam-bin to be installed with any version of opam-file-format.

@lefessan lefessan force-pushed the z-2020-12-06-ez_opam_file branch from 1cc7b9e to 0260909 Compare December 6, 2020 15:26
@camelus
Copy link
Contributor

camelus commented Dec 6, 2020

Commit: 0260909

A pull request by opam-seasoned @lefessan.

☀️ All lint checks passed 0260909
  • These packages passed lint tests: ez_opam_file.0.1.0

☀️ Installability check (+1)
  • new installable packages (1): ez_opam_file.0.1.0

@avsm
Copy link
Member

avsm commented Dec 6, 2020

This package is fine by me of course, but can we just fix this in opam-file-format @rjbou? A point release (2.1.1) shouldn't be incompatible with the previous one (2.1.0).

@rjbou
Copy link
Contributor

rjbou commented Dec 7, 2020

They are not incompatible, a lot of deprecated functions in 2.1.1, but still available and usable.

@lefessan
Copy link
Contributor Author

lefessan commented Dec 7, 2020

Is it possible to switch this discussion to opam-file-format Github issues and keep the focus here on the inclusion of this package, that would allow me to push new versions of drom and opam-bin with no conflicts anymore ?

@kit-ty-kate
Copy link
Member

I would rather prefer not to merge this PR and temporary merge #17788 instead. Is that ok for you?

@dra27
Copy link
Member

dra27 commented Dec 7, 2020

The description is misleading - opam-file-format 2.1.1 is compatible with anything requiring the previous releases. It contains a packaging error when built with Dune which causes it to expose a module called Flags which conflicts with core_kernel. drom-lib uses new features in opam-file-format 2.1.1, so of course it is incompatible with previous releases of opam-file-format. The linking issue is already merged in ocaml/opam-file-format#34. 2.1.2 should follow quite soon fixing this.

This package therefore back-ports new features in opam-file-format 2.1.1 to 2.1.0. I have no issue with that, but I wonder if @kit-ty-kate's idea combined with opam-file-format 2.1.2 would mean this library wouldn't be wanted for drom?

As to why we compatibly (in the limited OCaml definition!) added new features to a point release, we made a mistake releasing opam-file-format 2.1.0 before opam 2.1.0 itself. The alternative is to have opam 2.1 depend on opam-file-format 2.2 which I think is even more weird than our small semver violation!

@lefessan
Copy link
Contributor Author

lefessan commented Dec 7, 2020

Whatever happens with opam-file-format releases (former and next ones), this package is needed for drom and opam-bin to support multiple versions of opam-file-format.
Any package that wants to be independant of opam-file-format version will need this kind of code, so it shouldn't be in drom or opam-bin, but in an independant library such as this one.
So yes, I do prefer this PR to be merged. @kit-ty-kate

Note that, in my own recent packages, I use a convention to put the interface in a V1 module that should only change over time by additions of new functions and optional args. For incompatible changes, instead, I will create a V2, etc. module, so that packages that depend on V1 are not impacted by these changes. Of course, major releases will be allowed to break this naming schema. That's an attempt at solving such issues, I will tell you in a few months/years whether it was a good or bad idea...

@avsm
Copy link
Member

avsm commented Dec 7, 2020

The mistake's been made with opam-file-format, so we should unblock @lefessan with this (independent) package. I'm going ahead and merging this, and opam-file-format can be fixed elsewhere indeed.

@avsm avsm merged commit dbcec3b into ocaml:master Dec 7, 2020
@lefessan
Copy link
Contributor Author

lefessan commented Dec 7, 2020

Thanks @avsm !

@lefessan lefessan deleted the z-2020-12-06-ez_opam_file branch December 7, 2020 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants