Avoid deb_system and --install-layout deb

[sloretz]

Fixes #506

This PR avoids --install-layout deb and deb_system. I suspect these were never used before #504, and they have at least one bug that makes them unsuitable for colcon.

More info here: #506 (comment)

[nuclearsandwich]

I haven't onboarded the history of these changes so I don't want to muddy the waters with an ill-informed review but I did have a comment about comment wording.

[codecov]

Codecov Report

Merging #507 (8d6158d) into master (b1e064a) will decrease coverage by 0.08%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #507      +/-   ##
==========================================
- Coverage   81.79%   81.70%   -0.09%     
==========================================
  Files          60       60              
  Lines        3553     3564      +11     
  Branches      680      684       +4     
==========================================
+ Hits         2906     2912       +6     
- Misses        600      601       +1     
- Partials       47       51       +4     

Impacted Files	Coverage Δ
colcon_core/python_install_path.py	75.00% <0.00%> (ø)
colcon_core/task/python/build.py	36.02% <ø> (+0.19%)	⬆️
colcon_core/subprocess.py	64.53% <0.00%> (-1.36%)	⬇️
colcon_core/pytest/hooks.py	0.00% <0.00%> (ø)
colcon_core/task/__init__.py	100.00% <0.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b1e064a...8d6158d. Read the comment docs.

[cottsay]

Why was the --install-layout deb option there to begin with?

[sloretz]

Why was the --install-layout deb option there to begin with?

I don't know. It was added in the 39th commit to this repo 35a6ecc , which is before the first PR. Based on the commits to that point being quick and only seeming to add code, I'd guess Dirk was splitting an already working prototype into smaller pieces.

It might have been copied from catkin. There's reference to --install-layout deb in ros/catkin#50. The first commit I can find using setup.py install [...] --install-layout deb is this one: ros/catkin@bd27e5a. The very first commit using --install-layout deb is passing it to dh_auto_install: ros/catkin@a581aa3. It doesn't seem to come from rosbuild because the first commit with --install-layout deb references catkin: ros/ros@4ba1cdf

None of those commits have helped me understand why it's being used.

[cottsay]

Will switching to posix_prefix be sufficient to work around the current bug, and leaving the --install-layout deb part until we understand it better?

[sloretz]

Will switching to posix_prefix be sufficient to work around the current bug, and leaving the --install-layout deb part until we understand it better?

It will fix the bug, but the _append_install_layout code will never append --install-layout deb because posix_prefix won't return a path with dist-packages in it (hmm, maybe it would if the path to the install space contains the text dist-packages).

[cottsay]

I'm wondering if the original use case might have been to support building within a debian build, where the special environment variable is set and deb_system is implicitly chosen as the default. I think that we may have already broken that use case.

I understand that we're not currently invoking colcon in debian builds, but if was working before, maybe we shouldn't break it.

[sloretz]

I understand that we're not currently invoking colcon in debian builds, but if was working before, maybe we shouldn't break it.

I'm not sure how to test that. At the very least, building in a Debian build probably wouldn't need a symlink-install option, so we could at least remove the _append_install_layout that adds the option to the wrong verb from there.

[sloretz]

I understand that we're not currently invoking colcon in debian builds, but if was working before, maybe we shouldn't break it.

8d6158d adds it back for the non symlink install case.