-
Notifications
You must be signed in to change notification settings - Fork 17
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 support for custom toolsets and enforce root-relative paths #34
base: master
Are you sure you want to change the base?
Conversation
Better to split PRs.
|
local cfg_override = cfg | ||
cfg_override.project = cfg.workspace | ||
local includes = ninja.list(toolset.getincludedirs(cfg_override, filecfg.includedirs, filecfg.externalincludedirs, filecfg.frameworkdirs, filecfg.includedirsafter)) | ||
local forceincludes = ninja.list(toolset.getforceincludes(cfg_override)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That hack is superfluous with the other PR which handles it correctly (even if it requires change from premake-core)
@@ -384,7 +406,7 @@ local function compilation_rules(cfg, toolset, pch) | |||
p.outln(" description = link $out") | |||
p.outln("") | |||
end | |||
elseif toolset == p.tools.clang or toolset == p.tools.gcc then | |||
elseif toolset == p.tools.clang or toolset == p.tools.gcc or (toolset.clang_like ~= nil and toolset.clang_like()) then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be simpler to consider all but msc as "clang-like".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw, I've also hit this and tried to fix it: premake/premake-core#2260
@@ -348,7 +363,11 @@ local function compilation_rules(cfg, toolset, pch) | |||
local cxx = toolset.gettoolname(cfg, "cxx") | |||
local ar = toolset.gettoolname(cfg, "ar") | |||
local link = toolset.gettoolname(cfg, iif(cfg.language == "C", "cc", "cxx")) | |||
local rc = toolset.gettoolname(cfg, "rc") | |||
-- only compile rc files on windows |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked premake-core:
- They don't seems to filter out resource files for non-windows. (gmake/gmake2/codelite)
How about cross compilation?
BTW, you remove the rule itself, but not the call to the rule...
Alternative for user might be
filter {'system:windows'}
files { 'my_resource.rc' }
filter {}
Do you remember which error you hit with Emscripten? I have been using Ninja with https://github.com/tritao/premake-emscripten and so far have had no issues related to |
Changes
Ninja expects all paths to be relative to the root ninja folder
(see: Issue 997 in ninja-build/ninja)
Ninja expects paths to files to be relative to the root build file. Unfortunately, premake has no way of telling this to the toolsets, but we can work around this limitation by tricking them into generating paths relative to the workspace directory. See lines 255-262 and 273-280 for specifics.
RC files shouldn't be supported on targets other than windows
Since platforms like macos and linux don't integrate support for rc/res files, the "rc" rule shouldn't be generated. This removes the expectation that every toolset has a rc file command and allows support for custom toolsets, like emscripten, that don't support rc files.
Custom toolset support
Added simple support for custom toolset. Ninja will check for the function
clang_like()
on toolsets and if it returns true, will generate compilation rules using the gcc/clang branch of the conditional incompilation_rules(...)
.Associated Issues
Fixes #32