xmake-io / xmake

🔥 A cross-platform build utility based on Lua
https://xmake.io
Apache License 2.0
9.87k stars 776 forks source link

Improve to check and load toolchain #5466

Closed waruqi closed 1 month ago

waruqi commented 1 month ago

https://github.com/xmake-io/xmake/issues/5455

waruqi commented 1 month ago

We cannot call package:has_tool in on_load/on_fetch

SirLynix commented 1 month ago

how can we set compiler-specific flags in package without package:has_tool?

waruqi commented 1 month ago

how can we set compiler-specific flags in package without package:has_tool?

We can use package:has_tool and add flags in on_install

on_install(function (package)
    if package:has_tool(...) then
        package:add("cxflags", "xxx")
    end
end)

I'm trying to figure out how to get package:has_tool on on_load/on_fetch, but I haven't thought of a better way to do it yet.

If we check package toolchains in the on_load phase, then we can no longer support toolchain packages. e.g. llvm, mingw package

This is because these toolchain packages can only be used by other packages after they are installed, but by this time they have already been loaded.

SirLynix commented 1 month ago

would it be possible to load toolchains package before other packages with a dedicated callback? like on_toolchainload or something

waruqi commented 1 month ago

would it be possible to load toolchains package before other packages with a dedicated callback? like on_toolchainload or something

This will make the package configuration more complex. The current patch can fix #5455, but calling package:has_tool inside on_load doesn't get the exact result if a toolchain package is used.

on_load/package:has_tool can only use platform toolchains. https://github.com/xmake-io/xmake/pull/5466/commits/29b96dbce6ca18529e8d610e27fa47ced92f8f54

SirLynix commented 1 month ago

would it be possible to do it in two passes? fetch all toolchains packages, call on_load for them and then call on_load on all non-toolchain packages

waruqi commented 1 month ago

would it be possible to do it in two passes? fetch all toolchains packages, call on_load for them and then call on_load on all non-toolchain packages

It's hard to achieve, we need to download and install all toolchain packages first, then call other packages/on_load.

But to get all the package information, to determine if it's a toolchain, to get their dependencies, we need to load all the packages first.

Also, de-detecting the toolchain in on_load can affect performance. We can't support parallel loading either.

SirLynix commented 1 month ago

don't package toolchains have the "toolchain" kind? how about treating those package first, even if they switch to another kind in their on_load callback?

or maybe add another info to toolchains package (set_loadorder or something like that)

waruqi commented 1 month ago

don't package toolchains have the "toolchain" kind? how about treating those package first, even if they switch to another kind in their on_load callback?

or maybe add another info to toolchains package (set_loadorder or something like that)

current:

load_packages/on_load load all packages -> fetch -> download -> check toolchains -> install toolchain packages first -> install other packages

If we are detecting toolchains in on_load, we need to split load_packages:

check platform toolchain -> load_packages/on_load load toolchain packages -> download -> install -> check package toolchain -> load_packages/on_load load other packages -> download -> install

This results in a very complex implementation, and all actions that depend on load_packages are affected. e.g. xrepo search/export/import/install/download/fetch/env ...

we need to change a lot of code.

xmake/modules/utils/ci/packageskey.lua
52:    for _, instance in ipairs(package.load_packages(requires, {requires_extra = requires_extr
a})) do

xmake/modules/private/action/require/check.lua
43:    for _, instance in irpairs(package.load_packages(requires, {requires_extra = requires_ext
ra, nodeps = true})) do

xmake/modules/private/action/require/info.lua
88:    for _, instance in ipairs(package.load_packages(requires, {requires_extra = requires_extr
a})) do

xmake/modules/private/action/require/download.lua
28:import("private.action.require.impl.download_packages")
56:    download_packages(requires, {requires_extra = requires_extra, nodeps = true})

xmake/modules/private/action/require/list.lua
77:    for _, instance in ipairs(package.load_packages(requires, {requires_extra = requires_extr
a})) do

xmake/modules/private/action/require/register.lua
56:    local packages = package.load_packages(requires, {requires_extra = requires_extra})

xmake/modules/private/action/require/fetch.lua
52:    for _, instance in irpairs(package.load_packages(requires, {requires_extra = requires_ext
ra, nodeps = nodeps})) do

xmake/modules/private/xrepo/action/download.lua
80:function _download_packages(packages)
212:        _download_packages(packages)

xmake/modules/private/xrepo/action/env.lua
264:        for _, instance in ipairs(package.load_packages(requires, {requires_extra = requires
_extra})) do
282:        for _, instance in ipairs(package.load_packages(requires, {requires_extra = requires
_extra})) do

