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

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

Closed
SirLynix opened this issue Jul 17, 2021 · 11 comments
Closed

Comments

@SirLynix
Copy link
Member

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
Copy link
Member

waruqi commented Jul 17, 2021

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
Copy link
Member Author

SirLynix commented Jul 17, 2021

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
Copy link
Member

waruqi commented Jul 17, 2021

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
Copy link
Member Author

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:

  • Add set_allowedmodes and set_allowedarchs calls:
-- example
set_allowedmodes("asan", "debug", "releasedbg")
set_allowedarchs("x86", "x86_64")
  • Setting a mode outside of this list should trigger an error:
$ 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
  • The first value of the list should be the default value xmake takes when no configuration has been done, but this could also be set this way:
set_allowedmodes("asan", "debug", "releasedbg", { default = "debug" })
set_allowedarchs("x86", "x86_64", { debug = "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).

  • If allowed modes / archs is set, then it should override modes and arch list xmake use to generate VS (and others) projects.
  • If default mode / default arch is set, the generated projects should use it as default values too (if possible).

@SirLynix SirLynix changed the title XMake doesn't trigger an error when using an invalid mode [Feature request] XMake doesn't trigger an error when using an invalid mode Jul 17, 2021
@waruqi waruqi added this to the v2.5.6 milestone Jul 22, 2021
@waruqi
Copy link
Member

waruqi commented Jul 22, 2021

I have supported it, can you try it first? #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
Copy link
Member Author

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
Copy link
Member

waruqi commented Jul 22, 2021

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
Copy link
Member Author

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
Copy link
Member

waruqi commented Jul 22, 2021

I have improved it, try it again,

@SirLynix
Copy link
Member Author

Works great, I found no remaining issue. 👍

@waruqi
Copy link
Member

waruqi commented Jul 22, 2021

ok

@waruqi waruqi closed this as completed Jul 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants