From 0008920eed996009068fe9f71512c436577b6220 Mon Sep 17 00:00:00 2001 From: Bent Bisballe Nyeng Date: Wed, 2 Oct 2024 21:20:40 +0200 Subject: Ensure the initial task order is preserved. Fixes bad ordering during linking. --- src/build.cc | 43 ++++++++++++++++++++++++---------------- src/build.h | 10 +++++----- src/libctor.cc | 8 ++++---- src/task.cc | 10 +++++++--- src/task.h | 9 ++++----- src/task_cc.cc | 8 ++++++-- src/task_cc.h | 2 +- src/tasks.cc | 43 ++++++++++++++++++++-------------------- src/tasks.h | 17 ++++++++-------- src/unittest.cc | 2 +- src/unittest.h | 4 ++-- test/execute_test.cc | 18 +++++++++++------ test/tasks_test.cc | 55 ++++++++++++++++++++++++++-------------------------- 13 files changed, 125 insertions(+), 104 deletions(-) diff --git a/src/build.cc b/src/build.cc index ad30719..ea65656 100644 --- a/src/build.cc +++ b/src/build.cc @@ -4,12 +4,11 @@ #include "build.h" #include -#include #include #include -#include #include #include +#include #include "ctor.h" @@ -17,8 +16,8 @@ using namespace std::chrono_literals; int build(const ctor::settings& settings, const std::string& name, - const std::set>& tasks, - const std::set>& all_tasks, + const std::vector>& tasks, + const std::vector>& all_tasks, bool dryrun) { if(settings.verbose > 1) @@ -26,12 +25,13 @@ int build(const ctor::settings& settings, std::cout << "Building '" << name << "'\n"; } - std::set> dirtyTasks; + std::vector> dirtyTasks; for(auto task : tasks) { - if(task->dirty()) + if(task->dirty() && + std::find(dirtyTasks.begin(), dirtyTasks.end(), task) == dirtyTasks.end()) { - dirtyTasks.insert(task); + dirtyTasks.push_back(task); } } @@ -135,10 +135,10 @@ int build(const ctor::settings& settings, namespace { -std::set> getDepTasks(std::shared_ptr task) +std::vector> getDepTasks(std::shared_ptr task) { - std::set> tasks; - tasks.insert(task); + std::vector> tasks; + tasks.push_back(task); auto deps = task->getDependsTasks(); for(const auto& dep : deps) @@ -146,7 +146,10 @@ std::set> getDepTasks(std::shared_ptr task) auto depSet = getDepTasks(dep); for(const auto& dep : depSet) { - tasks.insert(dep); + if(std::find(tasks.begin(), tasks.end(), dep) == tasks.end()) + { + tasks.push_back(dep); + } } } @@ -156,7 +159,7 @@ std::set> getDepTasks(std::shared_ptr task) int build(const ctor::settings& settings, const std::string& name, - const std::set>& all_tasks, + const std::vector>& all_tasks, bool dryrun) { bool task_found{false}; @@ -167,10 +170,13 @@ int build(const ctor::settings& settings, task_found = true; auto depSet = getDepTasks(task); - std::set> ts; + std::vector> ts; for(const auto& task : depSet) { - ts.insert(task); + if(std::find(ts.begin(), ts.end(), task) == ts.end()) + { + ts.push_back(task); + } } auto ret = build(settings, name, ts, all_tasks, dryrun); @@ -195,11 +201,11 @@ int build(const ctor::settings& settings, int build(const ctor::settings& settings, const std::string& name, const std::vector& targets, - const std::set>& all_tasks, + const std::vector>& all_tasks, bool dryrun) { bool task_found{false}; - std::set> ts; + std::vector> ts; for(const auto& target : targets) { @@ -213,7 +219,10 @@ int build(const ctor::settings& settings, auto depSet = getDepTasks(task); for(const auto& task : depSet) { - ts.insert(task); + if(std::find(ts.begin(), ts.end(), task) == ts.end()) + { + ts.push_back(task); + } } } } diff --git a/src/build.h b/src/build.h index d74642c..500fb7f 100644 --- a/src/build.h +++ b/src/build.h @@ -4,7 +4,7 @@ #pragma once #include -#include +#include #include #include "task.h" @@ -17,19 +17,19 @@ struct settings; //! Dry-run returns number of dirty tasks but otherwise does nothing. int build(const ctor::settings& settings, const std::string& name, - const std::set>& tasks, - const std::set>& all_tasks, + const std::vector>& tasks, + const std::vector>& all_tasks, bool dryrun = false); //! Dry-run returns number of dirty tasks but otherwise does nothing. int build(const ctor::settings& settings, const std::string& name, - const std::set>& all_tasks, + const std::vector>& all_tasks, bool dryrun = false); //! Dry-run returns number of dirty tasks but otherwise does nothing. int build(const ctor::settings& settings, const std::string& name, const std::vector& targets, - const std::set>& all_tasks, + const std::vector>& all_tasks, bool dryrun = false); diff --git a/src/libctor.cc b/src/libctor.cc index 21792e1..3eb6c6f 100644 --- a/src/libctor.cc +++ b/src/libctor.cc @@ -15,7 +15,6 @@ #include #include #include -#include #include @@ -185,17 +184,18 @@ Options: if(list_files) { no_default_build = true; - std::set files; + std::vector files; for(std::size_t i = 0; i < numConfigFiles; ++i) { - files.insert(configFiles[i].file); + files.push_back(configFiles[i].file); } for(std::size_t i = 0; i < numExternalConfigFiles; ++i) { - files.insert(externalConfigFiles[i].file); + files.push_back(externalConfigFiles[i].file); } + std::sort(files.begin(), files.end()); for(const auto& file : files) { std::cout << file << "\n"; diff --git a/src/task.cc b/src/task.cc index 817ee3a..7813235 100644 --- a/src/task.cc +++ b/src/task.cc @@ -5,6 +5,7 @@ #include #include +#include Task::Task(const ctor::build_configuration& config, const ctor::settings& settings, const std::string& sourceDir) @@ -15,7 +16,7 @@ Task::Task(const ctor::build_configuration& config, const ctor::settings& settin { } -int Task::registerDepTasks(const std::set>& tasks) +int Task::registerDepTasks(const std::vector>& tasks) { for(const auto& depStr : depends()) { @@ -24,7 +25,10 @@ int Task::registerDepTasks(const std::set>& tasks) { if(*task == depStr) { - dependsTasks.insert(task); + if(std::find(dependsTasks.begin(), dependsTasks.end(), task) == dependsTasks.end()) + { + dependsTasks.push_back(task); + } found = true; } } @@ -155,7 +159,7 @@ std::string Task::compiler() const } } -std::set> Task::getDependsTasks() +std::vector> Task::getDependsTasks() { return dependsTasks; } diff --git a/src/task.h b/src/task.h index 9e99f25..6fe1686 100644 --- a/src/task.h +++ b/src/task.h @@ -6,7 +6,6 @@ #include #include #include -#include #include #include @@ -28,8 +27,8 @@ public: const std::string& sourceDir); virtual ~Task() = default; - int registerDepTasks(const std::set>& tasks); - virtual int registerDepTasksInner(const std::set>& tasks) { return 0; } + int registerDepTasks(const std::vector>& tasks); + virtual int registerDepTasksInner(const std::vector>& tasks) { return 0; } bool operator==(const std::string& dep); @@ -64,7 +63,7 @@ public: ctor::output_system outputSystem() const; std::string compiler() const; - std::set> getDependsTasks(); + std::vector> getDependsTasks(); virtual std::string source() const { return {}; } @@ -74,7 +73,7 @@ protected: virtual bool dirtyInner() { return false; } std::vector dependsStr; - std::set> dependsTasks; + std::vector> dependsTasks; const ctor::build_configuration& config; ctor::target_type target_type{ctor::target_type::automatic}; ctor::language source_language{ctor::language::automatic}; diff --git a/src/task_cc.cc b/src/task_cc.cc index 294c5ea..4eb07ae 100644 --- a/src/task_cc.cc +++ b/src/task_cc.cc @@ -6,6 +6,7 @@ #include #include #include +#include #include "ctor.h" #include "execute.h" @@ -72,13 +73,16 @@ TaskCC::TaskCC(const ctor::build_configuration& config, const ctor::settings& se std::filesystem::create_directories(targetFile().parent_path()); } -int TaskCC::registerDepTasksInner(const std::set>& tasks) +int TaskCC::registerDepTasksInner(const std::vector>& tasks) { for(const auto& task : tasks) { if(*task == _source.file) { - dependsTasks.insert(task); + if(std::find(dependsTasks.begin(), dependsTasks.end(), task) == dependsTasks.end()) + { + dependsTasks.push_back(task); + } } } diff --git a/src/task_cc.h b/src/task_cc.h index 70bb13c..6c4683a 100644 --- a/src/task_cc.h +++ b/src/task_cc.h @@ -19,7 +19,7 @@ public: const std::string& sourceDir, const ctor::source& source); virtual ~TaskCC() = default; - int registerDepTasksInner(const std::set>& tasks) override; + int registerDepTasksInner(const std::vector>& tasks) override; std::string name() const override; bool dirtyInner() override; diff --git a/src/tasks.cc b/src/tasks.cc index 22ad2d6..61c130b 100644 --- a/src/tasks.cc +++ b/src/tasks.cc @@ -70,11 +70,11 @@ const std::deque& getTargets(const ctor::settings& settings, return targets; } -std::set> taskFactory(const ctor::build_configuration& config, - const ctor::settings& settings, - const std::string& sourceDir) +std::vector> taskFactory(const ctor::build_configuration& config, + const ctor::settings& settings, + const std::string& sourceDir) { - std::set> tasks; + std::vector> tasks; std::filesystem::path targetFile(config.target); @@ -97,7 +97,7 @@ std::set> taskFactory(const ctor::build_configuration& con for(const auto& file : config.sources) { auto task = std::make_shared(config, settings, sourceDir, file); - tasks.insert(task); + tasks.push_back(task); objects.push_back(task->targetFile().string()); } } @@ -107,7 +107,7 @@ std::set> taskFactory(const ctor::build_configuration& con for(const auto& file : config.sources) { auto task = std::make_shared(config, settings, sourceDir, file); - tasks.insert(task); + tasks.push_back(task); objects.push_back(task->target()); } } @@ -127,8 +127,8 @@ std::set> taskFactory(const ctor::build_configuration& con case ctor::target_type::static_library: case ctor::target_type::unit_test_library: - tasks.insert(std::make_shared(config, settings, config.target, - objects, sourceDir)); + tasks.push_back(std::make_shared(config, settings, config.target, + objects, sourceDir)); break; #ifndef BOOTSTRAP case ctor::target_type::dynamic_library: @@ -138,14 +138,14 @@ std::set> taskFactory(const ctor::build_configuration& con std::cerr << "Dynamic library target must have 'lib' prefix\n"; exit(1); } - tasks.insert(std::make_shared(config, settings, config.target, - objects, sourceDir)); + tasks.push_back(std::make_shared(config, settings, config.target, + objects, sourceDir)); break; case ctor::target_type::executable: case ctor::target_type::unit_test: - tasks.insert(std::make_shared(config, settings, config.target, - objects, sourceDir)); + tasks.push_back(std::make_shared(config, settings, config.target, + objects, sourceDir)); break; case ctor::target_type::object: @@ -160,18 +160,19 @@ std::set> taskFactory(const ctor::build_configuration& con return tasks; } -std::shared_ptr getNextTask(const std::set>& allTasks, - std::set>& dirtyTasks) +std::shared_ptr getNextTask(const std::vector>& allTasks, + std::vector>& dirtyTasks) { for(auto dirtyTask = dirtyTasks.begin(); dirtyTask != dirtyTasks.end(); ++dirtyTask) { + auto task = *dirtyTask; //std::cout << "Examining target " << (*dirtyTask)->target() << "\n"; - if((*dirtyTask)->ready()) + if(task->ready()) { dirtyTasks.erase(dirtyTask); - return *dirtyTask; + return task; } } @@ -179,12 +180,12 @@ std::shared_ptr getNextTask(const std::set>& allTask return nullptr; } -std::set> getTasks(const ctor::settings& settings, - const std::vector names, - bool resolve_externals) +std::vector> getTasks(const ctor::settings& settings, + const std::vector names, + bool resolve_externals) { auto& targets = getTargets(settings, resolve_externals); - std::set> tasks; + std::vector> tasks; for(const auto& target : targets) { if(names.empty() || @@ -192,7 +193,7 @@ std::set> getTasks(const ctor::settings& settings, { std::vector objects; auto t = taskFactory(target.config, settings, target.path); - tasks.insert(t.begin(), t.end()); + tasks.insert(tasks.end(), t.begin(), t.end()); } } diff --git a/src/tasks.h b/src/tasks.h index ed1bf60..cc34f56 100644 --- a/src/tasks.h +++ b/src/tasks.h @@ -4,7 +4,6 @@ #pragma once #include -#include #include #include @@ -24,17 +23,17 @@ const std::deque& getTargets(const ctor::settings& settings, //! fulfilled. //! The returned task is removed from the dirty list. //! Return nullptr if no dirty task is ready. -std::shared_ptr getNextTask(const std::set>& allTasks, - std::set>& dirtyTasks); +std::shared_ptr getNextTask(const std::vector>& allTasks, + std::vector>& dirtyTasks); //! Get list of tasks filtered by name including each of their direct //! dependency tasks (ie. objects tasks from their sources). -std::set> getTasks(const ctor::settings& settings, - const std::vector names = {}, - bool resolve_externals = true); +std::vector> getTasks(const ctor::settings& settings, + const std::vector names = {}, + bool resolve_externals = true); //! Generate list of targets from a single configuration, including the final //! link target and all its objects files (if any). -std::set> taskFactory(const ctor::build_configuration& config, - const ctor::settings& settings, - const std::string& sourceDir); +std::vector> taskFactory(const ctor::build_configuration& config, + const ctor::settings& settings, + const std::string& sourceDir); diff --git a/src/unittest.cc b/src/unittest.cc index 1e32878..9e85187 100644 --- a/src/unittest.cc +++ b/src/unittest.cc @@ -8,7 +8,7 @@ #include "execute.h" #include "task.h" -int runUnitTests(std::set>& tasks, +int runUnitTests(std::vector>& tasks, const ctor::settings& settings) { bool ok{true}; diff --git a/src/unittest.h b/src/unittest.h index 2880319..6d1385e 100644 --- a/src/unittest.h +++ b/src/unittest.h @@ -3,7 +3,7 @@ // See accompanying file LICENSE for details. #pragma once -#include +#include #include class Task; @@ -12,5 +12,5 @@ namespace ctor { struct settings; } // namespace ctor:: -int runUnitTests(std::set>& tasks, +int runUnitTests(std::vector>& tasks, const ctor::settings& settings); diff --git a/test/execute_test.cc b/test/execute_test.cc index 03b3c2a..d5d40c9 100644 --- a/test/execute_test.cc +++ b/test/execute_test.cc @@ -2,10 +2,11 @@ #include #include -#include +#include #include #include +#include #include "paths.h" #include "tmpfile.h" @@ -34,6 +35,8 @@ public: void env() { + using namespace std::string_literals; + auto cur_path = std::filesystem::path(paths::argv_0).parent_path(); std::vector paths{{cur_path.string()}}; auto cmd = locate("testprog", paths); @@ -52,22 +55,25 @@ public: uASSERT_EQUAL(0, execute(cmd, {"envdump", tmp.get()}, env, false)); - std::set vars; + std::vector vars; { std::ifstream infile(tmp.get()); std::string line; while (std::getline(infile, line)) { - vars.insert(line); + vars.push_back(line); } } // Check the two explicitly set vars - uASSERT(vars.find("foo=bar") != vars.end()); - uASSERT(vars.find("bar=42") != vars.end()); + auto chk = std::find(vars.begin(), vars.end(), "foo=bar"s); + uASSERT(chk != vars.end()); + chk = std::find(vars.begin(), vars.end(), "bar=42"s); + uASSERT(chk != vars.end()); // Check the one that should have overwritten the existing one (probably LANG=en_US.UTF-8 or something) - uASSERT(vars.find("LANG=foo") != vars.end()); + chk = std::find(vars.begin(), vars.end(), "LANG=foo"s); + uASSERT(chk != vars.end()); // Check that other vars are also there (ie. the env wasn't cleared on entry) uASSERT(vars.size() > 3); diff --git a/test/tasks_test.cc b/test/tasks_test.cc index 5f1db26..3de6982 100644 --- a/test/tasks_test.cc +++ b/test/tasks_test.cc @@ -36,7 +36,7 @@ ctor::build_configurations ctorTestConfigs2(const ctor::settings&) REG(ctorTestConfigs1); REG(ctorTestConfigs2); -std::size_t count(const std::set>& tasks, +std::size_t count(const std::vector>& tasks, const std::string& name) { auto cnt{0u}; @@ -113,8 +113,7 @@ public: { auto tasks = getTasks(settings); uASSERT_EQUAL(6u, tasks.size()); - // Note: count() is used here because the order of - // std::set> is not deterministic. + // Note: count() is used here because the order doesn't matter uASSERT_EQUAL(1u, count(tasks, "target1"s)); uASSERT_EQUAL(1u, count(tasks, "target2"s)); uASSERT_EQUAL(1u, count(tasks, "target3"s)); @@ -142,8 +141,8 @@ public: ctor::settings settings{}; { // Zero (Empty) - std::set> allTasks; - std::set> dirtyTasks; + std::vector> allTasks; + std::vector> dirtyTasks; for(auto& task : dirtyTasks) { @@ -156,10 +155,10 @@ public: { // Zero (One task, no dirty) auto task1 = std::make_shared("task1", false); - std::set> allTasks; - allTasks.insert(task1); + std::vector> allTasks; + allTasks.push_back(task1); - std::set> dirtyTasks; + std::vector> dirtyTasks; for(auto& task : dirtyTasks) { @@ -172,11 +171,11 @@ public: { // One (One task, one dirty) auto task1 = std::make_shared("task1", true); - std::set> allTasks; - allTasks.insert(task1); + std::vector> allTasks; + allTasks.push_back(task1); - std::set> dirtyTasks; - dirtyTasks.insert(task1); + std::vector> dirtyTasks; + dirtyTasks.push_back(task1); for(auto& task : dirtyTasks) { @@ -191,12 +190,12 @@ public: auto task1 = std::make_shared("task1", false); auto task2 = std::make_shared("task2", true); - std::set> allTasks; - allTasks.insert(task1); - allTasks.insert(task2); + std::vector> allTasks; + allTasks.push_back(task1); + allTasks.push_back(task2); - std::set> dirtyTasks; - dirtyTasks.insert(task2); + std::vector> dirtyTasks; + dirtyTasks.push_back(task2); for(auto& task : dirtyTasks) { @@ -213,12 +212,12 @@ public: std::vector deps = {"task1"}; auto task2 = std::make_shared("task2", true, deps); - std::set> allTasks; - allTasks.insert(task1); - allTasks.insert(task2); + std::vector> allTasks; + allTasks.push_back(task1); + allTasks.push_back(task2); - std::set> dirtyTasks; - dirtyTasks.insert(task2); + std::vector> dirtyTasks; + dirtyTasks.push_back(task2); for(auto& task : dirtyTasks) { @@ -235,13 +234,13 @@ public: std::vector deps = {"task1"}; auto task2 = std::make_shared("task2", true, deps); - std::set> allTasks; - allTasks.insert(task2); - allTasks.insert(task1); + std::vector> allTasks; + allTasks.push_back(task2); + allTasks.push_back(task1); - std::set> dirtyTasks; - dirtyTasks.insert(task2); - dirtyTasks.insert(task1); + std::vector> dirtyTasks; + dirtyTasks.push_back(task2); + dirtyTasks.push_back(task1); for(auto& task : dirtyTasks) { -- cgit v1.2.3