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

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

Open
tfoote opened this issue Apr 20, 2016 · 8 comments
Labels

Comments

@tfoote
Copy link

tfoote commented Apr 20, 2016

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
Copy link
Author

tfoote commented Apr 20, 2016

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

@jacquelinekay
Copy link

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
Copy link
Member

@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
Copy link
Author

tfoote commented Apr 22, 2016

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
Copy link
Member

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
Copy link
Author

tfoote commented Apr 22, 2016

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
Copy link

wjwwood commented Apr 22, 2016

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants