Skip to content

Commit

Permalink
Encourage reading the opam file only once when pinning it
Browse files Browse the repository at this point in the history
  • Loading branch information
kit-ty-kate committed Mar 1, 2025
1 parent 892e126 commit 36113c8
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 18 deletions.
28 changes: 15 additions & 13 deletions src/client/opamAuxCommands.ml
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ let opams_of_dir_t files_in_source dir =
let name =
let open OpamStd.Option.Op in
p.pin_name >>+ fun () ->
OpamFile.OPAM.(name_opt (safe_read p.pin.pin_file))
OpamFile.OPAM.name_opt (Lazy.force p.pin_opam)
>>+ fun () ->
match files with
| [] | _::_::_ -> None
Expand Down Expand Up @@ -130,13 +130,14 @@ let name_and_dir_of_opam_file ?locked f =
then OpamFilename.dirname_dir srcdir
else srcdir
in
let opam = lazy (OpamFile.OPAM.safe_read (OpamFile.make f)) in
let name =
let open OpamStd.Option.Op in
OpamPinned.name_of_opam_filename ?locked srcdir f >>+ fun () ->
OpamFile.OPAM.(name_opt (safe_read (OpamFile.make f))) >>+ fun () ->
OpamFile.OPAM.name_opt (Lazy.force opam) >>+ fun () ->
name_from_project_dirname srcdir
in
name, srcdir
name, opam, srcdir

let resolve_locals_pinned st ?(recurse=false) ?subpath atom_or_local_list =
let pinned_packages_of_dir st dir =
Expand Down Expand Up @@ -213,15 +214,16 @@ let resolve_locals ?(quiet=false) ?locked ?recurse ?subpath
if OpamFilename.exists flocked then flocked, locked else f, None
in
match name_and_dir_of_opam_file ?locked f with
| Some n, srcdir ->
| Some n, opam, srcdir ->
{ pin_name = n;
pin_opam = opam;
pin = { pin_file = OpamFile.make f;
pin_locked = locked;
pin_url = target_dir srcdir;
pin_subpath = None;
}} :: to_pin,
(n, None) :: atoms
| None, _ ->
| None, _, _ ->
OpamConsole.error_and_exit `Not_found
"Could not infer package name from package definition file %s"
(OpamFilename.to_string f))
Expand Down Expand Up @@ -306,14 +308,12 @@ let autopin_aux st ?quiet ?(for_view=false) ?recurse ?subpath ?locked
(* For `opam show`, we need to check does the opam file changed to
perform a simulated pin if so *)
(not for_view ||
match
OpamSwitchState.opam_opt st pinned_pkg,
OpamFile.OPAM.read_opt nf.pin.pin_file
with
| Some opam0, Some opam ->
match OpamSwitchState.opam_opt st pinned_pkg with
| Some opam0 ->
let opam = Lazy.force nf.pin_opam in
let opam = OpamFile.OPAM.with_locked_opt nf.pin.pin_locked opam in
OpamFile.OPAM.equal opam0 opam
| _, _ -> false)
| _ -> false)
with Not_found -> false)
to_pin
in
Expand All @@ -338,12 +338,13 @@ let simulate_local_pinnings ?quiet ?(for_view=false) st to_pin =
let local_opams =
List.fold_left (fun map pin ->
let { pin_name = name;
pin_opam = lazy opam;
pin = { pin_file = file; pin_locked = locked;
pin_subpath = subpath; pin_url = target }} = pin
in
match
OpamPinCommand.read_opam_file_for_pinning
?locked ?quiet name file target
?locked ?quiet ~opam name file target
with
| None -> map
| Some opam ->
Expand Down Expand Up @@ -464,12 +465,13 @@ let autopin st ?(simulate=false) ?quiet ?locked ?recurse ?subpath
try
List.fold_left (fun (st, pins) pin ->
let { pin_name = name;
pin_opam = lazy opam;
pin = { pin_file = file; pin_locked = locked;
pin_subpath = subpath; pin_url = target }} = pin
in
match
OpamPinCommand.read_opam_file_for_pinning
?locked ?quiet name file target
?locked ?quiet ~opam name file target
with
| None -> st, pins
| Some opam ->
Expand Down
4 changes: 3 additions & 1 deletion src/client/opamAuxCommands.mli
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ val url_with_local_branch: url -> url

(** From an in-source opam file, return the corresponding package name if it can
be found, and the corresponding source directory *)
val name_and_dir_of_opam_file: ?locked:string -> filename -> name option * dirname
val name_and_dir_of_opam_file:
?locked:string -> filename ->
name option * OpamFile.OPAM.t Lazy.t * dirname

(** From a directory, retrieve its opam files and returns packages name, opam
file and subpath option *)
Expand Down
5 changes: 3 additions & 2 deletions src/client/opamPinCommand.ml
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,13 @@ let string_of_pinned ?(subpath_prefix=true) opam =
url)
(bold (OpamPackage.Version.to_string (OpamFile.OPAM.version opam)))

let read_opam_file_for_pinning ?locked ?(quiet=false) name f url =
let read_opam_file_for_pinning
?locked ?(quiet=false) name f ?(opam = OpamFile.OPAM.safe_read f) url =
let opam0 =
let dir = OpamFilename.dirname (OpamFile.filename f) in
let opam =
(OpamFormatUpgrade.opam_file_with_aux ~quiet ~dir ~files:false
~filename:f) (OpamFile.OPAM.safe_read f)
~filename:f) opam
in
if opam = OpamFile.OPAM.empty then None else Some opam
in
Expand Down
3 changes: 2 additions & 1 deletion src/client/opamPinCommand.mli
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ val parse_pins:
'files/' subdir (unless the file is directly below the specified, local
[url]), and returns it *)
val read_opam_file_for_pinning:
?locked:string -> ?quiet:bool -> name -> OpamFile.OPAM.t OpamFile.t -> url ->
?locked:string -> ?quiet:bool -> name -> OpamFile.OPAM.t OpamFile.t ->
?opam:OpamFile.OPAM.t -> url ->
OpamFile.OPAM.t option

(** The default version for pinning a package: depends on the state, what is
Expand Down
4 changes: 3 additions & 1 deletion src/state/opamPinned.ml
Original file line number Diff line number Diff line change
Expand Up @@ -226,9 +226,11 @@ let files_in_source ?locked ?(recurse=false) ?subpath d =
(* Ignore empty files *)
if (Unix.stat (OpamFilename.to_string f)).Unix.st_size = 0 then None
else
let file = OpamFile.make f in
Some { pin_name = name_of_opam_filename ?locked d f;
pin_opam = lazy (OpamFile.OPAM.safe_read file);
pin = {
pin_file = OpamFile.make f;
pin_file = file;
pin_locked = locked;
pin_subpath =
OpamStd.Option.map OpamFilename.SubPath.of_string subpath;
Expand Down
1 change: 1 addition & 0 deletions src/state/opamStateTypes.mli
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ type 'url _topin_opamfile = {
}
type ('name, 'url) _topin_name_and_opamfile = {
pin_name: 'name;
pin_opam: OpamFile.OPAM.t Lazy.t;
pin: 'url _topin_opamfile;
}
(**/**)
Expand Down

0 comments on commit 36113c8

Please sign in to comment.