Skip to content

[Draft] Updating Coinor.cmake to update deprecated method exec_program #148

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[Draft] Updating Coinor.cmake to update deprecated method exec_program #148

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2fb6af9
Update coinor.cmake
Avni2000 Apr 11, 2025
ec27b40
Update coinor.cmake
Avni2000 Apr 11, 2025
f71a58f
Update coinor.cmake - both done
Avni2000 Apr 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 27 additions & 23 deletions libraries.cmake/coinor.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,24 +146,26 @@ MACRO( OPENMS_CONTRIB_BUILD_COINOR)
endif()

message( STATUS "Configure COIN-OR library (./configure -C --prefix=${PROJECT_BINARY_DIR} ${STATIC_BUILD} ${SHARED_BUILD} --with-lapack=no --with-blas=no ${COINOR_EXTRA_FLAGS} CXX=${CMAKE_CXX_COMPILER} CC=${CMAKE_C_COMPILER})")
exec_program("./configure" "${COINOR_DIR}"
ARGS
-C
--prefix=${PROJECT_BINARY_DIR}
## Following two lines can be combined with prefix
## But maybe they avoid building the doc into share (wanted?)
#--libdir=${CONTRIB_BIN_LIB_DIR}
#--includedir=${CONTRIB_BIN_INCLUDE_DIR}
${STATIC_BUILD}
${SHARED_BUILD}
--with-lapack=no
--with-blas=no
${COINOR_EXTRA_FLAGS}
CXX=${CMAKE_CXX_COMPILER}
CC=${CMAKE_C_COMPILER}
#exec_program -> execute_process
execute_process(
COMMAND
./configure
-C
--prefix=${PROJECT_BINARY_DIR}
${STATIC_BUILD}
${SHARED_BUILD}
--with-lapack=no
--with-blas=no
${COINOR_EXTRA_FLAGS_LIST}
CXX=${CMAKE_CXX_COMPILER}
CC=${CMAKE_C_COMPILER}
WORKING_DIRECTORY ${COINOR_DIR}
OUTPUT_VARIABLE COINOR_CONFIGURE_OUT
RETURN_VALUE COINOR_CONFIGURE_SUCCESS
)
ERROR_VARIABLE COINOR_CONFIGURE_ERR
RESULT_VARIABLE COINOR_CONFIGURE_SUCCESS
)

Comment on lines +149 to +167
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Good transition from exec_program to execute_process

The replacement of the deprecated exec_program with execute_process is a positive improvement. You've correctly structured the command with appropriate parameters including WORKING_DIRECTORY, OUTPUT_VARIABLE, ERROR_VARIABLE, and RESULT_VARIABLE.

However, I noticed a potential issue: on line 159, you're using ${COINOR_EXTRA_FLAGS_LIST}, but this variable is defined as COINOR_EXTRA_FLAGS on lines 134/136. This mismatch could prevent the extra flags from being applied correctly.

- ${COINOR_EXTRA_FLAGS_LIST}
+ ${COINOR_EXTRA_FLAGS}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#exec_program -> execute_process
execute_process(
COMMAND
./configure
-C
--prefix=${PROJECT_BINARY_DIR}
${STATIC_BUILD}
${SHARED_BUILD}
--with-lapack=no
--with-blas=no
${COINOR_EXTRA_FLAGS_LIST}
CXX=${CMAKE_CXX_COMPILER}
CC=${CMAKE_C_COMPILER}
WORKING_DIRECTORY ${COINOR_DIR}
OUTPUT_VARIABLE COINOR_CONFIGURE_OUT
RETURN_VALUE COINOR_CONFIGURE_SUCCESS
)
ERROR_VARIABLE COINOR_CONFIGURE_ERR
RESULT_VARIABLE COINOR_CONFIGURE_SUCCESS
)
#exec_program -> execute_process
execute_process(
COMMAND
./configure
-C
--prefix=${PROJECT_BINARY_DIR}
${STATIC_BUILD}
${SHARED_BUILD}
--with-lapack=no
--with-blas=no
${COINOR_EXTRA_FLAGS}
CXX=${CMAKE_CXX_COMPILER}
CC=${CMAKE_C_COMPILER}
WORKING_DIRECTORY ${COINOR_DIR}
OUTPUT_VARIABLE COINOR_CONFIGURE_OUT
ERROR_VARIABLE COINOR_CONFIGURE_ERR
RESULT_VARIABLE COINOR_CONFIGURE_SUCCESS
)



## logfile
file(APPEND ${LOGFILE} ${COINOR_CONFIGURE_OUT})
Expand All @@ -177,13 +179,15 @@ MACRO( OPENMS_CONTRIB_BUILD_COINOR)

## make install
message( STATUS "Building and installing COIN-OR library (make install).. ")
exec_program(${CMAKE_MAKE_PROGRAM} "${COINOR_DIR}"
ARGS
install
#-j ${NUMBER_OF_JOBS} # the project has problems with multiple jobs. It tries to create folders at the same time and fails.
-j 1
execute_process(
COMMAND
${CMAKE_MAKE_PROGRAM}
install
"-j1"
WORKING_DIRECTORY ${COINOR_DIR}
# Explicitly pass as one argument
OUTPUT_VARIABLE COINOR_MAKE_OUT
RETURN_VALUE COINOR_MAKE_SUCCESS
RESULT_VARIABLE COINOR_MAKE_SUCCESS
)

## logfile
Expand Down