xmake-io / xmake

šŸ”„ A cross-platform build utility based on Lua
https://xmake.io
Apache License 2.0
9.82k stars 774 forks source link

[Feature request] XMake doesn't trigger an error when using an invalid mode #1523

Closed SirLynix closed 3 years ago

SirLynix commented 3 years ago

I have a xmake project using the following:

add_rules("mode.asan", "mode.debug", "mode.releasedbg")

So only asan, debug and releasedbg modes. When generating a VS project only those appears in the list for example.

However, when running xmake on a clean folder, it defaults to the "release" mode, which is not configured correctly since I only expect "releasedbg".

That's the first issue, I would expect xmake to default to either debug or releasedbg. Maybe a set_defaultmode would help?

The second issue is that when you make a typo, xmake doesn't care (notice the missing d):

lynix@SirLynixVanDesktop:/mnt/c/Projets/obs-kinect/obs-kinect$ xmake.exe f --mode=releasebg
checking for platform ... windows
checking for architecture ... x64
checking for Microsoft Visual Studio (x64) version ... 2019

XMake allows unknown mode without warning/error, which is weird.

This is kinda a follow-up of #1503, as xmake doesn't trigger any warning/error when an invalid (or unexpected) arch is set (for example ("x64" on Linux, which would mostly work but will fail for some things such as xmake run as detailed in #1503).

I think XMake should warn the user when a mistake like this is made, otherwise xmake behavior can be unsettling.

waruqi commented 3 years ago

Because users can set custom modes, xmake will not provide any mode definition standards.

$ xmake f -m mymode
if is_mode("mymode") then
    add_defines("xxxx")
end

or user can add custom mode rule, rule("xxxx_mode")

mode.asan mode.debug and so on are just some common mode rules provided by xmake to help users simplify, but they are not standard, and it does not mean that users can only set these modes.

Users can also not use them at all and define the debug/release mode rules by themselves.

Therefore, xmake will not do any check, because any mode is normal.

The user can use rule to define them, or directly use if is_mode() to configure them

SirLynix commented 3 years ago

It seems weird because if one doesn't use add_rules("mode.debug") in their project, then debug mode will just not work as expected (or not do what is expected from it).

set_allowedmodes and set_allowedarch calls would be great to help with this, as it could trigger an error if mode/arch isn't what's expected.

set_allowedmodes("asan", "debug", "releasedbg")
set_allowedarchs("x86", "x86_64")
$ xmake f --mode=release
> release is not a valid mode for this project: please use one of asan, debug, releasedbg

$ xmake f --arch=x64
> x64 is not a valid arch for this project, please use one of x86, x86_64

I know we can do that ourself in a script, but I'm thinking this could also be used by xmake to set the default mode/arch (to the first entry of allowedX for example), it would also help to set custom modes for VS projects (as I don't think this is possible otherwise?).

Plus this would not break compatibility as project not calling set_allowedX would behave like they do now.

What do you think?

Edit: alternative way of setting the default:

set_allowedmodes("asan", "debug", "releasedbg", { default = "debug" })
set_allowedarchs("x86", "x86_64", { debug = "x86_64" })
waruqi commented 3 years ago

It seems weird because if one doesn't use add_rules("mode.debug") in their project, then debug mode will just not work as expected (or not do what is expected from it).

add_rules("mode.debug") only provides a way to quickly configure common debug mode configurations, but it is not the only way.

Many of our projects will completely customize the debug mode and do not use mode.debug rules, for example:

if is_mode("debug") then
     set_optimize("none")
     add_defines("xxxx")
end

set_allowedmodes and set_allowedarch calls would be great to help with this, as it could trigger an error if mode/arch isn't what's expected.

Of course, if the project author wants to limit the collection of modes and architectures, I can also consider supporting it in the future.

SirLynix commented 3 years ago

Oh ok, I though builtin rules were somewhat special, but from xmake internals it seems all you have to do is add a rule named mode.xxx and add it to your project. I better understand why you allow every mode.

However it seems not to be true for arch, so yeah, here's a feature request:

$ xmake f --arch=x64

x64 is not a valid arch for this project, please use one of x86, x86_64

Or maybe:

set_allowedmodes("asan", "debug", "releasedbg")
set_allowedarchs("x86", "x86_64")

set_defaultmode("debug")
set_defaultarch("x86_64")

(adding set_defaultmode and set_defaultarch would allow one to set a default mode/arch without forcing a list of allowed values).

waruqi commented 3 years ago

I have supported it, can you try it first? https://github.com/xmake-io/xmake/pull/1527

add_allowedmodes("releasedbg", "debug", {default = "releasedbg"})
add_allowedplats("windows", "mingw", {default = "windows"})
add_allowedplats("windows", "mingw", "linux", "macosx")
add_allowedarchs("arm64", "x86_64", {plat = "macosx", default = "arm64"})
add_allowedarchs("i386", {plat = "linux"})
SirLynix commented 3 years ago

Amazing!

I'm trying to update to the allowed branch but it seems I can't

$ xmake update -vs allowed
pinging for the host(github.com) ... 46 ms
pinging for the host(gitlab.com) ... 35 ms
pinging for the host(gitee.com) ... 65535 ms
/usr/bin/git ls-remote --refs https://gitlab.com/tboox/xmake.git
error: unable to select version for range 'allowed'

Also, I see that you went for the {default=""} option, after thinking about it, wouldn't set_defaultmode and set_defaultarch be a better option as they allow one to set default mode/arch without forcing a list of them?

waruqi commented 3 years ago

I'm trying to update to the allowed branch but it seems I can't

xmake update github:xmake-io/xmake#allowed

Also, I see that you went for the {default=""} option, after thinking about it, wouldn't set_defaultmode and set_defaultarch be a better option as they allow one to set default mode/arch without forcing a list of them?

I have improved it.

set_defaultmode("releasedbg")
set_defaultplat("linux")
set_defaultarchs("macosx|arm64", "linux|i386", "armv7")

set_allowedmodes("releasedbg", "debug")
set_allowedplats("windows", "linux", "macosx")
set_allowedarchs("macosx|arm64", "macosx|x86_64", "linux|i386", "linux|x86_64")
SirLynix commented 3 years ago

It seems to work great, except for one thing.

set_defaultmode("debug")
set_allowedmodes("asan", "debug", "releasedbg")
set_allowedplats("windows", "linux", "macosx")
set_allowedarchs("windows|x64", "linux|x86_64", "macosx|x86_64")

I added this to my project, and then ran xmake project -k vsxmake

lynix@SirLynixVanDesktop:/mnt/c/Projets/Burgwar/BurgWar$ xmake.exe project -k vsxmake
checking for asan.x86 ...
checking for Microsoft Visual Studio (x86) version ... 2019
checking for Qt SDK directory ... C:/Projets/Libs/Qt/5.15.0/msvc2019_64
checking for Qt SDK version ... 5.15.0
checking for asan.x64 ...
checking for Microsoft Visual Studio (x64) version ... 2019
checking for Qt SDK directory ... C:/Projets/Libs/Qt/5.15.0/msvc2019_64
checking for Qt SDK version ... 5.15.0
checking for debug.x86 ...
checking for Microsoft Visual Studio (x86) version ... 2019
checking for Qt SDK directory ... C:/Projets/Libs/Qt/5.15.0/msvc2019_64
checking for Qt SDK version ... 5.15.0
checking for debug.x64 ...
checking for Microsoft Visual Studio (x64) version ... 2019
checking for Qt SDK directory ... C:/Projets/Libs/Qt/5.15.0/msvc2019_64
checking for Qt SDK version ... 5.15.0
checking for releasedbg.x86 ...
checking for Microsoft Visual Studio (x86) version ... 2019
checking for Qt SDK directory ... C:/Projets/Libs/Qt/5.15.0/msvc2019_64
checking for Qt SDK version ... 5.15.0
checking for releasedbg.x64 ...
checking for Microsoft Visual Studio (x64) version ... 2019
checking for Qt SDK directory ... C:/Projets/Libs/Qt/5.15.0/msvc2019_64
checking for Qt SDK version ... 5.15.0
create ok!

It's still generating x86 project here.

waruqi commented 3 years ago

I have improved it, try it again,

SirLynix commented 3 years ago

Works great, I found no remaining issue. šŸ‘

waruqi commented 3 years ago

ok