-
Notifications
You must be signed in to change notification settings - Fork 6
update opium #593
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
base: main
Are you sure you want to change the base?
update opium #593
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR modernizes dependency management by migrating from a shell script approach to using .opam.template files, and updates the opium dependency to a newer commit.
- Replaces shell script-based dependency pinning with dune's
.opam.templatemechanism - Updates opium and rock dependencies to commit
da1b22e2ee31889fea68a943b4a6358a633df0c6 - Updates transitive dependencies (yojson 3.0.0, cmdliner 2.1.0, and several other minor version bumps)
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/setup.sh | Removed - dependency pinning now handled via .opam.template |
| scripts/build.sh | Removed call to setup.sh script |
| .devcontainer/postCreate.sh | Removed call to setup.sh script |
| pool.opam.template | New file defining pin-depends for external dependencies |
| pool.opam | Added pin-depends section generated from template |
| pool.opam.locked | Updated dependency versions and opium commit hash |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pool.opam.template
Outdated
|
|
||
| ["conformist" "git+https://github.com/oxidizing/conformist.git#aa7b95d1f39215cdaab8cf96d765d63e41d5f8a6"] | ||
|
|
||
| ["letters" "git+https://github.com/mabiede/letters.git#bug/connection-close-wait"] |
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dependency letters is pinned as git+https://github.com/mabiede/letters.git#bug/connection-close-wait, which relies on a mutable git branch instead of an immutable commit. Because letters is a third-party package fetched at build time, a compromise of that GitHub repo or branch could inject malicious code into your builds without any change on your side. To mitigate this supply-chain risk, pin letters to a specific commit SHA (and update the lockfile accordingly) rather than a branch name.
pool.opam
Outdated
|
|
||
| ["conformist" "git+https://github.com/oxidizing/conformist.git#aa7b95d1f39215cdaab8cf96d765d63e41d5f8a6"] | ||
|
|
||
| ["letters" "git+https://github.com/mabiede/letters.git#bug/connection-close-wait"] |
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dependency letters is pinned as git+https://github.com/mabiede/letters.git#bug/connection-close-wait, which uses a mutable git branch instead of an immutable revision. Since this third-party repo is fetched during builds, an attacker who gains control of that branch could push malicious changes that are automatically incorporated into your application. To reduce supply-chain attack surface, pin letters to a specific commit SHA in this pin-depends entry.
| "git+https://github.com/mabiede/letters.git#bug/connection-close-wait" | ||
| ] | ||
| "letters.0.4.0" | ||
| "git+https://github.com/mabiede/letters.git#bug/connection-close-wait" |
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pin-depends entry for letters.0.4.0 uses git+https://github.com/mabiede/letters.git#bug/connection-close-wait, which points to a mutable git branch rather than an immutable commit. Because this third-party dependency is cloned at build time, a compromise or retargeting of that branch could silently introduce malicious code into your builds. To harden your dependency supply chain, pin this to a specific commit SHA instead of a branch name and keep the lockfile in sync.
remove —locked flag due to transitive dependency version missmatch between pool tool dependencies and dependencies of pinned packages. —locked flag was not present on the opam pin add commands therefore it is does not need to be present now anymore
This PR includes an update to opiums version due to an upstream PR (rgrinberg/opium#297) being merge include a fix we need.
Additionally i reworked how pinned versions are specified by using the
.opamfile rather than having opam cli commands in a shell script.Unfortunately dune does not have a built in way to specify pinned packages when using opam as the package manager. Therefore we have to add a section to the dune generated
.opamfile by using the<...>.opam.templatefile.Notes on the versioning of pinned dependencies:
In opam pinned dependencies need to have a version specified (see https://opam.ocaml.org/doc/Manual.html#opamfield-pin-depends and https://opam.ocaml.org/doc/Manual.html#Package-Formulas for more details on the exact format)
As most of our pinned dependencies are fixes made on top of already released versions i chose the following versioning scheme:
<LATEST_VERSION_PRIOR_TO_FIX>-<BRANCH_NAME>+<COMMIT_HASH>This versioning scheme is heavily inspired by by semantic version strings (see https://semver.org/#backusnaur-form-grammar-for-valid-semver-versions) and a valid version identifier by its conventions
Additionally this pattern interacts well with opam's version ordering (see https://opam.ocaml.org/doc/Manual.html#Version-ordering for the exact details)
This leads to a correct ordering of the previously released version "4.0.0" being smaller (<) than the fixed version "4.0.0-master+6c3c404"
This enables us to specify regular version constraints like
(>= 4.0.0)and have the modified, pinned version picked up by opam