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

DRY: Address New-Item calls in pre_install/post_install scripts #3166

Open
r15ch13 opened this issue Feb 24, 2019 · 6 comments
Open

DRY: Address New-Item calls in pre_install/post_install scripts #3166

r15ch13 opened this issue Feb 24, 2019 · 6 comments

Comments

@r15ch13
Copy link
Member

r15ch13 commented Feb 24, 2019

Should we address the repeated calls of New-Item in many pre_install/post_install scripts to keep them DRY? 😄
Most of them run Test-Path, display a message and create a directory or a file with no content.

These scripts could be cleanup up and be more readable in JSON format by using some simple functions like:

New-Directory -Path "$dir\conf.d" -Message "Creating 'conf.d' directory"
New-JsonFile -Path "$dir\bla.json" -Message "Creating 'bla.json' for ..." -Content "{ "bla": "blubb" }" -Encoding "utf-8"
New-EmptyFile -Path "$dir\empty.ini"
@Ash258
Copy link
Contributor

Ash258 commented Feb 24, 2019

Relate #2733

lib\ManifestHelpers with some usual persist checks would be nice. Documented, well named powershell module, which will be dot sourced in libexec-install or install.ps1
I would prefix it with Scoop- to preserve system wide naming / Aliases, ...

@r15ch13
Copy link
Member Author

r15ch13 commented Feb 24, 2019

One step ahead of me 😁
And I even answered your ticket ... wow, I am forgetful ...

@r15ch13
Copy link
Member Author

r15ch13 commented Feb 24, 2019

Import-Module supports prefixes, so we could use this instead of dot sourcing.

Import-Module -Name .\lib\ManifestHelpers.psm1 -prefix Scoop -Verbose

image

Example module lib\ManifestHelpers.psm1:

function Expand-7zip {
    <#
    .SYNOPSIS
    SOME DESCRIPTION
    ...
    #>
    param(
        [String] $Archive,
        [String] $Destination = '',
        [Switch] $Recurse
    )
    process {
        extract_7zip $Archive $Destination $Recurse
    }
}

function Expand-Msi {
    <#
    .SYNOPSIS
    SOME DESCRIPTION
    ...
    #>
    param(
        [String] $Archive,
        [String] $Destination = '',
        [Switch] $UseLessMsi
    )
    process {
        if ($UseLessMsi) {
            extract_lessmsi $Archive $Destination
        } else {
            extract_msi $Archive $Destination
        }
    }
}

function New-Directory {
    <#
    .SYNOPSIS
    SOME DESCRIPTION
    ...
    #>
    param(
        [String] $Path,
        [String] $Name,
        [String] $Message = ''
    )
    process {
        if (!(Test-Path (Join-Path -Path $Path -ChildPath $Name))) {
            if ($Message) {
                Write-Host $Message -f Yellow
            }
            New-Item -Path $Path -Name $name -ItemType Directory | Out-Null
        }
    }
}

function New-File {
    <#
    .SYNOPSIS
    SOME DESCRIPTION
    ...
    #>
    param(
        [String] $Path,
        [String] $Name,
        [String] $Encoding = 'utf-8',
        [String] $Content = $null
    )
    process {
        $file = Join-Path -Path $Path -ChildPath $Name
        if (!(Test-Path $file)) {
            if($Encoding -eq 'byte') {
                if((Get-Command Set-Content).parameters.ContainsKey('AsByteStream')) {
                    # PowerShell Core (6.0+) '-Encoding byte' is replaced by '-AsByteStream'
                    Set-Content -Path $file -AsByteStream -Value $Content
                }
                else {
                    Set-Content -Path $file -Encoding byte -Value $Content
                }
            } else {
                Set-Content -Path $file -Encoding $Encoding -Value $Content
            }
        }
    }
}

function Write-File {
    <#
    .SYNOPSIS
    SOME DESCRIPTION
    ...
    #>
    param(
        [String] $Path,
        [String] $Name,
        [String] $Encoding = 'utf-8',
        [String] $Content = $null
    )
    process {
        $file = Join-Path -Path $Path -ChildPath $Name
        if ((Test-Path $file)) {
            if($Encoding -eq 'byte') {
                if((Get-Command Set-Content).parameters.ContainsKey('AsByteStream')) {
                    # PowerShell Core (6.0+) '-Encoding byte' is replaced by '-AsByteStream'
                    Set-Content -Path $file -AsByteStream -Value $Content
                }
                else {
                    Set-Content -Path $file -Encoding byte -Value $Content
                }
            } else {
                Set-Content -Path $file -Encoding $Encoding -Value $Content
            }
        }
    }
}

@Ash258
Copy link
Contributor

Ash258 commented Feb 24, 2019

I like the idea of proper and powershell native code style, everything which make code more readable and reusable is awesome. But it should be adopted in whole scoop codebase or ditched. It's really mess when all code is dot sourced and just one module is real "module" with powershell standart codebase.

If this start some real Scoop refactor to adopt powershell standart and best practise, then Go for it.

Best would be to extract bucket folder, which will allow better maintanence of whole Scoop with less error domain.

So best scenario:

  1. Add this module and let know maintaners about adopting it
  2. Extract bucket / supportings
  3. Refactor install
  4. Rest of code adoption to new codebase with some beautifier / linter (PSSA 1.18 will bring all time needed rule for Correct Casing, which would save lot's of manual code edit. Add new powershell.useCorrectCasingsettings for new rule in PSSA 1.18: PSUseCorrectCasing PowerShell/vscode-powershell#1687, Add 'UseCorrectCasing' formatting rule for cmdlet/function name PowerShell/PSScriptAnalyzer#1117)
    • This will be long term goal with some core refactor and deduplication / optimalization of code.

@chawyehsu
Copy link
Member

Holy cow! Extracting core bucket always be the first work.

@Ash258
Copy link
Contributor

Ash258 commented Feb 26, 2019

Not always. For some hotfixes and small impact fixes it is not needed, but for future bigger refactoring / extending it's really crucial.

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

No branches or pull requests

3 participants