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

Dependency to other colcon packages in the ws #37

Open
francocipollone opened this issue Jul 6, 2023 · 4 comments
Open

Dependency to other colcon packages in the ws #37

francocipollone opened this issue Jul 6, 2023 · 4 comments

Comments

@francocipollone
Copy link
Contributor

francocipollone commented Jul 6, 2023

Summary

Hey there, I found really interesting this tool and I wanted to give it a shot
In order to try this out I created a colcon ws and I created a couple of poetry packages.
However, I found an issue when trying to identify dependencies among the pacakges via colcon graph or even the order of "building" when doing packages-up-to

I might probably be missing some configuration, I am not really poetry savvy

Replicating the issue

Basically I created a couple of packages in my ws:

  1. ros2 pkg create --build-type ament_python my_py_pkg
  2. ros2 pkg create --build-type ament_python my_py_pkg_2
  3. In the package xml of the second package I added the first one as a dependency.

So far (no poetry-ros so far) when doing colcon graph the dependencies are correctly identified:

my_py_pkg     +*
my_py_pkg_2    +
  1. Then, via poetry init I initalized the pyproject.toml for each package. (Also removed the setup.py file as it is no longer needed)

From here I proceeded to colcon build (previously installing colcon-poetry-ros extension) things are "ok"(things are built and installed).

However, when checking the dependencies via colcon graph the dependencies aren't well identified.

my_py_pkg     + 
my_py_pkg_2    +

Consequently, the ordering of the colcon build executing isn't expected.

@velovix
Copy link
Member

velovix commented Jul 7, 2023

Thanks for bringing this up. As you found, this tool does not interface properly with colcon graph. It's something I would like to address. Beyond making it more difficult to understand dependencies in your project, does this get in the way of properly running your ROS stack?

@francocipollone
Copy link
Contributor Author

Thanks for bringing this up. As you found, this tool does not interface properly with colcon graph. It's something I would like to address. Beyond making it more difficult to understand dependencies in your project, does this get in the way of properly running your ROS stack?

Yeah, actually colcon graph is just a tool for understanding the dependency graph in your workspace. Howeve when doing colcon build is expected that the dependency tree is followed accordingly. If I have a package that needs another package from my workspace I will need the latter to be built first.

Just to document an use-case in order to be more verbose, in my case we have:

  • A Cpp package that implements Foo: Let's call it package A
  • A Cpp package that adds python bindings of Foo: Let's call it package B
  • A pure python package that will consume the python bindings. Let's call it package C

If I do colcon build --packages-up-to C then only C gets built/installed. So the typical CI workflows doing build and test will fail.
A workaround can be added of course however this behavior attempts a bit against scalability in the workspace.

Thanks for the prompt answer @velovix 😄 . I will try to workaround this for the moment then

@francocipollone
Copy link
Contributor Author

francocipollone commented Jul 7, 2023

I might have found something: Here package_augmentation/poetry.py:

        if "build-system" in pyproject and "requires" in pyproject["build-system"]:
            build_deps = pyproject["build-system"]["requires"]
        else:
            build_deps = set()

        run_deps = project.get_dependencies(config.run_depends_extras.get())
        test_deps = project.get_dependencies(config.test_depends_extras.get())

        desc.dependencies["build_depends"] = set(
            create_dependency_descriptor(dep) for dep in build_deps
        )
        desc.dependencies["run_depends"] = set(
            create_dependency_descriptor(dep) for dep in run_deps
        )
        desc.dependencies["test_depends"] = set(
            create_dependency_descriptor(dep) for dep in test_deps

you are getting the build/run/test dependencies to be passed to colcon, right?

So if in the .toml of the package C I add:

[build-system]
requires = ["poetry-core", "A", "B"]

The dependencies are well obtained. And that fix the issue

Maybe here the package/colcon dependencies could be obtained from another key (not requires or even not in build-system) besides documenting these steps. Wdyt @velovix ? I could try to contribute with this feature if it is well scoped: I am not proficient working on colcon extensions but this seems trivial.

@francocipollone
Copy link
Contributor Author

francocipollone commented Jul 10, 2023

Maybe here the package/colcon dependencies could be obtained from another key (not requires or even not in build-system)

Just elaborating a bit over this idea:

We could indicate in the .toml file:

        [colcon-package]
        build_depend = ["A", "B"]

For doing this we should modify the code I linked and do something like(draft):

        if "colcon-package" in pyproject and "depend" in pyproject["colcon-package"]:
            build_depend = pyproject["colcon-package"]["build_depend"]
        else:
            build_depend = set()

        if "colcon-package" in pyproject and "exec_depend" in pyproject["colcon-package"]:
            exec_depend = pyproject["colcon-package"]["exec_depend"]
        else:
            exec_depend = set()

        if "colcon-package" in pyproject and "test_depend" in pyproject["colcon-package"]:
            test_depend = pyproject["colcon-package"]["test_depend"]
        else:
            test_depend = set()

        desc.dependencies["build_depends"] = set(
            create_dependency_descriptor(dep) for dep in build_depend
        )
        desc.dependencies["run_depends"] = set(
            create_dependency_descriptor(dep) for dep in exec_depend
        )
        desc.dependencies["test_depends"] = set(
            create_dependency_descriptor(dep) for dep in test_depend
        )

velovix pushed a commit that referenced this issue Jul 10, 2023
Related to #37 

### Summary

This PR introduces the ability to indicate the colcon/ros dependencies of the project via the `pyproject.toml` file.
Otherwise, dependencies for the package aren't loaded up, and the ordering when building isn't respected

### Use

Add to `pyproject.toml` the following section:

```toml
[colcon-package]
depend = ["another_pkg_1"]   # This will add to both `build_depend` and `exec_depend` following `package.xml` standards
build_depend = ["another_pkg_2"]
exec_depend = ["another_pkg_3"]
test_depend = ["another_pkg_4"]
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants