[MAJOR] Harden template ownership and development workflows - #29
Conversation
- Keep wrapper maintenance explicit and assemble Python packages entirely in build-owned trees. - Namespace nested feature options, deliver portable TensorRT discovery, and stage authoritative version and source-package metadata. - Preserve host ownership for Docker and Podman while adding VS Code and MATLAB container support plus isolated MATLAB library tooling. - Cover cleanup, packaging, containers, nested options, TensorRT, ROS facades, and late CPack builds with focused regressions and documentation.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 77106e61c9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if [[ "$wrap_update" == true ]]; then | ||
| cmake_args+=( "-DGTWRAP_BRANCH=$wrap_branch" -DGTWRAP_SYNC_TO_MASTER=ON ) | ||
| else | ||
| cmake_args+=( -DGTWRAP_SYNC_TO_MASTER=OFF ) | ||
| cmake_args+=( | ||
| "-DGTWRAP_BRANCH=$wrap_branch" | ||
| -DGTWRAP_MAINTENANCE_UPDATE=ON | ||
| -DGTWRAP_SYNC_TO_MASTER=ON |
There was a problem hiding this comment.
When a build directory has previously been configured with --wrap-update, a subsequent ordinary invocation or explicit --no-wrap-update appends neither cache variable as OFF, so the cached GTWRAP_MAINTENANCE_UPDATE=ON and GTWRAP_SYNC_TO_MASTER=ON remain active and CMake fetches/pulls the wrapper checkout again. This makes the advertised opt-in operation persist across later builds and can unexpectedly move a wrapper gitlink; explicitly disable or unset both values when wrap_update is false.
AGENTS.md reference: AGENTS.md:L55-L57
Useful? React with 👍 / 👎.
| [project] | ||
| name = "@PROJECT_NAME@" | ||
| version = "@PROJECT_VERSION@" | ||
| version = "@FULL_VERSION@" |
There was a problem hiding this comment.
When configuring from a supported SemVer tag with a prerelease identifier that is not recognized by PEP 440, such as v1.0.0-feature.x, HandleGitVersion.cmake produces FULL_VERSION=1.0.0-feature.x; placing it directly in pyproject.toml causes setuptools/packaging to reject the project version, so wheel and pip installation fail. Translate the resolved version to PEP 440 or validate it before generating Python metadata rather than assuming every accepted SemVer prerelease can be normalized.
Useful? React with 👍 / 👎.
- Derive PEP 440 package metadata from structured SemVer fields without changing release or CPack versions. - Preserve arbitrary prerelease labels through development-release local metadata in checked-in and fallback templates. - Cover canonical labels and real wheel metadata while documenting the version boundary.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41d341ab94
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| set(TENSORRT_ROOT "" CACHE PATH | ||
| "Compatibility TensorRT root directory hint.") |
There was a problem hiding this comment.
When a consumer using the supported CMake 3.15-era policy behavior supplies the documented compatibility hint as a normal variable (set(TENSORRT_ROOT "/sdk")), this cache assignment removes that normal binding and replaces it with an empty value before _TensorRT_root_hints is built, so discovery fails unless the hint was passed with -D or via the environment. CMake's CMP0126 documentation explicitly states that its OLD behavior removes a same-named normal variable when no cache entry existed; avoid initializing this cache entry in a way that discards the caller's value.
Useful? React with 👍 / 👎.
| while IFS= read -r line_; do | ||
| if [[ "${line_}" == *"${lookup_name_} ("* && | ||
| "${line_}" == *'x86-64'* && "${line_}" == *'=> '* ]]; then |
There was a problem hiding this comment.
When --opencv runs against a normal Linux OpenCV installation, discover_opencv_ passes an unversioned name such as libopencv_core.so, but ldconfig -p (whose help describes -p as printing the cache) ordinarily keys OpenCV entries by their versioned SONAME, such as libopencv_core.so.410. This exact "${lookup_name_} (" match therefore finds nothing and the helper exits with “Host library not found”; the test currently hides the failure by fabricating unrealistic unversioned OpenCV cache keys.
Useful? React with 👍 / 👎.
1 participant
Summary
Harden ownership boundaries across configuration, wrapper generation, source packaging, and container workflows. Wrapper configuration is non-mutating by default, Python packages are assembled entirely in build-owned trees, and CPack refreshes source-tree ownership at package time. The same batch adds portable TensorRT discovery and host-owned Docker/Podman development flows.
This is the main-branch hardening prerequisite for the v2 test-ownership migration; it does not move template conformance into TestField itself.
Main Changes
VERSIONin CPack output, refresh cache-proven build-tree exclusions when packaging begins, and deliver an optional portableFindTensorRT.cmaketo build-tree and installed consumers.Testing / Validation
feature.xwheel carried1.0.0.dev0+feature.x.5.gabc1234metadata.__init__.py,libtemplate_project.so, andtemplate_project.so; wheel and CMake-prefix installs imported with an emptyLD_LIBRARY_PATH, and native RUNPATHs remained loader-relative.CI_USE_SELF_HOSTEDvariable; local validation used CUDA 12.9,sm_120, and OptiX 9.0.Notes For Reviewers
cmake/HandleWrapper.cmakeis smaller and now owns only common resolution and explicitly granted maintenance; ordinary configure/build operations do not move the wrapper checkout or change gitlinks.cmake/HandlePythonWrapper.cmakereconstructs a disposable package from stable source inputs._wrapper_build.py, caches, and stale native artifacts are excluded from wheels and CMake installs.cmake/RefreshCPackSourceIgnores.cmake.inintentionally excludes only the active binary tree and caches owned by this exact checkout. Foreign caches and legitimate build-prefixed source directories remain package input.FindTensorRT.cmakeis delivered as an optional consumer module and does not enable TensorRT in the base template.scripts/use_system_matlab_libraries.shand its standalone test are not wired into CMake, CTest, wrappers, containers, ROS, tailoring, or TestField. No real--applyor--restoreoperation was performed.