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

Revert "jobs: Fixing environment required to run catkin test targets on pre-indigo catkin" #600

Merged
merged 1 commit into from
May 22, 2020
Merged

Revert "jobs: Fixing environment required to run catkin test targets on pre-indigo catkin" #600

merged 1 commit into from
May 22, 2020

Conversation

meyerj
Copy link
Contributor

@meyerj meyerj commented Mar 6, 2020

This pull requests suggest to revert #265 ([PR196+] jobs: Fixing environment required to run catkin test targets on pre-indigo catkin), which is supposed to be obsolete. According to the description and inline comments it was only required for ROS Hydro and earlier.

If complete removal is too disruptive, I suggest to only install the env-hook in ROS Hydro and earlier (not sure what would be a reliable way to detect this, before any workspace is sourced).

Rationale:

  • The CATKIN_TEST_RESULTS_DIR is not used anymore by catkin since remove CATKIN_TEST_RESULTS_DIR environment variable ros/catkin#728. Not sure about ROS_TEST_RESULTS_DIR, I did not check.
  • The additional env-hook get installed unconditionally and "pollutes" the devel- and install-spaces, although normally it should not do any harm.
  • In some cases it does harm, e.g. when building Debians using catkin_tools. catkin itself has flags like CATKIN_BUILD_BINARY_PACKAGE that prevents installation of common files in a shared install-space. 06-ctr-nuke.sh did not respect this and required manual removal from the staged DESTDIR to avoid that the file is added to multiple binary packages (see also related issue DESTDIR broken on master #280).

…on pre-indigo catkin"

This reverts commit 050de44 and partially c5daf4a.
@mikepurvis
Copy link
Member

This looks reasonable; there's no reason to be supporting pre-Indigo distros at this point, heck the master branch doesn't even work with Python 2 any more. :)

@mikepurvis mikepurvis merged commit 5fc3a5d into catkin:master May 22, 2020
@meyerj meyerj deleted the revert-pre-0.4.0-executor-ctr-hack branch May 24, 2020 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants