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

colcon mixin #646

Closed
tylerjw opened this issue Mar 22, 2021 · 4 comments
Closed

colcon mixin #646

tylerjw opened this issue Mar 22, 2021 · 4 comments

Comments

@tylerjw
Copy link
Contributor

tylerjw commented Mar 22, 2021

It would be nice if there was a way to set mixin arguments for colcon for ros2. It is much nicer than specifying cmake args specifically. To setup mixin you do this:

pip3 install colcon-mixin
colcon mixin add default https://raw.githubusercontent.com/colcon/colcon-mixin-repository/master/index.yaml
colcon mixin update default

That assumes colcon itself is installed.

Then you can do things like:

colcon build --mixin release ccache

And that's the same as:

colcon build --cmake args "-DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_COMPILER_LAUNCHER=ccache"

Maybe there could be a BUILDER_ARGS or COLCON_MIXIN that could be used to add these. Note that there are different mixin args that can apply to the build and test verbs of colcon.

@mathias-luedtke
Copy link
Member

Normally, the builders should not have builder-specific settings to keep them interchangeable.
I do not see an apparent need for this feature in CI (yet).

CCache is already configured by CCACHE_DIR and CMAKE_ARGS: -DCMAKE_BUILD_TYPE=Release is not too bad.

@tylerjw
Copy link
Contributor Author

tylerjw commented Mar 22, 2021

Right now I have one like this TARGET_CMAKE_ARGS: "-DCMAKE_BUILD_TYPE=RELWITHDEBINFO -DCMAKE_C_FLAGS='--coverage' -DCMAKE_CXX_FLAGS='--coverage' --no-warn-unused-cli", and it seems obnoxionsly long when locally I would just do --mixin rel-with-deb-info coverage if I wanted this.

To be fair there is no catkin equivalent.

I agree that ccache is done differently. I don't really understand why ccache now has cmake settings instead of through symlinks like before.

@mathias-luedtke
Copy link
Member

I don't really understand why ccache now has cmake settings instead of through symlinks like before.

Windows support?

and it seems obnoxionsly long when locally I would just do --mixin rel-with-deb-info coverage if I wanted this.

I see the point, but I am not yet convinced.
The coverage should be handled in #504

The mixins make sense for users, because it hard to memorize the arguments, but for CI this is not an issue.
I would rather have only one way to specify CMake settings (across all build systems and ROS versions).

@tylerjw
Copy link
Contributor Author

tylerjw commented Mar 22, 2021

The mixins make sense for users, because it hard to memorize the arguments, but for CI this is not an issue.
I would rather have only one way to specify CMake settings (across all build systems and ROS versions).

I'm really ok with this because for now atleast we are maintaining ros1 and ros2 versions of so many packages, the more similar the environment config is the better.

As to that PR, I tested it and couldn't get it work for me. I instead wrote my own solution that seems to work well for now (will happily use that when it is merged):

jobs:
  industrial_ci:
    strategy:
      matrix:
        env:
          - {
            CACHE_PREFIX: 'foxy-main-debug-ccov',
            ROS_DISTRO: foxy, ROS_REPO: main, CCOV_UPLOAD: true,
            CMAKE_ARGS: "-DCMAKE_BUILD_TYPE=Debug -DCMAKE_C_FLAGS='--coverage' -DCMAKE_CXX_FLAGS='--coverage'",
            AFTER_RUN_TARGET_TEST: './.ci.prepare_codecov',
            ADDITIONAL_DEBS: 'lcov'
          }
...
      - name: upaload codecov report
        uses: codecov/codecov-action@v1
        if: ${{ matrix.env.CCOV_UPLOAD }} 
        with:
          files: ${{ env.BASEDIR }}/coverage.info
#!/bin/bash
pushd $BASEDIR

BLUE='\033[0;34m'
NOCOLOR='\033[0m'

echo -e "${BLUE}Capture coverage info${NOCOLOR}"
lcov --capture --directory target_ws --output-file coverage.info

echo -e "${BLUE}Extract repository files${NOCOLOR}"
lcov --extract coverage.info "$BASEDIR/target_ws/src/$TARGET_REPO_NAME/*" --output-file coverage.info

echo -e "${BLUE}Filter out test files${NOCOLOR}"
lcov --remove coverage.info '*/test/*' --output-file coverage.info

echo -e "${BLUE}Output coverage data for debugging${NOCOLOR}" 
lcov --list coverage.info

popd

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