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

Improve link mechanism and order #1452

Closed
4 tasks done
waruqi opened this issue Jun 8, 2021 · 23 comments
Closed
4 tasks done

Improve link mechanism and order #1452

waruqi opened this issue Jun 8, 2021 · 23 comments

Comments

@waruqi waruqi added this to the v2.5.5 milestone Jun 8, 2021
@waruqi
Copy link
Member Author

waruqi commented Jun 8, 2021

Just a draft

Adjust the order of each configuration type

-- -la -ld -lb -lc -le -ls1 -ls2
add_syslinks("s1", "s2")
add_links("a")
add_links("b", "c", {order = 2})
add_links("d", {order = 1})
add_links("e")
-- -la -ld -lb -lc -le
add_ldflags("-la")
add_ldflags("-lb", "-lc", {order = 2})
add_ldflags("-ld", {order = 1})
add_ldflags("-le")

and add_includedirs, add_xxx etc, it supports all configurations

Adjust the link order between packages, deps, and options

-- -la -le -lf -lb -lc -ld -lg -ls1 -ls2
add_syslinks("s1", "s2")
add_links("a")
add_links("b", "c", {order = 4})
add_links("d", {order = 5})
add_links("package::e", {order = 1)
add_links("option::f", {order = 2)
add_links("dep::g")
add_packages("e")
add_options("f")
add_deps("g")

We can use namespaces such as package:: to individually adjust the link order of packages.

link group

--  -Wl,--start-group -la -lb -lc -Wl,--end-group -Wl,--start-group -lx -ly -lz -Wl,--end-group
add_links("a", "b", "c", {group = true})
add_syslinks("x", "y", "z", {group = true})

link group with deps, options and packages

--  -Wl,--start-group -la -lb -lc -ld -Wl,--end-group
add_links("a", "package::b", "dep::c", "option::d", {group = true}))
add_packages("b")
add_options("d")
add_deps("c")

link whole archive

--  -Wl,--whole-archive -la -lb -lc -Wl,--no-whole-archive
add_links("a", "b", "c", {whole = true})

link whole archive with deps, options and packages

--  -Wl,--whole-archive -la -lb -lc -ld -Wl,--no-whole-archive
add_links("a", "package::b", "dep::c", "option::d", {whole = true}))
add_packages("b")
add_options("d")
add_deps("c")

Explicitly specify the linked file name

libfoo.a libfoo.so in same directory

add_links("libfoo.a")
add_links("libfoo.so")
add_links("foo.lib")

@waruqi waruqi changed the title Improve link mechanism. Improve link mechanism and order Jun 9, 2021
@madhurajith
Copy link

Instead of repeating package, option, dependency using the "package::name" and setting the link order will it be better introduce "link_order" option for packages, options and dependencies?

Before,

add_links("package::e", {order = 1})
add_links("option::f", {order = 2})
add_links("dep::g")
add_packages("e")
add_options("f")
add_deps("g")

After,

add_packages("e", {link_order = 1})
add_options("f", {link_order = 2})
add_deps("g")

Instead of using a separate group flag, wouldn't it be better to use the "order" flag with two elements to denote the group? e.g.

add_syslinks("s1", "s2")
add_links("a")
add_links("b", "c", {order = 4})
add_links("d", {order = {5, 1}})
add_packages("e", {link_order = {1, 1}})
add_options("f", {link_order = {1, 2}})
add_deps("g", {link_order = 3, link_whole = true})
add_links("h", {order = {5, 2}})

This will end up with the command line,

-Wl,--start-group -le -lf -Wl,--end-group -Wl,--whole-archive -lg -Wl,--no-whole-archive -lb -lc -Wl,--start-group -ld -lh -Wl,--end-group -la -ls1 -ls2

@waruqi
Copy link
Member Author

waruqi commented Jun 11, 2021

Instead of repeating package, option, dependency using the "package::name" and setting the link order will it be better introduce "link_order" option for packages, options and dependencies?

I still want to consider other configurations in package, such as add_includedirs("package::xxx") Maybe there will be ldflags, cflags in the future. .

Instead of using a separate group flag, wouldn't it be better to use the "order" flag with two elements to denote the group? e.g.

This is a bit too complicated, or we can use same order to put it in a group without using two elements.

add_links("a", "b", {order = 5})
add_links("c", {order = 5})

@xq114
Copy link
Contributor

xq114 commented Jun 11, 2021

It's a bad idea using numbers to specify link orders. What matters is only the link order of some specific links and obviously a tree structure is more precise and robust.

@waruqi
Copy link
Member Author

waruqi commented Jun 11, 2021

It's a bad idea using numbers to specify link orders. What matters is only the link order of some specific links and obviously a tree structure is more precise and robust.

The main reason is that the order of links between add_packages/add_options/add_links cannot be controlled. Are there any other better ideas?

@madhurajith
Copy link

@xq114 How to define the tree structure? should it be a single command like,

set_link_order("e", "f", {"a", "d", "c"}, {"deps::f", "options::b"}, "k")

@waruqi
Copy link
Member Author

waruqi commented Jun 11, 2021

This does not conflict with tree inheritance, I only want to modify the link order between options/packages/links in the current target, and support link grouping.

target("test")
    -- need not `dep::xxx`, we only adjust the link order between options/packages/links in the current target
    add_linkorders("a", "b", "package::c", "option::d")
    add_linkorders("a", "b", "package::c", "option::d", {group = true})

how about this?

@waruqi
Copy link
Member Author

waruqi commented Jun 11, 2021

about whole link

target("test")
    add_deps("a", {wholelink = true}) -- only for dep("xxx"), -lxxx
    add_links("a", "b", "c", {wholelink = true})
    add_options("a", "b", "c", {wholelink = true})
    add_packages("a", "b", "c", {wholelink = true})

@xq114
Copy link
Contributor

xq114 commented Jun 11, 2021

@xq114 How to define the tree structure? should it be a single command like,

set_link_order("e", "f", {"a", "d", "c"}, {"deps::f", "options::b"}, "k")

You're right, I mean, we can use a couple of add_linkorders command to specify the link dependency tree. For example, the following code will generate a tree a->b->c->d c->e b->f

add_linkorders("a", "package::b", "c", "option::d")
add_linkorders("c", "package::e")
add_linkorders("b", "package::f)

If I want to modify the structure by adding g->a, just a modification to the first line would work, without rearranging the numbers denoting the position of linking.

Furthermore, it's better if a graph structure is supported, i.e. a->b->d a->c->d. The actual link order could be either a->b->c->d or a->c->b->d depending on implementation.

@madhurajith
Copy link

madhurajith commented Jun 11, 2021

Does it respect package add_deps() by default?

package("foo")
    add_deps("bar")

Will the link order will be?

-lfoor -lbar

@waruqi
Copy link
Member Author

waruqi commented Jun 11, 2021

Does it respect package add_deps() by default?

package("foo")
    add_deps("bar")

Will the link order will be?

-lfoor -lbar

right

@waruqi waruqi modified the milestones: v2.5.5, v2.5.6 Jun 23, 2021
@waruqi waruqi modified the milestones: v2.5.6, v2.5.7 Jul 26, 2021
This was referenced Aug 5, 2021
@waruqi waruqi modified the milestones: v2.5.7, v2.5.8 Aug 29, 2021
@xq114
Copy link
Contributor

xq114 commented Sep 6, 2021

Another solution comes from cmake and some other build systems: links can be treated as targets with dependency hierarchy. So the order of links can be derived from that.

add_link_deps("a", {"b", "c", "d"}) or add_link_deps("a", "b", "c", "d")
-- cmake: add_library(a IMPORTED); target_link_libraries(a b c d); target_link_libraries(<target> a)

Since dependency is specified, a simple topological sort can be applied to find the actual link order (and find circular dependencies). The implementation is similar to calculating the build order of targets.

For group links, another mechanism must be applied. General solution is

add_link_group or add_links(,{group=true})

a link group can be simplified as a node and topological sort still applies. A more tricky mechanism is to treat circular dependencies as link group (which means they require symbols from each other), e.g.

add_link_deps("a", "b")
add_link_deps("b", "a")
-- equal to add_links("a", "b", {group=true})

This is the solution of most build systems; if this kind of mechanism is chosen, a Tarjan's algorithm can be used to find the strong components for links. A simpler implementation is just to link these libraries multiple times: create a copy of the link every time it's required, and take all copies into account when calculating the link order, which reduced the usage of group link.

As for whole archive, it has nothing to do with link orders (in my opinion). We can just add whole archive flags for every links specified with {whole=true}, and remove all -Wl,--no-whole-archive -Wl,--whole-archive substrings from the final command.

@jorgenpt
Copy link

I would also love the ability to link a single library statically -- this can be achieved on clang/GCC using -Wl,-Bstatic -lsome_static_lib -Wl,-Bdynamic -lmy_other_lib -lmy_other_lib_2. :)

Maybe a syntax like add_links("some_static_lib", { static = true })?

@waruqi
Copy link
Member Author

waruqi commented Sep 16, 2021

I would also love the ability to link a single library statically -- this can be achieved on clang/GCC using -Wl,-Bstatic -lsome_static_lib -Wl,-Bdynamic -lmy_other_lib -lmy_other_lib_2. :)

Maybe a syntax like add_links("some_static_lib", { static = true })?

I will consider it

@waruqi
Copy link
Member Author

waruqi commented Sep 29, 2023

Now I have improved it. #4250

$ xmake update -s github:xmake-io/xmake#links

link group

--  -Wl,--start-group -la -lb -Wl,--end-group
add_linkgroups("a", "b", {group = true})

link group (whole archive)

--  -Wl,--whole-archive -la -lb -Wl,--no-whole-archive
add_linkgroups("a", "b", {whole = true})

link group (static search)

-- -Wl,-Bstatic -la -lb -Wl,-Bdynamic
add_linkgroups("a", "b", {static = true})

modify links orders

It will create graph and use topological sort to sort links.

and it will check link cycle in add_linkorders() and show warnings.

