BUILD.bazel files cause package identification problems

mjcarroll commented 3 years ago

Environment

OS Version: Ubuntu 20.04
Source or binary build? Source Edifice build with colcon

Description

Expected behavior: Build successfully completes
Actual behavior: Colcon package identification fails because it sees both cmake and bazel

Steps to reproduce

Install colcon as well as the colcon-bazel extension: https://github.com/colcon/colcon-bazel
Attempt to build the ignition edifice workspace with colcon

Output

An example of colcon list shows that ign-math and ign-utils are both ignored because they match against both CMake and Bazel.

$ colcon list
[0.368s] WARNING:colcon.colcon_core.package_identification:No valid Build content
[0.371s] WARNING:colcon.colcon_core.package_identification:_identify(src/ign-math) has multiple matches and therefore is being ignored: bazel, cmake
[0.435s] WARNING:colcon.colcon_core.package_identification:No valid Build content
[0.438s] WARNING:colcon.colcon_core.package_identification:_identify(src/ign-utils) has multiple matches and therefore is being ignored: bazel, cmake
ignition-cmake2 src/ign-cmake   (cmake)
ignition-common4    src/ign-common  (cmake)
ignition-fuel_tools6    src/ign-fuel-tools  (cmake)
ignition-gazebo5    src/ign-gazebo  (cmake)
ignition-gui5   src/ign-gui (cmake)
ignition-launch4    src/ign-launch  (cmake)
ignition-msgs7  src/ign-msgs    (cmake)
ignition-physics4   src/ign-physics (cmake)
ignition-plugin1    src/ign-plugin  (cmake)
ignition-rendering5 src/ign-rendering   (cmake)
ignition-sensors5   src/ign-sensors (cmake)
ignition-tools  src/ign-tools   (cmake)
ignition-transport10    src/ign-transport   (cmake)
sdformat11  src/sdformat    (cmake)

mjcarroll commented 3 years ago

As far as I can tell, there are a few ways to solve it.

1) If you aren't actively using bazel in colcon workspaces, remove the colcon-bazel extension 2) Alter the PRIORITY setting the colcon-bazel extension point to give CMake higher priority

Ref: https://github.com/colcon/colcon-core/blob/master/colcon_core/package_identification/__init__.py#L46
https://github.com/colcon/colcon-bazel/blob/master/colcon_bazel/package_identification/bazel.py#L26 3) Add a colcon.pkg to all ignition packages to make CMake explicit

mjcarroll commented 3 years ago

Another alternative is to use a colcon.meta file in the root of your colcon workspace.

{
  "paths": {
    "src/ign-utils": {
      "type": "cmake"
    },
    "src/ign-math": {
      "type": "cmake"
    },
    "src/ign-common": {
      "type": "cmake"
    },
  }
}

chapulina commented 3 years ago

I remember having this issue before:

https://github.com/ignitionrobotics/ign-plugin/pull/31#pullrequestreview-532959529

Option 1. sounds like a tough ask for all users and I'd like to avoid option 3 so we don't have one more file to maintain.

I didn't quite understand how a user can set the PRIORITY for option 2, is this something that can be set from ~/.colcon/defaults.yaml or something like that?

The colcon.meta option also sounds promising.

luca-della-vedova commented 3 years ago

I didn't quite understand how a user can set the PRIORITY for option 2, is this something that can be set from ~/.colcon/defaults.yaml or something like that?

The colcon.meta option also sounds promising.

+1 to both options, I also wonder if a different default priority for Bazel should be pushed upstream? While I understand that it would be an arbitrary choice to say what's higher priority between CMake and Bazel, it sounds like anything better than the current behavior (skipping the package altogether) would be better?

mjcarroll commented 3 years ago

I didn't quite understand how a user can set the PRIORITY for option 2, is this something that can be set from ~/.colcon/defaults.yaml or something like that?

Setting the priority wouldn't be a user choice. The bazel extension itself would have to be modified upstream so that it had a higher/lower priority than CMake so that one will always "win".

From my observations, colcon is used more widely with cmake/ros projects, so it seems like that would be the sane default, and users could use one of the approaches above to "force" bazel.

mjcarroll commented 3 years ago

I believe the colcon-bazel pull request will address this. You can install that version via pip:

 pip3 install -e git://github.com/colcon/colcon-bazel@reduce_priority#egg=colcon-bazel --upgrade --user

mxgrey commented 2 years ago

For future visitors to this issue, I needed to tweak @mjcarroll 's command to this:

pip3 install git+https://github.com/colcon/colcon-bazel@master#egg=colcon-bazel --upgrade --user

in particular I needed to use git+https: instead of only git:, otherwise GitHub hung up on me. The other changes in my command are just to (1) not leave an editable version of the package hanging around, and (2) use the master branch of colcon-bazel since the relevant PR has been merged.

gazebosim / gz-bazel