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

(spotify) Remove Windows Store version #2263

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 23 additions & 5 deletions automatic/spotify/tools/ChocolateyInstall.ps1
Copy link
Member

Choose a reason for hiding this comment

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

Please fix the indentation used on the lines changed to follow the editor config file.
The editor config file specifies that indentations should only be two spaces.

Copy link
Member Author

@pauby pauby Aug 29, 2023

Choose a reason for hiding this comment

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

The indentation is inconsistent. I'll update the file to match 2 spaces.

Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,24 @@ $arguments = @{
validExitCodes = @(0, 1641, 3010)
}

# check we can use Get-AppxPackage
if (Get-Command 'Get-AppxPackage' -ErrorAction SilentlyContinue) {
# there is likely going to be two packages returned for x86 and x64.
# we don't care about the architecture, just the version and they will both be the same.
$allAppxPackages = Get-AppxPackage
$installedAppXPackage = @($allAppxPackages | Where-Object -Property Name -eq 'SpotifyAB.SpotifyMusic')
if ($installedAppXPackage.Count -gt 0) {
if ($env:ChocolateyForce) {
#when you remove a package, you don't remove it per architecture. You just remove it for all architectures.
Write-Warning 'Attempting to remove Spotify installed from the Microsoft Store.'
Remove-AppxPackage -Package $installedAppXPackage[0].PackageFullName
Copy link
Member

Choose a reason for hiding this comment

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

IMO, we should not remove the Microsoft Store application in this package.

It is still two different applications, which goes against having one software for each package.

IMO, I think we should throw an exception instead, no matter what, possibly with details on how the user can uninstall the application manually or add a new package that only has the purpose of removing the Store version of Spotify (that we possibly can depend on in this package).

Copy link
Member Author

Choose a reason for hiding this comment

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

A package parameter? Warn them but if they add the package parameter it also uninstalls. Seems like the best of both worlds.

Copy link
Member

Choose a reason for hiding this comment

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

hmm. Add it behind a package parameter 🤔.

Yeah, could work as well. Let's go with that instead. I still do think a separate package could be beneficial though, but we can go with the parameter for now.

}
else {
throw "Cannot install the Spotify package because the Microsft Store version is installed. Please uninstall this manually or add the '--force' option to the command line."
}
}
}

# Download the installer
$arguments['file'] = Get-ChocolateyWebFile @arguments

Expand All @@ -24,10 +42,10 @@ schtasks.exe /Delete /TN $arguments['packageName'] /F
# Wait for Spotify to start, then kill it
$done = $false
do {
if (Get-Process Spotify -ErrorAction SilentlyContinue) {
Stop-Process -name Spotify
$done = $true
}
if (Get-Process Spotify -ErrorAction SilentlyContinue) {
Stop-Process -name Spotify
$done = $true
}

Start-Sleep -s 10
Start-Sleep -s 10
} until ($done)