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 Windows reinstall/uninstall issue #199

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
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
2 changes: 0 additions & 2 deletions features/uninstall-extensions.feature
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
Feature: Extensions can be uninstalled with PIE

# See https://github.com/php/pie/issues/190 for why this is non-Windows
@non-windows
Example: An extension can be uninstalled
Given an extension was previously installed
When I run a command to uninstall an extension
Expand Down
11 changes: 0 additions & 11 deletions src/Command/UninstallCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
namespace Php\Pie\Command;

use Composer\Composer;
use Composer\Util\Platform;
use Php\Pie\ComposerIntegration\ComposerIntegrationHandler;
use Php\Pie\ComposerIntegration\PieComposerFactory;
use Php\Pie\ComposerIntegration\PieComposerRequest;
Expand Down Expand Up @@ -54,16 +53,6 @@ public function configure(): void

public function execute(InputInterface $input, OutputInterface $output): int
{
if (Platform::isWindows()) {
/**
* @todo add support for uninstalling in Windows - see
* {@link https://github.com/php/pie/issues/190} for details
*/
$output->writeln('<comment>Uninstalling extensions on Windows is not currently supported.</comment>');

return 1;
}

if (! TargetPlatform::isRunningAsRoot()) {
$output->writeln('This command may need elevated privileges, and may prompt you for your password.');
}
Expand Down
1 change: 1 addition & 0 deletions src/Downloading/DownloadUrlMethod.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use Php\Pie\Platform\TargetPlatform;
use Php\Pie\Platform\WindowsExtensionAssetName;

/** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */
enum DownloadUrlMethod: string
{
case ComposerDefaultDownload = 'composer-default';
Expand Down
1 change: 1 addition & 0 deletions src/File/SudoCreate.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use function is_writable;
use function touch;

/** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */
final class SudoCreate
{
public static function file(string $filename): void
Expand Down
10 changes: 9 additions & 1 deletion src/File/SudoUnlink.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Php\Pie\File;

use Composer\Util\Platform;
use Php\Pie\Util\CaptureErrors;
use Php\Pie\Util\Process;
use Symfony\Component\Process\Exception\ProcessFailedException;
Expand All @@ -12,6 +13,7 @@
use function is_writable;
use function unlink;

/** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */
final class SudoUnlink
{
public static function singleFile(string $filename): void
Expand All @@ -20,7 +22,7 @@ public static function singleFile(string $filename): void
return;
}

if (is_writable($filename)) {
if (! Platform::isWindows() && is_writable($filename)) {
$capturedErrors = [];
$unlinkSuccessful = CaptureErrors::for(
static fn () => unlink($filename),
Expand All @@ -34,6 +36,12 @@ public static function singleFile(string $filename): void
return;
}

if (Platform::isWindows()) {
WindowsDelete::usingMoveToTemp($filename);

return;
}

if (! Sudo::exists()) {
throw FailedToUnlinkFile::fromNoPermissions($filename);
}
Expand Down
47 changes: 47 additions & 0 deletions src/File/WindowsDelete.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php

declare(strict_types=1);

namespace Php\Pie\File;

use Php\Pie\Util\Process;
use RuntimeException;

use function file_exists;
use function sprintf;
use function sys_get_temp_dir;
use function tempnam;

/** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */
final class WindowsDelete
{
/**
* Windows will hold a lock on the DLL, so we can't actually replace or
* delete the extension. However, Windows lets us move it out the way,
* so move it to the temp directory. It can then be cleaned up later.
*/
public static function usingMoveToTemp(string $filename): void
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note: not ideal, as it leaves a dangling temp file, but not sure a way around the lock on the file. Any suggestions to improve this are welcomed!

{
if (! file_exists($filename)) {
return;
}

$newLockedExtFilename = tempnam(sys_get_temp_dir(), 'pie');
if ($newLockedExtFilename === false) {
throw new RuntimeException(sprintf(
'Failed to create a temporary name for moving %s',
$filename,
));
}

Process::run(['move', $filename, $newLockedExtFilename]);

if (file_exists($filename)) {
throw new RuntimeException(sprintf(
'Failed to move %s to %s',
$filename,
$newLockedExtFilename,
));
}
}
}
2 changes: 1 addition & 1 deletion src/Installing/Ini/RemoveIniEntryWithFileGetContents.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ static function (string $path) use ($additionalIniDirectory): bool {
}

$regex = sprintf(
'/^(%s\w*=\w*%s)/m',
'/^(%s\s*=\s*%s)/m',
$package->extensionType() === ExtensionType::PhpModule ? 'extension' : 'zend_extension',
$package->extensionName()->name(),
);
Expand Down
18 changes: 5 additions & 13 deletions src/Installing/WindowsInstall.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
namespace Php\Pie\Installing;

use Php\Pie\Downloading\DownloadedPackage;
use Php\Pie\ExtensionType;
use Php\Pie\File\BinaryFile;
use Php\Pie\File\WindowsDelete;
use Php\Pie\Platform\TargetPlatform;
use Php\Pie\Platform\WindowsExtensionAssetName;
use RecursiveDirectoryIterator;
Expand All @@ -21,7 +21,6 @@
use function file_exists;
use function is_file;
use function mkdir;
use function sprintf;
use function str_replace;
use function strlen;
use function substr;
Expand Down Expand Up @@ -79,17 +78,6 @@ public function __invoke(
$output->writeln('<info>Copied extras:</info> ' . $destinationPathname);
}

/**
* @link https://github.com/php/pie/issues/20
*
* @todo this should be improved in future to try to automatically set up the ext
*/
$output->writeln(sprintf(
'<comment>You must now add "%s=%s" to your php.ini</comment>',
$downloadedPackage->package->extensionType() === ExtensionType::PhpModule ? 'extension' : 'zend_extension',
$downloadedPackage->package->extensionName()->name(),
));

$binaryFile = BinaryFile::fromFileWithSha256Checksum($destinationDllName);

($this->setupIniFile)(
Expand Down Expand Up @@ -124,6 +112,10 @@ private function copyExtensionDll(TargetPlatform $targetPlatform, DownloadedPack
$destinationDllName = $targetPlatform->phpBinaryPath->extensionPath() . DIRECTORY_SEPARATOR
. 'php_' . $downloadedPackage->package->extensionName()->name() . '.dll';

if (file_exists($destinationDllName)) {
WindowsDelete::usingMoveToTemp($destinationDllName);
}

if (! copy($sourceDllName, $destinationDllName) || ! file_exists($destinationDllName) && ! is_file($destinationDllName)) {
throw new RuntimeException('Failed to install DLL to ' . $destinationDllName);
}
Expand Down