From 611f09bb7bbeaf8d7fa6fc93442e81aa9644130e Mon Sep 17 00:00:00 2001 From: Merlyn Oppenheim Date: Fri, 11 Apr 2025 18:32:31 -0700 Subject: [PATCH 1/3] remove common/tasklib from LocalPackages because it made build really slow. (todo) break into seperate build config --- BuildConfigGen/Program.cs | 2 +- make.js | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/BuildConfigGen/Program.cs b/BuildConfigGen/Program.cs index 45915de2913..d2bf2298c11 100644 --- a/BuildConfigGen/Program.cs +++ b/BuildConfigGen/Program.cs @@ -52,7 +52,7 @@ public record ConfigRecord(string name, string constMappingKey, bool isDefault, public static readonly ConfigRecord Node20_229_14 = new ConfigRecord(name: nameof(Node20_229_14), constMappingKey: "Node20_229_14", isDefault: false, isNode: true, nodePackageVersion: "^20.3.1", isWif: false, nodeHandler: "Node20_1", preprocessorVariableName: "NODE20", enableBuildConfigOverrides: true, deprecated: false, shouldUpdateTypescript: true, overriddenDirectoryName: "Node20", writeNpmrc: true, mergeToBase: true); public static readonly ConfigRecord WorkloadIdentityFederation = new ConfigRecord(name: nameof(WorkloadIdentityFederation), constMappingKey: "WorkloadIdentityFederation", isDefault: false, isNode: true, nodePackageVersion: "^16.11.39", isWif: true, nodeHandler: "Node16", preprocessorVariableName: "WORKLOADIDENTITYFEDERATION", enableBuildConfigOverrides: true, deprecated: false, shouldUpdateTypescript: false, writeNpmrc: false); public static readonly ConfigRecord wif_242 = new ConfigRecord(name: nameof(wif_242), constMappingKey: "wif_242", isDefault: false, isNode: true, nodePackageVersion: "^20.3.1", isWif: true, nodeHandler: "Node20_1", preprocessorVariableName: "WIF", enableBuildConfigOverrides: true, deprecated: false, shouldUpdateTypescript: true, overriddenDirectoryName: "Wif", writeNpmrc: true); - public static readonly ConfigRecord LocalPackages = new ConfigRecord(name: nameof(LocalPackages), constMappingKey: "LocalPackages", isDefault: false, isNode: true, nodePackageVersion: "^20.3.1", isWif: false, nodeHandler: "Node20_1", preprocessorVariableName: "NODE20", enableBuildConfigOverrides: true, deprecated: false, shouldUpdateTypescript: true, overriddenDirectoryName: "LocalPackages", writeNpmrc: true, shouldUpdateLocalPkgs: true, useGlobalVersion: true, useAltGeneratedPath: true); + public static readonly ConfigRecord LocalPackages = new ConfigRecord(name: nameof(LocalPackages), constMappingKey: "LocalPackages", isDefault: false, isNode: true, nodePackageVersion: "^20.3.1", isWif: false, nodeHandler: "Node20_1", preprocessorVariableName: "NODE20", enableBuildConfigOverrides: true, deprecated: false, shouldUpdateTypescript: true, overriddenDirectoryName: "LocalPackages", writeNpmrc: true, shouldUpdateLocalPkgs: false, useGlobalVersion: true, useAltGeneratedPath: true); public static ConfigRecord[] Configs = { Default, Node16, Node16_225, Node20, Node20_228, Node20_229_1, Node20_229_2, Node20_229_3, Node20_229_4, Node20_229_5, Node20_229_6, Node20_229_7, Node20_229_8, Node20_229_9, Node20_229_10, Node20_229_11, Node20_229_12, Node20_229_13, Node20_229_14, WorkloadIdentityFederation, wif_242, LocalPackages }; } diff --git a/make.js b/make.js index 0ce83bf345d..f8ae98a06ed 100644 --- a/make.js +++ b/make.js @@ -263,6 +263,7 @@ CLI.serverBuild = async function(/** @type {{ task: string }} */ argv) { { if (!argv.skipPrebuildSteps) { + /* // temp: clone for now prior to merging these as subtrees if (!test('-d', 'task-lib')) { run("git clone https://github.com/microsoft/azure-pipelines-task-lib task-lib"); @@ -282,15 +283,17 @@ CLI.serverBuild = async function(/** @type {{ task: string }} */ argv) { // build task-lib cd(taskLibPath); - run("npm install", /*inheritStreams:*/true); - run("node make.js build", /*inheritStreams:*/true); + run("npm install", true); + run("node make.js build", true); + - await util.installNodeAsync('20'); // build task-lib cd(tasksCommonPath); - run("npm install", /*inheritStreams:*/true); - run("node make.js --build", /*inheritStreams:*/true); + run("npm install", true); + run("node make.js --build", true); + */ + } } From ef2ccd4233544eccf81d66621a604b3bfce92ab8 Mon Sep 17 00:00:00 2001 From: Merlyn Oppenheim Date: Fri, 11 Apr 2025 18:36:03 -0700 Subject: [PATCH 2/3] fixes for globalversion bumping: 1. global version wouldn't be bumped if task didn't have mapping file (now if task.json is modified, we check if version has changed to initiate global bump) 2. global version would always set patch to 0 when changing sprints -- should be the "maxpatch" --- BuildConfigGen/GitUtil.cs | 67 +++++++++++++++++++++++++++++ BuildConfigGen/Program.cs | 79 +++++++++++++++++++++++++++-------- BuildConfigGen/TaskVersion.cs | 5 +++ 3 files changed, 134 insertions(+), 17 deletions(-) diff --git a/BuildConfigGen/GitUtil.cs b/BuildConfigGen/GitUtil.cs index 2aad907bf27..2cba877f958 100644 --- a/BuildConfigGen/GitUtil.cs +++ b/BuildConfigGen/GitUtil.cs @@ -211,5 +211,72 @@ public static string FixupPath(string s) return s; } + + internal static bool HasUncommitedChanges(string filePath) + { + // this function generated by Co-pilot + + if (!File.Exists(filePath)) + { + throw new FileNotFoundException($"The file '{filePath}' does not exist."); + } + + string directory = Path.GetDirectoryName(filePath) ?? throw new Exception("Unable to determine the directory of the file."); + string relativePath = FixupPath(filePath); + + string[] output; + int exitCode = ProcessUtil.RunCommandWithExitCode("git", $"status --porcelain \"{relativePath}\"", directory, out output); + + if (exitCode != 0) + { + throw new Exception("Failed to check git status. Non-zero exit code."); + } + + // If the output contains any lines, it means there are uncommitted changes + return output.Length > 0; + } + + internal static string GetUnchangedContent(string filePath) + { + // this function generated by Co-pilot + + if (!File.Exists(filePath)) + { + throw new FileNotFoundException($"The file '{filePath}' does not exist."); + } + + string directory = Path.GetDirectoryName(filePath) ?? throw new Exception("Unable to determine the directory of the file."); + string relativePath = GetGitPath(filePath); + + string[] output; + int exitCode = ProcessUtil.RunCommandWithExitCode("git", $"show HEAD:\"{relativePath}\"", directory, out output); + + if (exitCode != 0) + { + throw new Exception("Failed to retrieve unchanged content. Non-zero exit code."); + } + + return string.Join(Environment.NewLine, output); + } + + private static string GetGitPath(string filePath) + { + // this function generated by Co-pilot + + if (!File.Exists(filePath)) + { + throw new FileNotFoundException($"The file '{filePath}' does not exist."); + } + + string directory = Path.GetDirectoryName(filePath) ?? throw new Exception("Unable to determine the directory of the file."); + string gitRoot = RunGitCommandScalar(directory, "rev-parse --show-toplevel"); + + if (!filePath.StartsWith(FixupPath(gitRoot))) + { + throw new Exception($"The file '{filePath}' is not within the Git repository root '{gitRoot}'."); + } + + return filePath.Substring(gitRoot.Length + 1).Replace(Path.DirectorySeparatorChar, '/'); + } } } diff --git a/BuildConfigGen/Program.cs b/BuildConfigGen/Program.cs index d2bf2298c11..e0facb7f5e4 100644 --- a/BuildConfigGen/Program.cs +++ b/BuildConfigGen/Program.cs @@ -140,7 +140,7 @@ private static void MainInner(string? task, string? configs, int? currentSprintN ? new NoDebugConfigGenerator() : new VsCodeLaunchConfigGenerator(gitRootPath, debugAgentDir); - int maxPatchForCurrentSprint = 0; + int maxPatchForCurrentSprint = -1; int currentSprint; if (currentSprintNullable.HasValue) @@ -183,7 +183,7 @@ private static void MainInner(string? task, string? configs, int? currentSprintN taskVersionInfo.Add(t.Value.Name, new TaskStateStruct()); IEnumerable configsList = FilterConfigsForTask(configs, t); HashSet targetConfigs = GetConfigRecords(configsList, writeUpdates); - UpdateVersionsForTask(t.Value.Name, taskVersionInfo[t.Value.Name], targetConfigs, currentSprint, globalVersionPath, globalVersion, generatedFolder); + UpdateVersionsForTask(t.Value.Name, taskVersionInfo[t.Value.Name], targetConfigs, currentSprint, globalVersionPath, globalVersion, generatedFolder, includeUpdatesForTasksWithoutVersionMap: true); bool taskTargettedForUpdating = allTasks || tasks.Where(x => x.Key == t.Value.Name).Any(); bool taskVersionsNeedUpdating = taskVersionInfo[t.Value.Name].versionsUpdated.Any(); @@ -225,7 +225,7 @@ private static void MainInner(string? task, string? configs, int? currentSprintN taskVersionInfo.Add(t.Value.Name, new TaskStateStruct()); IEnumerable configsList = FilterConfigsForTask(configs, t); HashSet targetConfigs = GetConfigRecords(configsList, writeUpdates); - UpdateVersionsForTask(t.Value.Name, taskVersionInfo[t.Value.Name], targetConfigs, currentSprint, globalVersionPath, globalVersion, generatedFolder); + UpdateVersionsForTask(t.Value.Name, taskVersionInfo[t.Value.Name], targetConfigs, currentSprint, globalVersionPath, globalVersion, generatedFolder, includeUpdatesForTasksWithoutVersionMap: false); CheckForDuplicates(t.Value.Name, taskVersionInfo[t.Value.Name].configTaskVersionMapping, checkGlobalVersion: true); } } @@ -254,7 +254,7 @@ private static void MainInner(string? task, string? configs, int? currentSprintN else { // this could fail if there is a task with a future-sprint version, which should not be the case. If that happens, CheckForDuplicates will throw - globalVersion = globalVersion.CloneWithMinorAndPatch(currentSprint, 0); + globalVersion = globalVersion.CloneWithMinorAndPatch(currentSprint, maxPatchForCurrentSprint); globalVersion = globalVersion.CloneWithMajor(taskMajorVersion); } } @@ -1244,7 +1244,7 @@ private static void CopyConfig(string gitRootPath, string taskTargetPathOrUnders } } - private static void UpdateVersionsForTask(string task, TaskStateStruct taskState, HashSet targetConfigs, int currentSprint, string globalVersionPath, TaskVersion? globalVersion, string generatedFolder) + private static void UpdateVersionsForTask(string task, TaskStateStruct taskState, HashSet targetConfigs, int currentSprint, string globalVersionPath, TaskVersion? globalVersion, string generatedFolder, bool includeUpdatesForTasksWithoutVersionMap) { string currentDir = Environment.CurrentDirectory; string gitRootPath = GetTasksRootPath(currentDir); @@ -1408,10 +1408,7 @@ private static void UpdateVersionsForTask(string task, TaskStateStruct taskState // if mergeToBase config existed previously, remove it if (old.TryGetValue(config, out var oldVersion)) { - if (!taskState.versionsUpdated.Contains(config)) - { - taskState.versionsUpdated.Add(config); - } + taskState.versionsUpdated.Add(config); } } else @@ -1427,14 +1424,55 @@ private static void UpdateVersionsForTask(string task, TaskStateStruct taskState taskState.configTaskVersionMapping.Add(config, targetVersion); - if (!taskState.versionsUpdated.Contains(config)) - { - taskState.versionsUpdated.Add(config); - } + taskState.versionsUpdated.Add(config); } } } } + + // make this conditional because HasTaskVersionChanged is expensive + if (includeUpdatesForTasksWithoutVersionMap) + { + if (!taskState.versionsUpdated.Any()) + { + // we'll get here if there is a task with no mapping file, so without checking git, we don't know if the task version has changed + // we'll check git on the base task to see if the task version changed (e.g. HEAD vs uncommited change) + + if (HasTaskVersionChanged(taskTargetPath)) + { + taskState.versionsUpdated.Add(Config.Default); + } + } + } + } + + private static bool HasTaskVersionChanged(string taskTargetPath) + { + string taskJsonPath = Path.Combine(taskTargetPath, "task.json"); + + if (!File.Exists(taskJsonPath)) + { + throw new Exception($"Task file not found: {taskJsonPath}"); + } + + if (GitUtil.HasUncommitedChanges(taskJsonPath)) + { + var unchangedContent = GitUtil.GetUnchangedContent(taskJsonPath); + + JsonNode taskJson = JsonNode.Parse(unchangedContent)!; + int major = taskJson["version"]!["Major"]!.GetValue(); + int minor = taskJson["version"]!["Minor"]!.GetValue(); + int patch = taskJson["version"]!["Patch"]!.GetValue(); + + TaskVersion versionInUnChangedTaskJson = new TaskVersion(major, minor, patch); + TaskVersion versionInChangedTaskJson = GetInputVersion(taskTargetPath); + + return !versionInUnChangedTaskJson.Equals(versionInChangedTaskJson); + } + else + { + return false; + } } private static void UpdateMaxPatchForSprint(TaskStateStruct taskState, int currentSprint, ref int maxPatchForCurrentSprint) @@ -1718,14 +1756,21 @@ private static JsonNode GetNodePath(JsonNode node, string path0, params string[] } - // todo rename - as this is not a struct. (refactor in a seperate PR) + // todo rename - as this is not a struct. (refactor/cleanup in a seperate PR) internal class TaskStateStruct { // todo - fix case public Dictionary configTaskVersionMapping { get; } - // todo - fix case - public HashSet versionsUpdated { get; } + private readonly HashSet _versionsUpdated; + + public HashSet versionsUpdated + { + get + { + return _versionsUpdated; + } + } public bool OnlyHasDefaultOrGlobalVersion { @@ -1751,7 +1796,7 @@ public bool OnlyHasDefaultOrGlobalVersion public TaskStateStruct() { this.configTaskVersionMapping = []; - this.versionsUpdated = []; + this._versionsUpdated = []; } } } diff --git a/BuildConfigGen/TaskVersion.cs b/BuildConfigGen/TaskVersion.cs index bf34a98e675..8b2a550f783 100644 --- a/BuildConfigGen/TaskVersion.cs +++ b/BuildConfigGen/TaskVersion.cs @@ -36,6 +36,11 @@ private TaskVersion(TaskVersion taskVersionToClone) public TaskVersion(int major, int minor, int overidePatch) { + if (overidePatch < 0) + { + throw new Exception($"bug, overridePatch must be >=0, got {overidePatch}"); + } + Major = major; Minor = minor; Patch = overidePatch; From 6930f34022e53a01e4a30de52cd8749e183a4ea3 Mon Sep 17 00:00:00 2001 From: Merlyn Oppenheim Date: Tue, 15 Apr 2025 19:09:41 -0700 Subject: [PATCH 3/3] do not block build when tasks other than the one specified via --task [taskname]] need updates fix for ensure task ouput is updated for shouldUpdateLocalPkgs and useAltGeneratedPath fix for shouldUpdateLocalPkgs not checking if package.json dependencies are up-to-date in buildoverride --- BuildConfigGen/Program.cs | 77 +++++++++++++++++++++++++++++++++------ 1 file changed, 66 insertions(+), 11 deletions(-) diff --git a/BuildConfigGen/Program.cs b/BuildConfigGen/Program.cs index e0facb7f5e4..f65e2af170a 100644 --- a/BuildConfigGen/Program.cs +++ b/BuildConfigGen/Program.cs @@ -19,6 +19,22 @@ internal class Knob public bool SourceDirectoriesMustContainPlaceHolders { get; init; } } + internal static class ConfigExtensions + { + public static bool UpdatesOuputUnconditionally(this Program.Config.ConfigRecord config) + { + return config.isNode + || config.shouldUpdateLocalPkgs + || config.mergeToBase + || config.useAltGeneratedPath; + } + + public static bool ManagePackageJsonInOverride(this Program.Config.ConfigRecord config) + { + return config.isNode || + config.shouldUpdateLocalPkgs; + } + } internal class Program { private const string filesOverriddenForConfigGoHereReadmeTxt = "FilesOverriddenForConfigGoHereREADME.txt"; @@ -52,7 +68,7 @@ public record ConfigRecord(string name, string constMappingKey, bool isDefault, public static readonly ConfigRecord Node20_229_14 = new ConfigRecord(name: nameof(Node20_229_14), constMappingKey: "Node20_229_14", isDefault: false, isNode: true, nodePackageVersion: "^20.3.1", isWif: false, nodeHandler: "Node20_1", preprocessorVariableName: "NODE20", enableBuildConfigOverrides: true, deprecated: false, shouldUpdateTypescript: true, overriddenDirectoryName: "Node20", writeNpmrc: true, mergeToBase: true); public static readonly ConfigRecord WorkloadIdentityFederation = new ConfigRecord(name: nameof(WorkloadIdentityFederation), constMappingKey: "WorkloadIdentityFederation", isDefault: false, isNode: true, nodePackageVersion: "^16.11.39", isWif: true, nodeHandler: "Node16", preprocessorVariableName: "WORKLOADIDENTITYFEDERATION", enableBuildConfigOverrides: true, deprecated: false, shouldUpdateTypescript: false, writeNpmrc: false); public static readonly ConfigRecord wif_242 = new ConfigRecord(name: nameof(wif_242), constMappingKey: "wif_242", isDefault: false, isNode: true, nodePackageVersion: "^20.3.1", isWif: true, nodeHandler: "Node20_1", preprocessorVariableName: "WIF", enableBuildConfigOverrides: true, deprecated: false, shouldUpdateTypescript: true, overriddenDirectoryName: "Wif", writeNpmrc: true); - public static readonly ConfigRecord LocalPackages = new ConfigRecord(name: nameof(LocalPackages), constMappingKey: "LocalPackages", isDefault: false, isNode: true, nodePackageVersion: "^20.3.1", isWif: false, nodeHandler: "Node20_1", preprocessorVariableName: "NODE20", enableBuildConfigOverrides: true, deprecated: false, shouldUpdateTypescript: true, overriddenDirectoryName: "LocalPackages", writeNpmrc: true, shouldUpdateLocalPkgs: false, useGlobalVersion: true, useAltGeneratedPath: true); + public static readonly ConfigRecord LocalPackages = new ConfigRecord(name: nameof(LocalPackages), constMappingKey: "LocalPackages", isDefault: false, isNode: false, nodePackageVersion: "^20.3.1", isWif: false, nodeHandler: "Node20_1", preprocessorVariableName: "NODE20", enableBuildConfigOverrides: true, deprecated: false, shouldUpdateTypescript: true, overriddenDirectoryName: "LocalPackages", writeNpmrc: true, shouldUpdateLocalPkgs: false, useGlobalVersion: true, useAltGeneratedPath: true); public static ConfigRecord[] Configs = { Default, Node16, Node16_225, Node20, Node20_228, Node20_229_1, Node20_229_2, Node20_229_3, Node20_229_4, Node20_229_5, Node20_229_6, Node20_229_7, Node20_229_8, Node20_229_9, Node20_229_10, Node20_229_11, Node20_229_12, Node20_229_13, Node20_229_14, WorkloadIdentityFederation, wif_242, LocalPackages }; } @@ -152,6 +168,8 @@ private static void MainInner(string? task, string? configs, int? currentSprintN currentSprint = GetCurrentSprint(); } + Console.WriteLine($"Current sprint: {currentSprint}"); + Dictionary taskVersionInfo = []; { @@ -199,7 +217,11 @@ private static void MainInner(string? task, string? configs, int? currentSprintN if (tasksNeedingUpdates.Count > 0) { - throw new Exception($"The following tasks have versions that need updating (needed for updating global version): {string.Join(", ", tasksNeedingUpdates)}. Please run 'node make.js build --task [taskname]' to update"); + Console.WriteLine($"The following tasks have versions that need updating (needed for updating global version); including in list of tasks to update: {string.Join(", ", tasksNeedingUpdates)}."); + + Console.WriteLine("before concat: " + string.Join(",", tasks.Select(x => x.Key).ToArray())); + tasks = ConcatAdditionalTasks(allTasksList: allTasksList, existingTasks: tasks, tasksToAppend: tasksNeedingUpdates); + Console.WriteLine("after concat: " + string.Join(",", tasks.Select(x => x.Key).ToArray())); } // bump patch number for global if any tasks invalidated or if there is no existing global version @@ -248,14 +270,12 @@ private static void MainInner(string? task, string? configs, int? currentSprintN { if (globalVersion.Minor == currentSprint) { - globalVersion = globalVersion.CloneWithMinorAndPatch(currentSprint, Math.Max(maxPatchForCurrentSprint, globalVersion.Patch)); - globalVersion = globalVersion.CloneWithMajor(taskMajorVersion); + globalVersion = new TaskVersion(taskMajorVersion, currentSprint, Math.Max(maxPatchForCurrentSprint, globalVersion.Patch)); } else { // this could fail if there is a task with a future-sprint version, which should not be the case. If that happens, CheckForDuplicates will throw - globalVersion = globalVersion.CloneWithMinorAndPatch(currentSprint, maxPatchForCurrentSprint); - globalVersion = globalVersion.CloneWithMajor(taskMajorVersion); + globalVersion = new TaskVersion(taskMajorVersion, currentSprint, maxPatchForCurrentSprint); } } } @@ -303,6 +323,41 @@ private static void MainInner(string? task, string? configs, int? currentSprintN } } + private static IEnumerable> ConcatAdditionalTasks( + IEnumerable> allTasksList, + IEnumerable> existingTasks, + List tasksToAppend) + { + List> newTasks = new(existingTasks); + + foreach (var taskNeedingUpdates in tasksToAppend) + { + bool taskExists = false; + + foreach (var existingTask in newTasks) + { + if (string.Equals(existingTask.Key, taskNeedingUpdates)) + { + taskExists = true; + break; + } + } + + if (!taskExists) + { + foreach (var taskToAdd in allTasksList) + { + if (string.Equals(taskToAdd.Key, taskNeedingUpdates, StringComparison.OrdinalIgnoreCase)) + { + newTasks.Add(taskToAdd); + } + } + } + } + + return newTasks; + } + private static string GetTasksRootPath(string inputCurrentDir) { string? currentDir = inputCurrentDir; @@ -613,14 +668,14 @@ private static void MainUpdateTask( taskConfigPath = Path.Combine(taskOutput, "task.json"); var taskConfigExists = File.Exists(taskConfigPath); - // only update task output if a new version was added, the config exists, the task contains preprocessor instructions, or the config targets Node (not Default) + // only update task output if a new version was added, the config exists, the task contains preprocessor instructions, or the config updates the output unconditionally // Note: CheckTaskInputContainsPreprocessorInstructions is expensive, so only call if needed if (versionUpdated || taskConfigExists + || config.UpdatesOuputUnconditionally() + || taskVersionState.OnlyHasDefaultOrGlobalVersion || HasTaskInputContainsPreprocessorInstructions(gitRootPath, taskTargetPath, config) - || config.isNode - || config.mergeToBase - || taskVersionState.OnlyHasDefaultOrGlobalVersion) +) { if (config.mergeToBase) { @@ -675,7 +730,7 @@ private static void MainUpdateTask( WriteInputTaskJson(taskTargetPath, taskVersionState.configTaskVersionMapping, "task.json"); WriteInputTaskJson(taskTargetPath, taskVersionState.configTaskVersionMapping, "task.loc.json"); - if (config.isNode) + if (config.ManagePackageJsonInOverride()) { GetBuildConfigFileOverridePaths(config, taskTargetPath, out string configTaskPath, out string readmePath, generatedFolder, task);