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

Allow xmake package to embed rules and scripts #2374

Closed
SirLynix opened this issue May 22, 2022 · 15 comments
Closed

Allow xmake package to embed rules and scripts #2374

SirLynix opened this issue May 22, 2022 · 15 comments

Comments

@SirLynix
Copy link
Member

Is your feature request related to a problem? Please describe.

We have several packages on xmake which may be requiring a set of specific rules, for example Qt packages (qt5base repository) are not compatibles with xmake Qt rules (which is why I provide my own rules currently, see here).

Sure, we can improve xmake embedded rules to handle such packages, but I think it would be better to allow packages to provides rules themselves, and perhaps even other kind of scripts (but rules would be great for a start).

Another example is my shader language library which I hope to put on xmake-repo soon, I'd like to be able to provide a rule similar to glsl2spirv to be able to call the shader compiler to produce binaries.

Describe the solution you'd like

Allowing packages to embed rules would be a great addition to xmake, as it would allow a package update to update rules, and won't need everyone to copy rules in their own projects.

Packages could copy their rules lua files to the package installation directory scripts/rules, which xmake would load when adding a package to a project.

add_requires("nzsl")

target("myproject")
    add_files("main.cpp")
    add_files("shader.nzsl", { rule = "nzsl::compile_shaders" }) -- reference rule from nzsl package

If required, we can make it explicit:

add_requires("nzsl", { import_rules = true })

Describe alternatives you've considered

  • Provide a compile_shader.lua file that people could copy in their project, but that means package updates can break it.

  • Make a pull request to embed the rules inside of xmake, but this means the package update has to wait the next xmake update to ensure both stay in sync.

Additional context

I think it would be great to give the possibility to embed other scripts, perhaps even make xmake packages that provide nothing but rules

@waruqi
Copy link
Member

waruqi commented May 22, 2022

I wouldn't consider it for now, it's too complex and I'd need to make a lot of changes. The current xmake rules implementation architecture cannot support it.

@SirLynix
Copy link
Member Author

From what I know about xmake internals I don't think it's that complex, we could store the presence of rules in the package manifest and know when processing them if a package has scripts or not, and simply run the scripts if required using import_rules or execute_scripts.

I can try to work on it and make a pull request if you wish.

@waruqi
Copy link
Member

waruqi commented May 22, 2022

I wouldn't consider it just yet and I don't accept other pr, sorry.

@xq114
Copy link
Contributor

xq114 commented May 22, 2022

+1 for this, it won't be a big problem I think. Just add some paths such as (xmake-repo\rules\abc-init.lua) and add xmake-repo\rules to the default script search path.

usage:

add_repositories("my-repo")

includes("abc-init.lua") -- from my-repo\rules\abc-init.lua

target("test")
	add_files("main.abc")

This makes xmake-repo more flexible and worth consideration.

@SirLynix
Copy link
Member Author

I like the include search path approach, but requiring a full xmake repository to add scripts seems a bit much.

Maybe something like

includes("compile-rule.lua", { package = "nzsl" })

Or even

includes("@nzsl/compile-rule.lua")

which would include from the package scripts folder.

@maximegmd
Copy link

This looks handy! Having the ability to add custom rules for codegen would help us as well

@arkena00
Copy link

arkena00 commented Jun 5, 2022

Would be awesome, no wait, amazing !!

I prefer to have one interface to handle all external code, what ever the code is. Let's call it sauron_require()

That's how i see things :
A package has all informations we need for any type of integration

  • source code (or just files)
  • built binaries
  • scripts
  • repository (for my xmake submodules :p)
add_requires("nzsl", { source, binary, scripts,  }) -- w/e the syntax

And of course a package could only provide binaries, or scripts

  • includes() to use internal code
  • add_requires() to use external code

Simple, flexible

@waruqi
Copy link
Member

waruqi commented Jul 1, 2022

I will consider implementing it, but at the moment the problem is not how to design the api. add_requires is sufficient.

The question now is how to implement it safely and securely. With the current mechanism, it would be complicated to implement and I don't have a good solution at the moment.

Since rules need to call the on_load interface, we need to load all the rules before we load all the targets, but the parsing of add_requires and the loading of the package configuration is relatively late, so how to load all the rules in requirements early and ensure that the repository version and state is a complex issue.

As the internal logic is already very complex, introducing this feature could complicate the existing implementation logic even more, so I would consider it more carefully.

@SirLynix
Copy link
Member Author

SirLynix commented Jul 1, 2022

A first shot would be the idea of @xq114, allow custom repositories to provide rules and include to fetch them. If it's simpler.

@waruqi
Copy link
Member

waruqi commented Jul 1, 2022

A first shot would be the idea of @xq114, allow custom repositories to provide rules and include to fetch them. If it's simpler.

Similarly, using includes is difficult to deal with, we need to select the right repository and we need to consider finding the rules from the repository with the specified commit when require-lock is turned on.

includes is invoked much earlier and no more information about the configuration of the repository is available.

@waruqi waruqi added this to the v2.7.2 milestone Oct 8, 2022
@waruqi
Copy link
Member

waruqi commented Oct 8, 2022

I have supported it in #2903

we need put all package rules to packages/x/xxx/rules directory, it will be installed to package installation directory.

Although we now support the reference package rule, it has some limitations.

  • With the rules inside the package, we cannot define on_load/after_load, but we can usually use on_config instead.

Add package rule

packages/z/zlib/rules/foo.lua

rule("foo")
    on_config(function (target)
        print("foo: on_config %s", target:name())
    end)

Apply package rule to target

We use add_rules("@packagename/rulename") to refer to the rules inside the specified package. for example add_rules("@zlib/foo")

add_requires("zlib", {system = false})
target("test")
    set_kind("binary")
    add_files("src/*.cpp")
    add_packages("zlib")
    add_rules("@zlib/foo")

Use package alias name

If a package alias exists, xmake will give preference to the package alias to get the rule.

add_requires("zlib", {alias = "zlib2", system = false})
target("test")
    set_kind("binary")
    add_files("src/*.cpp")
    add_packages("zlib2")
    add_rules("@zlib2/foo")

Add package rule deps

We can use add_deps("@bar") to add other rules relative to the current package directory.

However, we cannot add rule dependencies from other packages, they are completely isolated and we can only refer to rules from other packages imported by add_requires in the user project.

packages/z/zlib/rules/foo.lua

rule("foo")
    add_deps("@bar")
    on_config(function (target)
        print("foo: on_config %s", target:name())
    end)

packages/z/zlib/rules/bar.lua

rule("bar")
    on_config(function (target)
        print("bar: on_config %s", target:name())
    end)

@SirLynix
Copy link
Member Author

SirLynix commented Oct 8, 2022

This looks amazing, can't wait to test it! Thanks a lot for supporting this feature.

@waruqi
Copy link
Member

waruqi commented Oct 8, 2022

I have merged it.

@waruqi waruqi closed this as completed Oct 8, 2022
@SirLynix
Copy link
Member Author

Feature works well when using xmake in command-line but breaks with vs generator, this is probably because vs generator reconfigure target without using actions.config (thus not using _load_package_rules_for_target).

It breaks both vs and vsxmake generators when using those rules to handle unspecified extensions.

it can easily be fixed I think.

@waruqi
Copy link
Member

waruqi commented Oct 10, 2022

try it again.

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

5 participants