Alternative setup without submodules

[mathias-luedtke]

git submodules are a little bit cumbersome to use, especially to keep track with update.

For an easier maintenance it should be sufficient to have a client script like this:

install:
  - git clone https://github.com/ros-industrial/industrial_ci.git .ci_config
script:
  - source .ci_config/travis.sh
  # - source ./travis.sh

I will try both variants with ros_canopen.

[mathias-luedtke]

Explicit cloning seems to work: https://travis-ci.org/ros-industrial/ros_canopen/jobs/95636320

However, roslaunch-checks fail because of problems with catkin_tools and symlinks.
I had the same issues before, so I decided to use catkin_make.

[130s]

Yeah, we've already expressed a concern toward git submodule, and while I was experimenting I found it tolerable but indeed cumbersome at the same time.

install:
  - git clone https://github.com/ros-industrial/industrial_ci.git .ci_config
script:
  - source .ci_config/travis.sh
  # - source ./travis.sh

Interesting idea!

I'm almost ok for this. I just want a reassurance about the following; git cloning always checks out the HEAD, which means from industrial_ci side we cannot control its version to be used in client repos. As you can tell this would make debugging harder. But I'm not foreseeing how often industrial_ci will be updated either, so it may not be a big issue. I like to hear more thoughts.

However, roslaunch-checks fail because of problems with catkin_tools and symlinks.
I had the same issues before, so I decided to use catkin_make.

Without knowing catkin_tools and symlinks issue, let me ask if industrial_ci provides an option to choose either catkin_tools or catkin_make, do you think your issue will be gone? If so, I think that option is worth adding.

[mathias-luedtke]

I'm almost ok for this. I just want a reassurance about the following; git cloning always checks out the HEAD, which means from industrial_ci side we cannot control its version to be used in client repos.

You can clone a specific branch or tag with:

git clone https://github.com/ros-industrial/industrial_ci.git -b 0.1.0

if industrial_ci provides an option to choose either catkin_tools or catkin_make, do you think your issue will be gone?

It would be gone. Or if we copy the source instead of linking it.
But adding support for catkin_tools and catkin_make will bloat the code.
We should stick to one. Is there a particular reason for choosing catkin_tools in automated tests?

Without knowing catkin_tools and symlinks issue,

If I find some time I will prepare a test case and try to investigate which part of the toolchain needs to be fixed to get it working with catkin_tools as well.

[gavanderhoorn]

catkin_tools will conveniently expose problems with CMakeLists.txt where developers haven't been too careful with target names, specifying dependencies etc, as it defaults to completely isolated builds. This contrary to catkin_make, which aggregates all build scripts into one single CMake scope.

In my experience, if something builds with catkin_tools, it will build with catkin_make. The otherway around is not necessarily true.

[130s]

For "version control" of the CI config used

You can clone a specific branch or tag with:
git clone https://github.com/ros-industrial/industrial_ci.git -b 0.1.0

That's nice.
I unfortunately still have following concerns:

• With git clone -b VERSION maintainers at downstream repos need to manually update their repos with specific versions.
  • To update the version, maintainers need to manually edit the related file and open PR, which is a bit more complicated than updating by git submodule.
• With this, we still don't have a version control from server (industrial_ci) side.

I come to think that we don't need to stick to controlling the server's version now (sorry it was me who brought it up). I spread my thoughts here:

• Background of the issue: If CI failed on client side, root cause is either on the client side (code in each repo) or sometimes server's side (in CI config).
• Real issue: Harder to tell which side the root cause is in, if there's no way to tell the version of CI config.
• Resolution: Client maintainers can simply restart CI to test with the latest. They shouldn't need to care about the version of CI config.

So having git clone -b as an option is actually good, in case we need to debug with the specific version of CI conf.

That said, +1 for changing from git submodule to git clone.

[130s]

For catkin_tools or not?

@gavanderhoorn's summary tells more than I could do.

In my experience, if something builds with catkin_tools, it will build with catkin_make. The otherway around is not necessarily true.

This means that using catkin_tools, once the client repos employ industrial_ci, some may get exposed to new errors that they might have not seen with catkin_make. I think it's good ultimately because hidden errors may get fixed as long as patches are made. But is it something that we can force the maintainers to go through?

[gavanderhoorn]

@130s wrote:

[..] I think it's good ultimately because hidden errors may get fixed as long as patches are made. But is it something that we can force the maintainers to go through?

Yes, +1 from me. It may be inconvenient, but in that case the build scripts were faulty to begin with then.

And this is not just me being pedantic: if you've not properly setup your CMakeLists.txt, some builds will fail. Maybe not on your own machine, but on someone else's (due to target collisions, missing dependencies, anything), and / or on the ROS build farm. I'd rather have Travis alert me to those things, than having to re-release a bunch of times to get things right.

[mathias-luedtke]

@gavanderhoorn: catkin_make_isolated should fail in these cases as well, shouldn't it?
I don't want to fight for or against catkin_tools.
I just wanted to state that there is an issue with catkin_tools and roslaunch-check if the folder is symlinked.

[gavanderhoorn]

Yes, it should. catkin_make_isolated is orders of magnitude slower though. Takes the continuous out of CI a bit if you have to wait too long for the results :).

[shaun-edwards]

Have we committed to this change? With the acceptance of PR ros-industrial/industrial_core#128, I think we have. Should we update the README to reflect this new approach? I assume the old approach is still valid but discouraged?

[130s]

Should we update the README to reflect this new approach? I assume the old approach is still valid but discouraged?

README update is suggested #9.
I still left git submodule in the doc as an option instead of "deprecated", for when someone wants to specify the version of industrial_ci.

Please reopen if any topic still needs discussed in this ticket.