add_links("a", "b", "c", "d", "e")
-- e -> b -> a
add_linkorders("e", "b", "a")
-- e -> d
add_linkorders("e", "d")

sort links and linkgroups

add_links("a", "b", "c", "d", "e")
add_linkgroups("c", "d", {name = "foo", group = true})
add_linkorders("e", "linkgroup::foo")

sort links and frameworks

add_links("a", "b", "c", "d", "e")
add_frameworks("Foundation", "CoreFoundation")
add_linkorders("e", "framework::CoreFoundation")

examples

add_rules("mode.debug", "mode.release")

add_requires("libpng")

target("bar")
    set_kind("shared")
    add_files("src/foo.cpp")
    add_linkgroups("m", "pthread", {whole = true})

target("foo")
    set_kind("static")
    add_files("src/foo.cpp")
    add_packages("libpng", {public = true})

target("demo")
    set_kind("binary")
    add_deps("foo")
    add_files("src/main.cpp")
    if is_plat("linux", "macosx") then
        add_syslinks("pthread", "m", "dl")
    end
    if is_plat("macosx") then
        add_frameworks("Foundation", "CoreFoundation")
    end
    add_linkorders("framework::Foundation", "png16", "foo")
    add_linkorders("dl", "linkgroup::syslib")
    add_linkgroups("m", "pthread", {name = "syslib", group = true})

@Dozingfiretruck
Copy link

add_linkgroups("a", "b", {whole = true})如果同时需要-Wl,--start-group ,就有点歧义的感觉了,add_links("a", "b", {whole = true,group = true})或者 增加add_linkwhole("a", "b")?

@waruqi
Copy link
Member Author

waruqi commented Sep 30, 2023

add_linkgroups("a", "b", {whole = true})如果同时需要-Wl,--start-group ,就有点歧义的感觉了,add_links("a", "b", {whole = true,group = true})或者 增加add_linkwhole("a", "b")?

I improved it.

add_linkgroups("a", "b", {group = true})
add_linkgroups("a", "b", {whole = true})
add_linkgroups("a", "b", {group = true, whole = true})

and add_linkgroups("a", "b") is only for sorting links.

add_links("a", "b", "e")
add_linkorders("b", "linkgroup::foo")
add_linkgroups("c", "d", {name = "foo"})
-la -lb -lc -ld -le

@waruqi
Copy link
Member Author

waruqi commented Sep 30, 2023

I would also love the ability to link a single library statically -- this can be achieved on clang/GCC using -Wl,-Bstatic -lsome_static_lib -Wl,-Bdynamic -lmy_other_lib -lmy_other_lib_2. :)

Maybe a syntax like add_links("some_static_lib", { static = true })?

we can use add_linkgroups("a", "b", {static = true}) now.

@waruqi waruqi modified the milestones: todo, v2.8.5 Sep 30, 2023
@waruqi
Copy link
Member Author

waruqi commented Sep 30, 2023

xmake update -s dev

@waruqi waruqi closed this as completed Sep 30, 2023
@ivanallen
Copy link

在 package 中,报这个错误:

[root@70f6b8a8951d xmake-repo]# xmake l scripts/test.lua --shallow -vD spdk
error: ./packages/s/spdk/xmake.lua:35: global 'add_linkgroups' is not callable (a nil value)
stack traceback:
    [./packages/s/spdk/xmake.lua:35]: in main chunk

@waruqi
Copy link
Member Author

waruqi commented Oct 7, 2023

在 package 中,报这个错误:

[root@70f6b8a8951d xmake-repo]# xmake l scripts/test.lua --shallow -vD spdk
error: ./packages/s/spdk/xmake.lua:35: global 'add_linkgroups' is not callable (a nil value)
stack traceback:
    [./packages/s/spdk/xmake.lua:35]: in main chunk

暂不支持 package ,仅用于 target 下调整

@ivanallen
Copy link

在 package 中,报这个错误:

[root@70f6b8a8951d xmake-repo]# xmake l scripts/test.lua --shallow -vD spdk
error: ./packages/s/spdk/xmake.lua:35: global 'add_linkgroups' is not callable (a nil value)
stack traceback:
    [./packages/s/spdk/xmake.lua:35]: in main chunk

暂不支持 package ,仅用于 target 下调整

好的,请问这个有计划支持吗?期待一下。

@waruqi
Copy link
Member Author

waruqi commented Oct 27, 2023

在 package 中,报这个错误:

[root@70f6b8a8951d xmake-repo]# xmake l scripts/test.lua --shallow -vD spdk
error: ./packages/s/spdk/xmake.lua:35: global 'add_linkgroups' is not callable (a nil value)
stack traceback:
    [./packages/s/spdk/xmake.lua:35]: in main chunk

暂不支持 package ,仅用于 target 下调整

好的,请问这个有计划支持吗?期待一下。

I have supported it. #4314

we can also use add_linkorders to sort package links and add linkgroups/wholearchives.

package("libpng")
    add_linkorders("png16", "png", "linkgroup::foo")
    add_linkgroups("dl", {name = "foo", group = true})

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

6 participants