Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

install: totally rewritten on data persistting #2891

Closed
wants to merge 1 commit into from

Conversation

chawyehsu
Copy link
Member

@chawyehsu chawyehsu commented Dec 13, 2018

still WIP don't merge, needs code review and somebody to test.
Follow up #2890

To participate in the code review and tests, go visit https://github.com/h404bi/scoop-persist-test for test cases.

@Ash258
Copy link
Contributor

Ash258 commented Dec 13, 2018

Should be #2779 closed or merged, or ...

@chawyehsu
Copy link
Member Author

@Ash258 It maybe should be closed, the data persisting logic in pr #2779 is still confusing. Though I have tested for myself for many test cases, I need someone to help me to review and test this pull-request.

cc @PakornVl @dzxs @mpro7 who have left comments in related issues/pr.

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"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use $source.original or "$source.bak"?

@chawyehsu
Copy link
Member Author

This pull-request doesn't resolve #2724 (#2828 ), I'll take a look at it and try to fix it at the same time.

@chawyehsu chawyehsu closed this Dec 14, 2018
@chawyehsu chawyehsu deleted the patch-4 branch December 27, 2018 05:38
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants