geotiff: write VRT index atomically via temp-then-rename (#2965)

[brendancol]

Closes #2965

write_vrt in xrspatial/geotiff/_vrt.py wrote the final .vrt in place with open(vrt_path, 'w'). A crash, a full disk, or an interrupted write between open and the finished write could leave a truncated index at the final path.

The tiled writer makes this worse. _write_vrt_tiled promotes the staging tile directory to its final name with an atomic os.replace and only then writes the index, so the index was the last non-atomic step. A partial index would point at a complete, fully-promoted tile set, and a reader that picks it up sees a valid tile directory behind a malformed index.

Change

Route the index write through the existing _write_bytes helper in _writer.py. It writes to a temp file in the destination directory and os.replaces it into place. The temp file lives on the same filesystem as the destination, so the rename stays atomic. No new temp-then-rename code.

Backend coverage

Not applicable. write_vrt writes a filesystem index, it is not a backend-dispatched array op. The behavior is the same on every platform: os.replace is atomic on POSIX and is the standard atomic-rename primitive on Windows for same-directory targets.

Notes

_write_bytes names its temp file with a .tif.tmp suffix. That suffix is cosmetic for a VRT, the file is renamed to the real .vrt immediately and is only visible if the process dies mid-write, so I left the shared helper alone rather than reshaping it for one caller.

Test plan

• test_write_vrt_leaves_no_temp_file: a successful write leaves the .vrt and no .tmp artifacts in the directory.
• test_write_vrt_no_partial_file_on_write_failure: a forced failure during the write leaves no partial .vrt at the final path.
• test_write_vrt_failure_preserves_existing_file: a failed rewrite leaves a pre-existing .vrt byte-for-byte intact.
• Existing VRT write, metadata, nodata, and tiled-writer suites pass unchanged.

[brendancol]

PR Review: write VRT index atomically via temp-then-rename (#2965)

Reviewed the full diff and the surrounding code in _vrt.py, _writer.py, and the test helpers.

Blockers (must fix before merge)

None.

Suggestions (should fix, not blocking)

None blocking. One thing worth calling out explicitly in the PR/commit, not a code change: the old write used open(vrt_path, 'w'), i.e. the platform default text encoding. The new write encodes UTF-8 explicitly. That is the correct direction, since _read_vrt_xml already decodes the file as UTF-8 (.decode('utf-8')), so writer and reader now agree. On Windows the old default could have been cp1252, which would have mangled non-ASCII CRS WKT on round-trip. So this is a small latent-bug fix riding along with the atomicity change, not a regression. Fine to keep, just good to have on record.

Nits (optional improvements)

• test_write_vrt_failure_preserves_existing_file reads the file with bare open(vrt, "rb").read() twice (test_basic.py, in the new test). The handles get closed eventually by GC, but pathlib.Path(vrt).read_bytes() or a with block is cleaner and matches how the rest of the suite reads small files. Cosmetic.

What looks good

• Reuses the existing _write_bytes helper instead of hand-rolling a second temp-then-rename. The temp file lands in the destination directory, so the os.replace stays on one filesystem and is a true atomic rename.
• The local from ._writer import _write_bytes import avoids any import-cycle risk and matches the local-import style already used between these modules (eager.py does from .._vrt import write_vrt).
• The tests cover the cases that matter: a clean write leaves no temp litter, a failed write leaves no partial .vrt, and a failed rewrite leaves a pre-existing .vrt byte-for-byte intact. The monkeypatch targets writer_mod._write_bytes, which is the right binding since write_vrt looks the name up at call time.
• The write-site comment says why atomicity matters here specifically: the tiled writer promotes the tile dir before the index write.

Checklist

• Behavior matches the intended fix (atomic temp-then-rename)
• No backend dispatch involved; filesystem index writer
• No NaN / float concerns in this path
• Edge cases covered (write failure, rewrite over existing file)
• No dask chunk-boundary concerns
• No premature materialization or extra copies
• No benchmark needed (I/O index write, not a hot array path)
• README feature matrix not applicable (internal-only function, no API change)
• Write-site comment present and accurate

[brendancol]

Follow-up review (#2967)

Re-reviewed after the follow-up commit.

Blockers

None.

Suggestions

None.

Nits

The one nit from the first pass is resolved: test_write_vrt_failure_preserves_existing_file now reads the file inside with open(...) blocks instead of bare open(...).read().

What looks good

The production change is unchanged and correct: the VRT index now goes through _write_bytes, which writes a temp file in the destination directory and os.replaces it into place. Writer and reader agree on UTF-8 (_read_vrt_xml decodes UTF-8). Nothing else moved.

Nothing left to address. Good to merge once CI is green.