xmake/modules/private/action/require/impl/download_packages.lua
18:-- @file        download_packages.lua
116:function _download_packages(packages_download)
136:    runjobs("download_packages", function (index)
190:        local waitobjs = scheduler.co_group_waitobjs("download_packages")
240:    local packages = package.load_packages(requires, opt)
299:    _download_packages(packages_download)

xmake/modules/private/action/require/impl/import_packages.lua
30:    for _, instance in ipairs(package.load_packages(requires, opt)) do

xmake/modules/private/action/require/impl/install_packages.lua
118:                local extpackages = package.load_packages(extsources, extsources_extra)
672:    local packages = package.load_packages(requires, opt)

xmake/modules/private/action/require/impl/package.lua
1038:function _load_packages(requires, opt)
1063:                    local _, plaindeps = _load_packages(package:get("deps"), {requirepath =
 requirepath,
1353:function load_packages(requires, opt)
1357:    for _, package in ipairs((_load_packages(requires, opt))) do

xmake/modules/private/action/require/impl/uninstall_packages.lua
39:    for _, instance in ipairs(package.load_packages(requires, opt)) do

xmake/modules/private/action/require/impl/export_packages.lua
30:    for _, instance in ipairs(package.load_packages(requires, opt)) do
waruqi commented 1 month ago

I added more limit checks in on_load, and I will fix all packages in xmake-repo.

like this. https://github.com/xmake-io/xmake-repo/pull/4942

error: @programdir/core/main.lua:329: @programdir/core/package/package.lua:1225: we cannot call package:has_tool() in on_load(), please call it in on_check/on_install/on_test.

waruqi commented 1 month ago

But some packages will use has_tool to add deps. hmm..

https://github.com/xmake-io/xmake-repo/blob/027cda2fe8b64ec492093fcc7b8e88c404d1563e/packages/o/openmp/xmake.lua#L13

I don't know how to solve it.

SirLynix commented 1 month ago

Also, preventing on_load to set compiler-specific flags prevent them to add flags for found package, right?

waruqi commented 1 month ago

Also, preventing on_load to set compiler-specific flags prevent them to add flags for found package, right?

? we still can set compiler-specific flags in on_install for found package.

all flags will be saved to manifest.txt of installed package.

But we cannot set compiler-specific deps.

SirLynix commented 1 month ago

I mean for package not installed because they are found in pkgconfig, vcpkg, Conan.

To fix the whole issue, I wonder why it was working before? Also if we have no choice than to complicate package xmake.lua for toolchains, better complicate their description than all other packages.

waruqi commented 1 month ago

I wonder why it was working before?

It works now too, but it will break when it get the following two cases.

  1. load a host package, because platform toolchain (with host arch) has been not check yet. But I can fix it.
  2. all packages are using a remote toolchain package (e.g. llvm package), then, until llvm is installed, the on_load of all packages will not get the correct toolchain. I cannot solve it.

So while there is nothing issues with using has_tool/tool in on_load in most cases, it will cause problems in the two cases above.

Also, even complicating package descriptions, I haven't thought of a good way to support modifying package dependencies based on the compiler. For example the openmp package

waruqi commented 1 month ago

I'm trying to split the package installation phase and go ahead and install the toolchain package.

I'm not sure if it will work, and maybe break a lot of other things, but I'll try it as best I can.

https://github.com/xmake-io/xmake/pull/5481

waruqi commented 1 month ago

@SirLynix I tested it, it should work. Did you try it again?

SirLynix commented 1 month ago

I'm not home for a few days, I will test it as soon as possible. Thank you for the fix

SirLynix commented 1 month ago

I'm back, it looks like it broke utils.ci.packageskey: https://github.com/NazaraEngine/NazaraEngine/actions/runs/10489612059/job/29054625919

waruqi commented 1 month ago

try it again

SirLynix commented 1 month ago

It looks like it's working for windows here and macOS here (it looks like xmake update -s toolchain doesn't work on macOS for some reason but that's unrelated).

It seems to be working as well for macOS on NazaraEngine (here) however it fails with asan on the host version:

2024-08-21T13:44:21.0619960Z nzslc --version
2024-08-21T13:44:21.0656480Z dyld[4156]: Library not loaded: @rpath/libclang_rt.asan_osx_dynamic.dylib

maybe some toolchain envs aren't set for toolchains? that worked before

waruqi commented 1 month ago

I switched to dev version, it still does not work. https://github.com/waruqi/NazaraEngine/actions/runs/10504339795/job/29099574380

Maybe it has nothing to do with the current patch. But I'm not sure why at the moment.

SirLynix commented 1 month ago

maybe it changed on dev version before, I already do this for other toolchains to fix the asan issue: https://github.com/NazaraEngine/xmake-repo/blob/master/packages/n/nzsl/xmake.lua#L58-L74

waruqi commented 1 month ago

maybe it changed on dev version before, I already do this for other toolchains to fix the asan issue: https://github.com/NazaraEngine/xmake-repo/blob/master/packages/n/nzsl/xmake.lua#L58-L74

This doesn't seem to have anything to do with the current patch, it's a result of a previous change from v2.9.3.

But why is it broken now.

$ xmake l cli.bisect -g v2.9.3 -b v2.9.4 --gitdir=/Users/ruki/projects/personal/xmake -c "xrepo remove --all -y nzsl; xmake config --arch=x86_64 --mode=debug --static=no --asan=y --ccache=n --ffmpeg=y --shadernodes=y --tests=y --unitybuild=y --yes -vD -c"

10df5c458fcf66832c0ef437157c7397f2283b18 is the first bad commit
commit 10df5c458fcf66832c0ef437157c7397f2283b18
Author: ruki <waruqi@gmail.com>
Date:   Wed Jul 17 00:53:43 2024 +0800

    install rpath

 tests/actions/install/xmake.lua              |  1 +
 xmake/modules/target/action/install/main.lua | 25 +++++++++++++++++++++++++
 xmake/modules/utils/binary/rpath.lua         | 16 ++++++++++++++++
 3 files changed, 42 insertions(+)

https://github.com/xmake-io/xmake/commit/10df5c458fcf66832c0ef437157c7397f2283b18

waruqi commented 1 month ago

https://github.com/NazaraEngine/xmake-repo/blob/master/packages/n/nzsl/xmake.lua#L58-L74

try this patch. https://github.com/xmake-io/xmake/pull/5498

waruqi commented 1 month ago

try toolchain branch again.

SirLynix commented 1 month ago

it seems to works great 👍

I don't know if it's related but on the toolchain branch I can't run xmake repo --update on the CI: https://github.com/NazaraEngine/NazaraEngine/actions/runs/10526067618/job/29166118505#step:7:1

I can't reproduce it locally though

waruqi commented 1 month ago

I don't know if it's related but on the toolchain branch I can't run xmake repo --update on the CI:

It's xmake update -s issue on windows.

https://github.com/xmake-io/xmake/blob/3ba77c15ccf34187b6678f277f2dd418433493c8/xmake/actions/update/main.lua#L105

update version dev from official source ..
  => downloading https://github.com/xmake-io/xmake.git ..   => download https://github.com/xmake-io/xmake.git .. ok

  => install script to C:\hostedtoolcache\windows\xmake\dev\x64 .. ok
Waiting for xmake to exit... <----------------------- 
updating repositories .. 
Waiting for xmake to exit...
pinging the host(gitee.com) ... 65535 ms
pinging the host(github.com) ... 65535 ms
pinging the host(gitlab.com) ... 65535 ms
cloning repository(nazara-engine-repo): https://github.com/NazaraEngine/xmake-repo to D:\a\NazaraEngine\NazaraEngine\.xmake\windows\x64\repositories\nazara-engine-repo ..
git clone https://github.com/NazaraEngine/xmake-repo -c core.fsmonitor=false -c core.autocrlf=false D:\a\NazaraEngine\NazaraEngine\.xmake\windows\x64\repositories\nazara-engine-repo
Cloning into 'D:\a\NazaraEngine\NazaraEngine\.xmake\windows\x64\repositories\nazara-engine-repo'...
Waiting for xmake to exit...
cloning repository(build-artifacts): https://github.com/xmake-mirror/build-artifacts.git to D:\a\NazaraEngine\xmake-global\.xmake\repositories\build-artifacts ..
git clone https://github.com/xmake-mirror/build-artifacts.git -b main -c core.fsmonitor=false -c core.autocrlf=false D:\a\NazaraEngine\xmake-global\.xmake\repositories\build-artifacts
Cloning into 'D:\a\NazaraEngine\xmake-global\.xmake\repositories\build-artifacts'...
Waiting for xmake to exit...
Waiting for xmake to exit...
cloning repository(xmake-repo): https://github.com/xmake-io/xmake-repo.git to D:\a\NazaraEngine\xmake-global\.xmake\repositories\xmake-repo ..
git clone https://github.com/xmake-io/xmake-repo.git -b master -c core.fsmonitor=false -c core.autocrlf=false D:\a\NazaraEngine\xmake-global\.xmake\repositories\xmake-repo
Cloning into 'D:\a\NazaraEngine\xmake-global\.xmake\repositories\xmake-repo'...
Waiting for xmake to exit...
Waiting for xmake to exit...
waruqi commented 1 month ago

you can try , it should work. https://github.com/waruqi/NazaraEngine/actions/runs/10528140225/job/29172954590

    - name: Update xmake repository
      run: |
        xmake update -s toolchain
        Start-Sleep -Seconds 10
        xmake repo --update -vD
SirLynix commented 1 month ago

Seems good, thanks! https://github.com/NazaraEngine/NazaraEngine/actions/runs/10530933803/job/29181927448