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

Support for dynamic creation and injection of rules and targets in script scope #2879

Closed
waruqi opened this issue Sep 27, 2022 · 15 comments
Closed

Comments

@waruqi
Copy link
Member

waruqi commented Sep 27, 2022

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

#2874
#2866

Describe the solution you'd like

none

Describe alternatives you've considered

No response

Additional context

No response

@waruqi waruqi added this to the v2.7.2 milestone Sep 27, 2022
@waruqi
Copy link
Member Author

waruqi commented Sep 27, 2022

Clone and modify target dynamically

we must call target:clone() in after_load().

target("test")
    set_kind("binary")
    add_files("src/*.cpp")
    add_defines("TEST")
    after_load(function (target)
        import("core.project.project")
        local t = target:clone()
        t:name_set("test2")
        t:add("deps", "test")
        t:add("defines", "TEST2")
        t:set("link_before", function (target)
            print("link1", target:name())
            assert(target:dep("test"):data("linked"))
        end)
        project.target_add(t)
    end)
    before_link(function (target)
        print("link2", target:name())
        target:data_set("linked", true)
    end)

Clone and modify rule dynamically

rule("cppfront")
    set_extensions(".cpp2")
    on_load(function (target)
        local rule = target:rule("c++.build"):clone()
        rule:add("deps", "cppfront", {order = true})
        target:rule_add(rule)
    end)
    on_build_file(function (target, sourcefile, opt)
        print("build cppfront file")
        local objectfile = target:objectfile(sourcefile:gsub("cpp2", "cpp"))
        assert(not os.isfile(objectfile), "invalid rule order!")
    end)

target("test")
    set_kind("binary")
    add_rules("cppfront")
    add_files("src/*.cpp")
    add_files("src/*.cpp2")
    before_build_file(function (target, sourcefile, opt)
        local objectfile = target:objectfile(sourcefile)
        os.tryrm(objectfile)
    end)

@waruqi waruqi changed the title Support for dynamic creation and injection of rules and targets Support for dynamic creation and injection of rules and targets in script scope Sep 27, 2022
@davidchisnall
Copy link

This looks fantastic. I presume that targets can (should?) be created only in the on_load / after_load functions, or they may change dependency calculation in confusing ways (totally fine for my use case). Are there any other restrictions? When will the on_load / after_load hooks be executed for new targets? In particular, if I have:

  • Target A
  • Target B, which clones A to create target C
  • Target C, created by B's on_load function.

What (if any) is the ordering guarantee between A, B, and C's on_load / after_load functions? My expectation is that:

  • No ordering between A and B.
  • C's on_load will run after B's.
  • No ordering between A and C.

i.e. B must not rely on A's on_load running before it clones A.

@waruqi
Copy link
Member Author

waruqi commented Sep 27, 2022

you just call clone, add and access other targets in after_load.

@davidchisnall
Copy link

Right, but if I clone a target that has its own on_load and after_load implementations, when are these called? Presumably xmake has no knowledge of the fact that my after_load will call clone, so there's no guarantee that the target that will be the source of the clone's after_load is called before mine. Is there a guarantee that it will be called before the new (cloned) target? If I need to enforce some of this ordering, is there something that I can call on a target to make sure that its after_load is called before I clone it (does clone do this automatically)? Will on_load and after_load that are defined in the original target be called on the new one?

@waruqi
Copy link
Member Author

waruqi commented Sep 28, 2022

You can only call target:clone() in after_load, and the cloned target will not call any xx_load

@waruqi
Copy link
Member Author

waruqi commented Sep 28, 2022

I have merged this patch into dev. xmake update -s dev

@waruqi waruqi closed this as completed Sep 28, 2022
@davidchisnall
Copy link

I've tried to use this, but I get an error. As the first two lines in my target's after_load, I have:

