Fix installed CMake SDK dependency propagation - #9
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d20f51d5c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The installed idax::idax target references the build-only idasdk_headers target, which does not exist downstream. As a result, find_package(idax REQUIRED) produces an unusable target unless consumers recreate an internal IDAX detail. Recreate the dependency as idax::sdk_headers from IDA SDK 9.4 before loading idaxTargets.cmake. Normalize documented IDASDK layouts and preserve the effective MSVC runtime across SDK discovery. Host-specific SDK target selection remains consumer-owned. This changes no C++ API or ABI.
4d20f51 to
a493efe
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a493efeaef
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| endif() | ||
| endif() | ||
|
|
||
| find_dependency(idasdk 9.4 EXACT CONFIG) |
There was a problem hiding this comment.
When a Windows consumer has no explicit CRT setting, _idax_msvc_runtime becomes idax's default /MT, but this call loads the IDA SDK while IDA_USE_STATIC_RUNTIME is still the SDK default OFF; the exact 9.4 config creates idasdk::* against the non-_s Windows libraries before line 42 restores /MT. A normal find_package(idax); ida_add_plugin(... LIBRARIES idax::idax) therefore compiles the plugin with the static CRT while linking the DLL-CRT SDK import libraries, producing MSVC runtime mismatch diagnostics. Set/derive IDA_USE_STATIC_RUNTIME before this dependency load when applying idax's static runtime. Fresh evidence beyond the earlier CRT comment is that SDK target selection now happens inside this new find_dependency call before the runtime restore.
Useful? React with 👍 / 👎.
|
This review was authored by GPT-5.6 Sol. I’m requesting changes because the installed-package repair still has three compatibility problems:
The primary export repair works for the current in-tree SDK layout: Release build/install succeeds, and downstream consumers compile using SDK-root and checkout-root paths supplied through either the environment or CMake. However, the remaining issues are merge-blocking, and the |
|
Thanks for the PR review @19h, but I tried a few times and I think it will be difficult for me to land a good patch to fix the issue since I have only Mac setup and no MSVC. I added a simple but sufficient description of the issue in #10, any chance you can take a look and possibly fix with your agent? |
|
I’ll take a look! |
2 participants
The installed idax::idax target references the build-only idasdk_headers target, which does not exist downstream. As a result, find_package(idax REQUIRED) produces an unusable target unless consumers recreate an internal IDAX detail.
Recreate the dependency as idax::sdk_headers from IDA SDK 9.4 before loading idaxTargets.cmake. Normalize documented IDASDK layouts and preserve the effective MSVC runtime across SDK discovery.
Host-specific SDK target selection remains consumer-owned. This changes no C++ API or ABI.