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

Support opam lock file #166

Open
sim642 opened this issue Jul 21, 2021 · 4 comments
Open

Support opam lock file #166

sim642 opened this issue Jul 21, 2021 · 4 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@sim642
Copy link

sim642 commented Jul 21, 2021

I'm trying to set up a workflow that uses the dependencies specified by an .opam.locked file in the repository instead of using the unlocked .opam file directly. However this silently doesn't work correctly since the opam-pin input of the action causes the local package to be pinned unlocked. A subsequent job step opam install . --deps-only --locked therefore doesn't install the locked dependencies but the pinned unlocked ones, despite the --locked argument.

I think there are currently two workarounds:

  1. Make opam-pin input use lock file using environment variable by adding the following to this action's step:

    env:
      OPAMLOCKED: locked
  2. Disable opam-pin, such that installing with --locked actually uses the lock file? In my case this seems to make the opam-depext substep fail because it's then trying to install the depexts for the package with the same name actually published in opam repository, which is outdated and incompatible with the development version to run the workflow on.

    Even if it were compatible, it wouldn't still install depexts for locked depopts.

Maybe it makes sense to have an opam-locked input to this action (defaulting to false), which would properly make this action consider the opam lock file without having to manipulate the environment and still allow opam-pin and opam-depext substeps to work as expected.
Currently it can be surprising that even by using --locked in a subsequent step, the lock file isn't actually used and the reproducibility of the reproducible build isn't thus checked. Also this subtlety with opam lock files isn't mentioned in the README, so one has to dig deep to realize this.

@smorimoto
Copy link
Member

In addition to adding an input, one idea is to expose OPAMLOCKED by default if you have the *.opam.locked file in the project root, but I think this could cause more confusion. What do you think?

@smorimoto smorimoto added documentation Improvements or additions to documentation enhancement New feature or request labels Jul 21, 2021
@sim642
Copy link
Author

sim642 commented Jul 21, 2021

That would be a breaking change for everyone who already has a lock file in the repository but doesn't use it for CI with the current behavior. Especially since it would also interact poorly with ocaml-compiler coming from a matrix since the lock file fixes a particular OCaml version.

And having the default behavior change by presence of the lock file makes it more annoying to test a matrix of OCaml versions because then one has to similarly specify OPAMLOCKED to disable that. The use case I'm currently thinking about is having separate workflows on the same project: one on the locked state (for reproducibility) and one doing a version matrix on the unlocked state (for OCaml version and upgraded dependency compatibility).

An additional input with unchanged default behavior wouldn't require a new major release. Also this touches on a related issue with lock files: since the lock file specifies an OCaml version, it would be nice of ocaml-compiler wasn't a required input if opam-locked is enabled. Currently one has to manually specify the same value for ocaml-compiler as is locked, which is a minor duplication of the same information.

@dra27
Copy link
Member

dra27 commented Jul 27, 2021

Another alternative would simply be to turn off the opam-pin feature for the workflow where you're testing the locking and do the full set-up as part of the job commands (which is how it would have looked in v1). To be honest, the opam-pin and opam-depext features already feature-creep away from the core purpose of the action, which is to provide opam. They're very useful (the auto-pin and auto-depext features) as a convenience for a common use-case, but it makes me nervous pulling lockfile support in as well.

@sim642
Copy link
Author

sim642 commented Jul 27, 2021

Fair enough. It would indeed be easier to add lock file usage into the advanced configuration examples than engineer a new input to the action.

But if the core purpose of the action is to provide opam, then making the ocaml-compiler input optional would still make sense I suppose.

Bannerets added a commit to Bannerets/camlproto that referenced this issue Jun 29, 2022
Bannerets added a commit to Bannerets/camlproto that referenced this issue Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants