Skip to content

Add support for custom toolsets and enforce root-relative paths#34

Open
lochnessdragon wants to merge 4 commits into
jimon:masterfrom
lochnessdragon:master
Open

Add support for custom toolsets and enforce root-relative paths#34
lochnessdragon wants to merge 4 commits into
jimon:masterfrom
lochnessdragon:master

Conversation

@lochnessdragon

Copy link
Copy Markdown

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 in compilation_rules(...).

Associated Issues

Fixes #32

@Jarod42

Jarod42 commented Sep 8, 2024

Copy link
Copy Markdown
Collaborator

Better to split PRs.

  • getrelative should be fixed by the other PR (waiting change from premake core).
    So current hack to handle it will be rejected.

Comment thread ninja.lua
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))

Copy link
Copy Markdown
Collaborator

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)

Comment thread ninja.lua
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

Copy link
Copy Markdown
Collaborator

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".

Copy link
Copy Markdown
Contributor

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

Comment thread ninja.lua
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

@Jarod42 Jarod42 Sep 9, 2024

Copy link
Copy Markdown
Collaborator

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 {}

@tritao

tritao commented Sep 27, 2024

Copy link
Copy Markdown
Contributor

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.

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 rc.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support custom toolsets

3 participants