Update update.ps1 - #1862
Conversation
More robust backup handling Signed-off-by: seliSoft <seliSoft@users.noreply.github.com>
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| Write-Host -ForegroundColor Cyan "Copying directory conf, userdata and runtime to $TempBackupDirConf" | ||
| Copy-Item -Path $OHConf, $OHUserData, $OHRuntime -Destination $TempBackupDir -Recurse -Force -ErrorAction Stop | ||
| Write-Host -ForegroundColor Cyan "Copying directory conf, userdata and runtime to $TempBackupDir" |
There was a problem hiding this comment.
I think the text should be adapted....
wborn
left a comment
wborn
left a comment
There was a problem hiding this comment.
This PR was reviewed by AI first, before manual maintainer review.
The change addresses a valid problem: the temporary backup needs predictable conf, userdata, runtime, and home paths because those paths are used by the restore logic, while the source directories can be configured through environment variables.
There is one issue with the current implementation that should be addressed before merging. The code removes $TempBackupDir and then immediately tries to create child directories using that removed directory as the New-Item -Path. It also pre-creates the destinations before copying the source directories, which changes the behavior of Copy-Item and can introduce an additional directory level.
The existing CreateDirectory helper can be used to keep the fix simpler and make the intended backup layout explicit.
Since this backup is also the rollback mechanism when an update fails, it would be useful to manually verify the backup and restore paths with Windows PowerShell 5.1. The current CI build runs on Ubuntu and does not exercise this Windows-specific behavior.
| Write-Host -ForegroundColor Cyan "Copying directory conf, userdata and runtime to $TempBackupDirConf" | ||
| Copy-Item -Path $OHConf, $OHUserData, $OHRuntime -Destination $TempBackupDir -Recurse -Force -ErrorAction Stop | ||
| Write-Host -ForegroundColor Cyan "Copying directory conf, userdata and runtime to $TempBackupDir" | ||
| New-Item -Path $TempBackupDir -Name "conf" -ItemType "Directory" |
There was a problem hiding this comment.
$TempBackupDir was just removed above, so it does not normally exist when this tries to create conf relative to it.
Pre-creating $TempBackupDirConf also changes the semantics of the following Copy-Item: when the destination directory already exists, copying $OHConf can create another directory level below it.
The existing CreateDirectory helper could keep this simpler:
CreateDirectory $TempBackupDir
Copy-Item -Path $OHConf -Destination $TempBackupDirConf -Recurse -Force -ErrorAction Stop
Copy-Item -Path $OHUserData -Destination $TempBackupDirUserData -Recurse -Force -ErrorAction Stop
Copy-Item -Path $OHRuntime -Destination $TempBackupDirRuntime -Recurse -Force -ErrorAction Stop
CreateDirectory $TempBackupDirHomeThis still gives the backup the fixed conf, userdata, and runtime names expected by the restore logic, independent of the configured source directory names.
More robust backup handling
The upgrade failed on Windows 10, because the files are copied to a file named "home" instead of a the directory home. Simmilar thing happend to conf/userdata/runtime
With the changes the process is more robust and with the correct folders it is able to restore the files later.