Skip to content

Commit

Permalink
refactor: move [status_map] to own module (ocaml#9825)
Browse files Browse the repository at this point in the history
Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
  • Loading branch information
rgrinberg authored Jan 24, 2024
1 parent 8305a5b commit a379928
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 55 deletions.
6 changes: 3 additions & 3 deletions src/dune_rules/source_tree.ml
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ end = struct
let status ~status_map ~(parent_status : Source_dir_status.t) dir
: Source_dir_status.t option
=
match Sub_dirs.status status_map ~dir with
match Sub_dirs.Status_map.status status_map ~dir with
| Ignored -> None
| Status status ->
Some
Expand All @@ -269,7 +269,7 @@ end = struct

let physical ~dir ~dirs_visited ~dirs ~sub_dirs ~parent_status =
let status_map =
Sub_dirs.eval sub_dirs ~dirs:(List.map ~f:(fun (a, _) -> a) dirs)
Sub_dirs.Status_map.eval sub_dirs ~dirs:(List.map ~f:(fun (a, _) -> a) dirs)
in
List.fold_left
dirs
Expand Down Expand Up @@ -297,7 +297,7 @@ end = struct
| Some (df : Dune_file.t) ->
(* Virtual directories are not in [Readdir.t]. Their presence is only *)
let dirs = Sub_dirs.Dir_map.sub_dirs df.plain.for_subdirs in
let status_map = Sub_dirs.eval sub_dirs ~dirs in
let status_map = Sub_dirs.Status_map.eval sub_dirs ~dirs in
List.fold_left dirs ~init ~f:(fun acc fn ->
match status ~status_map ~parent_status fn with
| None -> acc
Expand Down
91 changes: 46 additions & 45 deletions src/dune_rules/sub_dirs.ml
Original file line number Diff line number Diff line change
@@ -1,11 +1,4 @@
open! Import
module Stanza = Dune_lang.Stanza

let status status_by_dir ~dir : Source_dir_status.Or_ignored.t =
match Filename.Map.find status_by_dir dir with
| None -> Ignored
| Some d -> Status d
;;

let default =
let standard_dirs = Predicate_lang.Glob.of_glob (Glob.of_string "[!._]*") in
Expand Down Expand Up @@ -35,46 +28,54 @@ let make ~dirs ~data_only ~ignored_sub_dirs ~vendored_dirs =
{ Source_dir_status.Map.normal = dirs; data_only; vendored = vendored_dirs }
;;

type status_map = Source_dir_status.t Filename.Map.t
module Status_map = struct
type t = Source_dir_status.t Filename.Map.t

let eval (t : _ Source_dir_status.Map.t) ~dirs =
(* This function defines the unexpected behavior of: (dirs foo)
(data_only_dirs bar)
let status status_by_dir ~dir : Source_dir_status.Or_ignored.t =
match Filename.Map.find status_by_dir dir with
| None -> Ignored
| Some d -> Status d
;;

In this setup, bar is actually ignored rather than being data only. Because
it was excluded from the total set of directories. *)
Filename.Set.of_list dirs
|> Filename.Set.to_map ~f:(fun _ -> ())
|> Filename.Map.filter_mapi ~f:(fun dir () : Source_dir_status.t option ->
let statuses =
Source_dir_status.Map.merge t default ~f:(fun pred standard ->
Predicate_lang.Glob.test pred ~standard dir)
|> Source_dir_status.Set.to_list
in
match statuses with
| [] -> None
| statuses ->
(* If a directory has a status other than [Normal], then the [Normal]
status is irrelevant so we just filter it out. *)
(match
List.filter statuses ~f:(function
| Source_dir_status.Normal -> false
| _ -> true)
with
| [] -> Some Normal
| [ status ] -> Some status
| statuses ->
(* CR-rgrinberg: this error needs a location *)
User_error.raise
[ Pp.textf
"Directory %s was marked as %s, it can't be marked as %s."
dir
(String.enumerate_and (List.map statuses ~f:Source_dir_status.to_string))
(match List.length statuses with
| 2 -> "both"
| _ -> "all these")
]))
;;
let eval (t : _ Source_dir_status.Map.t) ~dirs =
(* This function defines the unexpected behavior of: (dirs foo)
(data_only_dirs bar)
In this setup, bar is actually ignored rather than being data only. Because
it was excluded from the total set of directories. *)
Filename.Set.of_list dirs
|> Filename.Set.to_map ~f:(fun _ -> ())
|> Filename.Map.filter_mapi ~f:(fun dir () : Source_dir_status.t option ->
let statuses =
Source_dir_status.Map.merge t default ~f:(fun pred standard ->
Predicate_lang.Glob.test pred ~standard dir)
|> Source_dir_status.Set.to_list
in
match statuses with
| [] -> None
| statuses ->
(* If a directory has a status other than [Normal], then the [Normal]
status is irrelevant so we just filter it out. *)
(match
List.filter statuses ~f:(function
| Source_dir_status.Normal -> false
| _ -> true)
with
| [] -> Some Normal
| [ status ] -> Some status
| statuses ->
(* CR-rgrinberg: this error needs a location *)
User_error.raise
[ Pp.textf
"Directory %s was marked as %s, it can't be marked as %s."
dir
(String.enumerate_and (List.map statuses ~f:Source_dir_status.to_string))
(match List.length statuses with
| 2 -> "both"
| _ -> "all these")
]))
;;
end

type subdir_stanzas = (Loc.t * Predicate_lang.Glob.t) option Source_dir_status.Map.t

Expand Down
12 changes: 5 additions & 7 deletions src/dune_rules/sub_dirs.mli
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,12 @@ type subdir_stanzas
val or_default : subdir_stanzas -> Predicate_lang.Glob.t Source_dir_status.Map.t
val default : Predicate_lang.Glob.t Source_dir_status.Map.t

type status_map

val eval
: Predicate_lang.Glob.t Source_dir_status.Map.t
-> dirs:Filename.t list
-> status_map
module Status_map : sig
type t

val status : status_map -> dir:Filename.t -> Source_dir_status.Or_ignored.t
val eval : Predicate_lang.Glob.t Source_dir_status.Map.t -> dirs:Filename.t list -> t
val status : t -> dir:Filename.t -> Source_dir_status.Or_ignored.t
end

module Dir_map : sig
type t
Expand Down

0 comments on commit a379928

Please sign in to comment.