Fix sibs test not including parent library correctly

Refactor config parsing to reduce number of changes when introducing a
new platform to support

3 files changed, 152 insertions, 124 deletions

diff --git a/backend/BackendUtils.cpp b/backend/BackendUtils.cpp
index d5fad04..b724f51 100644
--- a/backend/BackendUtils.cpp
+++ b/backend/BackendUtils.cpp
@@ -91,7 +91,7 @@ namespace backend
                 if (!sibsConfig.getTestPath().empty() && isPathSubPathOf(pathNative.c_str(), sibsConfig.getTestPath()))
                 {
                     string filePathUtf8 = toUtf8(pathNative.c_str());
-                    ninjaProject->addTestSourceDir(filePathUtf8.c_str());
+                    ninjaProject->setTestSourceDir(filePathUtf8.c_str());
                 }
                 else if(recursive && !directoryToIgnore(pathNative, sibsConfig.getIgnoreDirs()) && _tinydir_strcmp(file->name, TINYDIR_STRING("sibs-build")) != 0)
                 {
diff --git a/backend/ninja/Ninja.cpp b/backend/ninja/Ninja.cpp
index ef6c365..3a2fa0f 100644
--- a/backend/ninja/Ninja.cpp
+++ b/backend/ninja/Ninja.cpp
@@ -252,11 +252,11 @@ namespace backend
             sourceFiles.push_back({ language, filePathStr });
     }
 
-    void Ninja::addTestSourceDir(const char *dir)
+    void Ninja::setTestSourceDir(const char *dir)
     {
         string dirStr = dir ? dir : "";
-        if(dir && !containsTestSourceDir(dirStr))
-            testSourceDirs.emplace_back(dirStr);
+        if(dir)
+            testSourceDir = dirStr;
     }
 
     void Ninja::addDependency(const std::string &binaryFile)
@@ -264,6 +264,12 @@ namespace backend
         if(!containsDependency(binaryFile))
             binaryDependencies.emplace_back(binaryFile);
     }
+
+    void Ninja::addDynamicDependency(const std::string &dependency)
+    {
+        if(!containsDynamicDependency(dependency))
+            dynamicDependencies.push_back(dependency);
+    }
     
     void Ninja::addSubProject(Ninja *subProject, SibsConfig *config, sibs::FileString &&buildPath)
     {
@@ -285,21 +291,21 @@ namespace backend
         return false;
     }
 
-    bool Ninja::containsTestSourceDir(const string &dir) const
+    bool Ninja::containsDependency(const string &dependency) const
     {
-        for(const string &testSourceDir : testSourceDirs)
+        for(const string &binaryDependency : binaryDependencies)
         {
-            if(testSourceDir == dir)
+            if(binaryDependency == dependency)
                 return true;
         }
         return false;
     }
 
-    bool Ninja::containsDependency(const string &dependency) const
+    bool Ninja::containsDynamicDependency(const std::string &dependency) const
     {
-        for(const string &binaryDependency : binaryDependencies)
+        for(const string &dynamicDependency : dynamicDependencies)
         {
-            if(binaryDependency == dependency)
+            if(dynamicDependency == dependency)
                 return true;
         }
         return false;
@@ -672,32 +678,41 @@ namespace backend
             return filepath.substr(0, it.base().base() - &filepath[0]);
     }
 
-    static string extractDynamicLibDirsFromLinkerFlags(const vector<string> &linkerFlags)
+    static string extractDynamicLibDirsFromLinkerFlag(const string &linkerFlag)
     {
         string result;
-        for(const string &flag : linkerFlags)
+
+        FileString flagNative;
+        if(linkerFlag.size() >= 2 && (linkerFlag[0] == '\'' || linkerFlag[0] == '"') && (linkerFlag.back() == '\'' || linkerFlag.back() == '"'))
+            flagNative = toFileString(StringView(&linkerFlag[1], linkerFlag.size() - 2));
+        else
+            flagNative = toFileString(linkerFlag);
+            
+        if(getFileType(flagNative.c_str()) == FileType::REGULAR)
         {
-            FileString flagNative;
-            if(flag.size() >= 2 && (flag[0] == '\'' || flag[0] == '"') && (flag.back() == '\'' || flag.back() == '"'))
-                flagNative = toFileString(StringView(&flag[1], flag.size() - 2));
-            else
-                flagNative = toFileString(flag);
-                
-            if(getFileType(flagNative.c_str()) == FileType::REGULAR)
+            Result<FileString> libFullPath = getRealPath(flagNative.c_str());
+            if(libFullPath)
             {
-                Result<FileString> libFullPath = getRealPath(flagNative.c_str());
-                if(libFullPath)
+                FileString libDir = getFileParentDirectory(libFullPath.unwrap());
+                if(!libDir.empty())
                 {
-                    FileString libDir = getFileParentDirectory(libFullPath.unwrap());
-                    if(!libDir.empty())
-                    {
-                        if(!result.empty());
-                            result += ":";
-                        result += toUtf8(libDir);
-                    }
+                    result = toUtf8(libDir);
                 }
             }
         }
+
+        return result;
+    }
+
+    static string extractDynamicLibDirsFromLinkerFlags(const vector<string> &linkerFlags)
+    {
+        string result;
+        for(const string &flag : linkerFlags)
+        {
+            if(!result.empty())
+                result += ":";
+            result += extractDynamicLibDirsFromLinkerFlag(flag);
+        }
         return result;
     }
 
@@ -866,6 +881,11 @@ namespace backend
                 dynamicLinkerFlags.push_back(linkerFlag);
             };
         }
+
+        for(const string &dynamicDependency : dynamicDependencies)
+        {
+            dynamicLinkerFlagCallback(dynamicDependency);
+        }
         
         vector<ninja::NinjaArg> cflags;
         auto cflagsCallbackFunc = [&cflags](const string &dependencyCflags)
@@ -1256,7 +1276,9 @@ namespace backend
         // TODO: For now zig projects (zig object files) are built with c/c++ compiler,
         // they should be built with zig if project only contains zig files.
         // But how to combine object files with zig? build-exe only wants to accept .zig files
-        string projectGeneratedBinaryFlags;
+        string projectGeneratedBinary = "\"";
+        projectGeneratedBinary += savePathUtf8;
+        projectGeneratedBinary += "/";
         if (!sourceFiles.empty() && !zigTest)
         {
             string noUndefinedFlag;
@@ -1268,10 +1290,6 @@ namespace backend
                     noUndefinedFlag = isCCompilerClang ? "-Wl,-undefined,error" : "-Wl,--no-undefined,--as-needed";
             }
 
-            string projectGeneratedBinary = "\"";
-            projectGeneratedBinary += savePathUtf8;
-            projectGeneratedBinary += "/";
-
             ninja::NinjaVariable zigObjectArgs("object_args");
             string objectZigArgs;
             if(onlyZigFiles)
@@ -1633,8 +1651,6 @@ namespace backend
                     assert(false);
                     return Result<bool>::Err("Unexpected error");
             }
-            
-            projectGeneratedBinaryFlags = allLinkerFlags + " " + projectGeneratedBinary;
         }
 
         if(!sourceFiles.empty())
@@ -1656,16 +1672,16 @@ namespace backend
             }
         }
 
-        Result<bool> buildTestResult = buildTests(projectGeneratedBinaryFlags, config, savePath, dependencyExportIncludeDirs);
+        Result<bool> buildTestResult = buildTests(allLinkerFlags, projectGeneratedBinary, config, savePath, dependencyExportIncludeDirs);
         if(!buildTestResult)
             return buildTestResult;
 
         return Result<bool>::Ok(true);
     }
 
-    Result<bool> Ninja::buildTests(const std::string &projectGeneratedBinaryFlags, const SibsConfig &config, const _tinydir_char_t *savePath, const string &parentDependencyExportIncludeDirs)
+    Result<bool> Ninja::buildTests(const std::string &parentLinkerFlags, const std::string &parentGeneratedLib, const SibsConfig &config, const _tinydir_char_t *savePath, const string &parentDependencyExportIncludeDirs)
     {
-        if(testSourceDirs.empty() || !config.shouldBuildTests())
+        if(testSourceDir.empty() || !config.shouldBuildTests())
             return Result<bool>::Ok(true);
 
         assert(getNinjaLibraryType(config.getPackageType()) != LibraryType::EXECUTABLE);
@@ -1682,105 +1698,115 @@ namespace backend
             parentExportIncludeDirs += getIncludeOptionFlag(config.getCompiler(), parentExportIncludeDir);
         }
 
-        for(const string &testSourceDir : testSourceDirs)
-        {
 #if OS_FAMILY == OS_FAMILY_POSIX
-            FileString testSourceDirNative = testSourceDir;
-            FileString projectConfFilePath = testSourceDir;
+        FileString testSourceDirNative = testSourceDir;
+        FileString projectConfFilePath = testSourceDir;
 #else
-            FileString testSourceDirNative = utf8To16(testSourceDir);
-            FileString projectConfFilePath = testSourceDirNative;
+        FileString testSourceDirNative = utf8To16(testSourceDir);
+        FileString projectConfFilePath = testSourceDirNative;
 #endif
-            projectConfFilePath += TINYDIR_STRING("/project.conf");
-
-            FileType projectConfFileType = getFileType(projectConfFilePath.c_str());
-            SibsTestConfig sibsTestConfig(config.getCompiler(), testSourceDirNative, config.getOptimizationLevel());
-            sibsTestConfig.platform = config.platform;
-            sibsTestConfig.setSanitize(config.getSanitize());
-            sibsTestConfig.zigTestFiles = move(config.zigTestFiles);
-            sibsTestConfig.zigTestAllFiles = config.zigTestAllFiles;
-            if(projectConfFileType == FileType::REGULAR)
-            {
-                Result<bool> result = Config::readFromFile(projectConfFilePath.c_str(), sibsTestConfig);
-                if(!result)
-                    return result;
-            }
+        projectConfFilePath += TINYDIR_STRING("/project.conf");
+
+        FileType projectConfFileType = getFileType(projectConfFilePath.c_str());
+        SibsTestConfig sibsTestConfig(config.getCompiler(), testSourceDirNative, config.getOptimizationLevel());
+        sibsTestConfig.platform = config.platform;
+        sibsTestConfig.setSanitize(config.getSanitize());
+        sibsTestConfig.zigTestFiles = move(config.zigTestFiles);
+        sibsTestConfig.zigTestAllFiles = config.zigTestAllFiles;
+        if(projectConfFileType == FileType::REGULAR)
+        {
+            Result<bool> result = Config::readFromFile(projectConfFilePath.c_str(), sibsTestConfig);
+            if(!result)
+                return result;
+        }
+
+        backend::Ninja ninja;
+        ninja.addGlobalIncludeDirs(parentExportIncludeDirs);
+        if(!parentLinkerFlags.empty())
+            ninja.addDependency(parentLinkerFlags);
 
-            backend::Ninja ninja;
-            ninja.addGlobalIncludeDirs(parentExportIncludeDirs);
-            if(!projectGeneratedBinaryFlags.empty())
-                ninja.addDependency(projectGeneratedBinaryFlags);
+        switch(config.getPackageType())
+        {
+            case PackageType::STATIC:
+                ninja.addDependency(parentGeneratedLib);
+                break;
+            case PackageType::LIBRARY:
+            case PackageType::DYNAMIC:
+                ninja.addDynamicDependency(parentGeneratedLib);
+                break;
+            default:
+                break;
+        }
 
-            bool zigTest = false;
-            if(config.zigTestAllFiles)
+        bool zigTest = false;
+        if(config.zigTestAllFiles)
+        {
+            backend::BackendUtils::collectSourceFiles(testSourceDirNative.c_str(), &ninja, sibsTestConfig);
+            // TODO: This can be optimized as well. No need to insert non-zig files if we are going to remove them.
+            // Maybe pass a filter callback function to @collectSourceFiles.
+            for(auto it = ninja.sourceFiles.begin(); it != ninja.sourceFiles.end(); )
             {
-                backend::BackendUtils::collectSourceFiles(testSourceDirNative.c_str(), &ninja, sibsTestConfig);
-                // TODO: This can be optimized as well. No need to insert non-zig files if we are going to remove them.
-                // Maybe pass a filter callback function to @collectSourceFiles.
-                for(auto it = ninja.sourceFiles.begin(); it != ninja.sourceFiles.end(); )
+                if(it->language != sibs::Language::ZIG)
                 {
-                    if(it->language != sibs::Language::ZIG)
-                    {
-                        it = ninja.sourceFiles.erase(it);
-                    }
-                    else
-                    {
-                        ++it;
-                    }
+                    it = ninja.sourceFiles.erase(it);
                 }
-                zigTest = true;
-            }
-            else if(!config.zigTestFiles.empty())
-            {
-                ninja.sourceFiles.reserve(config.zigTestFiles.size());
-                for(const sibs::FileString &testFile : config.zigTestFiles)
+                else
                 {
-                    ninja.addSourceFile(sibs::Language::ZIG, toUtf8(testFile.c_str()).c_str());
+                    ++it;
                 }
-                zigTest = true;
             }
-            else
+            zigTest = true;
+        }
+        else if(!config.zigTestFiles.empty())
+        {
+            ninja.sourceFiles.reserve(config.zigTestFiles.size());
+            for(const sibs::FileString &testFile : config.zigTestFiles)
             {
-                backend::BackendUtils::collectSourceFiles(testSourceDirNative.c_str(), &ninja, sibsTestConfig);
+                ninja.addSourceFile(sibs::Language::ZIG, toUtf8(testFile.c_str()).c_str());
             }
+            zigTest = true;
+        }
+        else
+        {
+            backend::BackendUtils::collectSourceFiles(testSourceDirNative.c_str(), &ninja, sibsTestConfig);
+        }
 
-            if(!ninja.getSourceFiles().empty())
+        if(!ninja.getSourceFiles().empty())
+        {
+            FileString buildPath = testSourceDirNative;
+            switch(sibsTestConfig.getOptimizationLevel())
             {
-                FileString buildPath = testSourceDirNative;
-                switch(sibsTestConfig.getOptimizationLevel())
-                {
-                    case OPT_LEV_DEBUG:
-                        buildPath += TINYDIR_STRING("/sibs-build/debug");
-                        break;
-                    case OPT_LEV_RELEASE:
-                        buildPath += TINYDIR_STRING("/sibs-build/release");
-                        break;
-                }
-                
-                Result<bool> buildFileResult = ninja.build(sibsTestConfig, buildPath.c_str());
-                if (!buildFileResult)
+                case OPT_LEV_DEBUG:
+                    buildPath += TINYDIR_STRING("/sibs-build/debug");
+                    break;
+                case OPT_LEV_RELEASE:
+                    buildPath += TINYDIR_STRING("/sibs-build/release");
+                    break;
+            }
+            
+            Result<bool> buildFileResult = ninja.build(sibsTestConfig, buildPath.c_str());
+            if (!buildFileResult)
+                return buildFileResult;
+            
+            // Main projects test should also have compilation database, so we can use it inside IDE
+            if(config.isMainProject())
+            {
+                buildFileResult = buildCompilationDatabase(buildPath.c_str(), testSourceDirNative);
+                if(!buildFileResult)
                     return buildFileResult;
-                
-                // Main projects test should also have compilation database, so we can use it inside IDE
-                if(config.isMainProject())
-                {
-                    buildFileResult = buildCompilationDatabase(buildPath.c_str(), testSourceDirNative);
-                    if(!buildFileResult)
-                        return buildFileResult;
-                }
+            }
 
-                if(!zigTest)
-                {
-                    FileString testExecutableName = buildPath;
-                    testExecutableName += TINYDIR_STRING("/");
-                    testExecutableName += toFileString(sibsTestConfig.getPackageName());
-                    Result<ExecResult> runTestResult = exec(testExecutableName.c_str(), true);
-                    if(!runTestResult)
-                        return Result<bool>::Err(runTestResult);
-                    
-                    if(runTestResult.unwrap().exitCode != 0)
-                        return Result<bool>::Err("Tests failed", runTestResult.unwrap().exitCode);
-                }
+            if(!zigTest)
+            {
+                FileString testExecutableName = buildPath;
+                testExecutableName += TINYDIR_STRING("/");
+                testExecutableName += toFileString(sibsTestConfig.getPackageName());
+                Result<ExecResult> runTestResult = exec(testExecutableName.c_str(), true);
+                if(!runTestResult)
+                    return Result<bool>::Err(runTestResult);
+                
+                if(runTestResult.unwrap().exitCode != 0)
+                    return Result<bool>::Err("Tests failed", runTestResult.unwrap().exitCode);
             }
         }
 
diff --git a/backend/ninja/Ninja.hpp b/backend/ninja/Ninja.hpp
index 8606708..2928fb3 100644
--- a/backend/ninja/Ninja.hpp
+++ b/backend/ninja/Ninja.hpp
@@ -43,25 +43,27 @@ namespace backend
 
         void addGlobalIncludeDirs(const std::string &globalIncludeDirs);
         void addSourceFile(sibs::Language language, const char *filepath);
-        void addTestSourceDir(const char *dir);
+        void setTestSourceDir(const char *dir);
         void addDependency(const std::string &binaryFile);
+        void addDynamicDependency(const std::string &dependency);
         void addSubProject(Ninja *subProject, sibs::SibsConfig *config, sibs::FileString &&buildPath);
         const std::vector<sibs::SourceFile>& getSourceFiles() const;
         sibs::Result<bool> build(const sibs::SibsConfig &config, const _tinydir_char_t *savePath, sibs::LinkerFlagCallbackFunc staticLinkerFlagCallbackFunc = nullptr, sibs::LinkerFlagCallbackFunc dynamicLinkerFlagCallback = nullptr, sibs::GlobalIncludeDirCallbackFunc globalIncludeDirCallback = nullptr);
     private:
         sibs::Result<bool> buildSubProjects(sibs::LinkerFlagCallbackFunc staticLinkerFlagCallbackFunc, sibs::LinkerFlagCallbackFunc dynamicLinkerFlagCallback, sibs::GlobalIncludeDirCallbackFunc globalIncludeDirCallback);
-        sibs::Result<bool> buildTests(const std::string &projectGeneratedBinaryFlags, const sibs::SibsConfig &config, const _tinydir_char_t *savePath, const std::string &parentDependencyExportIncludeDirs);
+        sibs::Result<bool> buildTests(const std::string &parentLinkerFlags, const std::string &parentGeneratedLib, const sibs::SibsConfig &config, const _tinydir_char_t *savePath, const std::string &parentDependencyExportIncludeDirs);
         bool containsSourceFile(const std::string &filepath) const;
-        bool containsTestSourceDir(const std::string &dir) const;
         bool containsDependency(const std::string &dependency) const;
+        bool containsDynamicDependency(const std::string &dependency) const;
         sibs::Result<bool> getLinkerFlags(const sibs::SibsConfig &config, sibs::LinkerFlagCallbackFunc staticLinkerFlagCallbackFunc, sibs::LinkerFlagCallbackFunc dynamicLinkerFlagCallback, sibs::GlobalIncludeDirCallbackFunc globalIncludeDirCallback, sibs::CflagsCallbackFunc cflagsCallbackFunc) const;
         sibs::Result<bool> compile(const _tinydir_char_t *buildFilePath);
         sibs::Result<bool> buildCompilationDatabase(const _tinydir_char_t *buildFilePath, const sibs::FileString &savePath);
     private:
         std::string customGlobalIncludeDirs;
+        std::string testSourceDir;
         std::vector<sibs::SourceFile> sourceFiles;
-        std::vector<std::string> testSourceDirs;
         std::vector<std::string> binaryDependencies;
+        std::vector<std::string> dynamicDependencies;
         std::vector<NinjaSubProject> subProjects;
     };
 }