Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 4, 2025

Addresses feedback from #6 regarding undefined ASAN_RUNTIME_DEP variable reference in the EXTRA_RUNTIME_DEPS CMake expression.

Changes

  • Removed $<$<CONFIG:Debug>:;${ASAN_RUNTIME_DEP}> from the generator expression since the variable is never defined
  • Simplified EXTRA_RUNTIME_DEPS to only include mimalloc DLLs for Release builds
  • Updated to use $<IF> syntax for cleaner generator expression

Before

-D EXTRA_RUNTIME_DEPS="$<$<AND:$<BOOL:${USE_MIMALLOC}>,$<CONFIG:Release>>:${MIMALLOC_INSTALL_DIR}/bin/mimalloc.dll;${MIMALLOC_INSTALL_DIR}/bin/mimalloc-redirect.dll>$<$<CONFIG:Debug>:;${ASAN_RUNTIME_DEP}>"

After

-D EXTRA_RUNTIME_DEPS="$<IF:$<AND:$<BOOL:${USE_MIMALLOC}>,$<CONFIG:Release>>,${MIMALLOC_INSTALL_DIR}/bin/mimalloc.dll;${MIMALLOC_INSTALL_DIR}/bin/mimalloc-redirect.dll,>"

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: asn5d <16312669+asn5d@users.noreply.github.com>
Copilot AI changed the title [WIP] WIP address feedback on ASAN_RUNTIME_DEP removal in mimalloc support PR Remove undefined ASAN_RUNTIME_DEP from runtime deps Dec 4, 2025
Copilot AI requested a review from asn5d December 4, 2025 15:51
@asn5d asn5d marked this pull request as ready for review December 4, 2025 17:34
@predictivephenomics predictivephenomics deleted a comment from Copilot AI Dec 4, 2025
@asn5d
Copy link
Member

asn5d commented Dec 4, 2025

@copilot ASAN_RUNTIME_DEP is already removed and pushed to feature/mimalloc branch. You mentioned about adding $ to make the generator expression clearer in PR #6

"$<IF:$AND:$BOOL:${USE_MIMALLOC},$CONFIG:Release,${MIMALLOC_INSTALL_DIR}/bin/mimalloc.dll;${MIMALLOC_INSTALL_DIR}/bin/mimalloc-redirect.dll,>

Copy link
Contributor Author

Copilot AI commented Dec 4, 2025

@copilot ASAN_RUNTIME_DEP is already removed and pushed to feature/mimalloc branch. You mentioned about adding $ to make the generator expression clearer in PR #6

"$<IF:$<AND:$BOOL:${USE_MIMALL...

I've verified that ASAN_RUNTIME_DEP is removed in both branches. Your comment appears to be cut off at "You mentioned about adding $" - could you clarify what additional change you're referring to?

@asn5d
Copy link
Member

asn5d commented Dec 4, 2025

@copilot I was referring to including $<IF> as you suggested in PR #6.

$<IF:$<AND:$<BOOL:${USE_MIMALLOC}>,$<CONFIG:Release>>,${MIMALLOC_INSTALL_DIR}/bin/mimalloc.dll;${MIMALLOC_INSTALL_DIR}/bin/mimalloc-redirect.dll,>

Co-authored-by: asn5d <16312669+asn5d@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 4, 2025

@copilot I was referring to including $<IF> as you suggested in PR #6.

`$<IF:$<AND:$BOOL:${USE_MIMALLOC},$CONFIG:Release>,${MIMALLOC_INSTALL_DIR}/bin/mimalloc.dll;${MIMALLOC_INSTALL_DIR}/bin/...

Updated the EXTRA_RUNTIME_DEPS to use $<IF> syntax in commit 81dd3db.

@asn5d asn5d merged commit dee3321 into feature/mimalloc Dec 4, 2025
@asn5d asn5d deleted the copilot/sub-pr-6-another-one branch December 4, 2025 18:49
asn5d added a commit that referenced this pull request Dec 4, 2025
* Add optional mimalloc allocator support for Release builds.

* Update RoiManager/CMakeLists.txt to check mimalloc target exists before linking.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update PluginManager/CMakeLists.txt to check mimalloc target exists before linking.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update CMakeLists.txt to show message that mimalloc will be linked for Release only.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Added link option /INCLUDE:mi_version in cvutil/CMakeLists.txt  only for MSVC

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Updated CMakeLists.txt to include platform specific import locations.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Fix generator expression in ExternalProject_Add for mimalloc (#8)

* Initial plan

* Replace $<CONFIG> with ${CMAKE_BUILD_TYPE} in ExternalProject_Add

Co-authored-by: asn5d <16312669+asn5d@users.noreply.github.com>

* Use hardcoded Release build type for mimalloc ExternalProject

Co-authored-by: asn5d <16312669+asn5d@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: asn5d <16312669+asn5d@users.noreply.github.com>

* Remove undefined ASAN_RUNTIME_DEP from runtime deps (#9)

* Initial plan

* Remove undefined ASAN_RUNTIME_DEP from EXTRA_RUNTIME_DEPS

Co-authored-by: asn5d <16312669+asn5d@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: asn5d <16312669+asn5d@users.noreply.github.com>

* Remove undefined ASAN_RUNTIME_DEP from runtime deps (#10)

* Initial plan

* Remove undefined ASAN_RUNTIME_DEP from EXTRA_RUNTIME_DEPS expression

Co-authored-by: asn5d <16312669+asn5d@users.noreply.github.com>

* Use $<IF> syntax for EXTRA_RUNTIME_DEPS generator expression

Co-authored-by: asn5d <16312669+asn5d@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: asn5d <16312669+asn5d@users.noreply.github.com>

---------

Co-authored-by: Seethepalli <7uz@ornl.gov>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
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