Cleanup is not $dir aware #2900
Comments
|
Since |
|
I see, how about |
|
I don't know. Maybe. This was first untested idea. |
|
I can't reproduce, since |
|
Should we revert e4e76d3 until this is sorted out? |
|
Hi, I hit this bug today. This is a potentially destructive operation; if there is a directory with that name in the current directory ( As an example for the effects, there is a junction called
|
**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.
Regression after e4e76d3
/cc @h404bi
The text was updated successfully, but these errors were encountered: