fix(snap): reapply changes removed by the merge of main

[Guillaumebeuzeboc]

The merge of main into dev/ros1_ros2_snap: 230b6ab removed a lot of the changes from the MR. Especially one from the Config.cmake.in that make the Target.cmake installed relatively to the Config.cmake

[facontidavide]

I have a problem with these:

        include("${CMAKE_CURRENT_LIST_DIR}/@PROJECT_NAME@Targets.cmake")

I need to take into account the case where not colcon/catkin is used.
The following command is suppose to work:

mkdir ws_plotjuggler
cd ws_plotjuggler

git clone https://github.com/facontidavide/PlotJuggler.git src/PlotJuggler
git clone https://github.com/PlotJuggler/plotjuggler-sample-plugins.git src/plotjuggler-samples

cmake -S src/PlotJuggler -B build/plotjuggler -DCMAKE_INSTALL_PREFIX="`pwd`/install"
cmake --build  build/plotjuggler --target install

cmake -S src/plotjuggler-samples -B build/plotjuggler-samples -DCMAKE_INSTALL_PREFIX="`pwd`/install"
cmake --build  build/plotjuggler-samples --target install

But with your code, I get the error:

-- The C compiler identification is GNU 9.4.0
-- The CXX compiler identification is GNU 9.4.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/lib/ccache/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/lib/ccache/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
CMake Error at /home/dfaconti/ws_PJ/build/plotjuggler/plotjugglerConfig.cmake:31 (include):
  include could not find requested file:

    /home/dfaconti/ws_PJ/build/plotjuggler/plotjugglerTargets.cmake
Call Stack (most recent call first):
  CMakeLists.txt:68 (find_package)


-- PlotJuggler FOUND
-- plotjuggler_INCLUDE_DIR: /home/dfaconti/ws_PJ/install/include
-- plotjuggler_LIBRARIES: plotjuggler_base
-- Configuring incomplete, errors occurred!

[Guillaumebeuzeboc]

Thank you for your answer.
The idea with:

        include("${CMAKE_CURRENT_LIST_DIR}/@PROJECT_NAME@Targets.cmake")

Is that the file Target.cmake is included relatively to the position of the Config.cmake. Using @CMAKE_INSTALL_PREFIX@ the path is hardcoded. So then if you move your build/ install/ directories (which snap does) everything is broken.

In the example you gave, there is something that is bothering me a little. When you run CMake on plotjuggler-sample, you don't provide any CMAKE_PREFIX_PATH (to point to your freshly "installed" plotjuggler).
Then during the run of CMake on plotjuggler-sample you have:

CMake Error at /home/dfaconti/ws_PJ/build/plotjuggler/plotjugglerConfig.cmake:31 (include):

Which is referring to your build repository. If I use the actual branch main (for PlotJuggler) I realized the build is using the same build/plotjuggler/plotjugglerConfig.cmake that is including then the install/lib/cmake/plotjuggler/plotjugglerTargets.cmake. So you end up using files from buid/ and install/ which sounds weird to me.

Instead, I would expect to specify CMAKE_PREFIX_PATH to point to the installed PlotJuggler:

cmake -S src/plotjuggler-samples -B build/plotjuggler-samples -DCMAKE_INSTALL_PREFIX="`pwd`/install" -DCMAKE_PREFIX_PATH="`pwd`/install/plotjuggler"

And then the correct Config.cmake (using the correct Target.cmake) is used

Then in the case of main and my branch it works.
Does it make sense ? I am not a CMake expert but from what I can see the Target.cmake relative to the path of Config.cmake seems to be standard (for example the zmq project)

edit: (sorry for the close/reopen, I hit the wrong shortcut)

[facontidavide]

I tried using the command you mentioned, but it still doesn't compile with your changes:

[dfaconti-Legion] ws_PJ$ cmake -S src/plotjuggler-samples -B build/plotjuggler-samples -DCMAKE_INSTALL_PREFIX="`pwd`/install" -DCMAKE_PREFIX_PATH="`pwd`/install/plotjuggler"

CMake Error at /home/dfaconti/ws_PJ/build/plotjuggler/plotjugglerConfig.cmake:31 (include):
  include could not find requested file:

    /home/dfaconti/ws_PJ/build/plotjuggler/plotjugglerTargets.cmake
Call Stack (most recent call first):
  CMakeLists.txt:68 (find_package)

Additionally, I think the correct command should be this one:

-DCMAKE_PREFIX_PATH="`pwd`/install

[artivis]

Hi there,

@facontidavide Regarding your last attempt, it looks like CMake is still using the *Config.cmake file from the build directory. You may have some CMake cache to clean.

CMake Error at /home/dfaconti/ws_PJ/**build**/plotjuggler/plotjugglerConfig.cmake:31 (include):

The following sequence worked just fine,

mkdir -p ws_plotjuggler/{src,build,install} && cd ws_plotjuggler

git clone https://github.com/facontidavide/PlotJuggler.git src/PlotJuggler
git clone https://github.com/PlotJuggler/plotjuggler-sample-plugins.git src/plotjuggler-samples

cmake -S src/PlotJuggler -B build/plotjuggler -DCMAKE_INSTALL_PREFIX="`pwd`/install"
cmake --build  build/plotjuggler --target install --parallel 12

cmake -S src/plotjuggler-samples -B build/plotjuggler-samples -DCMAKE_INSTALL_PREFIX="`pwd`/install" -DCMAKE_PREFIX_PATH="`pwd`/install"
cmake --build  build/plotjuggler-samples --target install --parallel 12

Alternatively, this works too,

cmake -S src/plotjuggler-samples -B build/plotjuggler-samples -DCMAKE_INSTALL_PREFIX="`pwd`/install" -Dplotjuggler_DIR:PATH="`pwd`/install"

[facontidavide]

yes, it works! thanks

[facontidavide]

It doesn't work

Staging plotjuggler 
+ snapcraftctl stage
Pulling plotjuggler-ros 
+ '[' '!' -d plotjuggler-ros-plugins ']'
+ vcs import
/bin/bash: line 37: /root/parts/plotjuggler/src/snap/local/plotjuggler.rosinstall: No such file or directory
Failed to run 'override-pull': Exit code was 1.
Build failed

[facontidavide]

Wait, this is in the automated build in https://snapcraft.io , not Github actions.
Waiting for the latter to complete

[artivis]

Looks like it is released on the beta channel,

$ sudo snap refresh plotjuggler --beta
plotjuggler (beta) 0588828 from Davide Faconti refreshed

The version is a little odd tho.

[facontidavide]

yes, and also I am not sure how to promote this to stable, since it is marked "devel grade" ( I just tried )

[facontidavide]

Unfortunately, the Snap doesn't work. It can't load rosbag nor rosbag2.

it throws an exception related to rosbag_storage

[artivis]

I am not sure how to promote this to stable, since it is marked "devel grade" ( I just tried )

The 'grade' selection is handled here.
The idea is that anything that is not a new git tag will be marked as 'grade: devel' so that it cannot be promoted to stable. So edge (& beta) are some sort of a rolling release, always on par with the main branch whereas candidate (& stable) will only be released on new git tag.
Similarly, the ci job pushes to edge on new commit and pushes to candidate on new tag (see here). This give the opportunity to test a new release before rolling it out to your users (promoting to stable).

We can of course revisit that if you'd like.

[artivis]

Unfortunately, the Snap doesn't work. It can't load rosbag nor rosbag2.

it throws an exception related to rosbag_storage

Having a look ;)

[facontidavide]

no, the devel/stable approach you described sounds very reasonable and correct.

My concerns is with rosbags now

[artivis]

I fixed the rosbag issue for ROS (1); finishing with ROS 2. PR coming soon ;)