build: fix windows auto deploy script#2306
Conversation
Modern Windows refuses to execute .cmd files from the current working
directory unless they're prefixed with `.\` (introduced as a security
default to prevent CWD command-hijacking). CMake's add_custom_command
strips the absolute path from COMMAND args that live inside
CMAKE_BINARY_DIR (the custom-build CWD), so the generated MSBuild
command was just `call robocopy_wrapper.cmd ...` - which fails with
"is not recognized as an internal or external command".
Fix: invoke through `cmd /c "<absolute-path>"`. Because the wrapper
path is now an argument to cmd rather than the COMMAND itself, CMake
preserves the absolute path verbatim and the generated MSBuild rule
becomes `cmd /c E:\path\to\robocopy_wrapper.cmd ...` - which works.
The variable rename (ROBOCOPY_WRAPPER -> ROBOCOPY_WRAPPER_PATH for the
file path, ROBOCOPY_WRAPPER for the invocation list) keeps the three
existing call sites untouched - they consume `${ROBOCOPY_WRAPPER}` as
a command-list expansion and pick up the new `cmd /c <path>` prefix
automatically.
Verified end-to-end with BuildRelease.bat ALL-WITH-AUTO-DEPLOYMENT:
both Skyrim VR and Skyrim Special Edition Data dirs receive the
plugin DLL + shaders without errors.
PREPARE_AIO and copy_shaders.stamp both staged shader files into
${AIO_DIR}/Shaders, with overlapping working sets:
PREPARE_AIO : everything under package/* and each feature path */*
copy_shaders : package/Shaders/* and each feature's Shaders/*
When MSBuild builds them in parallel both rules call cmake -E
copy_if_different on the same destination shader files. One process
opens the file for write while the other tries the same and gets
"Permission denied" on Windows (Windows file locking, not Unix-style
overwrite). Symptom seen at PREPARE_AIO during a parallel build:
Error copying file (if different) from
"...features/Terrain Helper/TerrainHelper.esp" to
".../build/ALL/aio/TerrainHelper.esp": Permission denied
(The visible failure happened on TerrainHelper.esp because that's
where the loser landed; the underlying contention was on shader
files copied earlier in the same parallel sweep.)
Fix: exclude /Shaders/ from PREPARE_AIO's input lists so it owns only
non-shader feature content, and copy_shaders.stamp owns Shaders/.
The two rules now have disjoint working sets - no contention possible.
Filter applied in three places:
- input-tracking glob (DEPENDS list)
- package source glob
- per-feature source glob
Bonus: PREPARE_AIO does less duplicate work, build is slightly faster.
Verified end-to-end with BuildRelease.bat ALL-WITH-AUTO-DEPLOYMENT
from a cleaned AIO dir: full configure + compile + parallel deploy
without any Permission denied or race symptoms; plugin deployed to
both Skyrim VR and Skyrim Special Edition Data dirs.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughCMake changes: Windows robocopy wrapper is invoked via ChangesAIO Shader Exclusion and Robocopy Invocation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
No actionable suggestions for changed features. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CMakeLists.txt`:
- Around line 427-440: The current change excludes "Shaders/" from the
_AIO_PACKAGE_FILES and from the per-feature _tmp glob, which breaks the
dependency chain (prepare_aio.stamp no longer updates on shader changes) and can
produce stale AIO zips; restore shader staging by either including "Shaders/" in
the package file lists or by explicitly adding the copy_shaders stamp as a
dependency of prepare_aio.stamp/AIO_ZIP targets. Update the CMake logic around
_AIO_PACKAGE_FILES and the FEATURE_PATHS loop (the file(GLOB_RECURSE ...) and
list(FILTER ... EXCLUDE REGEX "/Shaders/") lines) to stop excluding "/Shaders/"
or add a custom add_dependencies(prepare_aio.copy_shaders_target
copy_shaders.stamp) / add_dependencies(AIO_ZIP_PACKAGE prepare_aio_target) so
that copy_shaders.stamp is always considered when building AIO_ZIP_PACKAGE.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 45ffc61f-d6c2-4f2d-976b-347994b976a8
📒 Files selected for processing (1)
CMakeLists.txt
|
✅ A pre-release build is available for this PR: |
PR community-shaders#2306 excluded shaders from prepare_aio.stamp inputs to avoid a race with copy_shaders.stamp on the same destination files. Side effect: AIO_ZIP_PACKAGE only depended on PREPARE_AIO, so shader-only changes no longer triggered an archive rebuild and AIO_ZIP_TO_DIST builds could ship stale Shaders/ contents. Add copy_shaders.stamp to the AIO_ZIP_PACKAGE custom command's DEPENDS so the archive rebuilds whenever either the package files or the shaders change. Reported by CodeRabbit on PR community-shaders#2306.
Branch-preserving adaptation: keep this branch's manual AIO packaging target while applying the robocopy wrapper fix and excluding shader files from PREPARE_AIO's non-shader copy path.
Branch-preserving adaptation: keep this branch's manual AIO packaging target while applying the robocopy wrapper fix and excluding shader files from PREPARE_AIO's non-shader copy path.
Modern Windows refuses to execute .cmd files from the current working
directory unless they're prefixed with
.\(introduced as a securitydefault to prevent CWD command-hijacking). CMake's add_custom_command
strips the absolute path from COMMAND args that live inside
CMAKE_BINARY_DIR (the custom-build CWD), so the generated MSBuild
command was just
call robocopy_wrapper.cmd ...- which fails with"is not recognized as an internal or external command".
Fix: invoke through
cmd /c "<absolute-path>". Because the wrapperpath is now an argument to cmd rather than the COMMAND itself, CMake
preserves the absolute path verbatim and the generated MSBuild rule
becomes
cmd /c E:\path\to\robocopy_wrapper.cmd ...- which works.The variable rename (ROBOCOPY_WRAPPER -> ROBOCOPY_WRAPPER_PATH for the
file path, ROBOCOPY_WRAPPER for the invocation list) keeps the three
existing call sites untouched - they consume
${ROBOCOPY_WRAPPER}asa command-list expansion and pick up the new
cmd /c <path>prefixautomatically.
Verified end-to-end with BuildRelease.bat ALL-WITH-AUTO-DEPLOYMENT:
both Skyrim VR and Skyrim Special Edition Data dirs receive the
plugin DLL + shaders without errors.
Summary by CodeRabbit
Chores
Bug Fixes