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.

Sign up for GitHub

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

Move 'can_go_in_shared_cache' from Action to Rule, with the same default #10789

Closed
wants to merge 1 commit into from
Closed

Move 'can_go_in_shared_cache' from Action to Rule, with the same default #10789

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
26 changes: 4 additions & 22 deletions src/dune_engine/action.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,24 +328,17 @@ module Full = struct
{ action : t
; env : Env.t
; locks : Path.t list
; can_go_in_shared_cache : bool
; sandbox : Sandbox_config.t
}

let empty =
{ action = Progn []
; env = Env.empty
; locks = []
; can_go_in_shared_cache = true
; sandbox = Sandbox_config.default
}
{ action = Progn []; env = Env.empty; locks = []; sandbox = Sandbox_config.default }
;;

let combine { action; env; locks; can_go_in_shared_cache; sandbox } x =
let combine { action; env; locks; sandbox } x =
{ action = combine action x.action
; env = Env.extend_env env x.env
; locks = locks @ x.locks
; can_go_in_shared_cache = can_go_in_shared_cache && x.can_go_in_shared_cache
; sandbox = Sandbox_config.inter sandbox x.sandbox
}
;;
Expand All @@ -354,23 +347,12 @@ module Full = struct
include T
include Monoid.Make (T)

let make
?(env = Env.empty)
?(locks = [])
?(can_go_in_shared_cache = true)
?(sandbox = Sandbox_config.default)
action
=
{ action; env; locks; can_go_in_shared_cache; sandbox }
let make ?(env = Env.empty) ?(locks = []) ?(sandbox = Sandbox_config.default) action =
{ action; env; locks; sandbox }
;;

let map t ~f = { t with action = f t.action }
let add_env e t = { t with env = Env.extend_env t.env e }
let add_locks l t = { t with locks = t.locks @ l }

let add_can_go_in_shared_cache b t =
{ t with can_go_in_shared_cache = t.can_go_in_shared_cache && b }
;;

let add_sandbox s t = { t with sandbox = Sandbox_config.inter t.sandbox s }
end
3 changes: 0 additions & 3 deletions src/dune_engine/action.mli
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,14 +144,12 @@ module Full : sig
{ action : action
; env : Env.t
; locks : Path.t list
; can_go_in_shared_cache : bool
; sandbox : Sandbox_config.t
}

val make
: ?env:Env.t (** default [Env.empty] *)
-> ?locks:Path.t list (** default [[]] *)
-> ?can_go_in_shared_cache:bool (** default [true] *)
-> ?sandbox:Sandbox_config.t (** default [Sandbox_config.default] *)
-> action
-> t
Expand All @@ -168,7 +166,6 @@ module Full : sig
val add_env : Env.t -> t -> t
val add_locks : Path.t list -> t -> t
val add_sandbox : Sandbox_config.t -> t -> t
val add_can_go_in_shared_cache : bool -> t -> t

