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

Properly handle missing dune-project #227

Merged
merged 5 commits into from
Nov 8, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
- Properly detect all opam packages defined in the current repository, preventing it
from later pulling duplicates into the duniverse if they were part of the target packages
dependencies. (#203, @Leonidas-from-XIV)
- Properly report missing dune-project file when trying to determine the
to-be-genrated lockfile name (#227, @NathanReb)
NathanReb marked this conversation as resolved.
Show resolved Hide resolved

### Removed

Expand Down
8 changes: 7 additions & 1 deletion cli/lock.ml
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,14 @@ let lockfile_path ~explicit_lockfile ~target_packages repo =
| Some path -> Ok path
| None ->
Project.lockfile
~local_packages:(OpamPackage.Name.Set.elements target_packages)
~target_packages:(OpamPackage.Name.Set.elements target_packages)
repo
|> Result.map_error ~f:(function `Msg msg ->
Rresult.R.msgf
"Could not infer the target lockfile name: %s\n\
Try setting it explicitly using --lockfile or add a project \
Leonidas-from-XIV marked this conversation as resolved.
Show resolved Hide resolved
name in a root dune-project file."
msg)

let root_pin_depends local_opam_files =
OpamPackage.Name.Map.fold
Expand Down
4 changes: 3 additions & 1 deletion dune-project
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
(lang dune 2.6)
(lang dune 2.7)
(generate_opam_files true)
(name opam-monorepo)

(cram enable)

(source (github ocamllabs/opam-monorepo))
(license ISC)
(authors "Anil Madhavapeddy" "Nathan Rebours" "Lucas Pluvinage" "Jules Aguillon")
Expand Down
17 changes: 7 additions & 10 deletions lib/project.ml
Original file line number Diff line number Diff line change
Expand Up @@ -37,20 +37,17 @@ let dune_project t = Fpath.(t / "dune-project")
let name t =
let open Result.O in
let dune_project = dune_project t in
Dune_file.Raw.as_sexps dune_project >>= Dune_file.Project.name
Bos.OS.File.exists dune_project >>= function
| true -> Dune_file.Raw.as_sexps dune_project >>= Dune_file.Project.name
| false ->
Rresult.R.error_msgf "Missing dune-project file at the root: %a" Fpath.pp
dune_project

let lockfile ~name t = Fpath.(t / (name ^ Config.lockfile_ext))

let lockfile ?local_packages:lp t =
let lockfile ~target_packages t =
let open Result.O in
let local_packages =
match lp with
| Some lp -> Ok lp
| None -> local_packages ~recurse:false t >>| List.map ~f:fst
in

local_packages >>= fun names ->
match names with
match target_packages with
| [ name ] ->
let name = OpamPackage.Name.to_string name in
Ok (lockfile ~name t)
Expand Down
12 changes: 6 additions & 6 deletions lib/project.mli
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,16 @@ val name : t -> (string, [> `Msg of string ]) result
(** Returns the name of the project, as set in the dune-project. *)

val lockfile :
?local_packages:OpamPackage.Name.t list ->
target_packages:OpamPackage.Name.t list ->
t ->
(Fpath.t, [> `Msg of string ]) result
(** Returns the path to the opam-monorepo lockfile for the given project.
If the repo contains a single package, then it's the ["<package_name>.opam.locked"]
(** Returns the path to the opam-monorepo lockfile to generate for the given
project and lockfile target packages.
If there is a single target package, then it is the ["<package_name>.opam.locked"]
file at the root of the project.
If it contains multiple packages, then it's the ["<project_name>.opam.locked"] file
at the root of the project.
One can provide [local_packages] if they were already computed or if only a subset
of the local packages must be taken into account. *)
at the root of the project, where <project_name> is the name as defined in the
dune-project file. *)
Leonidas-from-XIV marked this conversation as resolved.
Show resolved Hide resolved

val local_lockfiles : t -> (Fpath.t list, Rresult.R.msg) result
(** Returns all the lockfiles located at the root of the project i.e. all
Expand Down
3 changes: 2 additions & 1 deletion opam-monorepo.opam
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ license: "ISC"
homepage: "https://github.com/ocamllabs/opam-monorepo"
bug-reports: "https://github.com/ocamllabs/opam-monorepo/issues"
depends: [
"dune" {>= "2.6"}
"dune" {>= "2.7"}
"ocaml" {>= "4.08.0"}
"dune-build-info"
"base"
Expand All @@ -29,6 +29,7 @@ depends: [
"sexplib"
"uri"
"alcotest" {with-test}
"odoc" {with-doc}
]
conflicts: [
"dune-build-info" {= "2.7.0" | = "2.7.1"}
Expand Down
2 changes: 2 additions & 0 deletions test/bin/missing-dune-project/dune
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
(cram
(deps %{bin:opam-monorepo}))
NathanReb marked this conversation as resolved.
Show resolved Hide resolved
21 changes: 21 additions & 0 deletions test/bin/missing-dune-project/run.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
Let us setup a simple project with two local packages, defined at the root
NathanReb marked this conversation as resolved.
Show resolved Hide resolved

$ cat > a.opam << EOF
> opam-version: "2.0"
> EOF

$ cat > b.opam << EOF
> opam-version: "2.0"
> EOF

The project has no dune-project file. That means that if we run `opam-monorepo lock`,
it will have more than onw target: `a` and `b`. It therefore has to determine the name of the
lockfile based on the project's name in the dune-project file. It's expected to fail but it should
to that nicely, letting the user know that it couldn't infer the lockfile and that they should
either explicitly specify it on the command line or add a valid dune-project file at the root.

$ opam-monorepo lock
==> Using 2 locally scanned packages as the targets.
opam-monorepo: Could not infer the target lockfile name: Missing dune-project file at the root: $TESTCASE_ROOT/dune-project
Try setting it explicitly using --lockfile or add a project name in a root dune-project file.
[1]
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.