build: drop /XO from robocopy auto-deploy#37
Merged
Conversation
The three robocopy invocations under AUTO_PLUGIN_DEPLOYMENT used /XO
("eXclude Older source") to skip files where the source is older than
the destination. In practice this silently lost real updates: any prior
deploy that touches the destination -- a manual `cp` for testing, a
package install, a previous build that ran later in wall time than the
source's git-checkout mtime -- leaves dest with a newer timestamp than
source, and /XO then refuses to overwrite even when source size and
content have changed.
Concrete failure: a fix to RunGrass.hlsl (commit f11bfaa) had source
size 33287 but the deployed file stayed at the previous 31877 bytes
across multiple full builds, because dest mtime was newer than source.
Compile errors in-game (Grass / Particle pixel shaders) followed.
Removing /XO falls back to robocopy's default detection (name + size
+ timestamp delta), which copies whenever any of those differ. The
git-HEAD-change block above already clears AIO and stamps on branch
switch, so the "protect intentional in-game edits across rebuilds"
property /XO was nominally providing was fragile anyway -- breaking
on any external touch to dest. Per-build cost increase is ~100ms of
extra IO on unchanged files; correctness wins.
📝 WalkthroughWalkthroughThis PR modifies Windows incremental deployment behavior in CMakeLists.txt by removing the ChangesWindows Incremental Deployment Robustness
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 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 docstrings
🧪 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
This PR fixes a Windows auto-deploy correctness issue in the CMake AUTO_PLUGIN_DEPLOYMENT workflow by removing Robocopy’s /XO (“exclude older”) flag so modified source files reliably overwrite deployed files even when the destination has a newer mtime.
Changes:
- Remove
/XOfrom the three Robocopy invocations used for incremental deploy (non-shaders, shader-only, and full-shader deploy). - Update inline comments to document the failure mode and explain the intended incremental-copy behavior.
- Clarify the git-HEAD-change invalidation behavior (clearing AIO + deploy stamps) as the mechanism for bulk redeploys.
|
✅ A pre-release build is available for this PR: |
alandtse
changed the title
alandtse
added a commit
that referenced
this pull request
May 25, 2026
Brings in: - 045e72e refactor: unify restart-required infrastructure (#39) - 5c00a38 build: drop /XO from robocopy auto-deploy (#37) The /XO drop is identical to our local cc65edc -- merge auto-resolved cleanly. The restart-required infrastructure (BootSnapshot, RestartSettings) will be adopted into ShadowCasterManager in a follow-up commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
alandtse
added a commit
that referenced
this pull request
May 28, 2026
Sync 13 upstream commits since bb6460d. Notable upstream changes: unified-water LOD/cell-transition fixes + move to core (drops the globals::game::waterSystem global), TAA banding/oversharpening fix, emat complex-material detection fix, ssgi specular depth-fade fix, IBL interior disable, and CI hygiene (cmake pin + useCloudCache=false, robocopy /XO drop already mirrored by our #37). Conflict resolutions: - Effect.hlsl: union — keep our relocated LLF/ISL includes (now after ShadowSampling) AND upstream's new GetEffectAmbientLighting / ExtractEffectLighting helpers; all call sites use the new API. - LightLimitFix.ini / VolumetricShadows.ini / VR.ini: keep our higher versions and (where present) the fork's [Nexus] autoupload section; upstream only stripped that metadata and did not change settings. Auto-merges kept fork-divergent CI intact: - nexus-upload.yaml unchanged (fork aio/matrix design; still uses alandtse/nexus-workflows). Upstream's migration to the official Nexus-Mods/upload-action was NOT adopted — tracked as a follow-up. - CMakeLists.txt kept (fork superset: MCP, exprtk, autoupload AIO filtering, cpp tests, Open Shaders branding). - release-build.yaml: upstream's UNEX_NEXUSMODS_SESSION_COOKIE removal landed on the dry-run call only (upload job is skipped in dry-run and the secret is optional, so this is safe). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/XO(eXclude Older) from the three robocopy invocations underAUTO_PLUGIN_DEPLOYMENTso changed source files actually reach the installWhy
/XOskips files where source is older than dest. In a normal git workflow that's usually safe — but as soon as anything else touches the destination (a manualcpduring debugging, a package install, even a previous build that ran later in wall time than the source's git checkout), dest gets a newer mtime and/XOthen refuses to overwrite even when the source file's size and content have changed.Concrete failure that prompted this
While debugging SLF I had RunGrass.hlsl fixed in source (commit
f11bfaa32, source size 33287 bytes with the corrected ShadowSampling include order), but the deployed file stayed at the previous 31877 bytes across multiple full builds. Mtime on dest (from a manualcpduring earlier diagnostics) was newer than the git-checkout mtime on source, so/XOskipped the copy. Grass and Particle pixel shaders failed to compile in-game:Same shape of bug can hit any contributor who: tests a package install over their dev build, copies files manually for any reason, or just has clock skew across filesystem boundaries.
What this changes
Without
/XO, robocopy uses its default name + size + timestamp delta — copies whenever any of those differ. Catches both newer-source and size-mismatch cases. The git-HEAD-change block above the deploy commands already clears AIO and deploy stamps on branch switch, so the bulk-redeploy story is unchanged.Trade-off
The property
/XOnominally provided ("don't clobber files the user intentionally edited in-game between builds at the same HEAD") was fragile anyway — broken by any external touch to dest. Removing it costs ~100ms of extra IO per build on unchanged files (robocopy still skips matching name+size+timestamp). Correctness wins.Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit