build: update bundled Syncthing to v2.1.3 - #273
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new version-detection logic relies on brittle token-splitting and should be made robust to avoid incorrect version parsing and unnecessary re-downloads.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the build tooling to bundle Syncthing v2.1.3 and improves the Windows download/extract script to better handle existing downloads and extraction layout.
Changes:
- Bumps the configured Syncthing download version from v2.0.3 to v2.1.3.
- Updates the download script to detect an already-downloaded Syncthing binary and replace it when the version differs.
- Changes extraction handling to copy/merge extracted contents into the target directory (instead of moving), preserving existing nested files.
File summaries
| File | Description |
|---|---|
| Taskfile.yml | Updates the pinned bundled Syncthing version to v2.1.3 for build tasks. |
| scripts/download-syncthing.ps1 | Adds version-aware “already downloaded” handling and switches extracted-content promotion from move to copy/merge. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| $versionOutput = & "$outdir/syncthing.exe" version 2>$null | Select-Object -First 1 | ||
| $currentVersion = ($versionOutput -split '\s+')[1] |
| # Optionally move binaries up from the nested folder | ||
| $extractedRoot = Join-Path $outdir "syncthing-windows-$arch-$Version" | ||
| if (Test-Path $extractedRoot) { | ||
| Move-Item -Path (Join-Path $extractedRoot '*') -Destination $outdir -Force | ||
| Copy-Item -Path (Join-Path $extractedRoot '*') -Destination $outdir -Recurse -Force |
|
I'm not sure what the point of this change is. The CI already updates the download binary so this isn't needed for anything. Sure we can refresh this from time to time but this doesn't affect anything in practice. I'm also against introducing additional moving parts for the downloader (with the version parsing and everything). This is just meant as a quick script to grab any bootstrap syncthing version for CI, which will always start from a clean state. The skip-if-exists logic exists so you're not trashing the server when testing locally - we don't need version-parsing there either. |
Summary
Testing
dotnet test src -c Release --no-restore