Add machine_install option#57
Conversation
There is not way to install to a machine as build artifacts stayed in build folder, add such an option while also updating CMakeLists.txt to install to correct paths.
There was a problem hiding this comment.
Pull Request Overview
Adds a new --machine_install option to the build script to install artifacts to system-default locations and updates all sample and library CMake targets to use the standard CMAKE_INSTALL_BINDIR and CMAKE_INSTALL_LIBDIR variables.
- Swapped hardcoded
bin/libdestinations in manyinstall()calls for${CMAKE_INSTALL_BINDIR}and${CMAKE_INSTALL_LIBDIR}. - Introduced
--machine_installin Build.py to skip custom install prefixes and rely on CMake’s defaults. - Bumped
cmake_minimum_requiredto 3.0.0 and changed several feature flags from ON to OFF by default.
Reviewed Changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| sample/**/CMakeLists.txt | Updated install destinations to use ${CMAKE_INSTALL_BINDIR}/${CMAKE_INSTALL_LIBDIR} |
| CMakeLists.txt | Increased minimum CMake version, added include changes, and toggled default options |
| Build.py | Added --machine_install option and conditional prefix logic |
Comments suppressed due to low confidence (3)
Build.py:168
- Add automated tests or CI checks to cover the
--machine_installcode path, verifying thatCMAKE_INSTALL_PREFIXis skipped and defaults to system paths when the flag is used.
if not options.machine_install:
CMakeLists.txt:21
- Consider adding
include(GNUInstallDirs)immediately after thecmake_minimum_requiredcall to ensureCMAKE_INSTALL_BINDIRandCMAKE_INSTALL_LIBDIRare defined before use.
cmake_minimum_required(VERSION 3.0.0)
libraries/debug/CMakeLists.txt:50
- [nitpick] Indentation here uses a mix of spaces/tabs. Align this line with the other
install()arguments for consistent formatting.
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
| parser.add_option("--c_flags", dest="c_flags", help="Set C Compiler Flags -DCMAKE_C_FLAGS=" " [Default empty]", default='') | ||
| parser.add_option("--cpp_flags", dest="cpp_flags", help="Set CPP Compiler Flags -DCMAKE_CXX_FLAGS=" " [Default empty]", default='') | ||
| # machine install to /usr/local/.... paths | ||
| parser.add_option("--machine_install", dest="machine_install", help="Install to machine to preferred destination path", default=False, action='store_true') |
There was a problem hiding this comment.
Update the script’s help text or README to document the new --machine_install flag so users know how to invoke system-default installs.
| @@ -52,19 +52,19 @@ set(CMAKE_CONFIGURATION_TYPES | |||
| set(CMAKE_CONFIGURATION_TYPES ${CMAKE_CONFIGURATION_TYPES} CACHE STRING "Available build configurations." FORCE) | |||
|
|
|||
| # Default options for the CMake GUI | |||
There was a problem hiding this comment.
Changing several feature flags from ON to OFF by default is a breaking change; document these new defaults in the project README or release notes to inform users of the updated behavior.
| # Default options for the CMake GUI | |
| # Default options for the CMake GUI | |
| # NOTE: If you change the default values of these feature flags, ensure that the changes are documented | |
| # in the project's README or release notes to inform users of the updated behavior. |
There is not way to install to a machine as build artifacts stayed in build folder, add such an option while also updating CMakeLists.txt to install to correct paths.