port to Qt Creator 4.14

[christian-rauch]

This PR ports the plugin to the newest Qt Creator 4.14.

I ported from the 4.10 branch step-by-step. My forked repo https://github.com/christian-rauch/ros_qtc_plugin contains branches for 4.11, 4.12 and 4.13.

@Levi-Armstrong There are many API breaking changes altogether. I verified that the 4.11, 4.12 and 4.13 branches can do the basic task of creating a workspace and compiling colcon packages, but I am mainly going to use the newest (4.14) release. Do you alternatively want to merge the 4.xx branches step-by-step?

Fixes #406

[Levi-Armstrong]

Do you alternatively want to merge the 4.xx branches step-by-step?

I want to make sure I understand the question, are asking if we want to do release of say 4.12, 4.13 and 4.14? If so, I would be more incline to just do the latest 4.14.

Questions: Is all of the existing functionality ported to 4.14 or is there more work to be done here?

[christian-rauch]

Are asking if we want to do release of say 4.12, 4.13 and 4.14?

Not a release, just having my 4.1x branches merge into the upstream project one-by-one, thus creating corresponding 4.1x branches under ros-industrial/ros_qtc_plugin. Having "4.12" commits in the 4.14 branch does not make sense as it would not compile with the corresponding Qt Creator version.

For a final binary release, I agree that it makes sense to only release the newest version (4.14).

Is all of the existing functionality ported to 4.14 or is there more work to be done here?

I use the plugin to create and build colcon workspaces (for ROS1 and ROS2). I have not used any other features, such as the templates or starting nodes via roslaunch through Qt Creator. So I cannot definitely say that all features are working. Since there are many API changes in between Qt Creator versions, I might have broken some functionality that I don't use regularly.

I would appreciate if you could test the plugin and give some feedback if all originally implemented features work. Maybe @vincent-hui is keen of testing the new plugin?

[Levi-Armstrong]

Not a release, just having my 4.1x branches merge into the upstream project one-by-one, thus creating corresponding 4.1x branches under ros-industrial/ros_qtc_plugin. Having "4.12" commits in the 4.14 branch does not make sense as it would not compile with the corresponding Qt Creator version.

Sounds good. I can create a branch for each using 4.10 branch as the base then we can merge them into the respective branches. This work?

[christian-rauch]

Yes. You could create a 4.11 branch from 4.10, I send a "4.11" PR and you can review the subset of changes. After merging 4.11, create a 4.12 branch from 4.11, and I will send a new "4.12" PR. And so on with 4.13 and 4.14.

[v4hn]

Thank you for your work porting all this @christian-rauch !
Building your 4.14 branch over here I found another minor issue in that qt-creator added QT_NO_JAVA_STYLE_ITERATORS from 4.11. and the plugin uses at least QHashIterator.
removing the directive in the qt-creator build scripts works, but it's probably better to migrate and avoid these classes.

At first glance all the functionality I usually use with the plugin works in your branch.
I've been wondering for a long time though, shouldn't I be able to add new headers/source files in the project tree on the left?
The corresponding entry "Add new..." in the context menu is disabled.

[christian-rauch]

Building your 4.14 branch over here I found another minor issue in that qt-creator added QT_NO_JAVA_STYLE_ITERATORS from 4.11. and the plugin uses at least QHashIterator.

I did not get a build error or warning. Do you know if/when this becomes deprecated or removed?

The corresponding entry "Add new..." in the context menu is disabled.

I just tested this with the old 4.9.2 release and I saw the Add New... enabled. In the newer 4.14 built, this is indeed disabled (greyed out), but it seems that Ctrl+N does the same.

In a standard CMake project with the same Qt Creator version, the Add New... feature is also disabled. So this seems to be something related to newer Qt Creator versions. Maybe this only works with other build systems.

[v4hn]

I did not get a build error or warning.

I do not run on Ubuntu or Debian, so my system installation does not include any Ubuntu-specific modifications.
Looking at a Ubuntu 18.04 machine, I am somewhat confused because grep -r QHashIterator /usr/include does not yield any output and I'm wondering where the classes are defined.
To me this looks like Ubuntu removed them altogether, but then this could not compile at all...

The two offending places seem to be in ros_project.cpp and in ros_utils.cpp.

Do you know if/when this becomes deprecated or removed?

The upstream comment says "Qt is likely to deprecate them". I don't know what that means.

I filed a pull-request with your branch to migrate to STL-style iterators.

[christian-rauch]

To me this looks like Ubuntu removed them altogether, but then this could not compile at all...

You will not find the definition via grep It is defined via a macro: Q_DECLARE_MUTABLE_ASSOCIATIVE_ITERATOR(Hash). You have to use an IDE or similar to find this.

The upstream comment says "Qt is likely to deprecate them". I don't know what that means.

If they are going to be removed sooner or later it makes sense to adapt this now.

I filed a pull-request with your branch to migrate to STL-style iterators.

Thanks for the PR. Can you rebase it one the 4.11 PR is open?

@Levi-Armstrong Can you create a 4.11 branch from 4.10 so we can kick-start merging this? I will finally rebase this 4.14 branch on the upstream 4.13, once my 4.13 branch has been merged

[Levi-Armstrong]

4.11 has been created.

[christian-rauch]

@Levi-Armstrong I tested and pushed the 4.13 myself to upstream without a dedicated PR and rebased this PR (4.14) on top of it. This should be it. The basic functionality (create and build workspace, change build type, create new ROS package) works. I have not tested other functionality.

@v4hn Feel free to test this with the official binary distribution of Qt Creator. You can simply use the pre-built library I mentioned at #406 (comment). Let me know of everything that used to work in 4.9.2 but broke with 4.14.0.

[v4hn]

Feel free to test this with the official binary distribution of Qt Creator.
You can simply use the pre-built library I mentioned at #406 (comment).

I always build from source, but thanks nonetheless. I just tested your 4.14 branch.

Let me know of everything that used to work in 4.9.2 but broke with 4.14.0.

It seems to work for my workspace with some non-breaking issues.

Aside from the disabled "Add new" entry in the context menu of the folders, I just noticed two other minor issues. Because I haven't used the "fully functional" version of the plugin at all I don't know whether these are new issues though (or even whether they are reproducible on Ubuntu).

• The "Remove" and "Rename" context menu entries are also disabled, forcing users to manage/restructure their workspaces through shell/file manager instead.
• If I build my catkin_tools workspace (catkin_tools version 0.5.0 using python3, version 0.6.1 is known to be broken with python3) and abort mid-build - either via Ctrl+Backspace or by clicking the red square in the build dialog, the catkin build process terminates correctly, but the underlying make jobs still continues (new compiler jobs keep showing up in htop). I don't have this problem when I Ctrl+C the catkin build command from the command line.

Thanks for your involvement @christian-rauch !

[christian-rauch]

The "Remove" and "Rename" context menu entries are also disabled, forcing users to manage/restructure their workspaces through shell/file manager instead.

The file I/O API changed. The methods were removed in 5c1f79d.

TL;DR: Filesystem operations are deactivated in the "Project" view, but work in the "File System" view.

Using a plain CMake project, e.g. https://github.com/stevenlovegrove/Pangolin, I am not able to add new files or rename/remove them in the "Projects" view either. These options are greyed out the same way as you observed with the ROS plugin. But The "File System" view allows the typical filesystem operations. Showing them as deactivated probably means that some plugins support file operations directly in the "Project" view and it can be implemented manually.

The ROS plugin has the same behaviour in this regard. So you have to use the "File System" view inside Qt Creator for all filesystem operations.

the underlying make jobs still continues

I am observing this too in the "old" 4.9.2 version. But I agree it would be useful if the plugin behaves in the same way as a Ctrl+C (SIGINT).

Once "4.14" is merged it would be useful to have these two issues opened on GitHub to better track them.

I may look into these issues if they become too annoying to me :-)

[Levi-Armstrong]

The "Remove" and "Rename" context menu entries are also disabled, forcing users to manage/restructure their workspaces through shell/file manager instead.

This is annoying that they did that but we may be able to enable it. I am good with merging and track these in issues since it looks like you can still Rename and Remove just have to switch to file view.

[v4hn]

**TL;DR:** Filesystem operations are deactivated in the "Project" view, but work in the "File System" view.

I agree with Levi. This is simply a stupid interface. For most use-cases these two views are basically the same. Yes, we could even have logic in the project view to adapt CMakeLists.txt/package.xml files as needed, but that's not important.

> the underlying make jobs still continues I am observing this too in the "old" 4.9.2 version. But I agree it would be useful if the plugin behaves in the same way as a Ctrl+C (SIGINT).

Digging through some levels of code it boils down to `ROSCatkinToolsStep` not reimplementing `ProjectExplorer::BuildStep::doCancel`. The default implementation in `AbstractProcessStep::doCancel` calls a `Core::Reaper` routine that looks like a neat abstraction, but sends `SIGKILL`. Because [SIGKILL does not kill child processes on linux](https://stackoverflow.com/questions/28830103/qprocesskill-does-not-kill-children-in-linux), this effectively fails to stop the build. A much better solution would indeed send `SIGINT` instead to have the catkin process terminate itself *as well as* its children.

I may look into these issues if they become too annoying to me :-)

Thank you for looking into these problems at all!

[v4hn]

A much better solution would indeed send SIGINT instead to have the catkin process terminate itself as well as its children.

This is not as easy as it looks though if we don't want to replicate a lot of structure because AbstractProcessStep hides the process in a d-ptr. I created an upstream ticket to discuss this shortcoming for this case.

[christian-rauch]

@v4hn I added a commit to "implement" the file I/O.

It turns out that the filesystem operations themselves are already implemented, the build system just has to report back which actions are supported and potentially implement additional functionality on top. But since the "Project" view simply shows the workspace structure, it was just a matter of returning "true" for all these actions.

I created an upstream ticket to discuss this shortcoming for this case.

Thanks for looking into this. I think there is nothing we can do in the plugin to circumvent this.

If the file I/O works for you, I am going to merge this.

[v4hn]

I added a commit to "implement" the file I/O.

Works like a charm and it's roughly what I hoped would be needed. Thanks so much!

Thanks for looking into this. I think there is nothing we can do in the plugin to circumvent this.

Not without upstream API additions. However, everyone I talked to seems to agree it's mostly a problem with catkin_tools (and apparently colcon as well) because they fail to forward SIGTERM to there children.
Playing around with various interactions, both tools only work from the command line because the shell explicitly creates a process group for them and their children and forward SIGINT to all processes in that group.
The only question for upstream is whether qt-creator should behave the same way as a shell w.r.t. the build and thus render any unknown issues with this void in all other build systems as well.

I did not test catkin_make but would be surprised if things work there.

[christian-rauch]

Works like a charm and it's roughly what I hoped would be needed. Thanks so much!

Nice, then I am going to merge this PR.

The only question for upstream is whether qt-creator should behave the same way as a shell w.r.t. the build and thus render any unknown issues with this void in all other build systems as well.

My take on this is that a process should forward signals to its child processes and correctly manage their life cycles. The proper solution would therefore to fix this issue in catkin/colcon. There might be many more IDEs that expect the build system to behave in this way.
@v4hn Can you open an issue in the catkin/colcon repo to request such behaviour?

Of course, the convenient solution would be to just make things work and emulate the terminal behaviour in the IDE, even if this means working around bad process behaviour.