include Monoid with type t := t
end
19 changes: 9 additions & 10 deletions src/dune_engine/build_system.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,6 @@ end = struct
let { Action.Full.action
; env
; locks
; can_go_in_shared_cache
; sandbox = _ (* already taken into account in [sandbox_mode] *)
}
=
Expand All @@ -273,7 +272,7 @@ end = struct
, Dep.Facts.digest facts ~env
, target_paths rule.targets.files @ target_paths rule.targets.dirs
, Action.for_shell action
, can_go_in_shared_cache
, true (* TODO: don't forget to remove this *)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO comment intentionnaly left here, as removing the true in the tuple changes the digest, and trips the tests. I wanted to make a separate commit that only removes this line and changes the hash to make it clean for the CI.

, List.map locks ~f:Path.to_string
, Execution_parameters.action_stdout_on_success execution_parameters
, Execution_parameters.action_stderr_on_success execution_parameters
Expand Down Expand Up @@ -336,9 +335,7 @@ end = struct
: Exec_result.t Fiber.t
=
let open Fiber.O in
let { Action.Full.action; env; locks; can_go_in_shared_cache = _; sandbox = _ } =
action
in
let { Action.Full.action; env; locks; sandbox = _ } = action in
let* dune_stats = Scheduler.stats () in
let deps =
Dep.Facts.paths
Expand Down Expand Up @@ -457,7 +454,9 @@ end = struct
;;

let execute_rule_impl ~rule_kind rule =
let { Rule.id = _; targets; mode; action; info = _; loc } = rule in
let { Rule.id = _; targets; mode; action; info = _; loc; can_go_in_shared_cache } =
rule
in
(* We run [State.start_rule_exn ()] entirely for its side effect, so one
might be tempted to use [Memo.of_non_reproducible_fiber] here but that is
wrong, because that would force us to rerun [execute_rule_impl] on every
Expand Down Expand Up @@ -534,7 +533,7 @@ end = struct
compute_rule_digest rule ~facts ~action ~sandbox_mode ~execution_parameters
in
let can_go_in_shared_cache =
action.can_go_in_shared_cache
can_go_in_shared_cache
&& (not
(always_rerun
|| is_action_dynamic
Expand Down Expand Up @@ -699,6 +698,7 @@ end = struct
Rule.make
~info:(if Loc.is_none loc then Internal else From_dune_file loc)
~targets:(Targets.File.create target)
~can_go_in_shared_cache:true
~mode:Standard
(Action_builder.record act.action deps ~f:build_dep)
in
Expand Down Expand Up @@ -758,8 +758,7 @@ end = struct
let observing_facts = () in
ignore observing_facts;
let digest =
let { Rule.Anonymous_action.action =
{ action; env; locks; can_go_in_shared_cache; sandbox }
let { Rule.Anonymous_action.action = { action; env; locks; sandbox }
; loc = _
; dir
; alias
Expand Down Expand Up @@ -791,7 +790,7 @@ end = struct
, dir
, alias
, capture_stdout
, can_go_in_shared_cache
, true (* TODO: don't forget to remove this *)
, sandbox )
in
(* It might seem superfluous to memoize the execution here, given that a
Expand Down
11 changes: 9 additions & 2 deletions src/dune_engine/rule.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ module T = struct
; mode : Mode.t
; info : Info.t
; loc : Loc.t
; can_go_in_shared_cache : bool
}

let compare a b = Id.compare a.id b.id
Expand All @@ -69,7 +70,13 @@ end
include T
include Comparable.Make (T)

let make ?(mode = Mode.Standard) ?(info = Info.Internal) ~targets action =
let make
?(mode = Mode.Standard)
?(info = Info.Internal)
?(can_go_in_shared_cache = true)
~targets
action
=
let action = Action_builder.memoize "Rule.make" action in
let report_error ?(extra_pp = []) message =
match info with
Expand Down Expand Up @@ -106,7 +113,7 @@ let make ?(mode = Mode.Standard) ?(info = Info.Internal) ~targets action =
(Path.build (Path.Build.relative targets.root "_unknown_")))
| Source_file_copy p -> Loc.in_file (Path.source p)
in
{ id = Id.gen (); targets; action; mode; info; loc }
{ id = Id.gen (); targets; action; mode; info; loc; can_go_in_shared_cache }
;;

let set_action t action =
Expand Down
2 changes: 2 additions & 0 deletions src/dune_engine/rule.mli
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ type t = private
; mode : Mode.t
; info : Info.t
; loc : Loc.t
; can_go_in_shared_cache : bool
}

include Comparable_intf.S with type key := t
Expand All @@ -72,6 +73,7 @@ val to_dyn : t -> Dyn.t
val make
: ?mode:Mode.t
-> ?info:Info.t
-> ?can_go_in_shared_cache:bool (** default [true]*)
-> targets:Targets.t
-> Action.Full.t Action_builder.t
-> t
Expand Down
2 changes: 1 addition & 1 deletion src/dune_rules/fetch_rules.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ let gen_rules_for_checksum_or_url (loc_url, (url : OpamUrl.t)) checksum =
let rule =
let info = Rule.Info.of_loc_opt (Some loc_url) in
fun { Action_builder.With_targets.build; targets } ->
Rules.Produce.rule (Rule.make ~info ~targets build)
Rules.Produce.rule (Rule.make ~info ~targets ~can_go_in_shared_cache:false build)
in
let make_target = make_target checksum_or_url in
let action ~target ~kind =
Expand Down
Loading