build: added MSVC target link for RelWithDebInfo#2071
Conversation
📝 WalkthroughWalkthroughThe CMakeLists.txt file has been updated to apply MSVC-specific linker optimizations for the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
No actionable suggestions for changed features. |
There was a problem hiding this comment.
Pull request overview
Fixes an MSVC build failure for the RelWithDebInfo configuration by aligning linker settings with third-party libraries built with /GL (e.g., FidelityFX backend), preventing /LTCG-related warnings from being treated as errors.
Changes:
- Add MSVC-only
RelWithDebInfolinker flags to enable/LTCG. - Disable incremental linking (
/INCREMENTAL:NO) forRelWithDebInfoto avoidLNK4075and subsequentLNK1218when/WXis enabled.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
CMakeLists.txt (2)
132-139: Consider centralizing this config incmake/XSEPlugin.cmake.MSVC compile/link profile rules are already managed there; moving this
RelWithDebInfopolicy there would reduce flag drift across files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CMakeLists.txt` around lines 132 - 139, Move the MSVC-specific RelWithDebInfo link options out of the top-level CMakeLists and into the central cmake/XSEPlugin.cmake so flags stay consistent: remove the MSVC conditional block that calls target_link_options(${PROJECT_NAME} PRIVATE "$<$<CONFIG:RelWithDebInfo>:/LTCG>" "$<$<CONFIG:RelWithDebInfo>:/INCREMENTAL:NO>") from this file and instead add equivalent logic in cmake/XSEPlugin.cmake (guarded by MSVC and applying the "$<$<CONFIG:RelWithDebInfo>:...>" generator expressions) so projects using XSEPlugin automatically get the /LTCG and /INCREMENTAL:NO RelWithDebInfo settings.
130-132: PR metadata follow-up: tighten title and add issue keyword.Suggested title:
fix(build): add msvc relwithdebinfo link flags
If this fixes a tracked bug, addCloses #<issue-id>(orFixes #<issue-id>) in the PR description.As per coding guidelines, provide suggestions for Conventional Commit Titles (
type(scope): description, lowercase, 50-char title limit) and Issue References (Fixes#123/ `Closes `#123).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CMakeLists.txt` around lines 130 - 132, Update the PR title to the suggested conventional-commit style "fix(build): add msvc relwithdebinfo link flags" and add an issue reference line like "Fixes #<issue-id>" or "Closes #<issue-id>" to the PR description; ensure the commit/PR title follows type(scope): description lowercase and stays under ~50 characters, and mention the change in the PR body referencing the MSVC-specific block (if(MSVC)) so reviewers know this targets RelWithDebInfo linker flags.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@CMakeLists.txt`:
- Around line 132-139: Move the MSVC-specific RelWithDebInfo link options out of
the top-level CMakeLists and into the central cmake/XSEPlugin.cmake so flags
stay consistent: remove the MSVC conditional block that calls
target_link_options(${PROJECT_NAME} PRIVATE "$<$<CONFIG:RelWithDebInfo>:/LTCG>"
"$<$<CONFIG:RelWithDebInfo>:/INCREMENTAL:NO>") from this file and instead add
equivalent logic in cmake/XSEPlugin.cmake (guarded by MSVC and applying the
"$<$<CONFIG:RelWithDebInfo>:...>" generator expressions) so projects using
XSEPlugin automatically get the /LTCG and /INCREMENTAL:NO RelWithDebInfo
settings.
- Around line 130-132: Update the PR title to the suggested conventional-commit
style "fix(build): add msvc relwithdebinfo link flags" and add an issue
reference line like "Fixes #<issue-id>" or "Closes #<issue-id>" to the PR
description; ensure the commit/PR title follows type(scope): description
lowercase and stays under ~50 characters, and mention the change in the PR body
referencing the MSVC-specific block (if(MSVC)) so reviewers know this targets
RelWithDebInfo linker flags.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 370c2a2d-a7ba-4fcb-ab9e-eb8498fca621
📒 Files selected for processing (1)
CMakeLists.txt
|
✅ A pre-release build is available for this PR: |
alandtse
changed the title
|
This is not user facing. It's not a fix. |
) (cherry picked from commit 26471ae)
fix build failure with --config RelWithDebInfo
error:
ffx_backend_dx11_x64.lib(ffx_dx11.obj) : MSIL .netmodule or module compiled with /GL found; restarting link with /LTCG; add /LTCG to the link command line to improve linker performance
LINK : warning LNK4075: ignoring '/INCREMENTAL' due to '/LTCG' specification [F:\Community-Shaders-Working-Folder\Github-Fork\skyrim-community-shaders\build\ALL\CommunityShaders.vcxproj]
LINK : error LNK1218: warning treated as error; no output file generated [F:\Community-Shaders-Working-Folder\Github-Fork\skyrim-community-shaders\build\ALL\CommunityShaders.vcxproj]
Summary by CodeRabbit