From 58d11ef80a8e42cddc03be4081d7e40699afa79e Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Mon, 22 Jun 2020 18:29:48 -0400 Subject: [PATCH] Progress towards passing FileTable or FileID or FileName --- src/Runner.Worker/ActionManager.cs | 2 ++ src/Runner.Worker/ActionManifestManager.cs | 19 +++++++++++++----- src/Runner.Worker/ExecutionContext.cs | 1 + src/Runner.Worker/Handlers/ScriptHandler.cs | 4 +++- .../ObjectTemplating/TemplateContext.cs | 17 ++++++++++++---- .../ObjectTemplating/TemplateEvaluator.cs | 7 ++++--- .../PipelineTemplateEvaluator.cs | 20 ++++++++++++++++--- 7 files changed, 54 insertions(+), 16 deletions(-) diff --git a/src/Runner.Worker/ActionManager.cs b/src/Runner.Worker/ActionManager.cs index 6acbb837a..5a4345aee 100644 --- a/src/Runner.Worker/ActionManager.cs +++ b/src/Runner.Worker/ActionManager.cs @@ -1280,6 +1280,8 @@ namespace GitHub.Runner.Worker public override ActionExecutionType ExecutionType => ActionExecutionType.Script; public override bool HasPre => false; public override bool HasPost => false; + + // TODO: Maybe add FileID here to pass to ScriptHandler for more helpful error message? } public sealed class CompositeActionExecutionData : ActionExecutionData diff --git a/src/Runner.Worker/ActionManifestManager.cs b/src/Runner.Worker/ActionManifestManager.cs index 3f6835aa6..255b0ac9c 100644 --- a/src/Runner.Worker/ActionManifestManager.cs +++ b/src/Runner.Worker/ActionManifestManager.cs @@ -100,7 +100,7 @@ namespace GitHub.Runner.Worker break; case "runs": - actionDefinition.Execution = ConvertRuns(executionContext, context, actionPair.Value, envComposite, fileId); + actionDefinition.Execution = ConvertRuns(executionContext, context, actionPair.Value, fileId, envComposite); break; default: @@ -235,7 +235,7 @@ namespace GitHub.Runner.Worker var context = CreateContext(executionContext, extraExpressionValues); try { - var evaluateResult = TemplateEvaluator.Evaluate(context, "runs-env", token, 0, fileID, omitHeader: true); + var evaluateResult = TemplateEvaluator.Evaluate(context, "runs-env", token, 0, fileID, omitHeader: false); context.Errors.Check(); // Mapping @@ -349,9 +349,12 @@ namespace GitHub.Runner.Worker IExecutionContext executionContext, TemplateContext context, TemplateToken inputsToken, - MappingToken envComposite = null, - Int32 fileID = default(Int32)) + Int32 fileID, + MappingToken envComposite = null + ) { + Trace.Info($"COMPOSITE ACTIONS FILEID: {fileID}"); + var runsMapping = inputsToken.AssertMapping("runs"); var usingToken = default(StringToken); var imageToken = default(StringToken); @@ -415,9 +418,15 @@ namespace GitHub.Runner.Worker case "steps": if (!string.IsNullOrEmpty(Environment.GetEnvironmentVariable("TESTING_COMPOSITE_ACTIONS_ALPHA"))) { + Trace.Info($"COMPOSITE ACTIONS FILEID inside steps case: {fileID}"); + Trace.Info($"COMPOSITE ACTIONS file name inside steps case {context.GetFileName(fileID)}"); + // File name is loaded correctly!! + // WHY DOESN'T THE CONTEXT GET PASED DOWN CORRECTLY? var steps = run.Value.AssertSequence("steps"); var evaluator = executionContext.ToPipelineTemplateEvaluator(); - stepsLoaded = evaluator.LoadCompositeSteps(steps, fileID); + // stepsLoaded = evaluator.LoadCompositeSteps(steps, context.GetFileTable(), fileID); + string fileName = context.GetFileName(fileID); + stepsLoaded = evaluator.LoadCompositeSteps(steps, fileName); break; } throw new Exception("You aren't supposed to be using Composite Actions yet!"); diff --git a/src/Runner.Worker/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs index b859c913a..09de26d86 100644 --- a/src/Runner.Worker/ExecutionContext.cs +++ b/src/Runner.Worker/ExecutionContext.cs @@ -275,6 +275,7 @@ namespace GitHub.Runner.Worker { // TODO: For UI purposes, look at figuring out how to condense steps in one node => maybe use the same previous GUID var newGuid = Guid.NewGuid(); + // TODO: maybe have to use record id to figure out the right place to place the error message? step.ExecutionContext = Root.CreateChild(newGuid, step.DisplayName, newGuid.ToString("N"), null, null); step.ExecutionContext.ExpressionValues["inputs"] = inputsData; diff --git a/src/Runner.Worker/Handlers/ScriptHandler.cs b/src/Runner.Worker/Handlers/ScriptHandler.cs index 051cd5fc7..8fcd8480a 100644 --- a/src/Runner.Worker/Handlers/ScriptHandler.cs +++ b/src/Runner.Worker/Handlers/ScriptHandler.cs @@ -291,7 +291,9 @@ namespace GitHub.Runner.Worker.Handlers // Error if (exitCode != 0) { - ExecutionContext.Error($"Process completed with exit code {exitCode}."); + // TODO: Maybe add FileID here to pass to ScriptHandler for more helpful error message? + // ExecutionContext.Error($"Process completed with exit code {exitCode} erroring in {GetFileName(Data.FileID)}"); + ExecutionContext.Error($"Process completed with exit code {exitCode}"); ExecutionContext.Result = TaskResult.Failed; } } diff --git a/src/Sdk/DTObjectTemplating/ObjectTemplating/TemplateContext.cs b/src/Sdk/DTObjectTemplating/ObjectTemplating/TemplateContext.cs index af8cf76ec..5ca06c5f5 100644 --- a/src/Sdk/DTObjectTemplating/ObjectTemplating/TemplateContext.cs +++ b/src/Sdk/DTObjectTemplating/ObjectTemplating/TemplateContext.cs @@ -137,9 +137,11 @@ namespace GitHub.DistributedTask.ObjectTemplating Int32? fileId, Int32? line, Int32? column, - Exception ex) + Exception ex, + String fileName = default(string)) { - var prefix = GetErrorPrefix(fileId, line, column); + TraceWriter.Info($"Error fileId: {fileId}"); + var prefix = GetErrorPrefix(fileId, line, column, fileName: fileName); Errors.Add(prefix, ex); TraceWriter.Error(prefix, ex); } @@ -203,9 +205,16 @@ namespace GitHub.DistributedTask.ObjectTemplating private String GetErrorPrefix( Int32? fileId, Int32? line, - Int32? column) + Int32? column, + string fileName = default(string)) { - var fileName = fileId.HasValue ? GetFileName(fileId.Value) : null; + TraceWriter.Info($"GetErrorPrefix FileID: {fileId}"); + + // if (String.IsNullOrEmpty(fileName)) { + // fileName = fileId.HasValue ? GetFileName(fileId.Value) : null; + // } + // var fileName = fileId.HasValue ? GetFileName(fileId.Value) : null; + fileName = "TESTING"; if (!String.IsNullOrEmpty(fileName)) { if (line != null && column != null) diff --git a/src/Sdk/DTObjectTemplating/ObjectTemplating/TemplateEvaluator.cs b/src/Sdk/DTObjectTemplating/ObjectTemplating/TemplateEvaluator.cs index 915fc3cbc..a27636d0c 100644 --- a/src/Sdk/DTObjectTemplating/ObjectTemplating/TemplateEvaluator.cs +++ b/src/Sdk/DTObjectTemplating/ObjectTemplating/TemplateEvaluator.cs @@ -28,10 +28,11 @@ namespace GitHub.DistributedTask.ObjectTemplating TemplateToken template, Int32 removeBytes, Int32? fileId, - Boolean omitHeader = false) + Boolean omitHeader = false, + String fileName = default(string)) { TemplateToken result; - + context.TraceWriter.Info("I'm in Evaluate!"); if (!omitHeader) { if (fileId != null) @@ -67,7 +68,7 @@ namespace GitHub.DistributedTask.ObjectTemplating } catch (Exception ex) { - context.Error(fileId, null, null, ex); + context.Error(fileId, null, null, ex, fileName: fileName); result = null; } diff --git a/src/Sdk/DTPipelines/Pipelines/ObjectTemplating/PipelineTemplateEvaluator.cs b/src/Sdk/DTPipelines/Pipelines/ObjectTemplating/PipelineTemplateEvaluator.cs index a54e382fd..8f15e7364 100644 --- a/src/Sdk/DTPipelines/Pipelines/ObjectTemplating/PipelineTemplateEvaluator.cs +++ b/src/Sdk/DTPipelines/Pipelines/ObjectTemplating/PipelineTemplateEvaluator.cs @@ -159,19 +159,33 @@ namespace GitHub.DistributedTask.Pipelines.ObjectTemplating return result; } + // public List LoadCompositeSteps( + // TemplateToken token, + // IReadOnlyList fileTable, + // Int32 fileID) + // { public List LoadCompositeSteps( TemplateToken token, - Int32 fileID - ) + String fileName) { + // HOW DO WE LOG INFORMATION IN THIS FILE?????? var result = default(List); if (token != null && token.Type != TokenType.Null) { + // Create New Context Object var context = CreateContext(null, null, setMissingContext: false); + + // Pass original filetable to the context: + // foreach (var f in fileTable) { + // context.GetFileId(f); + // } + // TODO: we might want to to have a bool to prevent it from filling in with missing context w/ dummy variables + + // TODO: context does not have the fileTable associated with it!!! try { - token = TemplateEvaluator.Evaluate(context, PipelineTemplateConstants.StepsInTemplate, token, 0, fileID, omitHeader: true); + token = TemplateEvaluator.Evaluate(context, PipelineTemplateConstants.StepsInTemplate, token, 0, null, omitHeader: false, fileName: fileName); context.Errors.Check(); result = PipelineTemplateConverter.ConvertToSteps(context, token); }