Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add windows mfc rule #216

Merged
merged 9 commits into from
Sep 6, 2018
Merged

add windows mfc rule #216

merged 9 commits into from
Sep 6, 2018

Conversation

xigalto
Copy link
Contributor

@xigalto xigalto commented Sep 4, 2018

usage:

target("test")
    add_files("*.cpp")
    add_files("*.rc")
    -- use sharedmfc
    add_rules("win.sdk.mfc.shared_app")
    -- or add_rules("win.sdk.mfc.static_app") to use staticmfc
target_end()

useage:

target("test")
    add_files("*.cpp")
    add_files("*.rc")
    -- use sharedmfc
    add_rules("win.sdk.mfcapp.shared")
    -- or add_rules("win.sdk.mfcapp.static") to use staticmfc
target_end()
end
target:set(flagsname, flags)
end

Copy link
Member

@waruqi waruqi Sep 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

xmake.lua是描述文件,仅用于描述规则的设置和定义,不要直接在外层定义function脚本,如果有需要写复杂的脚本逻辑,请参考: https://github.com/tboox/xmake/tree/master/xmake/rules/qt/moc

可以建个winsdk/mfc目录,专门用于存放mfc的规则,例如:

winsdk
  - dotnet
  - mfc
     - xmake.lua  -- mfc的规则描述配置(会被自动加载的),用于定义规则,可以在里面通过 import("mfc")导入脚本调用
     - mfc.lua -- mfc规则的复杂脚本逻辑的封装,通过独立模块的方式 开发接口调用,内部可以自定义私有实现

@@ -53,3 +116,60 @@ rule("win.sdk.application")
target:add("links", "cfgmgr32", "comdlg32", "setupapi", "strsafe", "shlwapi")
end)

-- define rule: sharedmfcapp
rule("win.sdk.mfcapp.shared")

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

此规则定义移动到独立 winsdk/mfc/xmake.lua里面吧

-- save local functions
local remove_exists_flags = _rule_remove_exists_flags
local check_is_unicode = _rule_check_is_unicode

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不要在外层描述域定义变量,也挪到独立mfc.lua模块中去吧


-- add mfc flags
target:values_set("project.vs.mfc", true)
end)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个就不用设置了,target:values_set设置是对用户开发设置接口,内部模块的私有数据传递,可以通过target:data_set来设置,而且此处 不需要额外设置了,在vcproj插件里面可以通过 if target:rule("win.sdk.mfc...") then的当时,直接判断是否应用了mfc规则。。

for key, define in pairs(defines) do
define = define:lower()
if define:find("unicode") or define:find("_unicode") then
return true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

可以通过 define:trim() == "unicode" 来判断,find并不保证完全匹配对,如果有带有 __UNICODE_xxx的其他宏 就会被干扰了。。而且plain text查找,尽量用:find("xxx", 1, true) ,默认是带lua 模式匹配的

function _rule_check_is_unicode(target)
local defines = target:get("defines")
for key, define in pairs(defines) do
define = define:lower()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defines是数组,应该用 ipairs, for _, define in ipairs(defines) do

-- add flags
target:add("ldflags", "-subsystem:windows", {force = true})
target:add("ldflags", ifelse(check_is_unicode(target), "-entry:wWinMainCRTStartup", "-entry:WinMainCRTStartup"), {force = true})

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ifelse 是早期的接口了,后面我一般尽量不去用了,建议用 if_true and A or B 方式来替代,会更好:

check_is_unicode(target) and "-entry:wWinMainCRTStartup" or "-entry:WinMainCRTStartup"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

添加entry入口,可以参考下面的代码,尽量先判断下用户在xmake.lua有没有自己改写entry,没有的话再设置默认入口

    -- set default driver entry if does not exist
    local entry = false
    for _, ldflag in ipairs(target:get("ldflags")) do
        ldflag = ldflag:lower()
        if ldflag:find("[/%-]entry:") then
            entry = true
            break
        end
    end
    if not entry then
        target:add("links", "WdfDriverEntry")
        target:add("ldflags", "-entry:FxDriverEntry" .. (is_arch("x86") and "@8" or ""), {force = true})
    end


-- set runtimelibrary
target:add("cxflags", ifelse(is_mode("debug"), "-MDd", "-MD"))
target:add("defines", "AFX", "_AFXDLL")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

此处也是,可以简化下:target:add("cxflags", is_mode("debug") and "-MDd" or "-MD")

不过这里用ifelse也没啥问题,ifelse(A, B, C)的一个问题是不管 B 跟 C,是否条件被满足,里面的代码都会预先执行掉

@codecov
Copy link

codecov bot commented Sep 4, 2018

Codecov Report

Merging #216 into dev will increase coverage by 0.01%.
The diff coverage is 68.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #216      +/-   ##
==========================================
+ Coverage   53.73%   53.75%   +0.01%     
==========================================
  Files         335      337       +2     
  Lines       15409    15428      +19     
==========================================
+ Hits         8280     8293      +13     
- Misses       7129     7135       +6
Impacted Files Coverage Δ
xmake/rules/winsdk/xmake.lua 21.42% <ø> (ø) ⬆️
xmake/rules/winsdk/mfc/env/xmake.lua 100% <100%> (ø)
xmake/rules/winsdk/mfc/xmake.lua 66.66% <66.66%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f330ff...e8af37c. Read the comment docs.

-- FIXME: before load need check of vs's minverion, if defined
--before_load(function (target)
--end)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

通用的环境预加载,可以通过 加个 win.sdk.mfc.env 的依赖规则,优先执行:

rule("win.sdk.mfc.env")
     -- FIXME: before load need check of vs's minverion, if defined
    --before_load(function (target)
    --end)

rule("win.sdk.mfcapp.shared")

    add_deps("win.sdk.mfc.env")

     -- after load
    after_load(function (target)
         -- apply mfc settings
        import("mfc").mfc_shared_app(target)        
    end)

 -- define rule: static mfcapp
rule("win.sdk.mfcapp.static")

    add_deps("win.sdk.mfc.env")

     -- after load
    after_load(function (target)
         -- apply mfc settings
        import("mfc").mfc_static_app(target)
    end)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个是想这样弄的,哈哈,就是当时没发现rule怎么继承另外的rule

-- Copyright (C) 2015 - 2018, TBOOX Open Source Group.
--
-- @author ruki
-- @file xmake.lua
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

author 可以标注下你的名字 😄


-- define rule: static mfcapp
rule("win.sdk.mfcapp.static")

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

规则名也改下吧,改成 win.sdk.mfc.static_appwin.sdk.mfc.shared_app吧,之后还能扩展支持mfc的静态库,动态库 win.sdk.mfc.static, win.sdk.mfc.shared

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个是这样计划的 win.sdk.mfcdll.static,win.sdk.mfcdll.shared,格式比较规整点

rename:
win.sdk.mfcapp.shared -> win.sdk.mfc.shared_app
win.sdk.mfcapp.static -> win.sdk.mfc.static_app

-- define rule: mfc
rule("win.sdk.mfc")

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个改成 win.sdk.mfc.env吧

for key, define in pairs(defines) do
define = define:lower()
if define:find("unicode") or define:find("_unicode") then
return "-entry:wWinMainCRTStartup"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

带unicode字样的macro defines说不定会有误判,还是改成 完整匹配 判断吧, define:trim() strip调用空格后判断就行了

        define = define:lower():trim()
        if define == "unicode" or define == "_unicode" then

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已调整,的确不太严谨,其他几个flag也加入了校验

@waruqi waruqi added this to the v2.2.2 milestone Sep 4, 2018
flag = flag:lower():trim()
if flag:find("^[/%-]?mt[d]?$") or flag:find("^[/%-]?md[d]?$") then
flags[key] = nil
end
Copy link
Member

@waruqi waruqi Sep 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里似乎有点问题,flags是个 array,首先应该是用 ipair去遍历,其次在遍历途中,直接改写flags自身,会有潜在的问题,尤其是设置成nil。

此处的 flags[key] = nil,也就是 flags[index] = nil,会在array中 产生空洞。。。导致下次遍历flags的时候,遇到nil元素后就提前截止了,也会影响 array的length值,建议先用一个临时table保存下。

并且可以通过table.wrap接口将string值wrap成array,统一处理,不需要分别判断table/string 分别处理

建议通过如下处理,更为安全可靠,不会丢失部分flags设置:

        local results = {}
        for _, flag in ipairs(table.wrap(flags)) do
            flag = flag:lower():trim()
            if not flag:find("^[/%-]?mt[d]?$") and not flag:find("^[/%-]?md[d]?$") then
                table.insert(results, flag)
            end
        end
        flags = results

if flag:find("[/%-]mt") or flag:find("[/%-]md") then
flags = {}
end
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

此处可通过 table.wrap(flags) 跟table的处理进行合并统一处理

if define:find("^[/%-]?mt[d]?$") or define:find("^[/%-]?md[d]?$") then
if type(defines) == "table" then
defines[key] = nil
else
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这几处同上。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:)好的,lua还不是很熟,我改下。

flag = flag:lower():trim()
if flag:find("^[/%-]?mt[d]?$") or flag:find("^[/%-]?md[d]?$") then
flags[key] = nil
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

此处的flags遍历内部直接改写,也处理下吧,我觉得还是有问题的。。

Copy link
Contributor Author

@xigalto xigalto Sep 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

额知道了,flags并没有被转化,转化的地方弄错了,明天去提交下

Copy link
Member

@waruqi waruqi Sep 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flags还在遍历过程中,去改flags的元素会有隐患,最好通过一个临时table先存储下修改后的结果,再到循环外更新整个flags

改成,

    local tmp = {} -- 新建个tmp数组存储新的结果
    for _, flag in ipairs(table.wrap(flags)) do -- 此处是array数组遍历,得用ipairs
        flag = flag:lower():trim()
        if not flag:find("^[/%-]?mt[d]?$") and not flag:find("^[/%-]?md[d]?$") then
            table.insert(tmp, flag)
        end
    end
    flags = tmp

而且flags是个array不是dictionary, 通过flags[] = nil 方式去设置nil,如果之后的值非nil,就被截断了,你可以验证下:

$ xmake lua
> a = {1, 2, 3, nil, 5, 6}
> #a  -- 这个时候a数组的长度只有3,即使通过ipairs去for循环遍历,也到3就终止了,5跟6就被丢弃了

-- remove exists md or mt
function _mfc_remove_mt_md_flags(target, flagsname)
local flags = table.wrap(target:get(flagsname))
for key, flag in pairs(flags) do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

此处改动 跟之前的没什么本质区别么,之前提的那个问题还是存在

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

应该不会吧,只要flags是个table就可以了,之前是因为放到for中了,flags并不一定是table。这个我测试过了是没问题的,你有疑惑的话可以写个demo测试下,自己加入MT,MD之类的看看是否过滤了就知道了

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我给你造个测试用例复现吧:

function main()
    local flags = table.wrap({"1", "2", "3", "-mt", "4", "5", "6"})
    for key, flag in pairs(flags) do
        flag = flag:lower():trim()
        if flag:find("^[/%-]?mt[d]?$") or flag:find("^[/%-]?md[d]?$") then
            flags[key] = nil
        end
    end
    for _, flag in ipairs(flags) do
        print(flag)
    end
     print(#flags)
end

你保存为 ./test.lua 然后直接运行测试:

$ xmake lua ./test.lua

正常结果应该是从{"1", "2", "3", "-mt", "4", "5", "6"} 移除 -mt 后得到结果 {"1", "2", "3", "4", "5", "6"}

但是实际输出的flags结果只有 {"1", "2", "3"},被截断了,flags最终长度也只有3, 你可以试试。

因为flags是个array数组,在luajit中,中间元素不能有nil值,否则结果未定义,有可能会被截断,所以我一般不会这么写,会有各种隐患和未定义行为。

@waruqi waruqi merged commit e4d6de3 into xmake-io:dev Sep 6, 2018
@waruqi
Copy link
Member

waruqi commented Sep 6, 2018

反向基于index遍历移除 倒也是可以的,多谢贡献 已merge

@waruqi
Copy link
Member

waruqi commented Sep 6, 2018

最近的pr更新我已经更新到 开发包了,https://github.com/tboox/xmake/releases/download/v2.2.1/xmake-v2.2.2-dev.exe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants