ros-industrial / industrial_ci

Easy continuous integration repository for ROS repositories
Apache License 2.0
249 stars 129 forks source link

industrial_ci simplification options ? #211

Open asmodehn opened 7 years ago

asmodehn commented 7 years ago

I have been doing CI for a while with ROS, before knowing about industrial_ci. I have decided to start using industrial_ci, mainly to merge maintenance effort, and make things more consistent.

After a quick try I find industrial_ci supports a lot of workflows (probably more than I need) and is overall quite complicated (as in "deeply nested in scripts"), probably more than needed. It makes it quite difficult to understand what is happening, and I think a build framework should be as simple as possible so users have no trouble understanding what is actually happening.

My usual package CI workflow is :

An example of travis file is there : https://github.com/pyros-dev/pyros-utils/blob/0.1.4/.travis.yml

Then the build script can be run locally as well to find out if something breaks or not (separately from docker). It only need the distro input and the "flow" (devel or install)

An example is there: https://github.com/pyros-dev/pyros-utils/blob/0.1.4/travis_checks.bash

I don't test the debian packaging as this will be done on the buildfarm later on.

This is what I found out is the optimum, only necessary steps are run, and it s still quite fast, faster than the ros buildfarm actually. By necessary I mean this is enough to make sure your install space works, and then you know you can rely on that to get your other code working.

Another point is the separation of concern :

I am not sure which are exactly the scenarios that industrial_ci is aiming at, but I would be glad if there was some ways to make it simpler to grasp for "casual CI users"...

Here are a few ideas for simplification :

Currently I see a lot of variable that can be set in travis file, but it might sometimes be better to just have a line int he travis file that a user can just comment out if he doesnt need it... it would make things more explicit : one would exactly know what is happening by looking at the travis file, without needing to dig into the shell scripts...

Thats for my 2 cents, feel free to ask any question you have about the simple CI setup I had before moving to industrial_ci.

mathias-luedtke commented 7 years ago

Thanks for your input :)

Then the build script can be run locally as well to find out if something breaks or not (separately from docker). It only need the distro input and the "flow" (devel or install)

The problem with local tests is that they won't fail in the developer's environment for most non-trivial errors. With run_ci (and docker!) you can run all of our tests locally in a clean (!) environment.

This is what I found out is the optimum, only necessary steps are run, and it s still quite fast, faster than the ros buildfarm actually.

Our scripts basically do the same things, but run additional checks and providing more configuration options that have been requested by users. With addressing these corner cases we can increase our user base and harden our scripts.

I am not sure which are exactly the scenarios that industrial_ci is aiming at

For robots like Care-O-bot packagse are spread over different repositories, some of them are released, some of them are not (yet) for some or all distros. Other projects are just private or even hosted on-premise, but this should be supported as well. For simple packages/repositories only the ROS distro name has to be specified.

Provide a list of travis.yml templates following most requested workflows. The user is supposed to just duplicate this and modify it in its package repository (comment out things he doesnt want, etc.).

full ack!

One nice and clean example could be this one. In addition it would be great to have a commented version that outlines the real-world use cases for certain options.

It makes it quite difficult to understand what is happening, and I think a build framework should be as simple as possible so users have no trouble understanding what is actually happening.

The documentation needs improvements for sure, it is a little bit outdated :-/

Provide a list of shell scripts that the user can decide to call or not from his travis. I dont see any problem in having nested script functionality as long as they are basic functionality working out of the box on a linux + ros system, since the user can test them locally, but the script should make it simple enough to understand and we should keep it minimal and rely on existing tools.)

As i said before, local tests are easy with run_ci, but this feature is not highlighted much in the documentation.

one would exactly know what is happening by looking at the travis file, without needing to dig into the shell scripts...

An additional remark: We don't target travis exclusively. The script should work with other services like Gitlab CI or Jenkins as well.

asmodehn commented 7 years ago

It looks like I ll need to try out run_ci... So far I was setting up my workspace and running catkin tools manually.

Docker, although definitely a good habit, is, I think, sometimes not necessary (I am already working in different VBox for different distros & usecases) and can make things more complicated than needed for newcomers (how to configure docker network, where do the log files go, how to debug a test running in container, etc.).

When I ll get to a travis template I like, I ll make a PR to add it.

asmodehn commented 7 years ago

After trying out the ci.sh script I must say I would rather have a script with a --help documenting all possible options, rather than relying on some environment variables (which meaning is hidden in some other file in another folder).

While travis encourages using environment variable in its yaml, and it makes things clear in such a setting, I think it is a bad habit for shell scripts and any other executables on a computer (one can never tell what are the option a program was run with). command line options are much better.

Of course a env variable can be used when calling an executable, but too few people actually realise that env variables are actually accessible from your program because your process is a child of another "bash" process, which happens to make these variables available when executing your code. Much less obvious than explicit command line arguments.

mathias-luedtke commented 7 years ago

All scripts accept the config as arguments as well, e.g run_ci ROS_DISTRO=indigo. This tools is meant for testing the lines from the travis.yml (or other CI config).