-
Notifications
You must be signed in to change notification settings - Fork 586
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
Seperate runInternal & handle error #2140
Merged
Merged
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,6 +54,10 @@ and [<NoComparison>] [<NoEquality>] TargetContext = | |
x.PreviousTargets |> List.tryFind (fun t -> t.Target.Name = name) | ||
member x.TryFindTarget name = | ||
x.AllExecutingTargets |> List.tryFind (fun t -> t.Name = name) | ||
member x.ErrorTargets = | ||
x.PreviousTargets |> List.choose (fun tres -> match tres.Error with | ||
| Some er -> Some (er, tres.Target) | ||
| None -> None) | ||
|
||
and [<NoComparison>] [<NoEquality>] TargetParameter = | ||
{ TargetInfo : Target | ||
|
@@ -446,17 +450,13 @@ module Target = | |
aligned "Total:" total null | ||
if not context.HasError then | ||
aligned "Status:" "Ok" null | ||
//Trace.setBuildState TagStatus.Success | ||
else | ||
alignedError "Status:" "Failure" null | ||
//Trace.setBuildState TagStatus.Failed | ||
else | ||
Trace.traceError "No target was successfully completed" | ||
//Trace.setBuildState TagStatus.Warning | ||
|
||
Trace.traceLine() | ||
|
||
|
||
/// Determines a parallel build order for the given set of targets | ||
let internal determineBuildOrder (target : string) = | ||
let _ = get target | ||
|
@@ -652,6 +652,11 @@ module Target = | |
|> Observable.subscribe (fun _ -> Environment.Exit 1) | ||
Process.killAllCreatedProcesses() |> ignore | ||
cts.Cancel() | ||
|
||
/// Optional `TargetContext` | ||
type OptionalTargetContext = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this can be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because it is returned, it can't be |
||
| Set of TargetContext | ||
| MaybeSet of TargetContext option | ||
|
||
/// Runs a target and its dependencies. | ||
let internal runInternal singleTarget parallelJobs targetName args = | ||
|
@@ -705,29 +710,12 @@ module Target = | |
|
||
if context.HasError && not context.CancellationToken.IsCancellationRequested then | ||
runBuildFailureTargets context | ||
else context | ||
else | ||
context | ||
|
||
let context = runFinalTargets {context with IsRunningFinalTargets=true} | ||
writeTaskTimeSummary watch.Elapsed context | ||
if context.HasError && not context.CancellationToken.IsCancellationRequested then | ||
let errorTargets = | ||
context.PreviousTargets | ||
|> List.choose (fun tres -> | ||
match tres.Error with | ||
| Some er -> Some (er, tres.Target) | ||
| None -> None) | ||
let targets = errorTargets |> Seq.map (fun (_er, target) -> target.Name) |> Seq.distinct | ||
let targetStr = String.Join(", ", targets) | ||
let errorMsg = | ||
if errorTargets.Length = 1 then | ||
sprintf "Target '%s' failed." targetStr | ||
else | ||
sprintf "Targets '%s' failed." targetStr | ||
let inner = AggregateException(AggregateException().Message, errorTargets |> Seq.map fst) | ||
BuildFailedException(context, errorMsg, inner) | ||
|> raise | ||
|
||
context | ||
context | ||
|
||
/// Creates a target in case of build failure (not activated). | ||
let createBuildFailure name body = | ||
|
@@ -759,13 +747,40 @@ module Target = | |
let t = get name // test if target is defined | ||
getFinalTargets().[name] <- false | ||
|
||
/// Runs a target and its dependencies, used for testing - usually not called in scripts. | ||
let internal getBuildFailedException (context:TargetContext) = | ||
let targets = context.ErrorTargets |> Seq.map (fun (_er, target) -> target.Name) |> Seq.distinct | ||
let targetStr = String.Join(", ", targets) | ||
let errorMsg = | ||
if context.ErrorTargets.Length = 1 then | ||
sprintf "Target '%s' failed." targetStr | ||
else | ||
sprintf "Targets '%s' failed." targetStr | ||
let inner = AggregateException(AggregateException().Message, context.ErrorTargets |> Seq.map fst) | ||
BuildFailedException(context, errorMsg, inner) | ||
|
||
let private getTargetContext (context:OptionalTargetContext) = | ||
match context with | ||
| Set c -> Some(c) | ||
| MaybeSet c -> c | ||
|
||
/// If `TargetContext option` is Some and has error, raise it as a BuildFailedException | ||
let raiseIfError (context:OptionalTargetContext) = | ||
let c = getTargetContext(context) | ||
if c.IsSome && c.Value.HasError && not c.Value.CancellationToken.IsCancellationRequested then | ||
getBuildFailedException c.Value | ||
|> raise | ||
context | ||
|
||
/// Runs a target and its dependencies and returns a `TargetContext` | ||
let runAndGetContext parallelJobs targetName args = runInternal false parallelJobs targetName args | ||
|
||
/// Runs a target and its dependencies | ||
let run parallelJobs targetName args = runInternal false parallelJobs targetName args |> ignore | ||
/// Runs a target and its dependencies and returns an `OptionalTargetContext` | ||
let runAndGetOptionalContext parallelJobs targetName args = runAndGetContext parallelJobs targetName args |> OptionalTargetContext.Set | ||
|
||
let internal runWithDefault allowArgs fDefault = | ||
/// Runs a target and its dependencies | ||
let run parallelJobs targetName args = runAndGetOptionalContext parallelJobs targetName args |> raiseIfError |> ignore | ||
|
||
let internal getRunFunction allowArgs defaultTarget = | ||
let ctx = Fake.Core.Context.forceFakeContext () | ||
let trySplitEnvArg (arg:string) = | ||
let idx = arg.IndexOf('=') | ||
|
@@ -790,11 +805,14 @@ module Target = | |
|
||
if DocoptResult.hasFlag "--list" results then | ||
listAvailable() | ||
None | ||
elif DocoptResult.hasFlag "-h" results || DocoptResult.hasFlag "--help" results then | ||
printfn "%s" TargetCli.targetCli | ||
printfn "Hint: Run 'fake run <build.fsx> target <target> --help' to get help from your target." | ||
None | ||
elif DocoptResult.hasFlag "--version" results then | ||
printfn "Target Module Version: %s" AssemblyVersionInformation.AssemblyInformationalVersion | ||
None | ||
else | ||
let target = | ||
match DocoptResult.tryGetArgument "<target>" results with | ||
|
@@ -833,23 +851,40 @@ module Target = | |
| None -> [] | ||
if not allowArgs && arguments <> [] then | ||
failwithf "The following arguments could not be parsed: %A\nTo forward arguments to your targets you need to use \nTarget.runOrDefaultWithArguments instead of Target.runOrDefault" arguments | ||
match target with | ||
| Some t -> runInternal singleTarget parallelJobs t arguments |> ignore | ||
| None -> fDefault singleTarget parallelJobs arguments | ||
match target, defaultTarget with | ||
| Some t, _ -> Some(fun () -> Some(runInternal singleTarget parallelJobs t arguments)) | ||
| None, Some t -> Some(fun () -> Some(runInternal singleTarget parallelJobs t arguments)) | ||
| None, None -> Some (fun () -> listAvailable() | ||
None) | ||
| Choice2Of2 e -> | ||
// To ensure exit code. | ||
raise <| exn (sprintf "Usage error: %s\n%s" e.Message TargetCli.targetCli, e) | ||
|
||
let private runFunction (targetFunction:(unit -> TargetContext option) Option) = | ||
match targetFunction with | ||
| Some f -> OptionalTargetContext.MaybeSet(f()) | ||
| _ -> OptionalTargetContext.MaybeSet(None) | ||
|
||
/// Runs the command given on the command line or the given target when no target is given & get context | ||
let runOrDefaultAndGetContext defaultTarget = | ||
getRunFunction false (Some(defaultTarget)) |> runFunction | ||
|
||
/// Runs the command given on the command line or the given target when no target is given | ||
let runOrDefault defaultTarget = | ||
runWithDefault false (fun singleTarget parallelJobs arguments -> | ||
runInternal singleTarget parallelJobs defaultTarget arguments |> ignore) | ||
runOrDefaultAndGetContext defaultTarget |> raiseIfError |> ignore | ||
|
||
/// Runs the command given on the command line or the given target when no target is given & get context | ||
let runOrDefaultWithArgumentsAndGetContext defaultTarget = | ||
getRunFunction true (Some(defaultTarget)) |> runFunction | ||
|
||
/// Runs the command given on the command line or the given target when no target is given | ||
let runOrDefaultWithArguments defaultTarget = | ||
runWithDefault true (fun singleTarget parallelJobs arguments -> | ||
runInternal singleTarget parallelJobs defaultTarget arguments |> ignore) | ||
runOrDefaultWithArgumentsAndGetContext defaultTarget |> raiseIfError |> ignore | ||
|
||
/// Runs the target given by the target parameter or lists the available targets & get context | ||
let runOrListAndGetContext() = | ||
getRunFunction false None |> runFunction | ||
|
||
/// Runs the target given by the target parameter or lists the available targets | ||
let runOrList() = | ||
runWithDefault false (fun _ _ _ -> listAvailable()) | ||
runOrListAndGetContext() |> raiseIfError |> ignore |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
raiseIfError
as there is noraiseIfErrorOption
?