Skip to content

Commit

Permalink
Rewritten data persisting feature (#2897)
Browse files Browse the repository at this point in the history
**Data matters, please review**

This is a rework of #2891 #2890 and #2882 . This will also fix #2724 and fix #2900 , close #2779 

**Introduction**

I add a function called `unlink_persist_data($dir)`, which recursively unlink all junction/hard link in the given directory. This affects some sub-commands which have interaction with junction/hard link:

- `scoop-install`: the persisting logic has been improved as follow:
  1. if there are data in the `persist` folder, also in app(`source`) folder, rename that one located in app folder (e.g. app's default setting files) with a `original` suffix, then create link from `persist` to  app
  2. if there are data in the `persist` folder, but no in app(`source`) folder, just create link from `persist` to  app
  3. if there is no data in the `persist` folder (e.g. fresh install), but there are data in app(`source`) folder (e.g. app's default setting files), we will just use that default setting files as the first-time persisting. So move that files from app folder to `persist` folder, then create link from `persist` to  app
  4. But what if if there is neither data in the `persist` folder (e.g. fresh install), nor in the app(`source`) folder (e.g. setting files will be created after first startup, like `Everthing.db`). We need to create empty persisting target in the `persist` folder. But by default we can't know if a persisting target is a file or a directory (e.g. `conf.d`). So we create a directory by default, and to avoid this, manifest maintainers should use `pre_install` to manually create the source file before persisting.
- `scoop-reset`: `reset` command uses the logic of `install`, but there is a problem: before re-persisting data, there have been `junction/hard link` in the app(`source`) folder. It will throw an exception of file exists #2724 . To fix this, we should unlink all old link before re-persisting, using `unlink_persist_data`.
- `scoop-uninstall`: `Remove-Item` can not remove `NTFS junction`, we need to unlink all persistting data, before uninstalling/deleting an app, but keeping persisting data.
- `scoop-cleanup`: like `uninstall`, `Remove-Item` can not remove `NTFS junction`, we need to unlink all persistting data, before deleting old versions of an app. Before PR #2882 , it use `fsutil.exe` to unlink junction, which is not friendly to restricted users (#2832 and #2881 ).

Beyond the logic improvement, there is a new feature now: it supports sub-folder data persisting, like:

```json
{
    "homepage": "https://scoop.sh",
    "description": "A dummy manifest for scoop tests.",
    "license": "Freeware",
    "version": "1.1.0",
    "url": "https://get.scoop.sh",
    "pre_install": [
        "if (!(test-path \"$dir\\dummy.txt\")) { new-item -force \"$dir\\dummy.txt\" -itemtype file | out-null }",
        "if (!(test-path \"$dir\\dummydir\")) { new-item \"$dir\\dummydir\" -itemtype directory | out-null }",
        "if (!(test-path \"$dir\\subdir\")) { new-item \"$dir\\subdir\" -itemtype directory | out-null }",
        "if (!(test-path \"$dir\\subdir\\subdummydir\")) { new-item \"$dir\\subdir\\subdummydir\" -itemtype directory | out-null }",
        "if (!(test-path \"$dir\\subdir\\subdummy.txt\")) { new-item -force \"$dir\\subdir\\subdummy.txt\" -itemtype file | out-null }",
    ],
    "persist": [
        "dummy.txt",
        "dummydir",
        "subdir\\subdummydir",
        "subdir\\subdummy.txt"
    ]
}
```
So no need to strip directories of source for target anymore.

To participate in the code review and tests, go visit https://github.com/h404bi/scoop-persist-test for test cases.
  • Loading branch information
chawyehsu authored and r15ch13 committed Jan 17, 2019
1 parent 3ffe824 commit d23552a
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 33 deletions.
62 changes: 42 additions & 20 deletions lib/install.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -1202,7 +1202,7 @@ function persist_def($persist) {
}

if (!$target) {
$target = fname($source)
$target = $source
}

return $source, $target
Expand All @@ -1222,39 +1222,61 @@ function persist_data($manifest, $original_dir, $persist_dir) {

write-host "Persisting $source"

# add base paths
if (!(Test-Path(fullpath "$dir\$source")) -or (is_directory(fullpath "$dir\$source"))) {
$source = New-Object System.IO.DirectoryInfo(fullpath "$dir\$source")
$target = New-Object System.IO.DirectoryInfo(fullpath "$persist_dir\$target")
} else {
$source = New-Object System.IO.FileInfo(fullpath "$dir\$source")
$target = New-Object System.IO.FileInfo(fullpath "$persist_dir\$target")
}
$source = fullpath "$dir\$source"
$target = fullpath "$persist_dir\$target"

if (!$target.Exists) {
# If we do not have data in the store we move the original
if ($source.Exists) {
Move-Item $source $target
} elseif($target.GetType() -eq [System.IO.DirectoryInfo]) {
# if there is no source and it's a directory we create an empty directory
ensure $target.FullName | out-null
# if we have had persist data in the store, just create link and go
if (Test-Path $target) {
# if there is also a source data, rename it (to keep a original backup)
if (Test-Path $source) {
Move-Item -Force $source "$source.original"
}
} elseif ($source.Exists) {
# (re)move original (keep a copy)
Move-Item $source "$source.original"
# we don't have persist data in the store, move the source to target, then create link
} elseif (Test-Path $source) {
# ensure target parent folder exist
$null = ensure (Split-Path -Path $target)
Move-Item $source $target
# we don't have neither source nor target data! we need to crate an empty target,
# but we can't make a judgement that the data should be a file or directory...
# so we create a directory by default. to avoid this, use pre_install
# to create the source file before persisting (DON'T use post_install)
} else {
$target = New-Object System.IO.DirectoryInfo($target)
}

# create link
if (is_directory $target) {
# target is a directory, create junction
& "$env:COMSPEC" /c "mklink /j `"$source`" `"$target`"" | out-null
attrib $source.FullName +R /L
attrib $source +R /L
} else {
# target is a file, create hard link
& "$env:COMSPEC" /c "mklink /h `"$source`" `"$target`"" | out-null
}
}
}
}

function unlink_persist_data($dir) {
# unlink all junction / hard link in the directory
Get-ChildItem -Recurse $dir | ForEach-Object {
$file = $_
if ($null -ne $file.LinkType) {
$filepath = $file.FullName
# directory (junction)
if ($file -is [System.IO.DirectoryInfo]) {
# remove read-only attribute on the link
attrib -R /L $filepath
# remove the junction
& "$env:COMSPEC" /c "rmdir /s /q $filepath"
} else {
# remove the hard link
& "$env:COMSPEC" /c "del $filepath"
}
}
}
}

# check whether write permission for Users usergroup is set to global persist dir, if not then set
function persist_permission($manifest, $global) {
if($global -and $manifest.persist -and (is_admin)) {
Expand Down
14 changes: 3 additions & 11 deletions libexec/scoop-cleanup.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
. "$psscriptroot\..\lib\versions.ps1"
. "$psscriptroot\..\lib\getopt.ps1"
. "$psscriptroot\..\lib\help.ps1"
. "$psscriptroot\..\lib\install.ps1"

reset_aliases

Expand All @@ -39,17 +40,8 @@ function cleanup($app, $global, $verbose) {
$version = $_
write-host " $version" -nonewline
$dir = versiondir $app $version $global
Get-ChildItem $dir | ForEach-Object {
$file = $_
# the file is a junction
if ($null -ne $file.LinkType -and $file -is [System.IO.DirectoryInfo]) {
# remove read-only attribute on the link
attrib -R /L $file
# remove the junction
$filepath = Resolve-Path $file
& "$env:COMSPEC" /c "rmdir /s /q $filepath"
}
}
# unlink all potential old link before doing recursive Remove-Item
unlink_persist_data $dir
Remove-Item $dir -ErrorAction Stop -Recurse -Force
}
write-host ''
Expand Down
2 changes: 2 additions & 0 deletions libexec/scoop-reset.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ $apps | ForEach-Object {
create_startmenu_shortcuts $manifest $dir $global $architecture
env_add_path $manifest $dir
env_set $manifest $dir $global
# unlink all potential old link before re-persisting
unlink_persist_data $original_dir
persist_data $manifest $original_dir $persist_dir
persist_permission $manifest $global
}
Expand Down
4 changes: 4 additions & 0 deletions libexec/scoop-uninstall.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ if(!$apps) { exit 0 }
env_rm $manifest $global

try {
# unlink all potential old link before doing recursive Remove-Item
unlink_persist_data $dir
Remove-Item -r $dir -ea stop -force
} catch {
if(test-path $dir) {
Expand All @@ -85,6 +87,8 @@ if(!$apps) { exit 0 }
write-host "Removing older version ($oldver)."
$dir = versiondir $app $oldver $global
try {
# unlink all potential old link before doing recursive Remove-Item
unlink_persist_data $dir
Remove-Item -r -force -ea stop $dir
} catch {
error "Couldn't remove '$(friendly_path $dir)'; it may be in use."
Expand Down
4 changes: 2 additions & 2 deletions test/Scoop-Install.Tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,10 @@ describe 'persist_def' -Tag 'Scoop' {
$target | should -be "test"
}

it 'should strip directories of source for target' {
it 'should handle sub-folder' {
$source, $target = persist_def "foo/bar"
$source | should -be "foo/bar"
$target | should -be "bar"
$target | should -be "foo/bar"
}

it 'should handle arrays' {
Expand Down

0 comments on commit d23552a

Please sign in to comment.