-
Notifications
You must be signed in to change notification settings - Fork 27
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
Depexts for opam-monorepo projects #152
Comments
cc @kit-ty-kate |
cc @emillon how should we handle this in the CI? My impression is that this file should be versioned along with the lockfile. We should then be able to pin it and run |
I think that the point of using lock files is to have something that represents the exact build plan. It's expected that it introduces some duplication, since it's a way to take a snapshot of the relevant bits of the opam repository. In my opinion that's a tradeoff you make when you opt into an opam-monorepo workflow. You become responsible for fixing issues like this (that opam-repository maintainers could do for you otherwise). I think there's a parallel to be made with dynamic vs static linking: when something is statically linked, it won't benefit from changes until it is linked again (here: until it's locked again). If we consider that conf packages are "independent" from the application, it can make sense not to lock them and instead keep the dependency resolved at install time. That means that opam-repository can fix (or break) your project by changing the depext definitions. This strategy can even be selected at lock time. Regarding creating an extra package-conf opam package, I don't think that this has any advantage compared to keeping the conf-x dependencies (the only difference is when a package has depexts itself, but you can either duplicate it in the lock file or extract a conf package, which is probably a good idea in any case). |
Regarding "how to handle that in CI", there's not much to change:
|
One thing I wish to clarify is that opam-monorepo isn't really intended to "lock" your depexts as well. The best it can do is lock a depext formula which will be interpreted by another tool and will result in the installation of a system package without further control on its source or version so I think it's "okay" if they are less tightly controlled by opam-monorepo. That being said I agree changes upstream in the conf packages might result in breaking an opam-monorepo project if we don't control their version tightly and I'm not sure I understand why the opam maintainers would want to fix an opam-monorepo package's opam file in the first place, even if it's not in sync with the conf package it derived its depexts from.
That's not actually true since the conf packages might be transitive dependencies and therefore not a part of the opam file. The extra package allow us to release without any modification to the opam files in the repo. |
Projects using opam-monorepo to vendor their dependencies need a proper way to handle transitive external dependencies.
Right now this is handled by hand, copying the depexts field from the lockfile to the opam file before the release and removing it afterward.
This is bad for two reasons:
conf-*
packages. This also means that it won't benefit from changes in saidconf-*
packages and that opam maintainers have to update opam-monorepo projects metadata when that happens.During discussions with the opam team, we decided that 1 could be solved by writing the depexts to a separate opam package, somehting along the lines of -conf.opam, generated during the lock step. The main package in an opam-monorepo should always depend on this package with a
{=version}
constraint and they should always be released together.Ideally opam file generation should also have a opam-monorepo mode where it automatically add the dependency from the main package to this one.
This extra package would be virtual (meaning dune-release must also be adapted to properly release this) and have all the external deps in the form of dependencies on
conf-*
packages whenever possible and would only contain adepexts
field if the project or some of its non conf dependencies had such fields.The text was updated successfully, but these errors were encountered: