[WIP] Persistence rework with backward compatibility (persist) #3212

r15ch13 · 2019-03-12T19:51:52Z

Reference discussions: #2897 #3209

Complete rework of the persist property including backward compatibility.

Introduces new persist-objects to JSON Schema ^{subject to change}

  "persist": [
      {
          "type": "file",
          "name": "config.dat",
          "content": "[Config]`nlanguage=eng",
          "encoding": "UTF-8",
          "method": "copy"
      },
      {
          "type": "directory",
          "name": "data",
          "method": "link"
      },
      {
          "type": "directory",
          "name": "plugins",
          "method": "merge"
      }
  ]

method
- merge: Merge new Languages/Plugins/Skins directory with current persistent directory to update those but don't lose changes made by the user
- copy: just copy directory or file (maybe the same as merge)
- link: create hardlink for files or junctions for directories
encoding: file encoding (not used for directories)
type: either file or directory
content: either string or array of strings
glue: characters to join content array (default \n)

Introduces new functions for Junction and Hardlink handling including Tests
- Drops $env:COMSPEC and attrib calls
Extract old persist_data behaviour to persist_data_old ^{subject to change}
More tests (more mocking)
Implement actual feature 😁

Cases that need to be adressed:

- Add is_junction() - Add create_junction() - Add remove_junction() - Add create_hardlink() - Add remove_hardlink()

- Extract old persist_data() functionality for backwards compatible with old manifests - Introduce new persist_data_new() stub

- Deprecates the String representation - Deprecates the StringArray representation - Introduces first version of Object representation

Ash258 · 2019-03-12T19:56:07Z

schema.json

+                                "enum": [
+                                    "file",
+                                    "directory",
+                                    "folder"


Why both folder and directory? Is it just alias or some actual differences?

Yeah, might drop it. Only directory should be fine.

Ash258 · 2019-03-12T20:04:46Z

Small notes without code review:

Make encoding UTF8 encoding default in code (eg. When not specified, use UTF-8), same for method link.

niheaven · 2019-03-13T02:34:10Z

lib/core.ps1

+
+function create_junction([String] $link, [String] $target) {
+    if (!(Test-Path $link) -and (is_directory $target)) {
+        New-Item -ItemType Junction -Path (Split-Path -Path $link) -Name (Split-Path -Leaf $link) -Target $target | Out-Null


There is needless to seperate Path and Name (refer to New-Item doc

niheaven · 2019-03-13T03:29:23Z

schema.json

+                            "required": [
+                                "encoding",
+                                "content"
+                            ]


These two properties should not be required, they could be ommited in most instances with no contents and encoding "UTF-8" (which is default in Windows 10 in despite of encoding with Default, ASCII, OEM in Out-File, but "UTF8" give "UTF-8 with BOM"...).

If app require different encoding, we can set through Get-Content | Out-File -Encoding, but it should be rare case.

niheaven · 2019-03-13T03:29:45Z

schema.json

+                {
+                    "description": "Deprecated, use object representation instead.",
+                    "items": {
+                        "$ref": "#/definitions/stringOrArrayOfStrings"


We have #/definitions/stringOrArrayOfStringsOrAnArrayOfArrayOfStrings, why not using it as former?

niheaven · 2019-03-13T03:30:06Z

lib/core.ps1

+    if(is_junction $link) {
+        $dirInfo = New-Object System.IO.DirectoryInfo($link)
+        $dirInfo.Attributes = $dirInfo.Attributes -band (-bnot [System.IO.FileAttributes]::ReadOnly)
+        $dirInfo.Delete()


Remove-Item xxx -Recurse -Force could remove read-only junction. So if we want to delete junction or hardlink, a single function that Remove-Item xxx -Recurse -Force is enough.

Yes, but I got an error message saying powershell is not in interactive mode, so I changed it to a simple delete be the object is already there.

Sent from my OnePlus5T using FastHub

I've tested with changing two remove_ function's contents to Remove-Item (after fix typo in L1216), and also with replacing all remove_junction and remove_hardlink with Remove-Item xxx -Recurse -Force | Out-Null (with a file existing check in link_current), and gotten no error. Maybe need more test.

PS. I've googled that without -Force may return the error that not in interactive mode. May help.

niheaven · 2019-03-13T06:08:27Z

lib/install.ps1

-                attrib -R /L $filepath
-                # remove the junction
-                & "$env:COMSPEC" /c "rmdir /s /q $filepath"
+                remove_junction | Out-Null


Small typo: missing $filename

niheaven · 2019-03-13T06:31:35Z

lib/core.ps1

+    if(is_junction $link) {
+        $dirInfo = New-Object System.IO.DirectoryInfo($link)
+        $dirInfo.Attributes = $dirInfo.Attributes -band (-bnot [System.IO.FileAttributes]::ReadOnly)
+        $dirInfo.Delete()


I've tested with changing two remove_ function's contents to Remove-Item (after fix typo in L1216), and also with replacing all remove_junction and remove_hardlink with Remove-Item xxx -Recurse -Force | Out-Null (with a file existing check in link_current), and gotten no error. Maybe need more test.

PS. I've googled that without -Force may return the error that not in interactive mode. May help.

rasa · 2019-03-13T06:57:39Z

Can most values default, so

  "persist": [
      {
          "name": "config.dat" // type=file, method=link
      },
      {
          "name": "data/" // type=directory, method=link
      }
  ]

works?

niheaven · 2019-03-13T08:12:42Z

Yes, it works, use a regex to judge if it contains "/" or "", as a shorcut of "type".

Ash258 · 2019-03-13T08:17:05Z

No need to use regex. Just "dir/".EndsWith('/') (-or "dir\\".EndsWith('\\') for people like me, who prefer windows like separator) should be enough.

niheaven · 2019-03-13T08:38:53Z

Remark for encoding: Out-File in PowerShell 3-5.1 use "ASCII" as default, and "UTF8" give a UTF-8 with BOM. The same function in PowerShell Core (6) use "UTF8NoBOM" as default, and "UTF8" give a UTF-8 without BOM. So PLEASE AVOID using "UTF8" as file contents encoding. "ASCII" is preferred.

chawyehsu · 2019-03-13T09:06:00Z

AFAIK previously, for restricted users, read-only junctions can interrupt uninstallation, and can not be removed by Remove-Item -r -force(#2832 #2881 #2882 #2890 ). Maybe need more tests under non-admin accounts.

niheaven · 2019-03-13T10:29:03Z

@h404bi Agree. I've tested with an admin account (with and without elevated privilege) and a non-admin test acount, and didn't gotten error with Remove-Item -Recurse -Force. This may need more tests.

Ash258 · 2019-05-04T21:17:46Z

Before Update:
file exists
copy file to persist directory

This is to prevent file overwriting instead of file editing? (mainly some qT apps ScoopInstaller/Extras#2018)

If yes i would suggest this tweak to description:

-Before update
+Before uninstallation

Persist doesn't seem to work properly (see ScoopInstaller/Scoop#3212) for now, so doing it manually Co-Authored-By: Jakub Čábera <[email protected]> Co-Authored-By: linsui <[email protected]>

r15ch13 added 3 commits March 12, 2019 20:36

Add functions for junction and hardlink handling including Tests

2f85050

- Add is_junction() - Add create_junction() - Add remove_junction() - Add create_hardlink() - Add remove_hardlink()

Create backwards compatible persist_data_old() function

2bcf544

- Extract old persist_data() functionality for backwards compatible with old manifests - Introduce new persist_data_new() stub

Add new 'persist'-object representation to schema.json

c2f1e8e

- Deprecates the String representation - Deprecates the StringArray representation - Introduces first version of Object representation

Ash258 reviewed Mar 12, 2019

View reviewed changes

r15ch13 added help-wanted work-in-progress labels Mar 12, 2019

niheaven reviewed Mar 13, 2019

View reviewed changes

niheaven added a commit to niheaven/scoop that referenced this pull request Mar 13, 2019

Apply helper function from ScoopInstaller#3212

f974052

niheaven added a commit to niheaven/scoop that referenced this pull request Mar 13, 2019

Add new schema from ScoopInstaller#3212 with small tweak

f9423d8

niheaven mentioned this pull request Mar 13, 2019

fix(persist): Using [Array] to persist file #3209

Closed

niheaven added a commit to niheaven/scoop that referenced this pull request Mar 14, 2019

Apply helper function from ScoopInstaller#3212

918626a

niheaven added a commit to niheaven/scoop that referenced this pull request Mar 14, 2019

Add new schema from ScoopInstaller#3212 with small tweak

8668d4d

This was referenced Mar 18, 2019

refactor(config): Move config handling to core.ps1 #3242

Merged

patch(persist): Persistence with type [Object] and [Array] #3248

Open

niheaven added a commit to niheaven/scoop that referenced this pull request Mar 28, 2019

Apply helper function from ScoopInstaller#3212

5e7eb60

niheaven added a commit to niheaven/scoop that referenced this pull request Mar 28, 2019

Add new schema from ScoopInstaller#3212 with small tweak

f1bd875

Deide mentioned this pull request Mar 29, 2019

XDG Base Directories #3285

Closed

Ash258 mentioned this pull request Apr 16, 2019

I think the "icons" folder in the "DrFolder" software should be placed in the "persist" folder. Ash258/Shovel-Ash258#17

Closed

niheaven added a commit to niheaven/scoop that referenced this pull request May 13, 2019

Apply helper function from ScoopInstaller#3212

8564fee

niheaven added a commit to niheaven/scoop that referenced this pull request May 13, 2019

Add new schema from ScoopInstaller#3212 with small tweak

49c541f

hoilc mentioned this pull request May 30, 2019

Doublecmd .msi instead portable edition? ScoopInstaller/Extras#2181

Closed

kidonng referenced this pull request in chawyehsu/dorado Jul 22, 2019

pandownload: Fix data persistence

f92c9b1

Ash258 mentioned this pull request Aug 18, 2019

feat(install): Initialize work on manifest helpers #3587

Closed

Ash258 mentioned this pull request Aug 27, 2019

QuickViewer: Add version 1.1.6 ScoopInstaller/Extras#2756

Merged

This was referenced Nov 13, 2019

keepassxc doesn't persist properly ScoopInstaller/Extras#2018

Closed

keepassxc: Persist configuration file ScoopInstaller/Extras#3162

Merged

Ash258 mentioned this pull request Nov 18, 2019

Support creating files in manifest #3752

Closed

mgziminsky mentioned this pull request Jun 18, 2020

die: Add version 2.05 ScoopInstaller/Extras#4295

Closed

zStruCat mentioned this pull request Feb 24, 2022

[Feature] Persist file that yet doestn't exist #4757

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Persistence rework with backward compatibility (persist) #3212

[WIP] Persistence rework with backward compatibility (persist) #3212

r15ch13 commented Mar 12, 2019 •

edited

Loading

Ash258 Mar 12, 2019

r15ch13 Mar 12, 2019

Ash258 commented Mar 12, 2019

niheaven Mar 13, 2019

niheaven Mar 13, 2019 •

edited

Loading

niheaven Mar 13, 2019

niheaven Mar 13, 2019

r15ch13 Mar 13, 2019

niheaven Mar 13, 2019 •

edited

Loading

niheaven Mar 13, 2019

niheaven Mar 13, 2019 •

edited

Loading

rasa commented Mar 13, 2019

niheaven commented Mar 13, 2019

Ash258 commented Mar 13, 2019 •

edited

Loading

niheaven commented Mar 13, 2019

chawyehsu commented Mar 13, 2019 •

edited

Loading

niheaven commented Mar 13, 2019

Ash258 commented May 4, 2019

[WIP] Persistence rework with backward compatibility (persist) #3212

Are you sure you want to change the base?

[WIP] Persistence rework with backward compatibility (persist) #3212

Conversation

r15ch13 commented Mar 12, 2019 • edited Loading

Cases that need to be adressed:

Ash258 Mar 12, 2019

Choose a reason for hiding this comment

r15ch13 Mar 12, 2019

Choose a reason for hiding this comment

Ash258 commented Mar 12, 2019

niheaven Mar 13, 2019

Choose a reason for hiding this comment

niheaven Mar 13, 2019 • edited Loading

Choose a reason for hiding this comment

niheaven Mar 13, 2019

Choose a reason for hiding this comment

niheaven Mar 13, 2019

Choose a reason for hiding this comment

r15ch13 Mar 13, 2019

Choose a reason for hiding this comment

niheaven Mar 13, 2019 • edited Loading

Choose a reason for hiding this comment

niheaven Mar 13, 2019

Choose a reason for hiding this comment

niheaven Mar 13, 2019 • edited Loading

Choose a reason for hiding this comment

rasa commented Mar 13, 2019

niheaven commented Mar 13, 2019

Ash258 commented Mar 13, 2019 • edited Loading

niheaven commented Mar 13, 2019

chawyehsu commented Mar 13, 2019 • edited Loading

niheaven commented Mar 13, 2019

Ash258 commented May 4, 2019

r15ch13 commented Mar 12, 2019 •

edited

Loading

niheaven Mar 13, 2019 •

edited

Loading

niheaven Mar 13, 2019 •

edited

Loading

niheaven Mar 13, 2019 •

edited

Loading

Ash258 commented Mar 13, 2019 •

edited

Loading

chawyehsu commented Mar 13, 2019 •

edited

Loading