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

Add new patcher: git 'init' patcher #471

Merged
merged 4 commits into from
Feb 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 0 additions & 5 deletions docs/troubleshooting/guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,6 @@ See the [system requirements]({{< relref "../getting-started/system-requirements
Composer Patches requires at least _some_ mechanism for applying patches. If you don't have any installed, you'll see a fair number of errors. You should install some combination of GNU `patch`, BSD `patch`, `git`, or other applicable software. macOS users commonly need to `brew install gpatch` to get a modern version of `patch` on their system.


## Set `preferred-install` to `source`

The Git patcher included with this plugin is the most reliable method of applying patches. However, the Git patcher won't even _attempt_ to apply a patch if the target directory isn't cloned from Git. To avoid this situation, you should either set `"preferred-install": "source"` or set specific packages to be installed from source in your Composer configuration. Patches may still be successfully applied without this setting, but if you're working on a team, you'll have much more consistent results with the Git patcher.


## Download patches securely

If you've been referred here, you're trying to download patches over HTTP without explicitly telling Composer that you want to do that. See the [`secure-http`]({{< relref "../usage/configuration.md#secure-http" >}}) documentation for more information.
Expand Down
22 changes: 3 additions & 19 deletions docs/usage/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ You probably don't need to change this value unless you're building a plugin tha
"disable-patchers": [
"\\cweagans\\Composer\\Patcher\\BsdPatchPatcher",
"\\cweagans\\Composer\\Patcher\\GitPatcher",
"\\cweagans\\Composer\\Patcher\\GitInitPatcher",
"\\cweagans\\Composer\\Patcher\\GnuGPatchPatcher",
"\\cweagans\\Composer\\Patcher\\GnuPatchPatcher"
]
Expand All @@ -147,30 +148,13 @@ You probably don't need to change this value unless you're building a plugin tha

For completeness, all of the patchers that ship with the plugin are listed above, but you should _not_ list all of them. If no patchers are available, the plugin will throw an exception during `composer install`.

`GitPatcher` and `GitInitPatcher` should be enabled and disabled together -- don't disable one without the other.

After changing this value, you should re-lock and re-apply patches to your project.


## Relevant configuration provided by Composer

### `preferred-install`

```json
{
[...],
"config": {
"preferred-install": "source"
}
}
```

**Default value**: `"dist"`

The relevant Composer documentation for this parameter can be found [here](https://getcomposer.org/doc/06-config.md#preferred-install).

If you're applying patches that were generated with `git`, setting `preferred-install` to `"source"` is **highly recommended** (either by changing the setting for all packages or by setting that value on a per-package basis as shown in the Composer documentation. This will allow the `GitPatcher` to apply patches as often as possible (the `GitPatcher` won't even _attempt_ to apply a patch if the target directory isn't managed by `git`). Git is the most reliable patcher available to Composer Patches. _Not_ changing this setting will result in other patchers attempting to apply patches. Historically, these patchers have had varying degrees of success depending on a number of factors, so it's better to use the `GitPatcher` when you're able to do so.

---

### `secure-http`

```json
Expand Down
2 changes: 2 additions & 0 deletions src/Capability/Patcher/CorePatcherProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use cweagans\Composer\Patcher\BsdPatchPatcher;
use cweagans\Composer\Patcher\GitPatcher;
use cweagans\Composer\Patcher\GitInitPatcher;
use cweagans\Composer\Patcher\GnuGPatchPatcher;
use cweagans\Composer\Patcher\GnuPatchPatcher;

Expand All @@ -16,6 +17,7 @@ public function getPatchers(): array
{
return [
new GitPatcher($this->composer, $this->io),
new GitInitPatcher($this->composer, $this->io),
new GnuPatchPatcher($this->composer, $this->io),
new GnuGPatchPatcher($this->composer, $this->io),
new BsdPatchPatcher($this->composer, $this->io),
Expand Down
60 changes: 0 additions & 60 deletions src/Command/DoctorCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -109,66 +109,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$io->write("");
$io->write("<info>Common configuration issues</info>");
$io->write("================================================================================");
$preferred_install_issues = false;
$pi = $composer->getConfig()->get('preferred-install');
$io->write(
str_pad("preferred-install is set:", 77) . (is_null($pi) ? " <warning>no</warning>" : "<info>yes</info>")
);

if (is_null($pi)) {
$preferred_install_issues = true;
}

if (is_string($pi)) {
$io->write(
str_pad("preferred-install set to 'source' for all/some packages:", 77) .
($pi === "source" ? "<info>yes</info>" : " <warning>no</warning>")
);
}

if (is_string($pi) && $pi !== "source") {
$preferred_install_issues = true;
}

if (is_array($pi)) {
$patched_packages = $plugin->getPatchCollection()->getPatchedPackages();
foreach ($patched_packages as $package) {
if (in_array($package, array_values($pi))) {
$io->write(
str_pad("preferred-install set to 'source' for $package:", 77) . "<info>yes</info>"
);
continue;
}

foreach ($pi as $pattern => $value) {
$pattern = strtr($pattern, ['*' => '.*', '/' => '\/']);
if (preg_match("/$pattern/", $package)) {
$io->write(
str_pad("preferred-install set to 'source' for $package:", 77) .
($value === "source" ? "<info>yes</info>" : " <warning>no</warning>")
);

if ($value !== "source") {
$preferred_install_issues = true;
}

break 2;
}
}

$preferred_install_issues = true;
}
}

if ($preferred_install_issues) {
$suggestions[] = [
"message" => "Setting 'preferred-install' to 'source' either globally or for each patched dependency " .
"is highly recommended for consistent results",
"link" =>
"https://docs.cweagans.net/composer-patches/troubleshooting/guide#set-preferred-install-to-source"
];
}

$has_http_urls = false;
foreach ($plugin->getPatchCollection()->getPatchedPackages() as $package) {
foreach ($plugin->getPatchCollection()->getPatchesForPackage($package) as $patch) {
Expand Down
42 changes: 42 additions & 0 deletions src/Patcher/GitInitPatcher.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?php

namespace cweagans\Composer\Patcher;

use Composer\IO\IOInterface;
use cweagans\Composer\Patch;

class GitInitPatcher extends GitPatcher
{
public function apply(Patch $patch, string $path): bool
{
// If the target is already a git repo, the standard Git patcher can handle it.
if (is_dir($path . '/.git')) {
return false;
}

$this->io->write("Creating temporary fake git repo in $path to apply patch", true, IOInterface::VERBOSE);

// Create a fake Git repo -- just enough to make Git think it's looking at a real repo.

Choose a reason for hiding this comment

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

👋

I think by not calling git init that it relies too much on the internal implementation of the .git directory. What if a later version requires some fourth file?

I suppose if this is documented somewhere by git it would be OK.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I think you're right. Changed this to just use git init in a14cba7.

$dirs = [
$path . '/.git',
$path . '/.git/objects',
$path . '/.git/refs',
];
foreach ($dirs as $dir) {
mkdir($dir);
}
file_put_contents($path . '/.git/HEAD', "ref: refs/heads/main");


// Use the git patcher to apply the patch.
$status = parent::apply($patch, $path);

// Clean up the fake git repo.
unlink($path . '/.git/HEAD');
foreach (array_reverse($dirs) as $dir) {
rmdir($dir);
}

return $status;
}
}
1 change: 1 addition & 0 deletions src/Patcher/GitPatcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ public function apply(Patch $patch, string $path): bool
// If the path isn't a git repo, don't even try.
// @see https://stackoverflow.com/a/27283285
if (!is_dir($path . '/.git')) {
$this->io->write("$path is not a git repo. Skipping Git patcher.", true, IOInterface::VERBOSE);
return false;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
{
"name": "cweagans/composer-patches-test-project",
"description": "Project for use in cweagans/composer-patches acceptance tests.",
"type": "project",
"license": "BSD-3-Clause",
"repositories": [
{
"type": "path",
"url": "../../../../"
}
],
"require": {
"cweagans/composer-patches": "*@dev",
"cweagans/composer-patches-testrepo": "~1.0"
},
"extra": {
"patches": {
"cweagans/composer-patches-testrepo": {
"Add a file": "https://patch-diff.githubusercontent.com/raw/cweagans/composer-patches-testrepo/pull/1.patch"
}
}
},
"config": {
"preferred-install": "dist",
"composer-patches": {
"disable-patchers": [
"\\cweagans\\Composer\\Patcher\\BsdPatchPatcher",
"\\cweagans\\Composer\\Patcher\\GnuGPatchPatcher",
"\\cweagans\\Composer\\Patcher\\GnuPatchPatcher"
]
},
"allow-plugins": {
"cweagans/composer-patches": true
}
}
}
1 change: 1 addition & 0 deletions tests/_data/fixtures/no-patchers-available/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"disable-patchers": [
"cweagans\\Composer\\Patcher\\BsdPatchPatcher",
"cweagans\\Composer\\Patcher\\GitPatcher",
"cweagans\\Composer\\Patcher\\GitInitPatcher",
"cweagans\\Composer\\Patcher\\GnuGPatchPatcher",
"cweagans\\Composer\\Patcher\\GnuPatchPatcher"
]
Expand Down
13 changes: 13 additions & 0 deletions tests/acceptance/ApplyGitPatchFromWebUsingInitCept.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?php

/**
* @var \Codeception\Scenario $scenario
*/

use cweagans\Composer\Tests\AcceptanceTester;

$I = new AcceptanceTester($scenario);
$I->wantTo('modify a package using a patch downloaded from the internet (exercising Git init patcher)');
$I->amInPath(codecept_data_dir('fixtures/apply-git-patch-from-web-using-init'));
$I->runComposerCommand('install', ['-vvv']);
$I->canSeeFileFound('./vendor/cweagans/composer-patches-testrepo/src/OneMoreTest.php');
3 changes: 2 additions & 1 deletion tests/unit/CoreDownloaderProviderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use Composer\Plugin\PluginInterface;
use cweagans\Composer\Capability\Downloader\CoreDownloaderProvider;
use cweagans\Composer\Downloader\ComposerDownloader;
use cweagans\Composer\Downloader\DownloaderInterface;

class CoreDownloaderProviderTest extends Unit
{
Expand All @@ -27,6 +28,6 @@ public function testGetDownloaders()
$downloaders = $downloaderProvider->getDownloaders();

$this->assertCount(1, $downloaders);
$this->assertInstanceOf(ComposerDownloader::class, $downloaders[0]);
$this->assertContainsOnlyInstancesOf(DownloaderInterface::class, $downloaders);
}
}
9 changes: 4 additions & 5 deletions tests/unit/CorePatcherProviderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@
use Composer\IO\NullIO;
use Composer\Plugin\PluginInterface;
use cweagans\Composer\Capability\Patcher\CorePatcherProvider;
use cweagans\Composer\Patcher\PatcherInterface;
use cweagans\Composer\Patcher\BsdPatchPatcher;
use cweagans\Composer\Patcher\GitPatcher;
use cweagans\Composer\Patcher\GitInitPatcher;
use cweagans\Composer\Patcher\GnuGPatchPatcher;
use cweagans\Composer\Patcher\GnuPatchPatcher;

Expand All @@ -25,10 +27,7 @@ public function testGetPatchers()

$patchers = $patcherProvider->getPatchers();

$this->assertCount(4, $patchers);
$this->assertInstanceOf(GitPatcher::class, $patchers[0]);
$this->assertInstanceOf(GnuPatchPatcher::class, $patchers[1]);
$this->assertInstanceOf(GnuGPatchPatcher::class, $patchers[2]);
$this->assertInstanceOf(BsdPatchPatcher::class, $patchers[3]);
$this->assertCount(5, $patchers);
$this->assertContainsOnlyInstancesOf(PatcherInterface::class, $patchers);
}
}
4 changes: 2 additions & 2 deletions tests/unit/CoreResolverProviderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Composer\Plugin\PluginInterface;
use cweagans\Composer\Capability\Resolver\CoreResolverProvider;
use cweagans\Composer\Resolver\PatchesFile;
use cweagans\Composer\Resolver\ResolverInterface;
use cweagans\Composer\Resolver\RootComposer;

class CoreResolverProviderTest extends Unit
Expand All @@ -24,7 +25,6 @@ public function testGetResolvers()
$resolvers = $resolverProvider->getResolvers();

$this->assertCount(2, $resolvers);
$this->assertInstanceOf(RootComposer::class, $resolvers[0]);
$this->assertInstanceOf(PatchesFile::class, $resolvers[1]);
$this->assertContainsOnlyInstancesOf(ResolverInterface::class, $resolvers);
}
}