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

fix(system): $dir parameter passing to Split-PathLikeEnvVar at strip_path with -Pattern switch #5937

Merged
merged 3 commits into from
May 6, 2024

Conversation

Capella87
Copy link
Contributor

@Capella87 Capella87 commented May 2, 2024

Description

Correct parameter's name, -Name to -Pattern. Corresponds to the new parameter name -Pattern for the Split-PathLikeEnvVar function.

Motivation and Context

Function Test-PathLikeEnvVar is changed to Split-PathLikeEnvVar recently, which also updated to provide -Pattern parameter with string array type to the function. But strip_path, which is now deprecated function and implemented to invoke warning and Split-PathLikeEnvVar function with $dir and $orig_path parameters, is still passing $dir parameter as -Name to the newer function. So $Pattern has $null value and causes null-valued expression error.

error

As we can see, the function could not receive the value to $Pattern correctly.

The error was that:

WARN  "env" will be deprecated. Please change your code/manifest to use "Get-EnvVar"
      -> :4:38
WARN  "strip_path" will be deprecated. Please change your code/manifest to use "Split-PathLikeEnvVar"
      -> :4:26
InvalidOperation: C:\Users\[redacted]\scoop\apps\scoop\current\lib\system.ps1:85
Line |
  85 |          $splitPattern = $Pattern.Split(';', [System.StringSplitOption|          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | You cannot call a method on a null-valued expression.

I discovered this issue when I invoked scoop update * to try to update SDL2, null-valued expression errors were occurred.

I fixed split_path function to pass the parameter as -Pattern correctly. Moreover, add casting related code, that's because Split-PathLikeEnvVar requires $Pattern as a string array, not a single string, It would be better to explicitly change $dir to an array.

I believe that we can troubleshoot this problem by updating strip_path function for now, we should also update manifests not to use deprecated functions like this ultimately.

Before:

function strip_path($orig_path, $dir) {
    Show-DeprecatedWarning $MyInvocation 'Split-PathLikeEnvVar'
    Split-PathLikeEnvVar -Name $dir -Path $orig_path
}

After:

function strip_path($orig_path, $dir) {
    Show-DeprecatedWarning $MyInvocation 'Split-PathLikeEnvVar'
    if ($dir -is [string]) {
        $dir = @($dir)
    }
    Split-PathLikeEnvVar -Pattern $dir -Path $orig_path
}

How Has This Been Tested?

Tested on both Windows 10 (22H2) and Windows 11(23H2)

  • Before:
PowerShell 7.4.2
Loading personal and system profiles took 828ms.
PS> scoop install extras/sdl2
Installing 'sdl2' (2.30.3) [64bit] from 'extras' bucket
Loading SDL2-devel-2.30.3-VC.zip from cache.
Loading SDL2-2.30.3.zip from cache.
Checking hash of SDL2-devel-2.30.3-VC.zip ... ok.
Checking hash of SDL2-2.30.3.zip ... ok.
Extracting SDL2-devel-2.30.3-VC.zip ... done.
Extracting SDL2-2.30.3.zip ... done.
Running installer script...
WARN  "env" will be deprecated. Please change your code/manifest to use "Get-EnvVar"
      -> :33:32
WARN  "strip_path" will be deprecated. Please change your code/manifest to use "Split-PathLikeEnvVar"
      -> :33:20
InvalidOperation: C:\Users\[redacted]\scoop\apps\scoop\current\lib\system.ps1:85
Line |
  85 |          $splitPattern = $Pattern.Split(';', [System.StringSplitOption …
     |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | You cannot call a method on a null-valued expression.
WARN  "env" will be deprecated. Please change your code/manifest to use "Set-EnvVar"
      -> :34:1
WARN  "env" will be deprecated. Please change your code/manifest to use "Get-EnvVar"
      -> :35:32
WARN  "strip_path" will be deprecated. Please change your code/manifest to use "Split-PathLikeEnvVar"
      -> :35:20
InvalidOperation: C:\Users\[redacted]\scoop\apps\scoop\current\lib\system.ps1:85
Line |
  85 |          $splitPattern = $Pattern.Split(';', [System.StringSplitOption …
     |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | You cannot call a method on a null-valued expression.
WARN  "env" will be deprecated. Please change your code/manifest to use "Set-EnvVar"
      -> :36:1
WARN  "strip_path" will be deprecated. Please change your code/manifest to use "Split-PathLikeEnvVar"
      -> :38:31
InvalidOperation: C:\Users\[redacted]\scoop\apps\scoop\current\lib\system.ps1:85
Line |
  85 |          $splitPattern = $Pattern.Split(';', [System.StringSplitOption …
     |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | You cannot call a method on a null-valued expression.
WARN  "strip_path" will be deprecated. Please change your code/manifest to use "Split-PathLikeEnvVar"
      -> :40:33
InvalidOperation: C:\Users\[redacted]\scoop\apps\scoop\current\lib\system.ps1:85
Line |
  85 |          $splitPattern = $Pattern.Split(';', [System.StringSplitOption …
     |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | You cannot call a method on a null-valued expression.
Linking ~\scoop\apps\sdl2\current => ~\scoop\apps\sdl2\2.30.3
'sdl2' (2.30.3) was installed successfully!

PS> scoop uninstall sdl2 --purge
Uninstalling 'sdl2' (2.30.3).
Running uninstaller script...
WARN  "env" will be deprecated. Please change your code/manifest to use "Get-EnvVar"
      -> :4:38
WARN  "strip_path" will be deprecated. Please change your code/manifest to use "Split-PathLikeEnvVar"
      -> :4:26
InvalidOperation: C:\Users\[redacted]\scoop\apps\scoop\current\lib\system.ps1:85
Line |
  85 |          $splitPattern = $Pattern.Split(';', [System.StringSplitOption …
     |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | You cannot call a method on a null-valued expression.
WARN  "env" will be deprecated. Please change your code/manifest to use "Get-EnvVar"
      -> :9:38
WARN  "strip_path" will be deprecated. Please change your code/manifest to use "Split-PathLikeEnvVar"
      -> :9:26
InvalidOperation: C:\Users\[redacted]\scoop\apps\scoop\current\lib\system.ps1:85
Line |
  85 |          $splitPattern = $Pattern.Split(';', [System.StringSplitOption …
     |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | You cannot call a method on a null-valued expression.
WARN  "strip_path" will be deprecated. Please change your code/manifest to use "Split-PathLikeEnvVar"
      -> :15:26
InvalidOperation: C:\Users\[redacted]\scoop\apps\scoop\current\lib\system.ps1:85
Line |
  85 |          $splitPattern = $Pattern.Split(';', [System.StringSplitOption …
     |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | You cannot call a method on a null-valued expression.
WARN  "strip_path" will be deprecated. Please change your code/manifest to use "Split-PathLikeEnvVar"
      -> :17:26
InvalidOperation: C:\Users\[redacted]\scoop\apps\scoop\current\lib\system.ps1:85
Line |
  85 |          $splitPattern = $Pattern.Split(';', [System.StringSplitOption …
     |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | You cannot call a method on a null-valued expression.
Unlinking ~\scoop\apps\sdl2\current
Removing persisted data.
'sdl2' was uninstalled.
  • After my fix:
PS C:\Users\[redacted]> scoop install sdl2
Installing 'sdl2' (2.30.3) [64bit] from 'extras' bucket
Loading SDL2-devel-2.30.3-VC.zip from cache
Checking hash of SDL2-devel-2.30.3-VC.zip ... ok.
Loading SDL2-2.30.3.zip from cache
Checking hash of SDL2-2.30.3.zip ... ok.
Extracting SDL2-devel-2.30.3-VC.zip ... done.
Extracting SDL2-2.30.3.zip ... done.
Running installer script...
WARN  "env" will be deprecated. Please change your code/manifest to use "Get-EnvVar"
      -> :33:32
WARN  "strip_path" will be deprecated. Please change your code/manifest to use "Split-PathLikeEnvVar"
      -> :33:20
WARN  "env" will be deprecated. Please change your code/manifest to use "Set-EnvVar"
      -> :34:1
WARN  "env" will be deprecated. Please change your code/manifest to use "Get-EnvVar"
      -> :35:32
WARN  "strip_path" will be deprecated. Please change your code/manifest to use "Split-PathLikeEnvVar"
      -> :35:20
WARN  "env" will be deprecated. Please change your code/manifest to use "Set-EnvVar"
      -> :36:1
WARN  "strip_path" will be deprecated. Please change your code/manifest to use "Split-PathLikeEnvVar"
      -> :38:31
WARN  "strip_path" will be deprecated. Please change your code/manifest to use "Split-PathLikeEnvVar"
      -> :40:33
Linking ~\scoop\apps\sdl2\current => ~\scoop\apps\sdl2\2.30.3
'sdl2' (2.30.3) was installed successfully!
PS C:\Users\[redacted]> scoop uninstall sdl2 --purge
Uninstalling 'sdl2' (2.30.3).
Running uninstaller script...
WARN  "env" will be deprecated. Please change your code/manifest to use "Get-EnvVar"
      -> :4:38
WARN  "strip_path" will be deprecated. Please change your code/manifest to use "Split-PathLikeEnvVar"
      -> :4:26
Removing ~\scoop\apps\sdl2\current\lib\pkgconfig from your path.
WARN  "env" will be deprecated. Please change your code/manifest to use "Set-EnvVar"
      -> :7:5
WARN  "env" will be deprecated. Please change your code/manifest to use "Get-EnvVar"
      -> :9:38
WARN  "strip_path" will be deprecated. Please change your code/manifest to use "Split-PathLikeEnvVar"
      -> :9:26
Removing ~\scoop\apps\sdl2\current from your path.
WARN  "env" will be deprecated. Please change your code/manifest to use "Set-EnvVar"
      -> :12:5
WARN  "strip_path" will be deprecated. Please change your code/manifest to use "Split-PathLikeEnvVar"
      -> :15:26
WARN  "strip_path" will be deprecated. Please change your code/manifest to use "Split-PathLikeEnvVar"
      -> :17:26
Unlinking ~\scoop\apps\sdl2\current
Removing persisted data.
'sdl2' was uninstalled.

Checklist:

  • I have read the Contributing Guide.
  • I have ensured that I am targeting the develop branch.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.
  • I have added an entry in the CHANGELOG.

@Capella87 Capella87 changed the title Fix $dir parameter passing to Split-PathLikeEnvVar at strip_path with -Pattern switch fix(system): Fix $dir parameter passing to Split-PathLikeEnvVar at strip_path with -Pattern switch May 3, 2024
@Capella87 Capella87 changed the title fix(system): Fix $dir parameter passing to Split-PathLikeEnvVar at strip_path with -Pattern switch fix(system): $dir parameter passing to Split-PathLikeEnvVar at strip_path with -Pattern switch May 3, 2024
CHANGELOG.md Outdated Show resolved Hide resolved
lib/system.ps1 Outdated Show resolved Hide resolved
Capella87 and others added 2 commits May 5, 2024 19:00
Co-authored-by: Hsiao-nan Cheung <niheaven@gmail.com>
Update the title to the suggested one.

Co-authored-by: Hsiao-nan Cheung <niheaven@gmail.com>
@Capella87
Copy link
Contributor Author

Capella87 commented May 5, 2024

I've added commits by your suggestions. Thank you!

I am wondering if I could ask you a question? I've made changes in several manifests to remove deprecated functions including strip_path, and I am about to write requests on ScoopInstaller/Extras. At least 6 manifests are updated. May I create an integrated pull requests about these changes over that manifests?

@niheaven
Copy link
Member

niheaven commented May 6, 2024

Yes, of cause. Please make one PR for all manifests.

@niheaven niheaven merged commit 5b86c30 into ScoopInstaller:develop May 6, 2024
2 checks passed
niheaven added a commit that referenced this pull request May 13, 2024
…ecated `strip_path()` (#5937)

Co-authored-by: Hsiao-nan Cheung <niheaven@gmail.com>
niheaven added a commit that referenced this pull request May 13, 2024
…ecated `strip_path()` (#5937)

Co-authored-by: Hsiao-nan Cheung <niheaven@gmail.com>
brian6932 pushed a commit to brian6932/Scoop that referenced this pull request Jun 1, 2024
…ecated `strip_path()` (ScoopInstaller#5937)

Co-authored-by: Hsiao-nan Cheung <niheaven@gmail.com>
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.

None yet

2 participants