Merge pull request #1188 from xmake-io/argv

fix add_defines bug

9 files changed, 108 insertions, 50 deletions

diff --git a/core/src/tbox/tbox b/core/src/tbox/tbox
-Subproject a175e44090dac8891769d28968a7c06191b1813
+Subproject 3b24fdb96bccec99b2fbb99f1d6b04f67f249da
diff --git a/core/src/xmake/os/args.c b/core/src/xmake/os/args.c
index adfe843a9..4f1c89f7f 100644
--- a/core/src/xmake/os/args.c
+++ b/core/src/xmake/os/args.c
@@ -38,43 +38,35 @@ static tb_void_t tb_os_args_append(tb_string_ref_t result, tb_char_t const* cstr
     // check
     tb_assert_and_check_return(size < TB_PATH_MAXN);
 
-    // wrap and escape characters
+    // need wrap quote?
     tb_char_t ch;
-    tb_size_t n = 0;
     tb_char_t const* p = cstr;
     tb_bool_t wrap_quote = tb_false;
-    tb_char_t buff[TB_PATH_MAXN];
-    tb_size_t m = tb_arrayn(buff);
-    while ((ch = *p) && n < m)
+    if (!nowrap)
     {
-        // escape '"' or '\\'
-        if (ch == '\"' || (escape && ch == '\\'))
+        while ((ch = *p))
         {
-            if (n < m) buff[n++] = '\\';
+            if (ch == ' ' || ch == '(' || ch == ')') wrap_quote = tb_true;
+            p++;
         }
-        else if (ch == ' ' || ch == '(' || ch == ')') wrap_quote = tb_true;
-        if (n < m) buff[n++] = ch;
-        p++;
     }
-    tb_assert_and_check_return(n < m);
-    buff[n] = '\0';
 
-    // wrap "" if exists escape characters and spaces?
-    if (wrap_quote && !nowrap)
+    // wrap begin quote
+    if (wrap_quote) tb_string_chrcat(result, '\"');
+
+    // escape characters
+    p = cstr;
+    while ((ch = *p))
     {
-        tb_string_chrcat(result, '\"');
-        tb_size_t i = 0;
-        tb_char_t ch;
-        for (i = 0; i < n; i++)
-        {
-            ch = buff[i];
-            if (ch == '\\') // escape the '\\' characters in ""
-                tb_string_chrcat(result, '\\');
-            tb_string_chrcat(result, ch);
-        }
-        tb_string_chrcat(result, '\"');
+        // escape '"' or '\\'
+        if (ch == '\"' || ((escape || wrap_quote) && ch == '\\'))
+            tb_string_chrcat(result, '\\');
+        tb_string_chrcat(result, ch);
+        p++;
     }
-    else if (n) tb_string_cstrncat(result, buff, n);
+
+    // wrap end quote
+    if (wrap_quote) tb_string_chrcat(result, '\"');
 }
 
 /* //////////////////////////////////////////////////////////////////////////////////////
diff --git a/core/src/xmake/os/argv.c b/core/src/xmake/os/argv.c
index f484fc6cf..980be8863 100644
--- a/core/src/xmake/os/argv.c
+++ b/core/src/xmake/os/argv.c
@@ -42,7 +42,17 @@ tb_int_t xm_os_argv(lua_State* lua)
     tb_char_t const* args = luaL_checkstring(lua, 1);
     tb_check_return_val(args, 0);
 
-    // done
+    // split only? do not escape
+    tb_bool_t splitonly = tb_false;
+    if (lua_istable(lua, 2))
+    {
+        lua_pushstring(lua, "splitonly");
+        lua_gettable(lua, 2);
+        splitonly = lua_toboolean(lua, -1);
+        lua_pop(lua, 1);
+    }
+
+    // parse argument list
     tb_string_t arg;
     do
     {
@@ -88,7 +98,7 @@ tb_int_t xm_os_argv(lua_State* lua)
             }
 
             // save this charactor to argument
-            if (!skip) tb_string_chrcat(&arg, ch);
+            if (splitonly || !skip) tb_string_chrcat(&arg, ch);
 
             // step and cancel escape
             if (escape == 1) escape++;
@@ -114,7 +124,5 @@ tb_int_t xm_os_argv(lua_State* lua)
 
     // exit arg
     tb_string_exit(&arg);
-
-    // ok
     return 1;
 }
diff --git a/tests/apis/add_defines/.gitignore b/tests/apis/add_defines/.gitignore
new file mode 100644
index 000000000..152105761
--- /dev/null
+++ b/tests/apis/add_defines/.gitignore
@@ -0,0 +1,8 @@
+# Xmake cache
+.xmake/
+build/
+
+# MacOS Cache
+.DS_Store
+
+
diff --git a/tests/apis/add_defines/src/main.cpp b/tests/apis/add_defines/src/main.cpp
new file mode 100644
index 000000000..d16b296dc
--- /dev/null
+++ b/tests/apis/add_defines/src/main.cpp
@@ -0,0 +1,14 @@
+#include <iostream>
+
+using namespace std;
+
+int main(int argc, char** argv)
+{
+    cout << TEST1 << endl;
+    cout << TEST2 << endl;
+    cout << TEST3 << endl;
+    cout << TEST4 << endl;
+    cout << TEST5 << endl;
+    cout << TEST6 << endl;
+    return 0;
+}
diff --git a/tests/apis/add_defines/test.lua b/tests/apis/add_defines/test.lua
new file mode 100644
index 000000000..a4a38b0ce
--- /dev/null
+++ b/tests/apis/add_defines/test.lua
@@ -0,0 +1,3 @@
+function main()
+    os.exec("xmake")
+end
diff --git a/tests/apis/add_defines/xmake.lua b/tests/apis/add_defines/xmake.lua
new file mode 100644
index 000000000..1e00f4f62
--- /dev/null
+++ b/tests/apis/add_defines/xmake.lua
@@ -0,0 +1,10 @@
+add_rules("mode.debug", "mode.release")
+
+target("test")
+    set_kind("binary")
+    add_files("src/*.cpp")
+    add_defines("TEST1=\"hello\"")
+    add_defines("TEST2=\"hello xmake\"")
+    add_defines("TEST3=3")
+    add_cxflags("-DTEST4=\"hello\"")
+    add_cxflags("-DTEST5=\"hello xmake\" -DTEST6=3")
diff --git a/tests/modules/os/test.lua b/tests/modules/os/test.lua
index 407421a5c..2b26cd939 100644
--- a/tests/modules/os/test.lua
+++ b/tests/modules/os/test.lua
@@ -63,25 +63,51 @@ end
 
 function test_argv(t)
     t:are_equal(os.argv(""), {})
+    -- $cli aa bb cc
     t:are_equal(os.argv("aa bb cc"), {"aa", "bb", "cc"})
+    -- $cli aa --bb=bbb -c
     t:are_equal(os.argv("aa --bb=bbb -c"), {"aa", "--bb=bbb", "-c"})
-    t:are_equal(os.argv("\"aa bb cc\" dd"), {"aa bb cc", "dd"})
-    t:are_equal(os.argv("\"aa(bb)cc\" dd"), {"aa(bb)cc", "dd"})
-    t:are_equal(os.argv("aa\\bb/cc dd"), {"aa\\bb/cc", "dd"})
-    t:are_equal(os.argv("\"aa\\\\bb/cc dd\" ee"), {"aa\\bb/cc dd", "ee"})
-    t:are_equal(os.argv("\"aa\\\\bb/cc (dd)\" ee"), {"aa\\bb/cc (dd)", "ee"})
-    t:are_equal(os.argv("-D__prefix__=\\\"tbox\\\""), {"-D__prefix__=\"tbox\""})
+    -- $cli "aa bb cc" dd
+    t:are_equal(os.argv('"aa bb cc" dd'), {"aa bb cc", "dd"})
+    -- $cli aa(bb)cc dd
+    t:are_equal(os.argv('"aa(bb)cc" dd'), {"aa(bb)cc", "dd"})
+    -- $cli aa\\bb/cc dd
+    t:are_equal(os.argv('aa\\bb/cc dd'), {"aa\\bb/cc", "dd"})
+    -- $cli "aa\\bb/cc dd" ee
+    t:are_equal(os.argv('"aa\\\\bb/cc dd" ee'), {"aa\\bb/cc dd", "ee"})
+    -- $cli "aa\\bb/cc (dd)" ee
+    t:are_equal(os.argv('"aa\\\\bb/cc (dd)" ee'), {"aa\\bb/cc (dd)", "ee"})
+    -- $cli -DTEST=\"hello\"
+    t:are_equal(os.argv('-DTEST=\\"hello\\"'), {'-DTEST="hello"'})
+    -- $cli -DTEST=\"hello\" -DTEST=\"hello\"
+    t:are_equal(os.argv('-DTEST=\\"hello\\" -DTEST2=\\"hello\\"'), {'-DTEST="hello"', '-DTEST2="hello"'})
+    -- $cli -DTEST="hello"
+    t:are_equal(os.argv('-DTEST="hello"'), {'-DTEST=hello'})
+    -- $cli -DTEST="hello world"
+    t:are_equal(os.argv('-DTEST="hello world"'), {'-DTEST=hello world'})
+    -- $cli -DTEST=\"hello world\"
+    t:are_equal(os.argv('-DTEST=\\"hello world\\"'), {'-DTEST="hello', 'world\"'})
+    -- $cli "-DTEST=\"hello world\"" "-DTEST2="\hello world2\""
+    t:are_equal(os.argv('"-DTEST=\\\"hello world\\\"" "-DTEST2=\\\"hello world2\\\""'), {'-DTEST="hello world"', '-DTEST2="hello world2"'})
+    -- $cli '-DTEST="hello world"' '-DTEST2="hello world2"'
+    t:are_equal(os.argv("'-DTEST=\"hello world\"' '-DTEST2=\"hello world2\"'"), {'-DTEST="hello world"', '-DTEST2="hello world2"'})
+    -- only split
+    t:are_equal(os.argv('-DTEST="hello world"', {splitonly = true}), {'-DTEST="hello world"'})
+    t:are_equal(os.argv('-DTEST="hello world" -DTEST2="hello world2"', {splitonly = true}), {'-DTEST="hello world"', '-DTEST2="hello world2"'})
 end
 
 function test_args(t)
     t:are_equal(os.args({}), "")
     t:are_equal(os.args({"aa", "bb", "cc"}), "aa bb cc")
     t:are_equal(os.args({"aa", "--bb=bbb", "-c"}), "aa --bb=bbb -c")
-    t:are_equal(os.args({"aa bb cc", "dd"}), "\"aa bb cc\" dd")
-    t:are_equal(os.args({"aa(bb)cc", "dd"}), "\"aa(bb)cc\" dd")
+    t:are_equal(os.args({"aa bb cc", "dd"}), '"aa bb cc" dd')
+    t:are_equal(os.args({"aa(bb)cc", "dd"}), '"aa(bb)cc" dd')
     t:are_equal(os.args({"aa\\bb/cc", "dd"}), "aa\\bb/cc dd")
-    t:are_equal(os.args({"aa\\bb/cc dd", "ee"}), "\"aa\\\\bb/cc dd\" ee")
-    t:are_equal(os.args({"aa\\bb/cc (dd)", "ee"}), "\"aa\\\\bb/cc (dd)\" ee")
-    t:are_equal(os.args("-D__prefix__=\"tbox\""), "-D__prefix__=\\\"tbox\\\"")
+    t:are_equal(os.args({"aa\\bb/cc dd", "ee"}), '"aa\\\\bb/cc dd" ee')
+    t:are_equal(os.args({"aa\\bb/cc (dd)", "ee"}), '"aa\\\\bb/cc (dd)" ee')
     t:are_equal(os.args({"aa\\bb/cc", "dd"}, {escape = true}), "aa\\\\bb/cc dd")
+    t:are_equal(os.args('-DTEST="hello"'), '-DTEST=\\"hello\\"')
+    t:are_equal(os.args({'-DTEST="hello"', '-DTEST2="hello"'}), '-DTEST=\\"hello\\" -DTEST2=\\"hello\\"')
+    t:are_equal(os.args('-DTEST=hello'), '-DTEST=hello') -- irreversible
+    t:are_equal(os.args({'-DTEST="hello world"', '-DTEST2="hello world2"'}), '"-DTEST=\\\"hello world\\\"" "-DTEST2=\\\"hello world2\\\""')
 end
diff --git a/xmake/core/tool/builder.lua b/xmake/core/tool/builder.lua
index 984b23992..3433813c9 100644
--- a/xmake/core/tool/builder.lua
+++ b/xmake/core/tool/builder.lua
@@ -364,23 +364,20 @@ end
 -- preprocess flags
 function builder:_preprocess_flags(flags)
 
-    -- remove repeat
-    flags = table.unique(flags)
-
-    -- split flag group, e.g. "-I /xxx" => {"-I", "/xxx"}
+    -- remove repeat first and split flags group, e.g. "-I /xxx" => {"-I", "/xxx"}
+    local unique = {}
     local results = {}
     for _, flag in ipairs(flags) do
         flag = flag:trim()
-        if #flag > 0 then
+        if #flag > 0 and not unique[flag] then
             if flag:find(" ", 1, true) then
-                table.join2(results, os.argv(flag))
+                table.join2(results, os.argv(flag, {splitonly = true}))
             else
                 table.insert(results, flag)
             end
+            unique[flag] = true
         end
     end
-
-    -- get it
     return results
 end