Skip to content

(#2398) Restore PowerShell v2 support #2411

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

Merged
merged 1 commit into from
Oct 21, 2021

Conversation

corbob
Copy link
Member

@corbob corbob commented Oct 20, 2021

Description Of Changes

Commit c408d12 introduced a change to fix a bug with the $env:TEMP variable. As well, commit f68a242 introduced handling of msps in the install helper.

However, they both introduced PowerShell 3+ syntax. This changes those to work with PowerShell 2+.

Motivation and Context

We currently support PowerShell 2+, so it's important that the PowerShell uses v2 compatible commands.

What Have I Done To Test This

This one is specific to PowerShell 2 and the import of the helpers\chocolateyInstaller.psm1. It can be tested either on a Windows 7 with PowerShell 2 system (this is a multi-hour waiting game to build and update a system if you don't already have one), or on a newer Windows system with the PowerShell 2 compatibility feature enabled by running powershell -version 2.

From PowerShell 2 run: Import-Module src\chocolatey.resources\helpers\chocolateyInstaller.psm1 -force from the root of the repository. This should result in no errors and no output.

Change Types Made

  • Bug fix (non-breaking change)
  • Feature / Enhancement (non-breaking change)
  • Breaking change (fix or feature that could cause existing functionality to change)

Related Issue

Fix #2398

Change Checklist

  • Requires a change to the documentation
  • Documentation has been updated
  • Tests to cover my changes, have been added
  • All new and existing tests passed.

@corbob corbob force-pushed the 2398_PS2_support branch 2 times, most recently from de7d1a8 to 71e84f9 Compare October 20, 2021 21:51
@corbob corbob marked this pull request as ready for review October 20, 2021 21:54
@corbob
Copy link
Member Author

corbob commented Oct 20, 2021

This was initially in draft as I found the second commit while doing up the testing for it as I found it was still giving the error after fixing the first one. It's ready now.

@corbob corbob requested a review from gep13 October 20, 2021 21:57
@pauby pauby requested review from pauby and AdmiringWorm and removed request for gep13 October 21, 2021 11:23
@gep13 gep13 changed the base branch from master to hotfix/0.11.3 October 21, 2021 20:42
Copy link
Member

@gep13 gep13 left a comment

Choose a reason for hiding this comment

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

LGTM!

@vexx32
Copy link
Member

vexx32 commented Oct 21, 2021

@corbob is there a reason you used -inotcontains instead of -notcontains for one of these? 🤔

@gep13
Copy link
Member

gep13 commented Oct 21, 2021

@vexx32 that is a great question!

@gep13
Copy link
Member

gep13 commented Oct 21, 2021

Based on this:

https://iannotes.wordpress.com/2011/12/25/powershell-comparison-operators/

They are equivalent, but it would be better to have them the same, right?

@vexx32
Copy link
Member

vexx32 commented Oct 21, 2021

Yep, they should be equivalent, but using different ones might confuse maintainers coming back to these code paths in the future.

@corbob
Copy link
Member Author

corbob commented Oct 21, 2021

Oh duh. For some reason I was thinking -notcontains would default to case sensitive, and for the one where we're comparing username it seemed reasonable to force it to case insensitive. Given that they're both the same, I'll change it back to -notcontains for the consistency.

PowerShell v2 doesn't recognize `-notin` as it was introduced in v3. This
changes to use `-notcontains` which was introduced in v2 and works similar
to `-notin`.
@corbob
Copy link
Member Author

corbob commented Oct 21, 2021

There we go, should be updated now 😄

Copy link
Member

@gep13 gep13 left a comment

Choose a reason for hiding this comment

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

LGTM!

@gep13 gep13 merged commit 9cc2419 into chocolatey:hotfix/0.11.3 Oct 21, 2021
@gep13
Copy link
Member

gep13 commented Oct 21, 2021

@corbob thanks for getting this fixed!

@corbob corbob deleted the 2398_PS2_support branch November 5, 2021 04:32
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.

Chocolateys PowerShell Installer Module uses non PowerShell 2 compatible syntax
3 participants