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

Ignore pinned packages when building dev-repo conflicts #398

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 @@ -7,6 +7,9 @@
### Deprecated

### Fixed
- Fix support for pinned packages. In that case, it is not necessary to add
dev-repo conflicts as `opam-monorepo` will always use the pinned repository.
(#398, #353, @samoht, @reynir, reported by @emillon)

### Removed

Expand Down
64 changes: 47 additions & 17 deletions lib/duniverse.ml
Original file line number Diff line number Diff line change
Expand Up @@ -67,19 +67,21 @@ module Repo = struct
dev_repo : Dev_repo.t;
url : unresolved Url.t;
hashes : OpamHash.t list;
pinned : bool;
}

let equal t t' =
OpamPackage.equal t.opam t'.opam
&& Dev_repo.equal t.dev_repo t'.dev_repo
&& Url.equal Git.Ref.equal t.url t'.url

let pp fmt { opam; dev_repo; url; hashes } =
let pp fmt { opam; dev_repo; url; hashes; pinned } =
let open Pp_combinators.Ocaml in
Format.fprintf fmt
"@[<hov 2>{ opam = %a;@ dev_repo = %a;@ url = %a;@ hashes = %a }@]"
"@[<hov 2>{ opam = %a;@ dev_repo = %a;@ url = %a;@ hashes = %a;@ \
pinned = %b; }@]"
Opam.Pp.raw_package opam string dev_repo (Url.pp Git.Ref.pp) url
(list Opam.Pp.hash) hashes
(list Opam.Pp.hash) hashes pinned

let from_package_summary ~get_default_branch ps =
let open Opam.Package_summary in
Expand All @@ -101,10 +103,11 @@ module Repo = struct
package;
dev_repo = Some dev_repo;
hashes;
pinned;
_;
} ->
let* url = url url_src in
Ok (Some { opam = package; dev_repo; url; hashes })
Ok (Some { opam = package; dev_repo; url; hashes; pinned })
| { dev_repo = None; package; _ } ->
Logs.warn (fun l ->
l
Expand All @@ -113,6 +116,8 @@ module Repo = struct
Opam.Pp.package package);
Ok None
| _ -> Ok None)

let is_pinned { pinned; _ } = pinned
end

type 'ref t = {
Expand All @@ -132,6 +137,24 @@ module Repo = struct
Dev_repo.repo_name dev_repo
|> Base.Result.map ~f:(function "dune" -> "dune_" | name -> name)

let log_url_selection ~dev_repo ~packages pinned_packages =
let url_to_string : unresolved Url.t -> string = function
| Git { repo; ref } -> Printf.sprintf "%s#%s" repo ref
| Other s -> s
in
let pp_package fmt { Package.opam; url; _ } =
Fmt.pf fmt "%a: %s" Opam.Pp.package opam (url_to_string url)
in
let pp_packages = Fmt.(list ~sep:(any "\n") pp_package) in
Logs.warn (fun l ->
l
"The following packages come from the same repository %s but are \
associated with different URLs:\n\
%a\n\
The URL for the pinned package(s) was selected: %a"
(Dev_repo.to_string dev_repo)
pp_packages packages pp_packages pinned_packages)

let from_packages ~dev_repo (packages : Package.t list) =
let open Result.O in
let provided_packages = List.map packages ~f:(fun p -> p.Package.opam) in
Expand All @@ -145,19 +168,26 @@ module Repo = struct
in
match urls with
| [ (url, hashes) ] -> Ok { dir; url; hashes; provided_packages }
| _ ->
let pp_hash = Fmt.of_to_string OpamHash.to_string in
(* this should not happen because we passed extra constraints
to the opam solver to avoid this situation *)
Fmt.failwith
"The following packages have the same `dev-repo' but are using \
different versions of the archive tarballs:\n\
%a\n\
This should not happen, please report the issue to \
https://github.com/tarides/opam-monorepo.\n\
%!"
Fmt.Dump.(list (pair (Url.pp string) (list pp_hash)))
urls
| _ -> (
match List.filter ~f:Package.is_pinned packages with
| [] ->
let pp_hash = Fmt.of_to_string OpamHash.to_string in
(* this should not happen because we passed extra constraints
to the opam solver to avoid this situation *)
Fmt.failwith
"The following packages have the same `dev-repo' but are using \
different versions of the archive tarballs:\n\
%a\n\
This should not happen, please report the issue to \
https://github.com/tarides/opam-monorepo.\n\
%!"
Fmt.Dump.(list (pair (Url.pp string) (list pp_hash)))
urls
| first_pin :: _ as pins ->
log_url_selection ~dev_repo ~packages pins;
let url = first_pin.url in
let hashes = first_pin.hashes in
Ok { dir; url; hashes; provided_packages })

let equal equal_ref t t' =
let { dir; url; hashes; provided_packages } = t in
Expand Down
1 change: 1 addition & 0 deletions lib/duniverse.mli
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ module Repo : sig
dev_repo : string;
url : unresolved Url.t;
hashes : OpamHash.t list;
pinned : bool;
}

val equal : t -> t -> bool
Expand Down
11 changes: 7 additions & 4 deletions lib/opam.ml
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ module Package_summary = struct
hashes : OpamHash.t list;
dev_repo : string option;
depexts : (OpamSysPkg.Set.t * OpamTypes.filter) list;
pinned : bool;
flags : Package_flag.t list;
has_build_commands : bool;
has_install_commands : bool;
Expand All @@ -207,23 +208,24 @@ module Package_summary = struct
hashes;
dev_repo;
depexts;
pinned;
flags;
has_build_commands;
has_install_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;@ has_build_commands = %B;@ \
has_install_commands = %B}@]"
dev_repo = %a;@ depexts = %a;@ pinned = %B;@ flags = %a;@ \
has_build_commands = %B;@ has_install_commands = %B}@]"
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 pinned (list Package_flag.pp) flags
has_build_commands has_install_commands

let from_opam package opam_file =
let from_opam package ~pinned opam_file =
let url_field = OpamFile.OPAM.url opam_file in
let url_src = Base.Option.map ~f:Url.from_opam_field url_field in
let hashes =
Expand All @@ -245,6 +247,7 @@ module Package_summary = struct
hashes;
dev_repo;
depexts;
pinned;
flags;
has_build_commands;
has_install_commands;
Expand Down
3 changes: 2 additions & 1 deletion lib/opam.mli
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,14 @@ module Package_summary : sig
hashes : OpamHash.t list;
dev_repo : string option;
depexts : (OpamSysPkg.Set.t * OpamTypes.filter) list;
pinned : bool;
flags : OpamTypes.package_flag list;
has_build_commands : bool;
has_install_commands : bool;
}

val pp : t Fmt.t
val from_opam : OpamPackage.t -> OpamFile.OPAM.t -> t
val from_opam : OpamPackage.t -> pinned:bool -> OpamFile.OPAM.t -> t

val is_safe_package : t -> bool
(** A package is safe when it is clear that it can be added to the lockfile
Expand Down
Loading
Loading