Skip to content

Fix incorrect regeneration for IDL targets#40148

Open
OneBlue wants to merge 1 commit intofeature/wsl-for-appsfrom
user/oneblue/idl-rebuild
Open

Fix incorrect regeneration for IDL targets#40148
OneBlue wants to merge 1 commit intofeature/wsl-for-appsfrom
user/oneblue/idl-rebuild

Conversation

@OneBlue
Copy link
Copy Markdown
Collaborator

@OneBlue OneBlue commented Apr 10, 2026

Summary of the Pull Request

#40130 introduced a regression that caused the idl targets to always be considered out of date. This changes solves this by explicitly setting the target stamp file and fixing the output of the dlldata file to only one command

PR Checklist

  • Closes: Link to issue #xxx
  • Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Tests: Added/updated if needed and all pass
  • Localization: All end user facing strings can be localized
  • Dev docs: Added/updated if needed
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

@OneBlue OneBlue requested a review from a team as a code owner April 10, 2026 02:33
Copilot AI review requested due to automatic review settings April 10, 2026 02:33
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a build regression where IDL-related CMake targets are always considered out-of-date, impacting incremental builds across dependent Windows components.

Changes:

  • Tracks dlldata_<platform>.c as an output from only the final proxy-IDL MIDL invocation to avoid conflicting/duplicated outputs.
  • Suppresses unwanted MIDL byproducts for “no proxy” IDLs by redirecting /iid, /proxy, and /dlldata outputs to nul.
  • Attempts to add a stamp-touch step for Visual Studio incremental up-to-date tracking on the IDL custom target.

Comment on lines +79 to +82
add_custom_target(${target}
COMMAND ${CMAKE_COMMAND} -E touch ${CMAKE_CURRENT_BINARY_DIR}/CMakeFiles/${target}
DEPENDS ${TARGET_OUTPUTS}
SOURCES ${idl_files_with_proxy} ${idl_files_no_proxy})
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

This custom target adds a COMMAND that touches a stamp file, but the stamp isn’t declared as an OUTPUT/BYPRODUCT. For non-MSBuild generators (e.g., Ninja/Makefiles), custom targets with commands and no tracked byproducts typically run every time the target is built, which would reintroduce the “always out of date” behavior. Also, the touched path is unquoted (breaks if the build dir has spaces) and uses CMakeFiles casing, while other stamp usage in this repo consistently touches/outputs ${CMAKE_CURRENT_BINARY_DIR}/CmakeFiles/<target> (e.g., cmake/FindMC.cmake, localization/CMakeLists.txt). Consider using the existing pattern: have the last MIDL add_custom_command produce the stamp as an OUTPUT (or set BYPRODUCTS on the target) and quote the path, keeping add_custom_target as DEPENDS-only.

Copilot uses AI. Check for mistakes.
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