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

Providing multiple mixins concatenates lists in reverse order #40

Open
asasine opened this issue Jun 14, 2022 · 3 comments
Open

Providing multiple mixins concatenates lists in reverse order #40

asasine opened this issue Jun 14, 2022 · 3 comments
Labels
bug Something isn't working

Comments

@asasine
Copy link

asasine commented Jun 14, 2022

When using multiple mixins that each define cmake-args, the order they are concatenated is in the reverse order they are listed in the mixin list.

Given these mixins:

index.yaml

mixin:
  - a.mixin
  - b.mixin

a.mixin

{
    "build": {
        "a": {
            "cmake-args": [
                "-DFOO=A"
            ]
        }
    }
}

b.mixin

{
    "build": {
        "b": {
            "cmake-args": [
                "-DFOO=B"
            ]
        }
    }
}

and this command:

colcon build --mixin a b

the executed cmake command for every package looks like this:

Invoking command in '/path/to/package/build/package': /usr/bin/cmake /path/to/package/src -DFOO=B -DFOO=A ...

Since the order provided to cmake is -DFOO=B -DFOO=A, the value of FOO is A when in cmake's configure step. This is the reverse of the order provided to --mixin a b as I expected. Is this a bug or by design?

@jeetee
Copy link

jeetee commented Nov 25, 2022

From reading https://github.com/colcon/colcon.readthedocs.org/blame/main/reference/mixin-arguments.rst#L31-L32 this seems to be a bug to me; I've come across the same when trying to overwrite build locations using two mixins; Currently you have to swap the call order to

colcon build --mixin specific_override general_case

While the docs seem to suggest it should be the other way around.

@cottsay cottsay added the bug Something isn't working label Feb 3, 2024
@cottsay
Copy link
Member

cottsay commented Feb 3, 2024

Hmm, this is a bit tricky. Simply reversing the application order causes list arguments to have the correct order, but messes up string arguments. The problem seems to center around how mixins PREPEND to list arguments.

We may need to pre-merge the mixin arguments before applying them to the final argument composition.

@sdmcgrath
Copy link

Hmm, this is a bit tricky. Simply reversing the application order causes list arguments to have the correct order, but messes up string arguments. The problem seems to center around how mixins PREPEND to list arguments.

@cottsay I tried to replicate @asasine's setup, but adding string arguments as well. e.g.

a.mixin

    "build": {
        "a": {
            "cmake-args": [
                "-DFOO=A"
            ],
            "cmake-target": "A"
        }
    }
}

b.mixin

    "build": {
        "b": {
            "cmake-args": [
                "-DFOO=B"
            ],
            "cmake-target": "B"
        }
    }
}

Executed colcon build --mixin a b and the output command had cmake_args=['-DFOO=B', '-DFOO=A'], cmake_target='A'
Given that the documentation suggests later mixins will overwrite earlier ones, I would expect cmake_target='B', and so it looks to me like string arguments are already messed up (in addition to the list argument append order).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

4 participants