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

Allow packages with no build commands #355

Merged
merged 1 commit into from
Dec 1, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 3 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

### Changed

- Treat packages with no build commands as if they can be built with dune (#355,
@gridbugs)

### Deprecated

### Fixed
Expand Down
21 changes: 17 additions & 4 deletions lib/opam.ml
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,14 @@ module Hash = struct
Format.fprintf fmt "%s:%s" kind contents
end

module Command = struct
type t = OpamTypes.command

let pp fmt (t : t) =
OpamPp.print OpamFormat.V.command t
|> OpamPrinter.FullPos.value |> Format.pp_print_string fmt
end

module Depexts = struct
let pp fmt t =
let pp_filter fmt filter =
Expand Down Expand Up @@ -196,18 +204,21 @@ module Package_summary = struct
dev_repo : string option;
depexts : (OpamSysPkg.Set.t * OpamTypes.filter) list;
flags : Package_flag.t list;
build_commands : OpamTypes.command list;
}

let pp fmt { package; url_src; hashes; dev_repo; depexts; flags } =
let pp fmt
{ package; url_src; hashes; dev_repo; depexts; flags; build_commands } =
let open Pp_combinators.Ocaml in
Format.fprintf fmt
"@[<hov 2>{ name = %a;@ version = %a;@ url_src = %a;@ hashes = %a;@ \
dev_repo = %a;@ depexts = %a;@ flags = %a }@]"
dev_repo = %a;@ depexts = %a;@ flags = %a;@ build_commands = %a }@]"
Pp.package_name package.name Pp.version package.version
(option ~brackets:true Url.pp)
url_src (list Hash.pp) hashes
(option ~brackets:true string)
dev_repo Depexts.pp depexts (list Package_flag.pp) flags
dev_repo Depexts.pp depexts (list Package_flag.pp) flags (list Command.pp)
build_commands

let from_opam package opam_file =
let url_field = OpamFile.OPAM.url opam_file in
Expand All @@ -220,14 +231,16 @@ module Package_summary = struct
in
let depexts = OpamFile.OPAM.depexts opam_file in
let flags = OpamFile.OPAM.flags opam_file in
{ package; url_src; hashes; dev_repo; depexts; flags }
let build_commands = OpamFile.OPAM.build opam_file in
{ package; url_src; hashes; dev_repo; depexts; flags; build_commands }

let has_flag flag { flags; _ } = List.mem flag ~set:flags
let is_compiler v = has_flag OpamTypes.Pkgflag_Compiler v

let is_virtual = function
| { url_src = None; _ } -> true
| { dev_repo = None | Some ""; _ } -> true
| { build_commands = []; _ } -> true
| _ -> false

let is_compiler_package { package; _ } =
Expand Down
1 change: 1 addition & 0 deletions lib/opam.mli
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ module Package_summary : sig
dev_repo : string option;
depexts : (OpamSysPkg.Set.t * OpamTypes.filter) list;
flags : OpamTypes.package_flag list;
build_commands : OpamTypes.command list;
}

val pp : t Fmt.t
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ dev-repo: "git+https://a.com/a.git"
depends: [
"dune"
]
build: [ "dune" "build" ]
url {
src: "https://a.com/a.0.1.tbz"
checksum: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ dev-repo: "git+https://a.com/a.git"
depends: [
"ocaml"
]
build: [ "make" ]
url {
src: "https://a.com/a.1.0.tbz"
checksum: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ dev-repo: "git+https://a.com/a.git"
depends: [
"ocaml"
]
build: [ "make" ]
url {
src: "https://a.com/a.1.1.tbz"
checksum: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ dev-repo: "git+https://a.com/a.git"
depends: [
"dune"
]
build: [ "dune" "build" ]
url {
src: "https://a.com/a.0.2.tbz"
checksum: [
Expand Down
1 change: 1 addition & 0 deletions test/bin/local-solver-picks-higher-version.t/run.t
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ solutions:
depends: [
"dune"
]
build: [ "dune" "build" ]
url {
src: "https://a.com/a.0.2.tbz"
checksum: [
Expand Down
1 change: 1 addition & 0 deletions test/bin/minimal-update.t/repo/packages/a/a.0.1/opam
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ opam-version: "2.0"
depends: [
"dune"
]
build: [ "dune" "build" ]
dev-repo: "git+https://github.com/a/a"
url {
src: "https://a.com/a.tbz"
Expand Down
1 change: 1 addition & 0 deletions test/bin/minimal-update.t/repo/packages/b/b.0.1/opam
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ opam-version: "2.0"
depends: [
"dune"
]
build: [ "dune" "build" ]
dev-repo: "git+https://github.com/b/b"
url {
src: "https://b.com/b.tbz"
Expand Down
1 change: 1 addition & 0 deletions test/bin/minimal-update.t/repo/packages/c/c.0.1/opam
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ opam-version: "2.0"
depends: [
"dune"
]
build: [ "dune" "build" ]
dev-repo: "git+https://github.com/c/c"
url {
src: "https://c.com/c.tbz"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
opam-version: "2.0"
dev-repo: "git+https://github.com/test-no-build-command/test"
depends: [
"with-dune"
]
dev-repo: "git+https://github.com/b/b"
url {
src: "https://test.com/test.tbz"
checksum: [
"sha256=0000000000000000000000000000000000000000000000000000000000000000"
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
opam-version: "2.0"
dev-repo: "git+https://github.com/test-no-build-command/test"
depends: [
"without-dune"
]
dev-repo: "git+https://github.com/b/b"
url {
src: "https://test.com/test.tbz"
checksum: [
"sha256=0000000000000000000000000000000000000000000000000000000000000000"
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
opam-version: "2.0"
dev-repo: "git+https://github.com/test-no-build-command/test"
depends: [
"dune"
]
build: [ "dune" "build" ]
url {
src: "https://test.com/test.tbz"
checksum: [
"sha256=0000000000000000000000000000000000000000000000000000000000000000"
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
opam-version: "2.0"
dev-repo: "git+https://github.com/test-no-build-command/test"
depends: []
build: [
["make"]
]
url {
src: "https://test.com/test.tbz"
checksum: [
"sha256=0000000000000000000000000000000000000000000000000000000000000000"
]
}
1 change: 1 addition & 0 deletions test/bin/no-build-command.t/repo/repo
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
opam-version: "2.0"
30 changes: 30 additions & 0 deletions test/bin/no-build-command.t/run.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
Test that packages with no build commands and no direct dependency on dune and
no build command are not assumed to not build with dune.

The repo contains 4 packages:
- with-dune: directly depends on dune and has a build command
- without-dune: doesn't depend on dune and has a build command
- depend-with-dune: depends on with-dune and doesn't have a build command
- depend-without-dune: depends on without-dune and doesn't have a build command

This test asserts that it's an error to generate a lockfile for a package
depending on depend-without-dune due to the transitive dependency on
without-dune, but that it's not an error to generate a lockfile for a package
which only depends on depend-with-dune, despite the latter not directly
depending on dune.

We setup the default base repository

$ gen-minimal-repo

Attempt to generate a lockfile for a package which depends on
depend-without-dune (this should fail due to the transitive dependency on
without-dune which has a build command but doesn't depend on dune)

$ opam-monorepo lock test-depend-without-dune.opam 2>&1 | grep -o "Doesn't build with dune"
Doesn't build with dune

Attempt to generate a lockfile for a package which depends only on
depend-with-dune

$ opam-monorepo lock test-depend-with-dune.opam > /dev/null
8 changes: 8 additions & 0 deletions test/bin/no-build-command.t/test-depend-with-dune.opam
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
opam-version: "2.0"
depends: [
"depend-with-dune"
]
x-opam-monorepo-opam-repositories: [
"file://$OPAM_MONOREPO_CWD/minimal-repo"
"file://$OPAM_MONOREPO_CWD/repo"
]
8 changes: 8 additions & 0 deletions test/bin/no-build-command.t/test-depend-without-dune.opam
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
opam-version: "2.0"
depends: [
"depend-without-dune"
]
x-opam-monorepo-opam-repositories: [
"file://$OPAM_MONOREPO_CWD/minimal-repo"
"file://$OPAM_MONOREPO_CWD/repo"
]
1 change: 1 addition & 0 deletions test/bin/opam-provided.t/repo/packages/b/b.1/opam
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ depends: [
"dune"
]
dev-repo: "git+https://github.com/b/b"
build: [ "dune" "build" ]
url {
src: "https://b.com/b.tbz"
checksum: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ depends: [
"dune"
"b"
]
build: [ "dune" "build" ]
dev-repo: "git+https://github.com/b/depends-on-b"
url {
src: "https://b.com/depends-on-b.tbz"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ dev-repo: "git+https://b.com/b.git"
depends: [
"dune"
]
build: [ "dune" "build" ]
tags: ["cross-compile"]
url {
src: "https://mirage.com/b.0.1-dune-mirage.tbz"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ dev-repo: "git+https://b.com/b.git"
depends: [
"dune"
]
build: [ "dune" "build" ]
url {
src: "https://dune.com/b.0.1-dune.tbz"
checksum: "sha256=0000000000000000000000000000000000000000000000000000000000000001"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
opam-version: "2.0"
dev-repo: "git+https://b.com/b.git"
build: [ "make" ]
url {
src: "https://b.com/b.0.1.tbz"
checksum: "sha256=0000000000000000000000000000000000000000000000000000000000000000"
Expand Down
3 changes: 3 additions & 0 deletions test/bin/require-cross-compiling-packages.t/run.t
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ original 0.1 release. There is a 0.1+dune port of it in opam-overlays:
depends: [
"dune"
]
build: [ "dune" "build" ]
url {
src: "https://dune.com/b.0.1-dune.tbz"
checksum: "sha256=0000000000000000000000000000000000000000000000000000000000000001"
Expand All @@ -25,6 +26,7 @@ maintainers created a 0.1+dune+mirage port in mirage-opam-overlays:
depends: [
"dune"
]
build: [ "dune" "build" ]
tags: ["cross-compile"]
url {
src: "https://mirage.com/b.0.1-dune-mirage.tbz"
Expand Down Expand Up @@ -72,6 +74,7 @@ to upstream the dune port before `0.2` so the `0.2` release builds with dune.
> depends: [
> "dune"
> ]
> build: [ "dune" "build" ]
> url {
> src: "https://b.com/b.0.2.tbz"
> checksum: "sha256=0000000000000000000000000000000000000000000000000000000000000003"
Expand Down
1 change: 1 addition & 0 deletions test/bin/simple-lock.t/repo/packages/b/b.1/opam
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ opam-version: "2.0"
depends: [
"dune"
]
build: [ "dune" "build" ]
dev-repo: "git+https://github.com/b/b"
url {
src: "https://b.com/b.tbz"
Expand Down
1 change: 1 addition & 0 deletions test/bin/simple-lock.t/repo/packages/c/c.1/opam
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ opam-version: "2.0"
depends: [
"dune"
]
build: [ "dune" "build" ]
dev-repo: "git+https://github.com/c/c"
url {
src: "https://c.com/c.tbz"
Expand Down
37 changes: 28 additions & 9 deletions test/lib/test_duniverse.ml
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,23 @@ let opam_factory ~name ~version =
OpamPackage.create name version

let summary_factory ?(name = "undefined") ?(version = "1") ?dev_repo ?url_src
?(hashes = []) ?(depexts = []) ?(flags = []) () =
?(hashes = []) ?(depexts = []) ?(flags = []) ?(build_commands = []) () =
let package = opam_factory ~name ~version in
{ Opam.Package_summary.package; dev_repo; url_src; hashes; depexts; flags }
{
Opam.Package_summary.package;
dev_repo;
url_src;
hashes;
depexts;
flags;
build_commands;
}

let dependency_factory ?(vendored = true) ?name ?version ?dev_repo ?url_src
?hashes ?depexts ?flags () =
?hashes ?depexts ?flags ?build_commands () =
let package_summary =
summary_factory ?name ?version ?dev_repo ?url_src ?hashes ?depexts ?flags ()
summary_factory ?name ?version ?dev_repo ?url_src ?hashes ?depexts ?flags
?build_commands ()
in
{ Opam.Dependency_entry.vendored; package_summary }

Expand Down Expand Up @@ -71,7 +80,9 @@ module Repo = struct
make_test ~name:"Regular"
~summary:
(summary_factory ~dev_repo:"d" ~url_src:(Other "u") ~name:"y"
~version:"v" ~hashes:[] ())
~version:"v" ~hashes:[]
~build_commands:[ ([], None) ]
())
~expected:
(Ok
(Some
Expand All @@ -88,7 +99,9 @@ module Repo = struct
~summary:
(summary_factory ~dev_repo:"d"
~url_src:(Git { repo = "r"; ref = None })
~name:"y" ~version:"v" ~hashes:[] ())
~name:"y" ~version:"v" ~hashes:[]
~build_commands:[ ([], None) ]
())
~expected:
(Ok
(Some
Expand Down Expand Up @@ -234,7 +247,9 @@ let test_from_dependency_entries =
~dependency_entries:
[
dependency_factory ~name:"x" ~version:"v" ~url_src:(Other "u")
~dev_repo:"d" ~hashes:[] ();
~dev_repo:"d" ~hashes:[]
~build_commands:[ ([], None) ]
();
]
~expected:
(Ok
Expand All @@ -258,9 +273,13 @@ let test_from_dependency_entries =
~dependency_entries:
[
dependency_factory ~name:"y" ~version:"v" ~url_src:(Other "u")
~dev_repo:"d" ~hashes:[] ();
~dev_repo:"d" ~hashes:[]
~build_commands:[ ([], None) ]
();
dependency_factory ~name:"y-lwt" ~version:"v" ~url_src:(Other "u")
~dev_repo:"d" ~hashes:[] ();
~dev_repo:"d" ~hashes:[]
~build_commands:[ ([], None) ]
();
]
~expected:
(Ok
Expand Down