Skip to content

Conversation

@tupek2
Copy link
Collaborator

@tupek2 tupek2 commented Dec 16, 2025

this enables smith to be built by other projects containing smith as a submodule

@tupek2 tupek2 requested a review from chapman39 December 16, 2025 20:18
Copy link
Collaborator

@chapman39 chapman39 left a comment

Choose a reason for hiding this comment

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

this looks good to me

@tupek2 tupek2 requested a review from btalamini December 16, 2025 21:58
Comment on lines 570 to 572
set(GRETL_SOURCE_DIR "${PROJECT_SOURCE_DIR}/gretl" CACHE PATH "")

if (NOT EXISTS "${GRETL_SOURCE_DIR}/CMakeLists.txt")
message(FATAL_ERROR
"The gretl repo is not present. "
"Either run the following command in your git repository: \n"
" git submodule update --init --recursive\n"
"Or add -DGRETL_SOURCE_DIR=/path/to/gretl to your CMake command." )
set(GRETL_SOURCE_DIR "${PROJECT_SOURCE_DIR}/smith/gretl" CACHE PATH "" FORCE)
Copy link
Member

Choose a reason for hiding this comment

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

I think something like this would be cleaner:

if(NOT DEFINED GRETL_SOURCE_DIR)
    set(GRETL_SOURCE_DIR "${PROJECT_SOURCE_DIR}/gretl" CACHE PATH "")
endif()

// then error if ${GRETL_SOURCE_DIR}/CMakeLists.txt doesnt exist

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think I cleaned it up a bit in the latest commit, but I still needed to try looking in /smith/gretl in some cases

@tupek2 tupek2 requested a review from lihanyu97 December 16, 2025 22:00
@tupek2 tupek2 requested a review from white238 December 17, 2025 23:38
@tupek2 tupek2 force-pushed the tupek/gretl_cmake branch 2 times, most recently from 99a270d to a6791cf Compare December 18, 2025 17:30
@tupek2 tupek2 closed this Dec 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants