src: Fixed FilePath::canonical() for UNIX

* fixed clang warnings
* fixed ApplicationSettings loaded from wrong location in tests
* added symlink within canonical path to tests

bin/test/data/FilePathTestSuite/parent/target

@@ -0,0 +1 @@
+../target

src/app/main.cpp

@@ -222,6 +222,22 @@ int main(int argc, char *argv[])
 	QScopedPointer<QtNetworkFactory> networkFactory;
 	QScopedPointer<QtViewFactory> viewFactory;
 
+	if (!commandLineParser.runWithoutGUI())
+	{
+		qtApp->setAttribute(Qt::AA_UseHighDpiPixmaps);
+
+		utility::loadFontsFromDirectory(ResourcePaths::getFontsPath(), ".otf");
+		utility::loadFontsFromDirectory(ResourcePaths::getFontsPath(), ".ttf");
+
+		networkFactory.reset(new QtNetworkFactory());
+		viewFactory.reset(new QtViewFactory());
+	}
+
+	Application::createInstance(version, viewFactory.data(), networkFactory.data());
+
+	ScopedFunctor f([](){
+		Application::destroyInstance();
+	});
 
 	if (commandLineParser.runWithoutGUI())
 	{
@@ -235,23 +251,6 @@ int main(int argc, char *argv[])
 			return 0;
 		}
 	}
-	else
-	{
-		qtApp->setAttribute(Qt::AA_UseHighDpiPixmaps);
-
-		utility::loadFontsFromDirectory(ResourcePaths::getFontsPath(), ".otf");
-		utility::loadFontsFromDirectory(ResourcePaths::getFontsPath(), ".ttf");
-
-		networkFactory.reset(new QtNetworkFactory());
-		viewFactory.reset(new QtViewFactory());
-	}
-
-
-	Application::createInstance(version, viewFactory.data(), networkFactory.data());
-
-	ScopedFunctor f([](){
-		Application::destroyInstance();
-	});
 
 	prefillPaths();
 	addLanguageModules();

src/lib/data/indexer/TaskBuildIndex.cpp

@@ -271,17 +271,11 @@ void TaskBuildIndex::updateIndexingDialog(
 	if (!sourcePath.empty())
 	{
 		std::stringstream ss;
-		ss << "[" << m_indexingFileCount
+		ss << "[" << m_indexingFileCount << "/" << sourceFileCount << "] Indexing file: " << sourcePath.str();
-		   << "/" << sourceFileCount
-		   << "] Indexing file: ";
-		ss << sourcePath.str();
 		MessageStatus(ss.str(), false, true).dispatch();
 	}
 
-	if (std::shared_ptr<DialogView> dialogView = Application::getInstance()->getDialogView())
+	Application::getInstance()->getDialogView()->updateIndexingDialog(
-	{
+		m_indexingFileCount, indexedSourceFileCount, sourceFileCount, sourcePath.str()
-		dialogView->updateIndexingDialog(
+	);
-			m_indexingFileCount, indexedSourceFileCount, sourceFileCount, sourcePath.str()
-		);
-	}
 }

src/lib/utility/commandline/commands/CommandlineCommand.cpp

@@ -11,6 +11,10 @@ Command::Command(const std::string name, CommandLineParser* parser)
 {
 }
 
+Command::~Command()
+{
+}
+
 const std::string Command::name()
 {
 	return m_name;
@@ -32,5 +36,4 @@ void Command::printHelp()
 	}
 }
 
-
 } // namespace cmd

src/lib/utility/commandline/commands/CommandlineCommand.h

@@ -8,13 +8,15 @@
 namespace po = boost::program_options;
 
 namespace commandline {
+
 enum class ReturnStatus;
 class CommandLineParser;
 
-
+class Command
-class Command {
+{
 public:
 	Command(const std::string name, CommandLineParser* parser = nullptr);
+	virtual ~Command();
 
 	const std::string name();
 

src/lib/utility/commandline/commands/CommandlineCommandConfig.cpp

@@ -83,6 +83,10 @@ CommandConfig::CommandConfig(CommandLineParser* parser)
 {
 }
 
+CommandConfig::~CommandConfig()
+{
+}
+
 void CommandConfig::setup()
 {
 	po::options_description options("Config Options");
@@ -112,7 +116,7 @@ void CommandConfig::setup()
 	m_options.add(options);
 }
 
-void printVector(std::string title, std::vector<FilePath> vec)
+void printVector(const std::string& title, const std::vector<FilePath>& vec)
 {
 	std::cout << "\n  " << title << ":";
 	if (vec.empty())
@@ -125,17 +129,8 @@ void printVector(std::string title, std::vector<FilePath> vec)
 	}
 }
 
-void CommandConfig::printSettings()
+void CommandConfig::printSettings(ApplicationSettings* settings)
 {
-	ApplicationSettings* settings = ApplicationSettings::getInstance().get();
-	settings->load(UserPaths::getAppSettingsPath());
-
-	if (settings == nullptr)
-	{
-		LOG_ERROR("Could not load application settings");
-		return;
-	}
-
 	std::cout << "Sourcetrail Settings:\n"
 			  << "\n  indexerthread count: " << settings->getIndexerThreadCount()
 			  << "\n  use-processes: " << settings->getMultiProcessIndexingEnabled()
@@ -144,9 +139,9 @@ void CommandConfig::printSettings()
 			  << "\n  jvm-path: " << settings->getJavaPath()
 			  << "\n  jvm-max-memory: " << settings->getJavaMaximumMemory()
 			  << "\n  maven-path: " << settings->getMavenPath().str();
-	printVector("global-header-search-paths", std::move(settings->getHeaderSearchPaths()));
+	printVector("global-header-search-paths", settings->getHeaderSearchPaths());
-	printVector("global-framework-search-paths", std::move(settings->getFrameworkSearchPaths()));
+	printVector("global-framework-search-paths", settings->getFrameworkSearchPaths());
-	printVector("jre-system-library-paths", std::move(settings->getJreSystemLibraryPaths()));
+	printVector("jre-system-library-paths", settings->getJreSystemLibraryPaths());
 	License license;
 	license.loadFromEncodedString(settings->getLicenseString(), AppPath::getAppPath());
 	std::cout << "\nValid license: " << license.isValid() << std::endl;
@@ -178,14 +173,18 @@ ReturnStatus CommandConfig::parse(std::vector<std::string>& args)
 		return ReturnStatus::CMD_QUIT;
 	}
 
-	if (args[0] == "show" || vm.count("show"))
+	ApplicationSettings* settings = ApplicationSettings::getInstance().get();
+	if (settings == nullptr)
 	{
-		printSettings();
+		LOG_ERROR("No application settings loaded");
 		return ReturnStatus::CMD_QUIT;
 	}
 
-	ApplicationSettings* settings = ApplicationSettings::getInstance().get();
+	if (args[0] == "show" || vm.count("show"))
-	settings->load(UserPaths::getAppSettingsPath());
+	{
+		printSettings(settings);
+		return ReturnStatus::CMD_QUIT;
+	}
 
 	parseAndSetValue(&ApplicationSettings::setMultiProcessIndexingEnabled, "use-processes", settings, vm);
 	parseAndSetValue(&ApplicationSettings::setLoggingEnabled, "logging-enabled", settings, vm);

src/lib/utility/commandline/commands/CommandlineCommandConfig.h

@@ -5,21 +5,25 @@
 #include <iostream>
 #include "utility/commandline/commands/CommandlineCommand.h"
 
+class ApplicationSettings;
 class CommandLineParser;
 
 namespace commandline
 {
 enum class ReturnStatus;
 
-class CommandConfig : public Command {
+class CommandConfig
+	: public Command
+{
 public:
 	CommandConfig(CommandLineParser* parser);
+	virtual ~CommandConfig();
 
 	virtual void setup();
 	virtual ReturnStatus parse(std::vector<std::string>& args);
-private:
-	void printSettings();
 
+private:
+	void printSettings(ApplicationSettings* settings);
 };
 
 } // namespace cmd

src/lib/utility/commandline/commands/CommandlineCommandIndex.cpp

@@ -11,6 +11,10 @@ CommandIndex::CommandIndex(CommandLineParser* parser)
 {
 }
 
+CommandIndex::~CommandIndex()
+{
+}
+
 void CommandIndex::setup()
 {
 	po::options_description options("Config Options");

src/lib/utility/commandline/commands/CommandlineCommandIndex.h

@@ -2,6 +2,7 @@
 #define COMMANDLINE_COMMAND_INDEX_H
 
 #include <memory>
+
 #include "boost/program_options.hpp"
 
 #include "utility/commandline/commands/CommandlineCommand.h"
@@ -12,9 +13,12 @@ namespace commandline {
 
 enum class ReturnStatus;
 
-class CommandIndex : public Command {
+class CommandIndex
+	: public Command
+{
 public:
 	CommandIndex(CommandLineParser* parser);
+	virtual ~CommandIndex();
 
 	virtual void setup();
 	virtual ReturnStatus parse(std::vector<std::string>& args);

src/lib/utility/file/FilePath.cpp

@@ -120,61 +120,45 @@ FilePath FilePath::absolute() const
 
 FilePath FilePath::canonical() const
 {
-	if (m_canonicalized)
+	if (m_canonicalized || !exists())
 	{
 		return FilePath(*this);
 	}
 
-	if (!exists())
-	{
-		return FilePath(*this);
-	}
-
-	boost::filesystem::path abs_p = boost::filesystem::absolute(m_path);
 	boost::filesystem::path canonicalPath;
+
+#if defined(_WIN32)
+	boost::filesystem::path abs_p = boost::filesystem::absolute(m_path);
 	for (boost::filesystem::path::iterator it = abs_p.begin(); it != abs_p.end(); ++it)
 	{
 		if (*it == "..")
 		{
-			// /a/b/.. is not necessarily /a if b is a symbolic link
+			canonicalPath = canonicalPath.parent_path();
-			if (boost::filesystem::is_symlink(canonicalPath))
-			{
-				canonicalPath /= *it;
-			}
-			// /a/b/../.. is not /a/b/.. under most circumstances
-			// We can end up with ..s in our result because of symbolic links
-			else if (canonicalPath.filename() == "..")
-			{
-				canonicalPath /= *it;
-			}
-			// Otherwise it should be safe to resolve the parent
-			else
-			{
-				canonicalPath = canonicalPath.parent_path();
-			}
 		}
 		else if (*it == ".")
 		{
-			// Ignore
+			continue;
 		}
 		else
 		{
-			// Just cat other path entries
 			canonicalPath /= *it;
-		}
 
-		if (boost::filesystem::is_symlink(canonicalPath))
+			if (boost::filesystem::is_symlink(canonicalPath))
-		{
-			boost::filesystem::path symlink = boost::filesystem::read_symlink(canonicalPath);
-			if (!symlink.empty())
 			{
-				// on Windows the read_symlink function discards the drive letter (this is a boost bug). Therefore
+				boost::filesystem::path symlink = boost::filesystem::read_symlink(canonicalPath);
-				// we need to make the path absolute again. We also have to discard the trailing \0 characters so
+				if (!symlink.empty())
-				// that we can continue appending to the path.
+				{
-				canonicalPath = utility::substrBeforeFirst(boost::filesystem::absolute(symlink).string(), '\0');
+					// on Windows the read_symlink function discards the drive letter (this is a boost bug). Therefore
+					// we need to make the path absolute again. We also have to discard the trailing \0 characters so
+					// that we can continue appending to the path.
+					canonicalPath = utility::substrBeforeFirst(boost::filesystem::absolute(symlink).string(), '\0');
+				}
 			}
 		}
 	}
+#else
+	canonicalPath = boost::filesystem::canonical(m_path);
+#endif
 
 	FilePath ret(canonicalPath);
 	ret.m_canonicalized = true;

src/test/FilePathTestSuite.h

@@ -170,6 +170,16 @@ public:
 		TS_ASSERT_EQUALS(path.absolute(), path.canonical());
 	}
 
+	void test_file_path_canonical_removes_symlinks()
+	{
+#ifndef _WIN32
+		FilePath pathA("data/FilePathTestSuite/parent/target/d.cpp");
+		FilePath pathB("data/FilePathTestSuite/target/d.cpp");
+
+		TS_ASSERT_EQUALS(pathB.absolute(), pathA.canonical());
+#endif
+	}
+
 	void test_file_path_compares_paths_with_posix_and_windows_format()
 	{
 #ifdef _WIN32

src/test/TestSuiteFixture.cpp

@@ -1,6 +1,6 @@
 #include "TestSuiteFixture.h"
 
-#include <sstream>
+#include <iostream>
 
 #include "utility/logging/FileLogger.h"
 #include "utility/logging/LogManager.h"
@@ -15,8 +15,6 @@ TestSuiteFixture::~TestSuiteFixture()
 {
 }
 
-#include <iostream>
-
 bool TestSuiteFixture::setUpWorld()
 {
 	LogManager* logManager = LogManager::getInstance().get();