Warn user that they are wrong if they colcon build after source install/setup

[emersonknapp]

It seems that many users will invoke colcon build in a shell after sourcing the setup files.

The thought process is "I am building - now I want to test run one of the binaries - now I want to rebuild".

However, this workflow leads to slow (and possibly incorrect?) results.

We could perhaps set a canary variable in the setup.bash file, and check for it in colcon build, then at least warn the user that it's not advised.

[dirk-thomas]

It seems that many users will invoke colcon build in a shell after sourcing the setup files.
The thought process is "I am building - now I want to test run one of the binaries - now I want to rebuild".

Unfortunately yes.

However, this workflow leads to slow (and possibly incorrect?) results.

I do understand the "possibly incorrect" part (since other packages are available to a package without depending on them). Can you elaborate how the second build is slower - and quantify what level of slower you are experiencing with what kind of workspace?

We could perhaps set a canary variable in the setup.bash file, and check for it in colcon build,

If the COLCON_PREFIX_PATH contains the current install prefix that could be a way to detect that case already.

[emersonknapp]

In Linux builds, I haven't measured it being slower. The main point I'm trying to make is that both you and @mjcarroll have told me now that this workflow should not be done. But, it's not documented anywhere that I know of and is a subtle detail, so we should have colcon warn the user that they're doing something incorrectly.

If COLCON_PREFIX_PATH is a good indicator, then we can use that to print the warning.

[dirk-thomas]

we should have colcon warn the user that they're doing something incorrectly.

Well, "incorrectly" is a strong word 😉 There are probably numerous cases where it won't harm you at all.

On the other hand there are also various other cases not related to sourcing the install space which would still result in possibly incorrect results:

• If you build once, set / change / unset any environment variable, and then build again.
• If you build once, then alter anything which affects information persisted in the CMakeCache.txt, and then build again.

Anyway, we can try adding the warning and see if it helps. It would be great if you could create a pull request for this.

[dirk-thomas]

It would be great if you could create a pull request for this.

@emersonknapp Are you still interested in adding this warning?

[emersonknapp]

I am interested - just haven't found the time to get up to speed on the relevant code. I can try to carve out a bit of time this week or next

[jonfink]

This is a bit of an aside but I think pertinent to this issue...

Unfortunate/incorrect/or otherwise, the series of events described above (sourcing an install/setup file interleaved with building) happen a lot and get quite nasty when working on multiple projects with different workspaces. Removing this (sometimes hidden) statefulness was one of the strengths of catkin-tools and I’ve been struggling to figure out how it could be integrated into colcon as we start to migrate.

@dirk-thomas Are there any plans to support storing this kind of meta-data in the workspace? Or thoughts on how this would fit into the extension system of colcon?

[dirk-thomas]

Removing this (sometimes hidden) statefulness was one of the strengths of catkin-tools and I’ve been struggling to figure out how it could be integrated into colcon as we start to migrate.

This seems to be an orthogonal topic to the one described in this ticket. Please consider to fill a separate ticket for this

Are there any plans to support storing this kind of meta-data in the workspace?

Please be more precise what exact meta data you are referring to. The parent workspaces are known when building in a workspace based on the current environment atm.

Or thoughts on how this would fit into the extension system of colcon?

When creating a separate ticket for this please include more information what exact behavior you are envisioning (not just referring to "what catkin_tools does"). I simply don't have enough context to give you any feedback on this without.

In general "rolling back" the environment changes a setup file causes are imo technically almost impossible. The setup files generated by catkin try to provide something like that but that logic has severe limitations so I don't think colcon should use a similar approach. The safest approach might be to just open a new (sub-)shell when ever you source a workspace - that allows you to simply close the (sub-)shell to get back to the original environment state. I think colcon-spawn-shell was aiming to provide a convenient way to do that.

[jonfink]

Removing this (sometimes hidden) statefulness was one of the strengths of catkin-tools and I’ve been struggling to figure out how it could be integrated into colcon as we start to migrate.

This seems to be an orthogonal topic to the one described in this ticket. Please consider to fill a separate ticket for this

I actually think this ticket is a symptom of statefulness in the build process that limits reproducibility. However, happy to open a new ticket.

Are there any plans to support storing this kind of meta-data in the workspace?

Please be more precise what exact meta data you are referring to. The parent workspaces are known when building in a workspace based on the current environment atm.

Or thoughts on how this would fit into the extension system of colcon?

When creating a separate ticket for this please include more information what exact behavior you are envisioning (not just referring to "what catkin_tools does"). I simply don't have enough context to give you any feedback on this without.

Sorry, more precisely I mean the profile concept of catkin_tools: collecting the workspace you want to extend, extra build arguments, extra cmake arguments, install destination, etc and storing in the .catkin_tools/profiles folder. Those files are then used to setup a clean environment (i.e., not polluted by whatever else a user might be doing in their shell) when executing builds.

For my team, that was a major breakthrough in terms of usability and working with many different workspaces so I'm interested in figuring out how to implement a similar concept as we migrate to ROS2 and need to use colcon.

In general "rolling back" the environment changes a setup file causes are imo technically almost impossible. The setup files generated by catkin try to provide something like that but that logic has severe limitations so I don't think colcon should use a similar approach. The safest approach might be to just open a new (sub-)shell when ever you source a workspace - that allows you to simply close the (sub-)shell to get back to the original environment state. I think colcon-spawn-shell was aiming to provide a convenient way to do that.

[dirk-thomas]

I actually think this ticket is a symptom of statefulness in the build process that limits reproducibility.

@jonfink I don't understand your comment. Can you please elaborate what you mean with this.

The build itself doesn't affect the environment at all. The only part which changes the environment is sourcing the setup file which sets up variables necessary to run / use the artifacts of the packages after they have been built and installed. While those variables are required to use the artifacts (e.g. extending PATH, PYTHONPATH, etc.) ultimately if you rebuild the workspace these changes can (not necessarily will) alter the build result. I don't think this fact is anything specific to the process used in ROS.

I mean the profile concept of catkin_tools: collecting the workspace you want to extend, extra build arguments, extra cmake arguments, install destination, etc and storing in the .catkin_tools/profiles folder.

All of these configuration would be covered by the concept of a profile as described in #168 which would indeed be very useful to have.

Those files are then used to setup a clean environment (i.e., not polluted by whatever else a user might be doing in their shell) when executing builds.

Maybe I am not aware of some functionality in catkin_tools but as far as I know it simply relies on the --rollback feature of the setup files generated by catkin. And if that is the only thing being done then you might miss some limitations of that approach. It won't magically restore a clean environment. If a previous sourced environment defined e.g. a completion function for bash that will not be reset. See https://github.com/ros/catkin/blob/15df8d27136976aa2240eae9aa232728887aebb8/cmake/templates/_setup_util.py.in#L68-L73 for the logic undoing environment variable changes catkin is aware of - which is only a subset. So a rebuild after sourcing a workspace in ROS 1 also has the risk of altering the result.

In ROS 2 we simply choose not to offer an incomplete rollback feature since we don't see any technical way to achieve a correct / complete rollback (beside of course using a subshell and closing that as mentioned above).