AutonomyLab / ardrone_autonomy

ROS driver for Parrot AR-Drone 1.0 and 2.0 quadrocopters
http://wiki.ros.org/ardrone_autonomy
BSD 3-Clause "New" or "Revised" License
357 stars 226 forks source link

git clone during build stage is bad practice and is known not to work on some architectures #189

Open tfoote opened 8 years ago

tfoote commented 8 years ago

Git cloning during the build step is poor practice. It means that building a package is tied to network connectivity, and the continued availability of the hosting server.

More pressing is that git clone is known not to work in some build environments such as QEMU.

This package is timing out repeatedly building on arm64 http://build.ros.org/job/Kbin_arm_dJv8__ardrone_autonomy__debian_jessie_arm64__binary/

06:54:55 [ 93%] Performing download step (git clone) for 'ardronelib'
06:54:55 cd /tmp/binarydeb/ros-kinetic-ardrone-autonomy-1.4.1/obj-aarch64-linux-gnu/devel/src && /usr/bin/cmake -P /tmp/binarydeb/ros-kinetic-ardrone-autonomy-1.4.1/obj-aarch64-linux-gnu/devel/tmp/ardronelib-gitclone.cmake

Also armhf http://build.ros.org/job/Kbin_arm_uXhf__ardrone_autonomy__ubuntu_xenial_armhf__binary/

tfoote commented 8 years ago

@jacquelinekay This also could be blacklisted for Kinetic arm builds if necessary to avoid the timeouts and errors on the farm.

jacquelinekay commented 8 years ago

ros-infrastructure/ros_buildfarm_config#38 will blacklist ardrone_autonomy for ARM Kinetic builds. The package was previously blacklisted for Indigo and Jade.

mani-monaj commented 8 years ago

@tfoote

Git cloning during the build step is poor practice. It means that building a package is tied to network connectivity, and the continued availability of the hosting server. More pressing is that git clone is known not to work in some build environments such as QEMU.

I agree with that. There are three alternatives that comes to my mind:

  1. Include Parrot SDK's source tree inside the driver's source repo: This was the initial approach, however it turned out to be problematic. Since the SDK is not maintained by Parrot, it was difficult to maintain patches to the SDK while its source code was mixed with the driver.
  2. Fetch the SDK from the external repo as a tarball archive instead of git cloning it: It does not solve the connectivity issue, but at least should be easier on the server.
  3. Package Parrot SDK as a third party catkin tool: The problem with this method is that the SDK's [rather complex] build system does not seem to implement install rules correctly. It may take some effort to fix those issues. Considering my time budget, that might not be feasible in the short term.

I will implement option 2 to fix some of the issues as soon as I get a chance.

This package is timing out repeatedly building on arm64

I am sorry about that. In addition to the git clone issue, there is an assembler error (when the Parrot SDK is being built) that is due to incompatibility of the h264 decoding code with the arm architecture. I think blacklisting arm builds is the best decision at the moment.

tfoote commented 8 years ago

Yeah, option 2 is probably the low hanging fruit.

For 1. Another option might be to create a standalone wrapper package for the sdk and depend on it from the driver package. That will separate the source code and decouple the release cycles.

mani-monaj commented 8 years ago

For 1. Another option might be to create a standalone wrapper package for the sdk and depend on it from the driver package. That will separate the source code and decouple the release cycles.

Thanks @tfoote

By 3. I actually meant to wrap our own fork of the SDK into a standalone package. Do you know if there is any other package that wraps a make based external project into a standalone package? Is the assumption that these are the only steps I need to take to wrap this package, correct?

tfoote commented 8 years ago

Ahh, yes, then what I was talking about was option 3. I do not know of any packages currently using just make. There are several pure cmake packages which have been demonstrated. And I think there are a couple that use cmake to call a third party makefile. @wjwwood might be able to point you to a good example.

wjwwood commented 8 years ago

Just follow the thing you linked to (http://wiki.ros.org/bloom/Tutorials/ReleaseThirdParty#Modifying_the_Upstream_Repository) and invoke additionally drop in a CMakeLists.txt that just invokes make for the all target and either make install or use cmake to install files for install target. Make sure it respects the CMAKE_INSTALL_PREFIX. You can add these invocations of make and stuff using this: https://cmake.org/cmake/help/v3.0/command/add_custom_command.html

mani-monaj commented 8 years ago

Thanks @tfoote and @wjwwood

I actually did the same thing in order to wrap Parrot's new SDK into a catkin package. The code is not yet released, but it follows what @wjwwood suggested more or less. The underlying build system is more complex though for that package. I will go on an adopt the same procedure to wrap the calls to make and also make sure it respects CMAKE_INSTALL_PREFIX. Thank you again.