From 5ffa1cbc125a1e79d4dde2c64203a4ca7530fbf9 Mon Sep 17 00:00:00 2001 From: Chris Blyth Date: Thu, 5 Jul 2018 10:45:03 +0100 Subject: [PATCH 1/4] Fix target results to show result based on the last target not all previous --- src/app/Fake.Core.Target/Target.fs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/app/Fake.Core.Target/Target.fs b/src/app/Fake.Core.Target/Target.fs index e36a4e0047d..63f4dda50ee 100644 --- a/src/app/Fake.Core.Target/Target.fs +++ b/src/app/Fake.Core.Target/Target.fs @@ -50,6 +50,10 @@ and [] [] TargetContext = member x.HasError = x.PreviousTargets |> List.exists (fun t -> t.Error.IsSome) + member x.LastTargetError = + x.PreviousTargets + |> List.last + |> fun x -> x.Error.IsSome member x.TryFindPrevious name = x.PreviousTargets |> List.tryFind (fun t -> t.Target.Name = name) member x.TryFindTarget name = @@ -320,7 +324,7 @@ module Target = let target = get name use t = Trace.traceFinalTarget target.Name (match target.Description with Some d -> d | _ -> null) (dependencyString target) let res = runSimpleContextInternal target context - if res.HasError + if res.LastTargetError then t.MarkFailed() else t.MarkSuccess() res @@ -336,7 +340,7 @@ module Target = let target = get name use t = Trace.traceFailureTarget target.Name (match target.Description with Some d -> d | _ -> null) (dependencyString target) let res = runSimpleContextInternal target context - if res.HasError + if res.LastTargetError then t.MarkFailed() else t.MarkSuccess() res @@ -508,7 +512,7 @@ module Target = if not context.HasError then use t = Trace.traceTarget target.Name (match target.Description with Some d -> d | _ -> null) (dependencyString target) let res = runSimpleContextInternal target context - if res.HasError + if res.LastTargetError then t.MarkFailed() else t.MarkSuccess() res From 74e60c9b7283c633027315ca7f8f0a85b65ef3fe Mon Sep 17 00:00:00 2001 From: Chris Blyth Date: Thu, 5 Jul 2018 10:45:48 +0100 Subject: [PATCH 2/4] Tweak trace logic to allow target traces to pass an option But maintain non breaking change for all other tag tracing --- src/app/Fake.Core.Target/Target.fs | 6 +++--- src/app/Fake.Core.Trace/Trace.fs | 20 +++++++++++++------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/app/Fake.Core.Target/Target.fs b/src/app/Fake.Core.Target/Target.fs index 63f4dda50ee..f1479e37b34 100644 --- a/src/app/Fake.Core.Target/Target.fs +++ b/src/app/Fake.Core.Target/Target.fs @@ -322,7 +322,7 @@ module Target = |> Seq.map (fun kv -> kv.Key) |> Seq.fold (fun context name -> let target = get name - use t = Trace.traceFinalTarget target.Name (match target.Description with Some d -> d | _ -> null) (dependencyString target) + use t = Trace.traceFinalTarget target.Name target.Description (dependencyString target) let res = runSimpleContextInternal target context if res.LastTargetError then t.MarkFailed() @@ -338,7 +338,7 @@ module Target = |> Seq.map (fun kv -> kv.Key) |> Seq.fold (fun context name -> let target = get name - use t = Trace.traceFailureTarget target.Name (match target.Description with Some d -> d | _ -> null) (dependencyString target) + use t = Trace.traceFailureTarget target.Name target.Description (dependencyString target) let res = runSimpleContextInternal target context if res.LastTargetError then t.MarkFailed() @@ -510,7 +510,7 @@ module Target = /// Runs a single target without its dependencies... only when no error has been detected yet. let internal runSingleTarget (target : Target) (context:TargetContext) = if not context.HasError then - use t = Trace.traceTarget target.Name (match target.Description with Some d -> d | _ -> null) (dependencyString target) + use t = Trace.traceTarget target.Name target.Description (dependencyString target) let res = runSimpleContextInternal target context if res.LastTargetError then t.MarkFailed() diff --git a/src/app/Fake.Core.Trace/Trace.fs b/src/app/Fake.Core.Trace/Trace.fs index d0fcd0e595d..f025e13a2cd 100644 --- a/src/app/Fake.Core.Trace/Trace.fs +++ b/src/app/Fake.Core.Trace/Trace.fs @@ -124,10 +124,11 @@ let traceHeader name = traceLine() /// Puts an opening tag on the internal tag stack -let openTagUnsafe tag (description:string) = +let openTagUnsafe tag description = let sw = System.Diagnostics.Stopwatch.StartNew() openTags.Value <- (sw, tag) :: openTags.Value - TraceData.OpenTag(tag, if System.String.IsNullOrEmpty description then None else Some description) |> CoreTracing.postMessage + let descriptionOption = if System.String.IsNullOrEmpty description then None else Some description + TraceData.OpenTag(tag, descriptionOption) |> CoreTracing.postMessage type ISafeDisposable = inherit System.IDisposable @@ -225,20 +226,25 @@ let traceEndTargetUnsafe name = [] let traceEndTarget name = traceEndTargetUnsafe name +let private optionDescriptionToNullable description = + match description with + | Some d -> d + | _ -> null + let traceTarget name description dependencyString = - traceStartTargetUnsafe name description dependencyString + traceStartTargetUnsafe name (optionDescriptionToNullable description) dependencyString asSafeDisposable (fun state -> traceEndTargetUnsafeEx state name) let traceFinalTarget name description dependencyString = - traceStartFinalTargetUnsafe name description dependencyString + traceStartFinalTargetUnsafe name (optionDescriptionToNullable description) dependencyString asSafeDisposable (fun state -> traceEndFinalTargetUnsafeEx state name) let traceFailureTarget name description dependencyString = - traceStartFailureTargetUnsafe name description dependencyString + traceStartFailureTargetUnsafe name (optionDescriptionToNullable description) dependencyString asSafeDisposable (fun state -> traceEndFailureTargetUnsafeEx state name) /// Traces the begin of a task -let traceStartTaskUnsafe task (description:string) = +let traceStartTaskUnsafe task description = openTagUnsafe (KnownTags.Task task) description /// Traces the begin of a task @@ -257,7 +263,7 @@ let traceEndTaskUnsafe task = traceEndTaskUnsafeEx TagStatus.Success task let traceEndTask task = traceEndTaskUnsafe task /// Wrap functions in a 'use' of this function -let traceTask name (description:string) = +let traceTask name description = traceStartTaskUnsafe name description asSafeDisposable (fun state -> traceEndTaskUnsafeEx state name) From 4482bbb7150002bbf1ee2f4c28780abc987f1ad9 Mon Sep 17 00:00:00 2001 From: Chris Blyth Date: Thu, 5 Jul 2018 13:34:52 +0100 Subject: [PATCH 3/4] Fixes following code review --- src/app/Fake.Core.Target/Target.fs | 71 ++++++++++++------------------ src/app/Fake.Core.Trace/Trace.fs | 11 ++--- 2 files changed, 30 insertions(+), 52 deletions(-) diff --git a/src/app/Fake.Core.Target/Target.fs b/src/app/Fake.Core.Target/Target.fs index f1479e37b34..22ff1bd3367 100644 --- a/src/app/Fake.Core.Target/Target.fs +++ b/src/app/Fake.Core.Target/Target.fs @@ -50,10 +50,6 @@ and [] [] TargetContext = member x.HasError = x.PreviousTargets |> List.exists (fun t -> t.Error.IsSome) - member x.LastTargetError = - x.PreviousTargets - |> List.last - |> fun x -> x.Error.IsSome member x.TryFindPrevious name = x.PreviousTargets |> List.tryFind (fun t -> t.Target.Name = name) member x.TryFindTarget name = @@ -70,6 +66,10 @@ and [] [] Target = SoftDependencies: string list; Description: TargetDescription option; Function : TargetParameter -> unit} + member x.DescriptionAsString = + match x.Description with + | Some d -> d + | _ -> null /// Exception for request errors #if !NETSTANDARD1_6 @@ -178,6 +178,14 @@ module Target = Trace.traceError <| sprintf " - %s" target.Value.Name failwithf "Target \"%s\" is not defined." name + /// Returns the DependencyString for the given target. + let internal dependencyString target = + if target.Dependencies.IsEmpty then String.Empty else + target.Dependencies + |> Seq.map (fun d -> (get d).Name) + |> String.separated ", " + |> sprintf "(==> %s)" + let internal runSimpleInternal context target = let watch = System.Diagnostics.Stopwatch.StartNew() let error = @@ -189,11 +197,16 @@ module Target = with e -> Some e watch.Stop() { Error = error; Time = watch.Elapsed; Target = target; WasSkipped = false } - let internal runSimpleContextInternal target context = + + let internal runSimpleContextInternal target context (traceStart: string -> string -> string -> Trace.ISafeDisposable) = + use t = traceStart target.Name target.DescriptionAsString (dependencyString target) let result = runSimpleInternal context target + if result.Error.IsSome then + t.MarkFailed() + else + t.MarkSuccess() { context with PreviousTargets = context.PreviousTargets @ [result] } - /// This simply runs the function of a target without doing anything (like tracing, stopwatching or adding it to the results at the end) let runSimple name args = let target = get name @@ -206,14 +219,6 @@ module Target = target |> runSimpleInternal ctx - /// Returns the DependencyString for the given target. - let internal dependencyString target = - if target.Dependencies.IsEmpty then String.Empty else - target.Dependencies - |> Seq.map (fun d -> (get d).Name) - |> String.separated ", " - |> sprintf "(==> %s)" - /// Returns the soft DependencyString for the given target. let internal softDependencyString target = if target.SoftDependencies.IsEmpty then String.Empty else @@ -266,7 +271,6 @@ module Target = getTargetDict().[targetName] <- { target with Dependencies = target.Dependencies @ [dependentTargetName] } - /// Appends the dependency to the list of soft dependencies. /// [omit] let internal softDependencyAtEnd targetName dependentTargetName = @@ -312,39 +316,23 @@ module Target = addTarget template name /// Creates a Target. - let create name body = addTargetWithDependencies [] body name + let create name body = addTargetWithDependencies [] body name /// Runs all activated final targets (in alphabetically order). /// [omit] let internal runFinalTargets context = getFinalTargets() - |> Seq.filter (fun kv -> kv.Value) // only if activated - |> Seq.map (fun kv -> kv.Key) - |> Seq.fold (fun context name -> - let target = get name - use t = Trace.traceFinalTarget target.Name target.Description (dependencyString target) - let res = runSimpleContextInternal target context - if res.LastTargetError - then t.MarkFailed() - else t.MarkSuccess() - res - ) context + |> Seq.filter (fun kv -> kv.Value) // only if activated + |> Seq.map (fun kv -> get kv.Key) + |> Seq.fold (fun context target -> runSimpleContextInternal target context Trace.traceFinalTarget) context /// Runs all build failure targets. /// [omit] let internal runBuildFailureTargets (context) = getBuildFailureTargets() - |> Seq.filter (fun kv -> kv.Value) // only if activated - |> Seq.map (fun kv -> kv.Key) - |> Seq.fold (fun context name -> - let target = get name - use t = Trace.traceFailureTarget target.Name target.Description (dependencyString target) - let res = runSimpleContextInternal target context - if res.LastTargetError - then t.MarkFailed() - else t.MarkSuccess() - res - ) context + |> Seq.filter (fun kv -> kv.Value) // only if activated + |> Seq.map (fun kv -> get kv.Key) + |> Seq.fold (fun context target -> runSimpleContextInternal target context Trace.traceFailureTarget) context /// List all targets available. let listAvailable() = @@ -510,12 +498,7 @@ module Target = /// Runs a single target without its dependencies... only when no error has been detected yet. let internal runSingleTarget (target : Target) (context:TargetContext) = if not context.HasError then - use t = Trace.traceTarget target.Name target.Description (dependencyString target) - let res = runSimpleContextInternal target context - if res.LastTargetError - then t.MarkFailed() - else t.MarkSuccess() - res + runSimpleContextInternal target context Trace.traceTarget else { context with PreviousTargets = context.PreviousTargets @ [{ Error = None; Time = TimeSpan.Zero; Target = target; WasSkipped = true }] } diff --git a/src/app/Fake.Core.Trace/Trace.fs b/src/app/Fake.Core.Trace/Trace.fs index f025e13a2cd..068e5d364ce 100644 --- a/src/app/Fake.Core.Trace/Trace.fs +++ b/src/app/Fake.Core.Trace/Trace.fs @@ -226,21 +226,16 @@ let traceEndTargetUnsafe name = [] let traceEndTarget name = traceEndTargetUnsafe name -let private optionDescriptionToNullable description = - match description with - | Some d -> d - | _ -> null - let traceTarget name description dependencyString = - traceStartTargetUnsafe name (optionDescriptionToNullable description) dependencyString + traceStartTargetUnsafe name description dependencyString asSafeDisposable (fun state -> traceEndTargetUnsafeEx state name) let traceFinalTarget name description dependencyString = - traceStartFinalTargetUnsafe name (optionDescriptionToNullable description) dependencyString + traceStartFinalTargetUnsafe name description dependencyString asSafeDisposable (fun state -> traceEndFinalTargetUnsafeEx state name) let traceFailureTarget name description dependencyString = - traceStartFailureTargetUnsafe name (optionDescriptionToNullable description) dependencyString + traceStartFailureTargetUnsafe name description dependencyString asSafeDisposable (fun state -> traceEndFailureTargetUnsafeEx state name) /// Traces the begin of a task From e99340207570eae798a80ff469c1aaa4c2d208f2 Mon Sep 17 00:00:00 2001 From: Chris Blyth Date: Thu, 5 Jul 2018 14:18:40 +0100 Subject: [PATCH 4/4] Tweaks to make the code look nicer --- src/app/Fake.Core.Target/Target.fs | 10 +++++----- src/app/Fake.Core.Trace/Trace.fs | 3 +-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/app/Fake.Core.Target/Target.fs b/src/app/Fake.Core.Target/Target.fs index 22ff1bd3367..24edef92ebc 100644 --- a/src/app/Fake.Core.Target/Target.fs +++ b/src/app/Fake.Core.Target/Target.fs @@ -198,7 +198,7 @@ module Target = watch.Stop() { Error = error; Time = watch.Elapsed; Target = target; WasSkipped = false } - let internal runSimpleContextInternal target context (traceStart: string -> string -> string -> Trace.ISafeDisposable) = + let internal runSimpleContextInternal (traceStart: string -> string -> string -> Trace.ISafeDisposable) context target = use t = traceStart target.Name target.DescriptionAsString (dependencyString target) let result = runSimpleInternal context target if result.Error.IsSome then @@ -316,7 +316,7 @@ module Target = addTarget template name /// Creates a Target. - let create name body = addTargetWithDependencies [] body name + let create name body = addTargetWithDependencies [] body name /// Runs all activated final targets (in alphabetically order). /// [omit] @@ -324,7 +324,7 @@ module Target = getFinalTargets() |> Seq.filter (fun kv -> kv.Value) // only if activated |> Seq.map (fun kv -> get kv.Key) - |> Seq.fold (fun context target -> runSimpleContextInternal target context Trace.traceFinalTarget) context + |> Seq.fold (fun context target -> runSimpleContextInternal Trace.traceFinalTarget context target) context /// Runs all build failure targets. /// [omit] @@ -332,7 +332,7 @@ module Target = getBuildFailureTargets() |> Seq.filter (fun kv -> kv.Value) // only if activated |> Seq.map (fun kv -> get kv.Key) - |> Seq.fold (fun context target -> runSimpleContextInternal target context Trace.traceFailureTarget) context + |> Seq.fold (fun context target -> runSimpleContextInternal Trace.traceFailureTarget context target) context /// List all targets available. let listAvailable() = @@ -498,7 +498,7 @@ module Target = /// Runs a single target without its dependencies... only when no error has been detected yet. let internal runSingleTarget (target : Target) (context:TargetContext) = if not context.HasError then - runSimpleContextInternal target context Trace.traceTarget + runSimpleContextInternal Trace.traceTarget context target else { context with PreviousTargets = context.PreviousTargets @ [{ Error = None; Time = TimeSpan.Zero; Target = target; WasSkipped = true }] } diff --git a/src/app/Fake.Core.Trace/Trace.fs b/src/app/Fake.Core.Trace/Trace.fs index 068e5d364ce..c23b43d50ea 100644 --- a/src/app/Fake.Core.Trace/Trace.fs +++ b/src/app/Fake.Core.Trace/Trace.fs @@ -127,8 +127,7 @@ let traceHeader name = let openTagUnsafe tag description = let sw = System.Diagnostics.Stopwatch.StartNew() openTags.Value <- (sw, tag) :: openTags.Value - let descriptionOption = if System.String.IsNullOrEmpty description then None else Some description - TraceData.OpenTag(tag, descriptionOption) |> CoreTracing.postMessage + TraceData.OpenTag(tag, if System.String.IsNullOrEmpty description then None else Some description) |> CoreTracing.postMessage type ISafeDisposable = inherit System.IDisposable