Skip to content
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

Jump to bottom

Fypp call update for windows compatability #742

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

Fypp call update for windows compatability #742

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9e4fff2
fypp call update
chuckyvt Oct 29, 2023
c2fab31
Help string
chuckyvt Oct 29, 2023
629850c
Update to include fypp
chuckyvt Dec 6, 2023
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
13 changes: 8 additions & 5 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,14 @@ if(NOT DEFINED CMAKE_MAXIMUM_RANK)
set(CMAKE_MAXIMUM_RANK 4 CACHE STRING "Maximum array rank for generated procedures")
endif()

# --- find preprocessor
find_program(FYPP fypp)
if(NOT FYPP)
message(FATAL_ERROR "Preprocessor fypp not found! Please install fypp following the instructions in https://fypp.readthedocs.io/en/stable/fypp.html#installing")
endif()
# Find python. Python 3.5 or higher required for ftypp
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Find python. Python 3.5 or higher required for ftypp
# Find python. Python 3.5 or higher required for fypp

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CMake wasn't find fypp. I had assumed (incorrectly) that the find_file function would find the fypp package if it had been installed. Doing some trials, its seem it will be difficult to reliably find fypp across the various installation options, without the user manually specifying the path via -DCMAKE_INCLUDE_PATH or similar.

Based on that, I think its worth considering including fypp in the stdlib package. Its a single py file, so relatively compact. and the fypp license allows it. The pull request has been updated to reflect this.

Understand if don't want to add, in that case I can close the pull request.

find_package(Python3 3.5 REQUIRED COMPONENTS Interpreter)

# Find fypp file
find_file(FYPP fypp.py REQUIRED HINTS ${CMAKE_CURRENT_SOURCE_DIR}/fypp )

# Cmd line string to run fypp
SET(FYPP ${Python3_EXECUTABLE} ${FYPP})

# Custom preprocessor flags
if(DEFINED CMAKE_MAXIMUM_RANK)
Expand Down
Loading
Loading