From 444526eb8209a9608bf6409c904cb1a35f72680e Mon Sep 17 00:00:00 2001 From: Matthew Asplund Date: Tue, 15 Nov 2022 20:53:43 -0600 Subject: [PATCH] Fix bug missing cache for build graph loader (#151) Co-authored-by: Matthew Asplund --- .../CSharp/ConsoleApplication/PackageLock.sml | 2 +- .../Core/Source/Build/BuildLoadEngine.h | 17 +- .../UnitTests/Build/BuildLoadEngineTests.h | 238 +++++++++++++++++- .../gen/Build/BuildLoadEngineTests.gen.h | 1 + 4 files changed, 250 insertions(+), 8 deletions(-) diff --git a/Samples/CSharp/ConsoleApplication/PackageLock.sml b/Samples/CSharp/ConsoleApplication/PackageLock.sml index b9ae2396..8484d6c3 100644 --- a/Samples/CSharp/ConsoleApplication/PackageLock.sml +++ b/Samples/CSharp/ConsoleApplication/PackageLock.sml @@ -7,7 +7,7 @@ Closures: { } Build0: { CSharp: [ - { Name: "Soup.CSharp", Version: "0.7.0" } + { Name: "Soup.CSharp", Version: "0.7.1" } ] } } \ No newline at end of file diff --git a/Source/Client/Core/Source/Build/BuildLoadEngine.h b/Source/Client/Core/Source/Build/BuildLoadEngine.h index e6764838..c370bf4d 100644 --- a/Source/Client/Core/Source/Build/BuildLoadEngine.h +++ b/Source/Client/Core/Source/Build/BuildLoadEngine.h @@ -72,7 +72,7 @@ namespace Soup::Core PackageLookupMap _packageLookup; // Mapping from build dependency to Package path to graph id - std::map _knownGraphSet; + std::map _knownBuildGraphSet; public: /// @@ -89,7 +89,7 @@ namespace Soup::Core _recipeCache(recipeCache), _packageGraphLookup(), _packageLookup(), - _knownGraphSet() + _knownBuildGraphSet() { } @@ -464,7 +464,7 @@ namespace Soup::Core } else { - Log::Diag("Recipe closure already loaded: " + languagePackageName); + Log::Diag("Recipe already loaded: " + languagePackageName); dependencyTypeProjects.push_back(PackageChildInfo(dependency, false, findKnownPackage->second.first, -1)); } } @@ -546,11 +546,11 @@ namespace Soup::Core } // Check if the package has already been processed from another graph - auto findKnownGraph = _knownGraphSet.find(dependencyProjectRoot); - if (findKnownGraph != _knownGraphSet.end()) + auto findKnownGraph = _knownBuildGraphSet.find(dependencyProjectRoot); + if (findKnownGraph != _knownBuildGraphSet.end()) { // Verify the project name is unique - Log::Diag("Graph closure already loaded: " + dependencyProjectRoot.ToString()); + Log::Diag("Graph already loaded: " + dependencyProjectRoot.ToString()); return PackageChildInfo(dependency, true, -1, findKnownGraph->second); } else @@ -588,6 +588,11 @@ namespace Soup::Core graphId, PackageGraph(graphId, childPackageId, _hostBuildGlobalParameters)); + // Keep track of the build graphs we have already seen + auto insertKnown = _knownBuildGraphSet.emplace( + dependencyProjectRoot, + graphId); + // Update the child project id return PackageChildInfo(dependency, true, -1, graphId); } diff --git a/Source/Client/Core/UnitTests/Build/BuildLoadEngineTests.h b/Source/Client/Core/UnitTests/Build/BuildLoadEngineTests.h index 56b1c698..282435c2 100644 --- a/Source/Client/Core/UnitTests/Build/BuildLoadEngineTests.h +++ b/Source/Client/Core/UnitTests/Build/BuildLoadEngineTests.h @@ -383,7 +383,7 @@ namespace Soup::Core::UnitTests "DIAG: Load Recipe: C:/WorkingDirectory/MyPackage/Recipe.sml", "DIAG: Load Recipe: C:/Users/Me/.soup/packages/C++/PackageA/3.3.3/Recipe.sml", "DIAG: Load Recipe: C:/Users/Me/.soup/packages/C++/PackageB/4.4.4/Recipe.sml", - "DIAG: Recipe closure already loaded: C++|PackageB", + "DIAG: Recipe already loaded: C++|PackageB", }), testListener->GetMessages(), "Verify log messages match expected."); @@ -1348,6 +1348,7 @@ namespace Soup::Core::UnitTests "Verify package graph matches expected."); } + // [[Fact]] void Load_OtherDependency_PackageLock_ExplicitLanguage() { @@ -1539,5 +1540,240 @@ namespace Soup::Core::UnitTests packageProvider, "Verify package graph matches expected."); } + + // [[Fact]] + void Load_RuntimeDependency_PackageLock_ExplicitLanguageOverride_MultipleReferences_ReuseBuildGraph() + { + // Register the test listener + auto testListener = std::make_shared(); + auto scopedTraceListener = ScopedTraceListenerRegister(testListener); + + // Register the test file system + auto fileSystem = std::make_shared(); + auto scopedFileSystem = ScopedFileSystemRegister(fileSystem); + + // Create the Recipe to build + fileSystem->CreateMockFile( + Path("C:/WorkingDirectory/MyPackage/Recipe.sml"), + std::make_shared(std::stringstream(R"( + Name: "MyPackage" + Language: "C#|1" + Dependencies: { + Runtime: [ + "../Package1/" + ] + } + )"))); + + fileSystem->CreateMockFile( + Path("C:/WorkingDirectory/Package1/Recipe.sml"), + std::make_shared(std::stringstream(R"( + Name: "Package1" + Language: "C#|1" + )"))); + + fileSystem->CreateMockFile( + Path("C:/Users/Me/.soup/packages/C#/Soup.CSharp/2.2.3/Recipe.sml"), + std::make_shared(std::stringstream(R"( + Name: "Soup.CSharp" + Language: "C#|1" + )"))); + + // Create the package lock + fileSystem->CreateMockFile( + Path("C:/WorkingDirectory/MyPackage/PackageLock.sml"), + std::make_shared(std::stringstream(R"( + Version: 3 + Closures: { + Root: { + CSharp: [ + { Name: "MyPackage", Version: "../MyPackage/", Build: "Build0" } + { Name: "Package1", Version: "../Package1/", Build: "Build0" } + ] + } + Build0: { + CSharp: [ + { Name: "Soup.CSharp", Version: "2.2.3" } + ] + } + } + )"))); + + // Register the test process manager + auto processManager = std::make_shared(); + auto scopedProcessManager = ScopedProcessManagerRegister(processManager); + + auto builtInLanguages = std::map( + { + { + "C++", + BuiltInLanguagePackage( + "Cpp", + "Soup.Cpp", + SemanticVersion(1, 1, 1), + Path("Soup.Cpp.dll")) + }, + { + "C#", + BuiltInLanguagePackage( + "CSharp", + "Soup.CSharp", + SemanticVersion(2, 2, 2), + Path("Soup.CSharp.dll")) + }, + }); + auto arguments = RecipeBuildArguments(); + arguments.GlobalParameters = ValueTable( + std::map({ + { + "ArgumentValue", + Value(true), + }, + })); + arguments.WorkingDirectory = Path("C:/WorkingDirectory/MyPackage/"); + auto hostBuildGlobalParameters = ValueTable( + std::map({ + { + "HostValue", + Value(true), + }, + })); + auto recipeCache = RecipeCache(); + auto uut = BuildLoadEngine( + builtInLanguages, + arguments, + hostBuildGlobalParameters, + recipeCache); + + auto packageProvider = uut.Load(); + + // Verify expected logs + Assert::AreEqual( + std::vector({ + "DIAG: Load PackageLock: C:/WorkingDirectory/MyPackage/PackageLock.sml", + "INFO: Package lock loaded", + "DIAG: Load Recipe: C:/WorkingDirectory/MyPackage/Recipe.sml", + "DIAG: Load Recipe: C:/WorkingDirectory/Package1/Recipe.sml", + "DIAG: Load Recipe: C:/Users/Me/.soup/packages/C#/Soup.CSharp/2.2.3/Recipe.sml", + "DIAG: Load PackageLock: C:/Users/Me/.soup/locks/C#/Soup.CSharp/2.2.3/PackageLock.sml", + "INFO: PackageLock file does not exist", + "DIAG: Graph already loaded: C:/Users/Me/.soup/packages/C#/Soup.CSharp/2.2.3", + }), + testListener->GetMessages(), + "Verify log messages match expected."); + + // Verify expected file system requests + Assert::AreEqual( + std::vector({ + "Exists: C:/WorkingDirectory/MyPackage/PackageLock.sml", + "OpenReadBinary: C:/WorkingDirectory/MyPackage/PackageLock.sml", + "Exists: C:/WorkingDirectory/MyPackage/Recipe.sml", + "OpenReadBinary: C:/WorkingDirectory/MyPackage/Recipe.sml", + "Exists: C:/WorkingDirectory/Package1/Recipe.sml", + "OpenReadBinary: C:/WorkingDirectory/Package1/Recipe.sml", + "GetCurrentDirectory", + "Exists: C:/Users/Me/.soup/packages/C#/Soup.CSharp/2.2.3/Recipe.sml", + "OpenReadBinary: C:/Users/Me/.soup/packages/C#/Soup.CSharp/2.2.3/Recipe.sml", + "GetCurrentDirectory", + "Exists: C:/Users/Me/.soup/locks/C#/Soup.CSharp/2.2.3/PackageLock.sml", + "GetCurrentDirectory", + }), + fileSystem->GetRequests(), + "Verify file system requests match expected."); + + // Verify expected process requests + Assert::AreEqual( + std::vector({ + "GetCurrentProcessFileName", + }), + processManager->GetRequests(), + "Verify process manager requests match expected."); + + // Verify expected package graph + Assert::AreEqual( + PackageProvider( + 1, + PackageGraphLookupMap( + { + { + 1, + PackageGraph( + 1, + 1, + ValueTable( + std::map({ + { + "ArgumentValue", + Value(true), + }, + }))) + }, + { + 2, + PackageGraph( + 2, + 3, + ValueTable( + std::map({ + { + "HostValue", + Value(true), + }, + }))) + }, + }), + PackageLookupMap( + { + { + 1, + PackageInfo( + 1, + Path("C:/WorkingDirectory/MyPackage/"), + recipeCache.GetRecipe(Path("C:/WorkingDirectory/MyPackage/Recipe.sml")), + std::nullopt, + PackageChildrenMap({ + { + "Build", + { + PackageChildInfo(PackageReference(std::nullopt, "Soup.CSharp", SemanticVersion(2, 2, 3)), true, -1, 2), + } + }, + { + "Runtime", + { + PackageChildInfo(PackageReference(Path("../Package1/")), false, 2, -1), + } + }, + })) + }, + { + 2, + PackageInfo( + 2, + Path("C:/WorkingDirectory/Package1/"), + recipeCache.GetRecipe(Path("C:/WorkingDirectory/Package1/Recipe.sml")), + std::nullopt, + PackageChildrenMap({ + { + "Build", + { + PackageChildInfo(PackageReference(std::nullopt, "Soup.CSharp", SemanticVersion(2, 2, 3)), true, -1, 2), + } + }, + })) + }, + { + 3, + PackageInfo( + 3, + Path("C:/Users/Me/.soup/packages/C#/Soup.CSharp/2.2.3"), + recipeCache.GetRecipe(Path("C:/Users/Me/.soup/packages/C#/Soup.CSharp/2.2.3/Recipe.sml")), + Path("C:/testlocation/Extensions/Soup.CSharp/2.2.2/Soup.CSharp.dll"), + PackageChildrenMap()) + }, + })), + packageProvider, + "Verify package graph matches expected."); + } }; } diff --git a/Source/Client/Core/UnitTests/gen/Build/BuildLoadEngineTests.gen.h b/Source/Client/Core/UnitTests/gen/Build/BuildLoadEngineTests.gen.h index a77743ad..96278dc7 100644 --- a/Source/Client/Core/UnitTests/gen/Build/BuildLoadEngineTests.gen.h +++ b/Source/Client/Core/UnitTests/gen/Build/BuildLoadEngineTests.gen.h @@ -15,6 +15,7 @@ TestState RunBuildLoadEngineTests() state += Soup::Test::RunTest(className, "Load_BuildDependency_PackageLock", [&testClass]() { testClass->Load_BuildDependency_PackageLock(); }); state += Soup::Test::RunTest(className, "Load_BuildDependency_PackageLock_Override", [&testClass]() { testClass->Load_BuildDependency_PackageLock_Override(); }); state += Soup::Test::RunTest(className, "Load_OtherDependency_PackageLock_ExplicitLanguage", [&testClass]() { testClass->Load_OtherDependency_PackageLock_ExplicitLanguage(); }); + state += Soup::Test::RunTest(className, "Load_RuntimeDependency_PackageLock_ExplicitLanguageOverride_MultipleReferences_ReuseBuildGraph", [&testClass]() { testClass->Load_RuntimeDependency_PackageLock_ExplicitLanguageOverride_MultipleReferences_ReuseBuildGraph(); }); return state; } \ No newline at end of file