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

Handle target name clashes in subprojects (Support namespace) #3380

Closed
paul-reilly opened this issue Feb 17, 2023 · 14 comments
Closed

Handle target name clashes in subprojects (Support namespace) #3380

paul-reilly opened this issue Feb 17, 2023 · 14 comments

Comments

@paul-reilly
Copy link
Contributor

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

Steps to reproduce the situation:

  • include other xmake projects contained in subdirectories with e.g. includes("external/**/xmake.lua")
  • if those other xmake projects contain a target with the same name/type, they are treated as the same target

Say we have multiple targets called tests in this sort of setup using git submodules in the canonical external subdirectory:

.
├── doxyfile
├── external
│   └── mylib
│       ├── src
│       │   ├── foo.cpp
│       │   ├── foo.h
│       │   └── main.cpp
│       ├── tests
│       │   └── test.cpp
│       └── xmake.lua
├── README.md
├── src
│   └── main.cpp
├── tests
│   └── test.cpp
└── xmake.lua

We cannot use set_default(false) for the tests targets because there then doesn't seem to be a way of telling xmake which tests target to build.

Describe the solution you'd like

I don't know what would be ideal here.

  • The CMake solution of having targets namespaced in original CMakeLists.txt is not ideal because it requires editing the file in third party dependency.
  • A way of avoiding clashes while still being able to just include a subproject would be better.

Describe alternatives you've considered

Current solutions:

Be careful with naming targets:
target("common") -> target("myproj_common")
target("tests") -> target("mylibtests")

Additional context

No response

@waruqi
Copy link
Member

waruqi commented Feb 17, 2023

target("common") -> target("myproj_common")

There is no better solution, this is the recommended practice

@paul-reilly
Copy link
Contributor Author

paul-reilly commented Feb 18, 2023

At the moment if we have two targets called foo in different directories that have been includes() by an xmake file, they are treated as the same target. If there are no symbol clashes then there's no linker error either. Linker flags are merged.

In the (near) future when xmake is used as the build system in many more projects that are then used as git submodules, it's something that will happen more often. The included projects could contain a hierarchy of nested xmake.lua files so there's a point where the tool needs to take over the responsibility of tracking these clashes imo.

  • I think xmake should at least warn/inform when it encounters duplicate target names:
    e.g.
Info: Target 'foo' from 'external/theirlib/subdir/anothersubdir/xmake.lua' has been merged with target 'foo' from './xmake.lua' into 'libfoo.so'. If this was not intentional you can rename one of the targets.

It's something to consider at least.

@waruqi
Copy link
Member

waruqi commented Feb 18, 2023

I think xmake should at least warn/inform when it encounters duplicate target names:

Merge the configuration of a target with the same name between different xmake.lua. It is also one of the features of xmake, so I can't provide a warning.

This allows us to configure the same target in different xmake.lua.

I'll consider improving it, but I don't have a good solution yet, maybe separating the namespace of the target based on set_project, or adding a separate interface to set the namespace

@paul-reilly
Copy link
Contributor Author

There's no rush for a solution, I understand now that merging targets is a feature. It's not intended as a feature for difference projects though and this is just something to plan for, for when xmake is used in a sizeable percentage of projects on GitHub/Lab/ee!

@SirLynix
Copy link
Member

Maybe something like namespace?

target("foo")

namespace("xx")

target("foo") -- is xx_foo

@xq114
Copy link
Contributor

xq114 commented Feb 18, 2023

I think namespace is better, since if an external project mylib is not provided with set_project, we can still use the following code to avoid confliction. (set_project can be interpreted as a default namespace though)

namespace("mylib")
includes("ext/mylib")
namespace_end()

target("foo")
    add_deps("mylib::foo")

Despite the syntax change, the produced files can still conflict, thus some extra concern on build/install file structure should be present, e.g.

-- xmake install -o dist
dist
- bin
- include
- lib
- mylib
- ...

@xq114
Copy link
Contributor

xq114 commented Feb 18, 2023

Btw it's not a big problem in xmake. Write a (local) package for each conflicting dependency is enough to solve the problem.

@waruqi waruqi changed the title Handle target name clashes in subprojects Handle target name clashes in subprojects (Support namespace) Feb 20, 2023
@waruqi waruqi mentioned this issue Sep 1, 2024
10 tasks
@waruqi waruqi added this to the v3.0.0 milestone Sep 1, 2024
@microdee
Copy link
Contributor

microdee commented Oct 6, 2024

If it's still debated I'm advocating namespaces but in a way that it's still "ok" to write in lua.
I know it's nowhere near polished and may even go against XMake guidelines but I cooked up a solution for my own project (currently it does nothing, only the project model is set up).

In there I have an API which allows to have things like

local ns = NS.use()
-- infer namespace of the parent folder of this script
-- (for demo purposes let's assume now it refers to "core.stuff.namespace")

local ns_bar = NS.use("core.foo.bar")
-- use another namespace

target(ns:n("myLib"))
-- ns:n("myLib") simply adds a name to the specified namespace (it yields "core.stuff.namespace.myLib" in this case)

    add_deps(ns_bar:full("otherLib"))
    -- "otherLib" doesn't have to be directly in "core.foo.bar" namespace, it can be in any parent namespaces as well and it will be returned accordingly,
    -- like if it's declared in "core.foo" then this yields "core.foo.otherLib"

    add_deps(ns:full("coreLib"))
    -- add a target from the current or from any of the parent namespaces
    -- like if we defined `ns:n("coreLib")` in a namespace "core", then ns:full("coreLib") in namespace "core.stuff.namespace" will yield "core.coreLib"

-- also as an amenity I've added scoped/namespaced object storage so one can do
ns.scope.mystuff = { foo = "bar" }
ns:get("mystuff").foo -- -> "bar"
ns:push("deeper") -- now the current namespace is "core.stuff.namespace.deeper"
ns:get("mystuff").foo -- -> "bar" (still gets the correct value from parent namespace)

Here's how it's implemented https://github.com/microdee/rats.nest/blob/main/_build/xmake/rats/namespaces.lua

If XMake will take in namespaces as a vanilla concept it will most probably look completely different, I just wanted to provide an example. We would need none of this of course if Lua had namespaces as a language feature, so that's why the workaround. A setup like this also shouldn't be applied only to target names, but should allow the developer to use it in any string or any context really.

@wpchom
Copy link

wpchom commented Dec 18, 2024

I think namespace is better, since if an external project mylib is not provided with set_project, we can still use the following code to avoid confliction. (set_project can be interpreted as a default namespace though)

namespace("mylib")
includes("ext/mylib")
namespace_end()

target("foo")
    add_deps("mylib::foo")

Despite the syntax change, the produced files can still conflict, thus some extra concern on build/install file structure should be present, e.g.

-- xmake install -o dist
dist
- bin
- include
- lib
- mylib
- ...

it has a little flavor like gn, includes the path within deps reduces the statements of includes(...), and makes the dependency declarations more straightforward.

group("foo") {
    deps = [ "external/mylib/:foo" ]
}

like this, describe the path & target of the dependency in one line.

target("foo")
    add_deps("external/mylib/:foo")

@waruqi
Copy link
Member

waruqi commented Dec 20, 2024

I think namespace is better, since if an external project mylib is not provided with set_project, we can still use the following code to avoid confliction. (set_project can be interpreted as a default namespace though)

namespace("mylib")
includes("ext/mylib")
namespace_end()

target("foo")
    add_deps("mylib::foo")

Despite the syntax change, the produced files can still conflict, thus some extra concern on build/install file structure should be present, e.g.

-- xmake install -o dist
dist
- bin
- include
- lib
- mylib
- ...

How do we handle nested namespaces and target references within a namespace?

@waruqi
Copy link
Member

waruqi commented Dec 20, 2024

enter namespaces

namespace("test")

includes("foo")
includes("bar")

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

    namespace("test1")
        target("hello1")
            -- ...
    namespace_end()
namespace_end()

or

namespace("test", function()
    includes("foo")
    includes("bar")

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

    namespace("test1", function ()
        target("hello1")
            -- ...
    end)
end)

access namespaces

add_deps("test::hello", "test::test1::hello1") -- in root
add_deps("hello", "test1::hello1") -- in test namespace
add_deps("test::hello", "hello1") -- in test1 namespace

@waruqi
Copy link
Member

waruqi commented Jan 6, 2025

try #6001

xmake update -s github:xmake-io/xmake#namespace

Roadmap

  • target
  • option
  • rule
  • package
  • toolchain
  • task
  • xpack

Target (inner access)

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

namespace("ns1", function ()
    target("foo")
        set_kind("static")
        add_files("src/foo.cpp")

    namespace("ns2", function()
        target("bar")
            set_kind("static")
            add_files("src/bar.cpp")
    end)

    target("test")
        set_kind("binary")
        add_deps("foo", "ns2::bar")
        add_files("src/main.cpp")
end)

Target (root values)

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

add_defines("ROOT")

namespace("ns1", function ()
    add_defines("NS1_ROOT")
    target("foo")
        set_kind("static")
        add_files("src/foo.cpp")
        add_defines("FOO")

    namespace("ns2", function ()
        add_defines("NS2_ROOT")
        target("bar")
            set_kind("static")
            add_files("src/bar.cpp")
            add_defines("BAR")
    end)
end)

target("test")
    set_kind("binary")
    add_deps("ns1::foo", "ns1::ns2::bar")
    add_files("src/main.cpp")
    add_defines("TEST")

Target (includes)

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

add_defines("ROOT")

namespace("ns1", function ()
    add_defines("NS1_ROOT")
    target("foo")
        set_kind("static")
        add_files("src/foo.cpp")
        add_defines("FOO")

    includes("src")
end)

target("test")
    set_kind("binary")
    add_deps("ns1::foo", "ns1::ns2::bar")
    add_files("src/main.cpp")
    add_defines("TEST")

Option

$ xmake f --opt0=y
$ xmake f --ns1::opt1=y
$ xmake f --ns1::ns2::opt2=y
add_rules("mode.debug", "mode.release")

option("opt0", {default = true, defines = "OPT0", description = "option0"})

namespace("ns1", function ()
    option("opt1", {default = true, defines = "NS1_OPT1", description = "option1"})

    target("foo")
        set_kind("static")
        add_files("src/foo.cpp")
        add_options("opt1")

    namespace("ns2", function()
        option("opt2", {default = true, defines = "NS2_OPT2", description = "option2"})
        target("bar")
            set_kind("static")
            add_files("src/bar.cpp")
            add_options("opt2")
    end)

    target("test")
        set_kind("binary")
        add_deps("foo", "ns2::bar")
        add_files("src/main.cpp")
        add_options("opt0", "opt1", "ns2::opt2")
end)

Rule

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

rule("rule0")
    on_load(function (target)
        target:add("defines", "RULE0")
    end)

namespace("ns1", function ()
    rule("rule1")
        on_load(function (target)
            target:add("defines", "NS1_RULE1")
        end)

    target("foo")
        set_kind("static")
        add_files("src/foo.cpp")
        add_rules("rule1")

    namespace("ns2", function()
        rule("rule2")
            on_load(function (target)
                target:add("defines", "NS2_RULE2")
            end)

        target("bar")
            set_kind("static")
            add_files("src/bar.cpp")
            add_rules("rule2")
    end)

    target("test")
        set_kind("binary")
        add_deps("foo", "ns2::bar")
        add_files("src/main.cpp")
        add_rules("rule0", "rule1", "ns2::rule2")
end)

Task

xmake task0
xmake ns1::task1
xmake ns1::ns2::task2
task("task0")
    set_menu {options = {}}
    on_run(function ()
        print("task0")
    end)

namespace("ns1", function ()
    task("task1")
        set_menu {options = {}}
        on_run(function ()
            print("NS1_TASK1")
        end)

    namespace("ns2", function()
        task("task2")
            set_menu {options = {}}
            on_run(function ()
                print("NS2_TASK2")
            end)
    end)
end)

Toolchain

toolchain("toolchain0")
    on_load(function (toolchain)
        toolchain:add("defines", "TOOLCHAIN0")
    end)

namespace("ns1", function ()
    toolchain("toolchain1")
        on_load(function (toolchain)
            toolchain:add("defines", "NS1_TOOLCHAIN1")
        end)

    target("foo")
        set_kind("static")
        add_files("src/foo.cpp")
        set_toolchains("toolchain1")

    namespace("ns2", function()
        toolchain("toolchain2")
            on_load(function (toolchain)
                toolchain:add("defines", "NS2_TOOLCHAIN2")
            end)

        target("bar")
            set_kind("static")
            add_files("src/bar.cpp")
            set_toolchains("toolchain2")
    end)

    target("test")
        set_kind("binary")
        add_deps("foo", "ns2::bar")
        add_files("src/main.cpp")
        set_toolchains("toolchain0", "toolchain1", "ns2::toolchain2")
end)

Package

add_requires("package0", {system = false})

package("package0")
    on_load(function (package)
        package:add("defines", "PACKAGE0")
    end)
    on_install(function (package) end)

namespace("ns1", function ()

    add_requires("package1", {system = false})

    package("package1")
        on_load(function (package)
            package:add("defines", "NS1_PACKAGE1")
        end)
        on_install(function (package) end)

    target("foo")
        set_kind("static")
        add_files("src/foo.cpp")
        add_packages("package1")

    namespace("ns2", function()

        add_requires("package2", {system = false})

        package("package2")
            on_load(function (package)
                package:add("defines", "NS2_PACKAGE2")
            end)
            on_install(function (package) end)

        target("bar")
            set_kind("static")
            add_files("src/bar.cpp")
            add_packages("package2")
    end)

    target("test")
        set_kind("binary")
        add_deps("foo", "ns2::bar")
        add_files("src/main.cpp")
        add_packages("package0", "package1", "ns2::package2")
end)
$ xmake build -r ns1::test
[ 33%]: cache compiling.release ns1::ns2::src/bar.cpp
[ 41%]: cache compiling.release ns1::src/foo.cpp
[ 50%]: cache compiling.release ns1::src/main.cpp
[ 58%]: archiving.release ns1::ns2::libbar.a
[ 75%]: archiving.release ns1::libfoo.a
[ 91%]: linking.release ns1::test
[100%]: build ok, spent 1.325s

@waruqi waruqi modified the milestones: v3.0.0, v2.9.8 Jan 6, 2025
@ifarbod
Copy link
Contributor

ifarbod commented Jan 8, 2025

This is a game-changer! Good stuff [@]waruqi!

@waruqi
Copy link
Member

waruqi commented Jan 9, 2025

Done, and xmake/core also uses it now.

xmake/core/xmake.lua

Lines 135 to 141 in a85de0a

if namespace then
namespace("tbox", function ()
includes("src/tbox")
end)
else
includes("src/tbox")
end

@waruqi waruqi closed this as completed Jan 9, 2025
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

7 participants