scoop-cleanup: handle with junction better #2882
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rasa
added
review-needed
r15ch13
pushed a commit
that referenced
this pull request
Jan 17, 2019
**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.
31 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The work is done, now needs code review and somebody to test.
This will fix #2881 , close #2832 .
checklist:
test case
{ "homepage": "https://scoop.sh", "description": "A dummy manifest for scoop-cleanup tests.", "license": "Freeware", "version": "1.0.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 }" ], "persist": [ "dummy.txt", "dummydir" ] }go visit https://github.com/h404bi/scoop-persist-test for test cases.