Skip to content

build: drop /XO from robocopy auto-deploy#2417

Merged
SkrubbySkrubInAShrub merged 1 commit into
community-shaders:devfrom
SkrubbySkrubInAShrub:alandtse-lift-3
May 25, 2026
Merged

build: drop /XO from robocopy auto-deploy#2417
SkrubbySkrubInAShrub merged 1 commit into
community-shaders:devfrom
SkrubbySkrubInAShrub:alandtse-lift-3

Conversation

@SkrubbySkrubInAShrub

@SkrubbySkrubInAShrub SkrubbySkrubInAShrub commented May 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Drop /XO (eXclude Older) from the three robocopy invocations under AUTO_PLUGIN_DEPLOYMENT so changed source files actually reach the install
  • Update inline comments to document the trap (any prior touch to dest can give it a newer mtime than git-checkout source, which silently disables deploy)

Why

/XO skips 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 manual cp during 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 /XO then 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 manual cp during earlier diagnostics) was newer than the git-checkout mtime on source, so /XO skipped the copy. Grass and Particle pixel shaders failed to compile in-game:

LightLimitFix/LightLimitFix.hlsli(85,3-28): error X3000: unrecognized identifier 'DirectionalShadowLightData'

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

Before After
`/E /XD ... /COPY:DAT /XO /R:1 /W:1 ...` `/E /XD ... /COPY:DAT /R:1 /W:1 ...`

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 /XO nominally 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

  • `BuildRelease.bat ALL-WITH-AUTO-DEPLOYMENT` produces a deployed install that matches `build//aio` byte-for-byte
  • Manually `touch` a file in the deployed install to give it a newer mtime, rebuild, confirm the source file overwrites it
  • Verify unchanged files still skip on subsequent builds (robocopy's default behavior)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Fixed incremental deployment logic to properly detect and copy updated content, resolving an issue where destination file timestamps could prevent source changes from being applied during builds.

Review Change Stack

## Summary

- Drop `/XO` (eXclude Older) from the three robocopy invocations under
`AUTO_PLUGIN_DEPLOYMENT` so changed source files actually reach the
install
- Update inline comments to document the trap (any prior touch to dest
can give it a newer mtime than git-checkout source, which silently
disables deploy)

## Why

`/XO` skips 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 manual `cp` during 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 `/XO` then 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 manual `cp` during
earlier diagnostics) was newer than the git-checkout mtime on source, so
`/XO` skipped the copy. Grass and Particle pixel shaders failed to
compile in-game:

> `LightLimitFix/LightLimitFix.hlsli(85,3-28): error X3000: unrecognized
identifier 'DirectionalShadowLightData'`

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

| Before | After |
|---|---|
| \`/E /XD ... /COPY:DAT /XO /R:1 /W:1 ...\` | \`/E /XD ... /COPY:DAT
/R:1 /W:1 ...\` |

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 `/XO` nominally 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

- [ ] \`BuildRelease.bat ALL-WITH-AUTO-DEPLOYMENT\` produces a deployed
install that matches \`build/<preset>/aio\` byte-for-byte
- [ ] Manually \`touch\` a file in the deployed install to give it a
newer mtime, rebuild, confirm the source file overwrites it
- [ ] Verify unchanged files still skip on subsequent builds (robocopy's
default behavior)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Chores**
* Adjusted Windows plugin deployment configuration to improve
synchronization between deployed and built files. Updated build system
documentation comments.

<!-- review_stack_entry_start -->

[![Review Change
Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/alandtse/open-shaders/pull/37?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)

<!-- review_stack_entry_end -->

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@coderabbitai

coderabbitai Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: ff3f5d2c-3d8f-46ac-ab2a-a0a4412ebca0

📥 Commits

Reviewing files that changed from the base of the PR and between 6c369ad and 734c3e2.

📒 Files selected for processing (1)
  • CMakeLists.txt

📝 Walkthrough

Walkthrough

This PR updates the AUTO_PLUGIN_DEPLOYMENT CMake logic to remove the /XO robocopy flag from per-build AIO and shader copies, preventing changed source files from being skipped when destination timestamps are newer. Supporting comments clarify the timestamp-based behavior and the role of git-HEAD-change clearing.

Changes

Robocopy Deployment Flag Fix

Layer / File(s) Summary
Robocopy timestamp behavior rationale
CMakeLists.txt
Comments updated to explain how /XO can skip copying when destination mtimes are newer, and how the git-HEAD-change block ensures stale timestamps don't prevent redeploy on repository changes.
Incremental deployment flag removal
CMakeLists.txt
Main incremental robocopy invocation removes /XO flag with detailed rationale about preventing timestamp-based copy skipping during per-build deployment.
Shader deployment flag removals
CMakeLists.txt
Both fast shader-only and full shader robocopy invocations remove /XO flag, applying the same timestamp-prevention fix across shader-specific deployment paths.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • community-shaders/skyrim-community-shaders#2306: Both PRs modify CMakeLists.txt's Windows AUTO_PLUGIN_DEPLOYMENT robocopy-based incremental deploy behavior for AIO/shader copying—build: fix windows auto deploy script #2306 changes how the robocopy wrapper is invoked and stages shader copies to avoid race issues, while the main PR changes the robocopy flags (removing /XO) in the same deploy steps to ensure changed sources aren't skipped.
  • community-shaders/skyrim-community-shaders#1756: The main PR's CMake changes adjust the robocopy invocation flags used by the fast shader-only and full shader AUTO_PLUGIN_DEPLOYMENT deploy paths, which are exactly the shader deployment workflows/targets (e.g., COPY_SHADERS/shader-only vs shader-full stamps) introduced and wired up in #1756.
  • community-shaders/skyrim-community-shaders#2251: Both PRs modify the same CMakeLists.txt deployment/staging logic under AUTO_PLUGIN_DEPLOYMENTbuild: force AIO rebuild on HEAD change or manual delete #2251 clears AIO + deploy/shader stamp files on HEAD/change to force redeploy, while the main PR changes the subsequent robocopy copy steps (removing /XO for AIO/shader copies) to prevent timestamp-based skipping—so they directly touch the same incremental deployment control flow.

Suggested reviewers

  • doodlum
  • jiayev

Poem

🐰 A rabbit's robocopy ode:
Old flags that skipped with haughty pride,
Now /XO sits shuffled aside!
Timestamps yield to source's call—
Fresh shaders flow to victory hall. ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main change: removing the /XO flag from robocopy auto-deploy. It directly reflects the core modification in the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

Copy link
Copy Markdown

No actionable suggestions for changed features.

@github-actions

Copy link
Copy Markdown

✅ A pre-release build is available for this PR:
Download

@SkrubbySkrubInAShrub SkrubbySkrubInAShrub changed the title build: drop /XO from robocopy auto-deploy (#37) build: drop /XO from robocopy auto-deploy May 25, 2026
@SkrubbySkrubInAShrub SkrubbySkrubInAShrub merged commit 19483b3 into community-shaders:dev May 25, 2026
13 checks passed
IgorAlanAlbuquerque pushed a commit to IgorAlanAlbuquerque/skyrim-community-shaders that referenced this pull request May 29, 2026
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.

3 participants