Add machine_install option #57
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
| ) | ||
| 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.
Suggested change
| # 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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.