Skip to content

Fix install paths - #348

Open
christophfroehlich wants to merge 5 commits into
mainfrom
install-path
Open

christophfroehlich wants to merge 5 commits into
mainfrom
install-path

Conversation

@christophfroehlich

Copy link
Copy Markdown
Collaborator

This was completely coded by GPT-5.3-Codex and needs proper review and testing.

Summary

Fix Python generated-parameter module installation/import behavior across all colcon layouts (standard, isolated, and merge-install), resolving intermittent prerelease import failures.

Problem

Tests could fail with:

  • ModuleNotFoundError: No module named 'generate_parameter_module_example.admittance_parameters'
  • ModuleNotFoundError: No module named 'generate_parameter_module_example.custom_validation'

Root causes were path/layout mismatches in generation output handling and empty package __init__.py files in install space.

Changes

Validation

All scenarios pass for generate_parameter_module_example:

  • Standard layout: 48 passed, 0 failed
  • Isolated layout (--build-base/--install-base): 48 passed, 0 failed
  • Merge-install layout (--merge-install): 48 passed, 0 failed

Generated module is now written to the correct install location in each layout.

@christophfroehlich christophfroehlich added the check-prerelease Runs the pre-release workflow without downstream dependencies label Apr 17, 2026

@saikishor saikishor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understood, LGTM

@tylerjw

tylerjw commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator

Looks like this needs to be rebased. What more do you want to do here before we land this?

@christophfroehlich

Copy link
Copy Markdown
Collaborator Author

There were several related issues and I hoped that any of the the reporters would review it ;)
@Benjamin-Tan @sea-bass @imcelroy @xguay @MarqRazz @emulate-jonathanmanning @Flova

@christophfroehlich
christophfroehlich marked this pull request as ready for review October 6, 2026 20:37
@tylerjw

tylerjw commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

It looks like this breaks Python packages. The new source-directory skip leaves the generated module outside the package's import path. The build succeeds but then the python import fails.

@MarqRazz

MarqRazz commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Thanks for pulling these issues together @christophfroehlich! I tested this on Lyrical and can confirm @tylerjw's report: with --symlink-install, colcon puts build/<pkg> (a symlink to src) ahead of site-packages, so Python imports the source package, and this PR no longer writes the generated module there. The example only passes because it has no source __init__.py.

I took a different approach in #393 to generate from setuptools' build_py/develop commands instead of parsing sys.argv. It passes regular, symlink, merge-install, overlay, and custom --install-base builds plus pip install -e, and fixes #192, #255 and #257. Since it replaces the approach here rather than building on it, I opened it separately. Happy to adjust however you and @tylerjw prefer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

check-prerelease Runs the pre-release workflow without downstream dependencies

Projects

None yet

Development

Successfully merging this pull request may close these issues.

python setup_helper ignores --install-base flag ModuleNotFoundError python unit test Store codegen result for python code in build folder

4 participants