rule("firmware")
    after_load(function (target)
        import("core.project.project")
        local component = project.target("cherimcu.allocator"):clone()```
...

I then get:

error: please call target:clone() in after_load().

It appears that you must call target:clone() after the target's after_load has been called, but I have no way of enforcing that ordering. This is partly why I was asking about ordering earlier. I believe the correct fix here would be to force the target that you're cloning to execute its after_load hooks when you try to clone it, rather than asserting that you have.

The target that I'm cloning does not implement after_load, so it would also be fine to simply disallow implementing after_load in targets that will be cloned (you can always move the after_load script to the script that operates on the clone).

@davidchisnall
Copy link

If I comment out that check, then I later hit an error that I am adding my cloned target to the target whose after_load method I am executing:

target:add("deps", cloned_target_name)

And then it does not show up when I call target:deps().

@waruqi
Copy link
Member Author

waruqi commented Sep 29, 2022

I've tried to use this, but I get an error. As the first two lines in my target's after_load, I have:

rule("firmware")
    after_load(function (target)
        import("core.project.project")
        local component = project.target("cherimcu.allocator"):clone()```
...

I then get:

error: please call target:clone() in after_load().

It appears that you must call target:clone() after the target's after_load has been called, but I have no way of enforcing that ordering. This is partly why I was asking about ordering earlier. I believe the correct fix here would be to force the target that you're cloning to execute its after_load hooks when you try to clone it, rather than asserting that you have.

The target that I'm cloning does not implement after_load, so it would also be fine to simply disallow implementing after_load in targets that will be cloned (you can always move the after_load script to the script that operates on the clone).

please try it again. xmake update -s dev

@waruqi
Copy link
Member Author

waruqi commented Sep 29, 2022

If I comment out that check, then I later hit an error that I am adding my cloned target to the target whose after_load method I am executing:

target:add("deps", cloned_target_name)

And then it does not show up when I call target:deps().

it works for me.

target("hello")
    set_kind("binary")
    add_files("src/*.cpp")

target("test")
    set_kind("binary")
    add_files("src/*.cpp")
    add_defines("TEST")
    after_load(function (target)
        import("core.project.project")
        local t = project.target("hello"):clone()
        t:name_set("hello_cloned")
        t:add("defines", "TEST2")
        target:add("deps", "hello_cloned")
        project.target_add(t)
        print(table.keys(target:deps()))
    end)
{
  "hello_cloned"
}
 1
[ 25%]: cache compiling.release src/main.cpp
[ 25%]: cache compiling.release src/main.cpp
[ 25%]: cache compiling.release src/main.cpp
[ 50%]: linking.release hello_cloned
[ 50%]: linking.release hello
[ 83%]: linking.release test
[100%]: build ok!

@davidchisnall
Copy link

Thanks. This was the key line:

        t:name_set("hello_cloned")

I was using t:set("name", "hello_cloned"), from the previous example, and that didn't work. It showed up when I called t:name() and saw the old name. Changing this makes it work.

@davidchisnall
Copy link

It appears that on_load things in rules for cloned targets are not invoked, so my version still doesn't actually work. I think that a cloned target probably wants to export a mechanism for calling these: they shouldn't be called at clone time, because they might depend on properties set on the cloned target. It might be fine to call them after the after_load method that created them is called.

Without this, it's not possible to use stateful rules with cloned targets.

@waruqi
Copy link
Member Author

waruqi commented Sep 30, 2022

The new target being cloned inherits exactly the state of the target, which has previously called on_load and should not be called again.

Only when a new empty target is created should on_load be called

@davidchisnall
Copy link

Sorry, it looks as if it's the after_load hook in the rule that isn't called. I have an after_load that adds a compiler flag based on either the target name or a custom property if it is set. This appears to be called with the original target but not my cloned one.

Again, this is why I wanted a clearly defined (and documented) order of all of the operations with respect to cloning.

@waruqi
Copy link
Member Author

waruqi commented Sep 30, 2022

Sorry, it looks as if it's the after_load hook in the rule that isn't called. I have an after_load that adds a compiler flag based on either the target name or a custom property if it is set. This appears to be called with the original target but not my cloned one.

Again, this is why I wanted a clearly defined (and documented) order of all of the operations with respect to cloning.

Can you provide an example/xmake.lua?

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