Skip to content

Commit

Permalink
Fix bug missing cache for build graph loader (#151)
Browse files Browse the repository at this point in the history 
Co-authored-by: Matthew Asplund <[email protected]>
  • Loading branch information
@mwasplund
mwasplund and Matthew Asplund authored Nov 16, 2022
1 parent 568e02a commit 444526e
Show file tree
Hide file tree
Showing 4 changed files with 250 additions and 8 deletions.
2 changes: 1 addition & 1 deletion Samples/CSharp/ConsoleApplication/PackageLock.sml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Closures: {
}
Build0: {
CSharp: [
{ Name: "Soup.CSharp", Version: "0.7.0" }
{ Name: "Soup.CSharp", Version: "0.7.1" }
]
}
}
17 changes: 11 additions & 6 deletions Source/Client/Core/Source/Build/BuildLoadEngine.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ namespace Soup::Core
PackageLookupMap _packageLookup;

// Mapping from build dependency to Package path to graph id
std::map<Path, PackageGraphId> _knownGraphSet;
std::map<Path, PackageGraphId> _knownBuildGraphSet;

public:
/// <summary>
Expand All @@ -89,7 +89,7 @@ namespace Soup::Core
_recipeCache(recipeCache),
_packageGraphLookup(),
_packageLookup(),
_knownGraphSet()
_knownBuildGraphSet()
{
}

Expand Down Expand Up @@ -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));
}
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
Expand Down
238 changes: 237 additions & 1 deletion Source/Client/Core/UnitTests/Build/BuildLoadEngineTests.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
Expand Down Expand Up @@ -1348,6 +1348,7 @@ namespace Soup::Core::UnitTests
"Verify package graph matches expected.");
}


// [[Fact]]
void Load_OtherDependency_PackageLock_ExplicitLanguage()
{
Expand Down Expand Up @@ -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<TestTraceListener>();
auto scopedTraceListener = ScopedTraceListenerRegister(testListener);

// Register the test file system
auto fileSystem = std::make_shared<MockFileSystem>();
auto scopedFileSystem = ScopedFileSystemRegister(fileSystem);

// Create the Recipe to build
fileSystem->CreateMockFile(
Path("C:/WorkingDirectory/MyPackage/Recipe.sml"),
std::make_shared<MockFile>(std::stringstream(R"(
Name: "MyPackage"
Language: "C#|1"
Dependencies: {
Runtime: [
"../Package1/"
]
}
)")));

fileSystem->CreateMockFile(
Path("C:/WorkingDirectory/Package1/Recipe.sml"),
std::make_shared<MockFile>(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<MockFile>(std::stringstream(R"(
Name: "Soup.CSharp"
Language: "C#|1"
)")));

// Create the package lock
fileSystem->CreateMockFile(
Path("C:/WorkingDirectory/MyPackage/PackageLock.sml"),
std::make_shared<MockFile>(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<MockProcessManager>();
auto scopedProcessManager = ScopedProcessManagerRegister(processManager);

auto builtInLanguages = std::map<std::string, BuiltInLanguagePackage>(
{
{
"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<std::string, Value>({
{
"ArgumentValue",
Value(true),
},
}));
arguments.WorkingDirectory = Path("C:/WorkingDirectory/MyPackage/");
auto hostBuildGlobalParameters = ValueTable(
std::map<std::string, Value>({
{
"HostValue",
Value(true),
},
}));
auto recipeCache = RecipeCache();
auto uut = BuildLoadEngine(
builtInLanguages,
arguments,
hostBuildGlobalParameters,
recipeCache);

auto packageProvider = uut.Load();

// Verify expected logs
Assert::AreEqual(
std::vector<std::string>({
"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<std::string>({
"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<std::string>({
"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<std::string, Value>({
{
"ArgumentValue",
Value(true),
},
})))
},
{
2,
PackageGraph(
2,
3,
ValueTable(
std::map<std::string, Value>({
{
"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.");
}
};
}
1 change: 1 addition & 0 deletions Source/Client/Core/UnitTests/gen/Build/BuildLoadEngineTests.gen.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

0 comments on commit 444526e

Please sign in to comment.