feat(build): change cxxflags from string to array

README.md

@@ -108,20 +108,24 @@ simple_lib = { git = "https://github.com/cppship/demo_lib.git", commit = "25dbed
 scnlib = "1.1.2"
 
 [profile]
-# by default, apply to all profiles
-cxxflags = "-Wall -Wextra -Werror -Wno-unused-parameter -Wno-missing-field-initializers"
-definitions = ["A", "B"]
+definitions = ["BOOST_PROCESS_USE_STD_FS"]
+
+[profile.'cfg(not(compiler = "msvc"))']
+cxxflags = ["-Wall", "-Wextra", "-Werror", "-Wno-unused-parameter", "-Wno-missing-field-initializers"]
+
+[profile.'cfg(compiler = "msvc")']
+cxxflags = ["/Zc:__cplusplus", "/Zc:preprocessor", "/MP"]
 
 [profile.debug]
 # appends to cxxflags in [profile]
-cxxflags = "-g"
+cxxflags = ["-g"]
 
 # merged with definitions in [profile]
 definitions = ["C"]
 
 [profile.release]
 # appends to cxxflags in [profile]
-cxxflags = "-O3 -DNDEBUG"
+cxxflags = ["-O3", "-DNDEBUG"]
 ```
 
 ## header-only lib

cppship.toml

@@ -18,7 +18,7 @@ spdlog = "1.11.0"
 definitions = ["BOOST_PROCESS_USE_STD_FS"]
 
 [profile.'cfg(not(compiler = "msvc"))']
-cxxflags = "-Wall -Wextra -Werror -Wno-unused-parameter -Wno-missing-field-initializers"
+cxxflags = ["-Wall", "-Wextra", "-Werror", "-Wno-unused-parameter", "-Wno-missing-field-initializers"]
 
 [profile.'cfg(compiler = "msvc")']
-cxxflags = "/Zc:__cplusplus /Zc:preprocessor /MP"
+cxxflags = ["/Zc:__cplusplus", "/Zc:preprocessor", "/MP"]

include/cppship/cmake/generator.h

@@ -49,6 +49,7 @@ class CmakeGenerator {
     void emit_footer_();
 
 private:
+    void fill_default_profile_();
     void fill_profile_(Profile profile);
 
 private:

include/cppship/core/profile.h

@@ -58,12 +58,12 @@ enum class ProfileCondition {
 };
 
 struct ProfileConfig {
-    std::string cxxflags;
+    std::vector<std::string> cxxflags;
     std::vector<std::string> definitions;
 };
 
 struct ProfileOptions {
-    std::string cxxflags;
+    std::vector<std::string> cxxflags;
     std::vector<std::string> definitions;
 
     // TODO: refine me

lib/cmake/generator.cpp

@@ -7,7 +7,6 @@
 #include "cppship/core/manifest.h"
 #include "cppship/exception.h"
 #include "cppship/util/log.h"
-#include "cppship/util/string.h"
 
 #include <boost/algorithm/string/join.hpp>
 #include <fmt/core.h>
@@ -24,6 +23,8 @@ using namespace cppship;
 using namespace cppship::cmake;
 using namespace fmt::literals;
 
+using boost::join;
+
 std::vector<cmake::Dep> cmake::collect_cmake_deps(
     const std::vector<DeclaredDependency>& declared_deps, const ResolvedDependencies& deps)
 {
@@ -99,32 +100,8 @@ enable_testing()
 
 )";
 
-    const auto& default_profile = mManifest.default_profile();
-    if (!default_profile.cxxflags.empty() || !default_profile.definitions.empty()) {
-        mOut << "# cpp options\n";
-        mOut << fmt::format("add_compile_options({})\n", default_profile.cxxflags);
-        mOut << fmt::format("add_compile_definitions({})\n", boost::join(default_profile.definitions, " "));
-
-        if (auto conf = default_profile.config.find(ProfileCondition::msvc); conf != default_profile.config.end()) {
-            mOut << fmt::format(R"(
-if(MSVC)
-    add_compile_options({})
-    add_compile_definitions({})
-endif()
-)",
-                conf->second.cxxflags, boost::join(conf->second.definitions, " "));
-        }
-
-        if (auto conf = default_profile.config.find(ProfileCondition::non_msvc); conf != default_profile.config.end()) {
-            mOut << fmt::format(R"(
-if(NOT MSVC)
-    add_compile_options({})
-    add_compile_definitions({})
-endif()
-)",
-                conf->second.cxxflags, boost::join(conf->second.definitions, " "));
-        }
-    }
+    mOut << "# cpp options\n";
+    fill_default_profile_();
 
     mOut << "\n# profile cpp options\n";
     fill_profile_(Profile::debug);
@@ -366,16 +343,50 @@ include(CPack)
 )";
 }
 
+namespace {
+
+inline std::string format_condition(ProfileCondition condition)
+{
+    switch (condition) {
+    case ProfileCondition::msvc:
+        return "MSVC";
+
+    case ProfileCondition::non_msvc:
+        return "NOT MSVC";
+    }
+
+    std::abort();
+}
+
+}
+
+void CmakeGenerator::fill_default_profile_()
+{
+    const auto& default_profile = mManifest.default_profile();
+    if (!default_profile.cxxflags.empty()) {
+        mOut << fmt::format("add_compile_options({})\n", join(default_profile.cxxflags, " "));
+    }
+    if (!default_profile.definitions.empty()) {
+        mOut << fmt::format("add_compile_definitions({})\n", join(default_profile.definitions, " "));
+    }
+
+    for (const auto& [condition, conf] : default_profile.config) {
+        mOut << fmt::format(R"(
+if({})
+    add_compile_options({})
+    add_compile_definitions({})
+endif()
+)",
+            format_condition(condition), join(conf.cxxflags, " "), join(conf.definitions, " "));
+    }
+}
+
 void CmakeGenerator::fill_profile_(Profile profile)
 {
     const auto& options = mManifest.profile(profile);
     const auto& profile_str = to_string(profile);
 
-    for (const auto& opt : util::split(options.cxxflags, boost::is_space())) {
-        if (opt.empty()) {
-            continue;
-        }
-
+    for (const auto& opt : options.cxxflags) {
         mOut << fmt::format("add_compile_options($<$<CONFIG:{}>:{}>)\n", profile_str, opt);
     }
     for (const auto& def : options.definitions) {

lib/core/manifest.cpp

@@ -24,6 +24,20 @@ template <class T = toml::value> T get(const toml::value& value, const std::stri
     return toml::find<T>(value, key);
 }
 
+std::vector<std::string> get_list(const toml::value& value, const std::string& key)
+{
+    if (value.is_uninitialized() || !value.contains(key)) {
+        return {};
+    }
+
+    const auto& content = value.at(key);
+    if (!content.is_array()) {
+        throw Error { fmt::format("invalid manifest: {} should be a table", key) };
+    }
+
+    return get<std::vector<std::string>>(content);
+}
+
 CxxStd get_cxx_std(const toml::value& value)
 {
     const auto val = toml::find_or<int>(value, "std", 17);
@@ -110,8 +124,7 @@ void check_dependency_dups(const std::vector<DeclaredDependency>& deps, const st
     }
 }
 
-ProfileOptions parse_profile_options(
-    const toml::value& manifest, const std::string& key, const std::string& cxxflags_default = "")
+ProfileOptions parse_profile_options(const toml::value& manifest, const std::string& key)
 {
     const auto profile = toml::find_or(manifest, key, {});
     ProfileOptions result;
@@ -126,22 +139,22 @@ ProfileOptions parse_profile_options(
             const auto conf = boost::replace_all_copy(profile_key, " ", "");
             if (conf == R"(cfg(compiler="msvc"))") {
                 result.config[ProfileCondition::msvc] = {
-                    .cxxflags = toml::find_or<std::string>(val, "cxxflags", ""),
-                    .definitions = toml::find_or<std::vector<std::string>>(val, "definitions", {}),
+                    .cxxflags = get_list(val, "cxxflags"),
+                    .definitions = get_list(val, "definitions"),
                 };
             } else if (conf == R"(cfg(not(compiler="msvc")))") {
                 result.config[ProfileCondition::non_msvc] = {
-                    .cxxflags = toml::find_or<std::string>(val, "cxxflags", ""),
-                    .definitions = toml::find_or<std::vector<std::string>>(val, "definitions", {}),
+                    .cxxflags = get_list(val, "cxxflags"),
+                    .definitions = get_list(val, "definitions"),
                 };
             } else {
                 throw Error { fmt::format("invalid config {}", profile_key) };
             }
         }
     }
 
-    result.cxxflags = toml::find_or<std::string>(profile, "cxxflags", cxxflags_default);
-    result.definitions = toml::find_or<std::vector<std::string>>(profile, "definitions", {});
+    result.cxxflags = get_list(profile, "cxxflags");
+    result.definitions = get_list(profile, "definitions");
 
     return result;
 }

tests/core/manifest.cpp

@@ -245,15 +245,16 @@ name = "abc"
 version = "0.1.0"
 
 [profile]
-cxxflags = "-Wall"
+cxxflags = ["-Wall"]
 definitions = ["A", "B"]
     )");
 
     prof = meta.default_profile();
-    ASSERT_EQ(prof.cxxflags, "-Wall");
-    ASSERT_EQ(prof.definitions.size(), 2);
+    ASSERT_EQ(prof.cxxflags.size(), 1);
+    ASSERT_EQ(prof.cxxflags[0], "-Wall");
 
     ranges::sort(prof.definitions);
+    ASSERT_EQ(prof.definitions.size(), 2);
     ASSERT_EQ(prof.definitions[0], "A");
     ASSERT_EQ(prof.definitions[1], "B");
 }
@@ -266,25 +267,27 @@ version = "0.1.0"
     )");
 
     auto prof = meta.profile(Profile::debug);
-    EXPECT_EQ(prof.cxxflags, "");
+    ASSERT_TRUE(prof.cxxflags.empty());
     ASSERT_TRUE(prof.definitions.empty());
 
     meta = mock_manifest(R"([package]
 name = "abc"
 version = "0.1.0"
 
 [profile.debug]
-cxxflags = "-Wall"
+cxxflags = ["-Wall", "-Wextra"]
 definitions = ["A", "B"]
     )");
 
     prof = meta.profile(Profile::debug);
-    EXPECT_EQ(prof.cxxflags, "-Wall");
-    EXPECT_EQ(prof.definitions.size(), 2);
+    ASSERT_EQ(prof.cxxflags.size(), 2);
+    ASSERT_EQ(prof.cxxflags[0], "-Wall");
+    ASSERT_EQ(prof.cxxflags[1], "-Wextra");
 
     ranges::sort(prof.definitions);
-    EXPECT_EQ(prof.definitions[0], "A");
-    EXPECT_EQ(prof.definitions[1], "B");
+    ASSERT_EQ(prof.definitions.size(), 2);
+    ASSERT_EQ(prof.definitions[0], "A");
+    ASSERT_EQ(prof.definitions[1], "B");
 }
 
 TEST(manifest, ProfileRelease)
@@ -295,25 +298,31 @@ version = "0.1.0"
     )");
 
     auto prof = meta.profile(Profile::release);
-    EXPECT_EQ(prof.cxxflags, "");
+    ASSERT_TRUE(prof.cxxflags.empty());
     ASSERT_TRUE(prof.definitions.empty());
 
     meta = mock_manifest(R"([package]
 name = "abc"
 version = "0.1.0"
 
 [profile.release]
-cxxflags = "-Wall"
+cxxflags = ["-Wall"]
 definitions = ["A", "B"]
     )");
 
     prof = meta.profile(Profile::release);
-    EXPECT_EQ(prof.cxxflags, "-Wall");
-    EXPECT_EQ(prof.definitions.size(), 2);
+    ASSERT_EQ(prof.cxxflags.size(), 1);
+    ASSERT_EQ(prof.cxxflags[0], "-Wall");
 
     ranges::sort(prof.definitions);
-    EXPECT_EQ(prof.definitions[0], "A");
-    EXPECT_EQ(prof.definitions[1], "B");
+    ASSERT_EQ(prof.definitions.size(), 2);
+    ASSERT_EQ(prof.definitions[0], "A");
+    ASSERT_EQ(prof.definitions[1], "B");
+
+    for (const auto& prof : { meta.default_profile(), meta.profile(Profile::debug) }) {
+        ASSERT_TRUE(prof.cxxflags.empty());
+        ASSERT_TRUE(prof.definitions.empty());
+    }
 }
 
 TEST(manifest, ProfileCfg)
@@ -323,23 +332,25 @@ name = "abc"
 version = "0.1.0"
 
 [profile.'cfg(not(compiler = "msvc"))']
-cxxflags = "-Wall"
+cxxflags = ["-Wall"]
 
 [profile.'cfg(compiler = "msvc")']
-cxxflags = "/MP"
+cxxflags = ["/MP"]
 definitions = ["A"]
     )");
 
     auto prof = meta.default_profile();
-    EXPECT_EQ(prof.cxxflags, "");
+    ASSERT_TRUE(prof.cxxflags.empty());
     ASSERT_TRUE(prof.definitions.empty());
     ASSERT_TRUE(prof.config.contains(ProfileCondition::msvc));
     ASSERT_TRUE(prof.config.contains(ProfileCondition::non_msvc));
 
-    EXPECT_EQ(prof.config[ProfileCondition::msvc].cxxflags, "/MP");
-    EXPECT_EQ(prof.config[ProfileCondition::msvc].definitions.size(), 1);
+    ASSERT_EQ(prof.config[ProfileCondition::msvc].cxxflags.size(), 1);
+    EXPECT_EQ(prof.config[ProfileCondition::msvc].cxxflags[0], "/MP");
+    ASSERT_EQ(prof.config[ProfileCondition::msvc].definitions.size(), 1);
     EXPECT_EQ(prof.config[ProfileCondition::msvc].definitions[0], "A");
 
-    EXPECT_EQ(prof.config[ProfileCondition::non_msvc].cxxflags, "-Wall");
+    ASSERT_EQ(prof.config[ProfileCondition::non_msvc].cxxflags.size(), 1);
+    EXPECT_EQ(prof.config[ProfileCondition::non_msvc].cxxflags[0], "-Wall");
     EXPECT_EQ(prof.config[ProfileCondition::non_msvc].definitions.size(), 0);
 }