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

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

Closed
wants to merge 28 commits into from

Conversation

Ash258
Copy link
Contributor

@Ash258 Ash258 commented Aug 7, 2019

Generic function for dealing with persitence checks as starter for widely used repeated codes.

Manifest for testing
{
    "version": "1.0",
    "url": [
        "https://github.com/sharkdp/bat/releases/download/v0.12.1/bat-v0.12.1-x86_64-pc-windows-msvc.zip",
        "https://raw.githubusercontent.com/lukesampson/scoop-extras/master/scripts/vscode-install-context.reg",
        "https://raw.githubusercontent.com/lukesampson/scoop-extras/master/scripts/vscode-uninstall-context.reg"
    ],
    "pre_install": [
        "    # Nested file with two lines",
        "Test-Persistence 'bin\\nested\\file' -Content 'new', 'line'",
        "    # File4 should be empty",
        "Test-Persistence 'file1', 'file2', 'file3', 'file4' -Content 'new', 'new2', 'new3'",
        "    # Empty",
        "'file5', 'file6', 'file7', 'file8' | Test-Persistence",
        "    # One file with 2 lines",
        "'file9' | Test-Persistence -Content 'alfa', 'new'",
        "Test-Persistence 'alfa.json' -Execution { Write-Host 'This file will not exists', \"[$currentFile]\" -f magenta }",
        "",
        "    # File will be automatically checked against $dir and if exists it will use it.",
        "Edit-File 'vscode-install-context.reg' '$code', '&Code' 'pathToVSCODE.exe', 'Code &Insiders'",
        "Edit-File \"$dir\\vscode-install-context.reg\" 'pathToVSCODE.exe', 'Code &Insiders' 'SECOND REPLACE USING $dir', 'REPLACED AGAIN'",
        "Edit-File 'file9' '^new$' 'completelyNew' -Regex",
        "Edit-File \"$dir\\file9\" 'letely' 'COSI' -Regex",
        "Edit-File 'file9' '^(alfa)$' '${1}NNNNN' -Regex",
        "    # Replace with empty string",
        "Edit-File 'vscode-uninstall-context.reg' 'Windows Registry'",
        "Edit-File \"$dir\\vscode-uninstall-context.reg\" '^\\[-HKEY_.*\\\\command' -Regex",
        "",
        "Remove-AppDirItem 'LICENS*', 'README.md'"
    ]
}

Persistence check

Ditto manifest:

-    "pre_install": [
-        "if (!(Test-Path \"$persist_dir\\Ditto.db\")) {",
-        "    Write-Host 'File Ditto.db does not exists. Creating.' -f Yellow",
-        "    Start-Process -FilePath \"$dir\\Ditto.exe\"",
-        "    while (!(Test-Path \"$dir\\Ditto.db\")) {",
-        "        Start-Sleep -Milliseconds 500",
-        "    }",
-        "    Start-Sleep 1",
-        "    Stop-Process -Name Ditto",
-        "}",
-        "$file = 'Ditto.Settings'",
-        "if (!(Test-Path \"$persist_dir\\$file\")) {",
-        "    Write-Host 'File' $file 'does not exists. Creating.' -f Yellow",
-        "    $CONT = @('[Ditto]', 'DBPath3=Ditto.db')",
-        "    Set-Content \"$dir\\$file\" ($CONT -join \"`r`n\") -Encoding Ascii",
-        "}"
-    ],
+    "pre_install": [
+        "Test-Persistence 'Ditto.db' -Execution {",
+        "    Start-Process -FilePath \"$dir\\Ditto.exe\"",
+        "    while (!(Test-Path \"$dir\\Ditto.db\")) {",
+        "        Start-Sleep -Milliseconds 500",
+        "    }",
+        "    Start-Sleep 1",
+        "    Stop-Process -Name Ditto",
+        "}",
+        "Test-Persistence 'Ditto.Settings' @('[Ditto]', 'DBPath3=Ditto.db')"
+    ],

Java shortcut wrapper

Classyshark:

-    "installer": {
-        "script": "Set-Content \"$dir\\ClassyShark.bat\" (@('@echo off', 'start javaw.exe -jar %~dp0\\ClassyShark.jar %*') -join \"`r`n\") -Encoding ASCII"
-    },
+    "installer": {
+        "script": "New-JavaShortcutWrapper 'ClassyShark'"
+    },

Remove files from $dir

- "Remove-Item \"$dir\\`$*\", \"$dir\\Uninstall*\" -Force -Recurse"
- '$*', 'Uninstall*', 'src' | ForEach-Object { "Remove-Item \"$dir\\$_\" -Force -Recurse }"
- Remove-Item \"$dir\\*\" -Include '$*', 'Uninstall*', 'src' -Force -Recurse
+ "Remove-AppDirItem '$*', 'Uninstall*'"

Import-module is hapenning in main scoop and also in install.ps1 to keep install.ps1 working as standalone module without need to import install.ps1 with all references. (So if you want to import it in external script you can just do . '...\install.ps1' instead gci '...\lib' | % { . $_.Fullname }

- $global: variables are mandatory for function scoped variables to be available in modules.

@niheaven
Copy link
Member

niheaven commented Aug 8, 2019

Is this for some codes that might be widely used in manifests?

@r15ch13
Copy link
Member

r15ch13 commented Aug 16, 2019

Hm, I don't know if we should use global variables. 🤔

I was thinking about adding helper functions like:

Create-NewTextFile -Path "$dir\blah.ini" -Value "blah" -Encoding utf-8 -
Create-NewTextFile -Path "$dir\blah.ini" -Value { @('[global]', '[Software\grepWin\History]') -join "`r`n" } -Encoding utf-8 -
Create-NewTextFile -Path "$dir\blah.conf" -ScriptBlock { easyrsa init-pki }
Create-NewDirectory -Value "$persist_dir\blah"

Each function will report if the file exists or was created.

Maybe most of the pre_install scripts containing Test-Path/New-Item/Set-Content invocations could be declared in JSON?

Something like this:

"files": [
  {
     "name": "blah.ini",
     "path": "$dir",
     "content": "$null",
     "encoding": "utf-8",
     "lineending": "crlf"
  }
],
"directories": [
  {
     "name": "blah",
     "path": "$persist_dir"
  }
]

@Ash258
Copy link
Contributor Author

Ash258 commented Aug 18, 2019

Manifest declaration is not really handy as it is completely unergonomic. Instead of oneliner New-item / set-content / ... you will have 5 lines of json object.

For global i will see what I can do about this.

@Deide
Copy link

Deide commented Aug 18, 2019

Is there ever a reason to create files (in pre_install or what have you) that are not meant for persistence? Ie. would this not be covered by the proposed persistence schema updates?

@Ash258
Copy link
Contributor Author

Ash258 commented Aug 18, 2019

Proposed persisted schema solution in #3212 does not cover every use case. New scheme cover only Static and deterministic content. When you need to setup something complex you need to use use persist check manually. As example ditto and execution of it's executable to create db.

@Ash258
Copy link
Contributor Author

Ash258 commented Aug 18, 2019

@r15ch13 Tried with environemnt variables as they are only strings all the time and then prevent leakage of them again.

@Deide
Copy link

Deide commented Aug 18, 2019

I see what you mean. I was thinking the ditto.db could be created with New-Item somehow

Though now I wonder if it might not be be a good idea to have that whole function as some persistence field. Something along the lines of runProgramIfNotPresent, but with a better name; a lot of applications generate their configs and some data files at first run so it might be a good way to deal with all those pre_install scripts easily.

You think this would be too jarring for some applications?

@Ash258
Copy link
Contributor Author

Ash258 commented Aug 18, 2019

No. Keeping it within properties will make things worse. There is no reason to ditch one line powershell code and distribute it into complex and misleading properties.

As lots of configurations files exists and needs to be edited manually (HoneyView) or copied (cmder) This generic function will keep manifests code clean and easy to write / maintain instead of thinking about complex properties structure with mutually exclusive properties to deal with.

#3212 Will take care of empty / simply / static / files creations and other will be handled using helpers. Easy, maintainable, no need to look for reference.

@Deide
Copy link

Deide commented Aug 18, 2019

I see your point, but I was actually referring to replacing 11 lines of Powershell with one manifest property. And instead of a new property itself, it could even be relegated to a new value for Method.

Like:

"persist": [
    {
        "type": "file",
        "name": "ditto.db",
        "method": "run ditto.exe"
    },
    ...
]

In the end, it's probably not a good trade-off to base/normalise a whole feature around some edge cases, and I too am not that sure "run ditto.exe" is all that intuitive. Thought it worth discussing anyway.

@Ash258
Copy link
Contributor Author

Ash258 commented Aug 18, 2019

Best possible solution is to get varibles from session state. It will require to manually register all needed variables explicitly, but it is not using global, object could be saved as well, not leaking anything to upper scope and it is safe.

@linsui
Copy link
Contributor

linsui commented Oct 22, 2019

How about vbs instead of bat for Java shortcut wrapper? There will be a black popup when run bat.

@Ash258
Copy link
Contributor Author

Ash258 commented Nov 18, 2019

@linsui Show me code

Added some additonal checks for requirements. Module usage was depricated due to context problem (TLDR; It was not possible to use core functions in Execution block of Test-Persistence, which is really not user friendly). This PR now require #3721 and follow ups on #3748. I added abort functions just as temporary reminder to replace it with throw.

Manifest for testing
{
    "version": "1.0",
    "url": "https://github.com/sharkdp/bat/releases/download/v0.12.1/bat-v0.12.1-x86_64-pc-windows-msvc.zip",
    "pre_install": [
        "    # Nested file with two lines",
        "Test-Persistence 'bin\\nested\\file' -Content 'new', 'line'",
        "    # File4 should be empty",
        "Test-Persistence 'file1', 'file2', 'file3', 'file4' -Content 'new', 'new2', 'new3'",
        "    # Empty",
        "'file5', 'file6', 'file7', 'file8' | Test-Persistence",
        "    # One file with 2 lines",
        "'file9' | Test-Persistence -Content 'alfa', 'new'",
        "Test-Persistence 'alfa.json' -Execution { Write-Host 'This file will not exists' }",
        "Assert-Administrator",
        "Assert-WindowsVersion '10'",
        "Assert-DotNetFramework '4.5'"
    ],
    "uninstaller": {
        "script": [
            "Assert-Administrator",
            "Write-Host 'After assert'"
        ]
    }
}

@linsui
Copy link
Contributor

linsui commented Nov 18, 2019

Here is a example. I don't know how to make it work for cli, but it works well for gui. So I use vbs in shortcuts and bat in bin.

example
"pre_install": [
        "Expand-7zipArchive \"$dir\\$fname\" -Switches 'icons\\xxhdpi\\icon.png'",
        "Set-Content \"$dir\\RunXDM.bat\" 'start javaw -Xmx1024m -jar %~dp0\\xdman.jar %*' -Encoding ASCII",
        "Set-Content \"$dir\\RunXDM.vbs\" \"CreateObject(`\"WScript.Shell`\").Run `\"javaw -Xmx1024m -jar xdman.jar`\",0\" -Encoding ASCII",
        "$stream = [System.IO.File]::OpenWrite(\"$dir\\icon.ico\")",
        "$bitmap = [System.Drawing.Image]::FromFile(\"$dir\\icons\\xxhdpi\\icon.png\")",
        "([System.Drawing.Icon]::FromHandle($bitmap.GetHicon())).Save($stream)"
    ],
    "bin": "RunXDM.bat",
    "shortcuts": [
        [
            "RunXDM.vbs",
            "Xtreme Download Manager",
            "",
            "icon.ico"
        ]
    ],

Set-Content -LiteralPath (Join-Path $dir "$f.bat") -Value "@start javaw.exe -jar `"%~dp0$f.jar`" %*" -Encoding Ascii -Force

Why force is needed here?

@Ash258
Copy link
Contributor Author

Ash258 commented Dec 31, 2019

I cleanuped some more WIP helpers and kept only basic ones.

I will add more of them in other PR. As basic this is enough

@linsui VBS has not way how to cleanly pass parameters in universal way. Parameter handling have to be done manually, which is completely non future proof.

@Ash258 Ash258 mentioned this pull request Aug 21, 2020
21 tasks
Ash258 added a commit to Ash258/Scoop-Core that referenced this pull request Aug 21, 2020
@Ash258 Ash258 closed this Mar 8, 2022
@Ash258 Ash258 deleted the helpers branch March 8, 2022 05:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants