Merging multiple mixins _packages-skip_ lists

[jrgnicho]

I'm trying to configure my colcon workspace to skip packages from two (for now) skip.mixin files in my workspace, the files are as follows:

colcon_ws/src/some_repo1/some_repo1_skip.mixin

{
  "build": {
    "skip": {
      "packages-skip": ["some_repo1_ros_pkg1",
                        "some_repo1_ros_pkg2",
                        "some_repo1_ros_pkg3"
                        ],
    }
  }
}

colcon_ws/src/some_repo2/some_repo2_skip.mixin

{
  "build": {
    "skip": {
      "packages-skip": ["some_repo2_ros_pkg1",
                        "some_repo2_ros_pkg2",
                        "some_repo2_ros_pkg3"
                        ],
    }
  }
}

Then I have a workspace level colcon_ws/index.yaml file

mixin:
    - src/some_repo1/some_repo1_skip.mixin
    - src/some_repo2/some_repo2_skip.mixin

I need different name for the mixing files otherwise it complains that the names are the same.

Then I add the mixin to the colcon workspace as follows

colcon mixin add skip file://`pwd`/index.yaml
colcon mixin update skip

Then I list the mixin with colcon mixin list and get the following warning

WARNING:colcon.colcon_mixin.mixin:Mixin 'skip' from file '/home/username/.colcon/mixin/skip/some_repo2_skip.mixin' is overwriting another mixin with the same name

And then when I build with

colcon build --mixin skip --symlink-install

colcon tries to build the packages in the mixin that was overwritten.

So my question is if there's a way to merge the entries in the packages-skip lists from all of the same mixins that appear in the files listed in the index.yaml file ?

[dirk-thomas]

I need different name for the mixing files otherwise it complains that the names are the same.

The file containing the paths of both mixin files is an index. It is being used to e.g. download mixin files using colcon mixin update. Since all mixin files from a specific index file are downloaded to the same directory the basename of each mixin file must be unique.

To avoid this limitation the full path of the mixin file could be somehow used to generate unique filenames even if their basenames are the same.

is there's a way to merge the entries in the packages-skip lists from mixins?

No, at the moment there is no logic merging the content of mixins with the same name. The last one simply overwrite the previous ones.

For values of type bool, numeric, and str the current semantic is sufficient. For collections like dict, list, and set` it would be possible to also merge the data instead of using only the last.

For some options the order of arguments doesn't make a difference (as in your case --packages-skip). But for some options (like --mixin) the order is important. The problem I see is that the mixins from different index index files have no order between them. So if the implementation would enforce some deterministic order (e.g. alphabetical over the registered index files) that might not be the order the user wants / expects.

[jrgnicho]

To avoid the need to make each copied mixin file name unique would it be easier to instead create a single mixin file that contains all of the entries from all the mixin files listed in the index?

In regards to the merging and the order that the individual entries are appended to a list, I think it would make sense to honor the order listed in the index.yaml

If these feature/changes are acceptable I could try to make them myself I would just need some guidance.

[dirk-thomas]

To avoid the need to make each copied mixin file name unique would it be easier to instead create a single mixin file that contains all of the entries from all the mixin files listed in the index?

Since each file is downloaded individually they are also stored individually - a 1-to-1 mapping.

In regards to the merging and the order that the individual entries are appended to a list, I think it would make sense to honor the order listed in the index.yaml

Honoring the ordering in a single index file is an obvious choice. But there are multiple index files and there is no specific order defined between them.

If these feature/changes are acceptable I could try to make them myself I would just need some guidance.

That would be great. I just don't know at the moment how exactly that's should be implemented.

[jrgnicho]

Since each file is downloaded individually they are also stored individually - a 1-to-1 mapping.

What I'm saying is that when one calls colcon mixin update [mixin-name] instead of copying each mixin 1 by 1 the implementation could create a single mixin file with all the entries in each mixin listed. From that point on the information for the mixin would be retrieved from that new combined mixin file when one calls colcon build --mixin [mixin-name] ....
This would also help with honoring the order listed as there's only one file and all of the entries in it should be organized in the order that was specified in the index.yaml

[dirk-thomas]

What I'm saying is that when one calls colcon mixin update [mixin-name] instead of copying each mixin 1 by 1 the implementation could create a single mixin file with all the entries in each mixin listed.

That alone does not solve the ordering problem across indices. The same merge could also be performed on-the-fly when the data is read. The challenge stays the same independent when the merge is being performed.

It would be possible to only merge mixins with the same name within the same index (where an order is defined) and keep the existing error message about using only the last if they are defined in different indices. That merging should still happen on-usage time.

A pull request for that kind of change would be great to try the idea.

[jrgnicho]

@dirk-thomas If you (or anyone) could provide some guidance for where it would be right to make that change it really help

[dirk-thomas]

If you (or anyone) could provide some guidance for where it would be right to make that change it really help

The logic is contained in this repository and the current decision to use only the last key is embedded here:

colcon-mixin/colcon_mixin/mixin/__init__.py

Line 102 in 3b336dd

if name in mixins_by_verb[verb_key]:

To implement the desired logic to allow merging information from the same index (which provides an order) the logic probably has to be refactored in order to allow that kind of context awareness,

[jrgnicho]

I see, since the value type in the mixins_by_verb dictionary is yet another dictionary it won't be as straight forward to keep the order as dictionaries sort keys by name I think. I'll take a look, thanks

[dirk-thomas]

I'll take a look

@jrgnicho Did you have a chance to look into this?

[jrgnicho]

@dirk-thomas unfortunately I've been swamped lately and haven't had a change to work on this. However, in this project, I'm using a custom script to more or less create a single mixin file that is then added to colcon.