open-rmf / rmf

Root repository for the RMF software
Apache License 2.0
238 stars 59 forks source link

Removing datamodel_code_generator from main installation step #375

Closed aaronchongth closed 1 year ago

aaronchongth commented 1 year ago

Bug fix

Fixed bug

Fixes https://github.com/open-rmf/rmf/issues/372.

Fix applied

Verify warning instead of crash

Using podman,

podman run -it docker.io/library/ros:humble-ros-base-jammy

# inside the image
mkdir -p ~/ws/src
cd ~/ws/src
git clone https://github.com/open-rmf/rmf_api_msgs
cd ~/ws
rosdep install --from-paths src --ignore-src --rosdistro humble -y
source /opt/ros/humble/setup.bash
colcon build
# only emits warning, still builds fine
aaronchongth commented 1 year ago

There is https://github.com/open-rmf/rmf_api_msgs too, I can update the readme there too.

for rmf-web, I think we are installing into the virtual environment and not system so it shouldn't matter right?

koonpeng commented 1 year ago

There is https://github.com/open-rmf/rmf_api_msgs too, I can update the readme there too.

for rmf-web, I think we are installing into the virtual environment and not system so it shouldn't matter right?

It half matters because rmf-web uses a venv that shares the system packages, the venv packages should have priority but in rare cases there may be problems with the transitive dependencies (actually this also applies to fastapi, uvicorn etc). I don't see any solutions though so this might be the best workaround.

After we merge this, what else do we need to do to make the build farm also use these instructions?

aaronchongth commented 1 year ago

Right now the buildfarm doesn't care, it is actually optional https://github.com/open-rmf/rmf_api_msgs/blob/main/rmf_api_msgs/CMakeLists.txt#L95, to allow folks to use models in python (ok this is rather important still if someone wants to use binaries and python to interact with rmf-web) But right now, nothing needs to be done.

We'll need to start the conversation again, regarding migrating away from pip for anything rmf related.