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.

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

Fix CMake integration of NMODL generation. #1554

Merged
merged 2 commits into from
Nov 7, 2024
Merged

Fix CMake integration of NMODL generation. #1554

merged 2 commits into from
Nov 7, 2024

Conversation

1uc
Copy link
Collaborator

@1uc 1uc commented Nov 6, 2024

The code for the code-generator NMODL is itself generated from Jinja templates. This requires CMake integration to ensure it happens automatically.

The following has been changed about the integration:

  • The CMake code needs a list of the generated files. This list is stored in a CMake file and included. This file must exist before running code_generator.py to generate the NMODL code, because the command to generate the files needs to know which file it will be generating. Therefore, we split the script into two phases: one to generate the list of generated files, and another to generate the files.

  • The list of generated files depends on the build flags. Therefore, this file (code_generator.cmake) can't be checked into the Git repo. The code_generator.cmake is created during the configure phase, and now lives in the build directory.

  • CMake code was lifted from src/language/CMakeLists.txt to the top-level CMakeLists.txt to avoid any scoping issues for the lists of generated files.

  • Removed pattern of yielding tasks while creating a list tasks in favour of returning the list of tasks directly.

@1uc 1uc force-pushed the 1uc/fix-cmake-deps branch from f399b0c to d72500b Compare November 6, 2024 20:20
@1uc 1uc marked this pull request as ready for review November 6, 2024 20:37
@1uc
Copy link
Collaborator Author

1uc commented Nov 6, 2024

Second attempt at fixing the issue. The previous was a rather short-lived "solution".

The code for the code-generator NMODL is itself generated from Jinja
templates. This requires CMake integration to ensure it happens
automatically.

The following has been changed about the integration:

  * The CMake code needs a list of the generated files. This list is
    stored in a CMake file and included. This file must exist before
    running `code_generator.py` to generate the NMODL code, because the
    command to generate the files needs to know which file it will be
    generating. Therefore, we split the script into two phases: one to
    generate the list of generated files, and another to generate the
    files.

  * The list of generated files depends on the build flags. Therefore,
    this file (`code_generator.cmake`) can't be checked into the Git
    repo. The `code_generator.cmake` is created during the configure
    phase, and now lives in the build directory.

  * CMake code was lifted from `src/language/CMakeLists.txt` to the
    top-level `CMakeLists.txt` to avoid any scoping issues for the
    lists of generated files.

  * Removed pattern of yielding tasks while creating a list tasks in
    favour of returning the list of tasks directly.
@1uc 1uc force-pushed the 1uc/fix-cmake-deps branch from ce0f0c7 to 2f31622 Compare November 7, 2024 06:41
Copy link
Contributor

@pramodk pramodk left a comment

Choose a reason for hiding this comment

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

LGTM this approach as well.

@1uc 1uc merged commit 49fdfe6 into master Nov 7, 2024
12 checks passed
@1uc 1uc deleted the 1uc/fix-cmake-deps branch November 7, 2024 12:57
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.

2 participants