From c70216629da6e514ce069f5c2c474b13afffc796 Mon Sep 17 00:00:00 2001 From: Jesse Leite Date: Wed, 6 Nov 2024 00:33:31 -0500 Subject: [PATCH 1/7] Spec out `addon: true` for starter kits that should live on as composer updatable addons. --- tests/StarterKits/InstallTest.php | 62 +++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/tests/StarterKits/InstallTest.php b/tests/StarterKits/InstallTest.php index 10c85e1c2ee..fc697639a8d 100644 --- a/tests/StarterKits/InstallTest.php +++ b/tests/StarterKits/InstallTest.php @@ -209,6 +209,68 @@ public function it_restores_existing_repositories_after_successful_install() $this->assertEquals($expectedRepositories, $composerJson['repositories']); } + #[Test] + public function it_installs_as_living_addon_with_custom_config() + { + $this->setConfig([ + 'addon' => true, // With `addon: true`, kit should live on as composer updatable addon + 'export_paths' => [ + 'copied.md', + ], + ]); + + $this->assertFileDoesNotExist(base_path('copied.md')); + $this->assertFileDoesNotExist($this->kitVendorPath()); + + $this->installCoolRunnings(); + + $this->assertFileExists(base_path('copied.md')); + $this->assertComposerJsonDoesntHave('repositories'); + + // Keep addon around + $this->assertFileExists($this->kitVendorPath()); + + // But ensure we still delete backup composer.json, which is only used for error handling purposes + $this->assertFileDoesNotExist(base_path('composer.json.bak')); + } + + #[Test] + public function it_leaves_custom_repository_for_living_addon() + { + $this->setConfig([ + 'addon' => true, // With `addon: true`, kit should live on as composer updatable addon + 'export_paths' => [ + 'copied.md', + ], + ]); + + $this->assertFileDoesNotExist(base_path('copied.md')); + $this->assertFileDoesNotExist($this->kitVendorPath()); + $this->assertComposerJsonDoesntHave('repositories'); + + $this->installCoolRunnings([], [ + 'outpost.*' => Http::response(['data' => ['price' => null]], 200), + 'github.com/*' => Http::response('', 200), + '*' => Http::response('', 404), + ]); + + $this->assertFileExists(base_path('copied.md')); + + // Keep addon around + $this->assertFileExists($this->kitVendorPath()); + + // As well as custom repository, which will be needed for composer updates, if it was needed for install + $composerJson = json_decode($this->files->get(base_path('composer.json')), true); + $this->assertCount(1, $composerJson['repositories']); + $this->assertEquals([[ + 'type' => 'vcs', + 'url' => 'https://github.com/statamic/cool-runnings', + ]], $composerJson['repositories']); + + // But delete backup composer.json, which is only used for error handling purposes + $this->assertFileDoesNotExist(base_path('composer.json.bak')); + } + #[Test] public function it_fails_if_starter_kit_config_does_not_exist() { From 5a8649c963cdb5066364f57b2f4278c7c2a8ce85 Mon Sep 17 00:00:00 2001 From: Jesse Leite Date: Wed, 6 Nov 2024 00:33:59 -0500 Subject: [PATCH 2/7] Blink config, no reason to read it twice. --- src/StarterKits/Installer.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/StarterKits/Installer.php b/src/StarterKits/Installer.php index a17f8adfc37..1b5a783baf2 100644 --- a/src/StarterKits/Installer.php +++ b/src/StarterKits/Installer.php @@ -672,7 +672,9 @@ protected function starterKitPath(?string $path = null): string */ protected function config(?string $key = null): mixed { - $config = collect(YAML::parse($this->files->get($this->starterKitPath('starter-kit.yaml')))); + $config = Blink::once('starter-kit-config', function () { + return collect(YAML::parse($this->files->get($this->starterKitPath('starter-kit.yaml')))); + }); if ($key) { return $config->get($key); From 109317a9e97c1f9a607339bfd0acddfb1060d10f Mon Sep 17 00:00:00 2001 From: Jesse Leite Date: Wed, 6 Nov 2024 00:34:04 -0500 Subject: [PATCH 3/7] Pass tests. --- src/StarterKits/Installer.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/StarterKits/Installer.php b/src/StarterKits/Installer.php index 1b5a783baf2..be7b6313c72 100644 --- a/src/StarterKits/Installer.php +++ b/src/StarterKits/Installer.php @@ -548,7 +548,7 @@ function () { */ public function removeStarterKit(): self { - if ($this->disableCleanup) { + if ($this->isAddon() || $this->disableCleanup) { return $this; } @@ -589,7 +589,7 @@ protected function completeInstall(): self */ protected function removeRepository(): self { - if ($this->fromLocalRepo || ! $this->url) { + if ($this->isAddon() || $this->fromLocalRepo || ! $this->url) { return $this; } @@ -682,4 +682,12 @@ protected function config(?string $key = null): mixed return $config; } + + /** + * Should starter kit be treated as a regular addon, and live on for future composer updates, etc? + */ + protected function isAddon(): bool + { + return (bool) $this->config('addon'); + } } From bdfce397c963742cf41787e0f8a761aab0d698ed Mon Sep 17 00:00:00 2001 From: Jesse Leite Date: Wed, 6 Nov 2024 11:35:46 -0500 Subject: [PATCH 4/7] Refactor to `updatable: true`. --- src/StarterKits/Installer.php | 10 +++++----- tests/StarterKits/InstallTest.php | 12 ++++++------ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/StarterKits/Installer.php b/src/StarterKits/Installer.php index be7b6313c72..ec2a6ebf02e 100644 --- a/src/StarterKits/Installer.php +++ b/src/StarterKits/Installer.php @@ -548,7 +548,7 @@ function () { */ public function removeStarterKit(): self { - if ($this->isAddon() || $this->disableCleanup) { + if ($this->isUpdatable() || $this->disableCleanup) { return $this; } @@ -589,7 +589,7 @@ protected function completeInstall(): self */ protected function removeRepository(): self { - if ($this->isAddon() || $this->fromLocalRepo || ! $this->url) { + if ($this->isUpdatable() || $this->fromLocalRepo || ! $this->url) { return $this; } @@ -684,10 +684,10 @@ protected function config(?string $key = null): mixed } /** - * Should starter kit be treated as a regular addon, and live on for future composer updates, etc? + * Should starter kit be treated as an updatable package, and live on for future composer updates, etc? */ - protected function isAddon(): bool + protected function isUpdatable(): bool { - return (bool) $this->config('addon'); + return (bool) $this->config('updatable'); } } diff --git a/tests/StarterKits/InstallTest.php b/tests/StarterKits/InstallTest.php index fc697639a8d..868d3f11ebc 100644 --- a/tests/StarterKits/InstallTest.php +++ b/tests/StarterKits/InstallTest.php @@ -210,10 +210,10 @@ public function it_restores_existing_repositories_after_successful_install() } #[Test] - public function it_installs_as_living_addon_with_custom_config() + public function it_installs_as_living_package_with_custom_config() { $this->setConfig([ - 'addon' => true, // With `addon: true`, kit should live on as composer updatable addon + 'updatable' => true, // With `updatable: true`, kit should live on as composer updatable package 'export_paths' => [ 'copied.md', ], @@ -227,7 +227,7 @@ public function it_installs_as_living_addon_with_custom_config() $this->assertFileExists(base_path('copied.md')); $this->assertComposerJsonDoesntHave('repositories'); - // Keep addon around + // Keep package around $this->assertFileExists($this->kitVendorPath()); // But ensure we still delete backup composer.json, which is only used for error handling purposes @@ -235,10 +235,10 @@ public function it_installs_as_living_addon_with_custom_config() } #[Test] - public function it_leaves_custom_repository_for_living_addon() + public function it_leaves_custom_repository_for_living_packages_that_need_it() { $this->setConfig([ - 'addon' => true, // With `addon: true`, kit should live on as composer updatable addon + 'updatable' => true, // With `updatable: true`, kit should live on as composer updatable package 'export_paths' => [ 'copied.md', ], @@ -256,7 +256,7 @@ public function it_leaves_custom_repository_for_living_addon() $this->assertFileExists(base_path('copied.md')); - // Keep addon around + // Keep package around $this->assertFileExists($this->kitVendorPath()); // As well as custom repository, which will be needed for composer updates, if it was needed for install From c299481ab8121e89ac1091e15ee0a387134055fd Mon Sep 17 00:00:00 2001 From: Jesse Leite Date: Tue, 26 Nov 2024 16:14:34 -0500 Subject: [PATCH 5/7] Add test coverage to ensure package remains in `require` (not `require-dev`). --- tests/StarterKits/InstallTest.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/StarterKits/InstallTest.php b/tests/StarterKits/InstallTest.php index 676a69dad17..62946eff8b1 100644 --- a/tests/StarterKits/InstallTest.php +++ b/tests/StarterKits/InstallTest.php @@ -14,6 +14,7 @@ use Statamic\Facades\Blink; use Statamic\Facades\Config; use Statamic\Facades\YAML; +use Statamic\Support\Arr; use Statamic\Support\Str; use Tests\Fakes\Composer\FakeComposer; use Tests\TestCase; @@ -56,6 +57,7 @@ public function it_installs_starter_kit() $this->assertFalse(Blink::has('starter-kit-repository-added')); $this->assertFileDoesNotExist($this->kitVendorPath()); $this->assertFileDoesNotExist(base_path('composer.json.bak')); + $this->assertComposerJsonDoesntHavePackage('statamic/cool-runnings'); $this->assertComposerJsonDoesntHave('repositories'); $this->assertFileExists(base_path('copied.md')); } @@ -229,6 +231,7 @@ public function it_installs_as_living_package_with_custom_config() // Keep package around $this->assertFileExists($this->kitVendorPath()); + $this->assertComposerJsonHasPackage('require', 'statamic/cool-runnings'); // But ensure we still delete backup composer.json, which is only used for error handling purposes $this->assertFileDoesNotExist(base_path('composer.json.bak')); @@ -258,6 +261,7 @@ public function it_leaves_custom_repository_for_living_packages_that_need_it() // Keep package around $this->assertFileExists($this->kitVendorPath()); + $this->assertComposerJsonHasPackage('require', 'statamic/cool-runnings'); // As well as custom repository, which will be needed for composer updates, if it was needed for install $composerJson = json_decode($this->files->get(base_path('composer.json')), true); @@ -1665,6 +1669,21 @@ private function assertFileDoesntHaveContent($expected, $path) $this->assertStringNotContainsString($expected, $this->files->get($path)); } + private function assertComposerJsonHasPackage($requireKey, $package) + { + $composerJson = json_decode($this->files->get(base_path('composer.json')), true); + + $this->assertTrue(Arr::has($composerJson, "{$requireKey}.{$package}")); + } + + private function assertComposerJsonDoesntHavePackage($package) + { + $composerJson = json_decode($this->files->get(base_path('composer.json')), true); + + $this->assertFalse(Arr::has($composerJson, "require.{$package}")); + $this->assertFalse(Arr::has($composerJson, "require-dev.{$package}")); + } + private function assertComposerJsonHasPackageVersion($requireKey, $package, $version) { $composerJson = json_decode($this->files->get(base_path('composer.json')), true); From 261f6894cbe9566736fb12d2f7a0ed40c3563569 Mon Sep 17 00:00:00 2001 From: Jesse Leite Date: Tue, 26 Nov 2024 16:15:08 -0500 Subject: [PATCH 6/7] =?UTF-8?q?Pass=20test.=20(We=20don=E2=80=99t=20really?= =?UTF-8?q?=20have=20reason=20to=20`requireDev`=20here=20anymore.)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/StarterKits/Installer.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/StarterKits/Installer.php b/src/StarterKits/Installer.php index 5ad2e866b4b..9a2add59b23 100644 --- a/src/StarterKits/Installer.php +++ b/src/StarterKits/Installer.php @@ -250,7 +250,7 @@ function () { : $this->package; try { - Composer::withoutQueue()->throwOnFailure()->requireDev($package); + Composer::withoutQueue()->throwOnFailure()->require($package); } catch (ProcessException $exception) { $this->rollbackWithError("Error installing starter kit [{$package}].", $exception->getMessage()); } @@ -555,7 +555,7 @@ public function removeStarterKit(): self spin( function () { if (Composer::isInstalled($this->package)) { - Composer::withoutQueue()->throwOnFailure(false)->removeDev($this->package); + Composer::withoutQueue()->throwOnFailure(false)->remove($this->package); } }, 'Cleaning up temporary files...' From 9ab6e5144843da88c58cb8a548aa39790a91937d Mon Sep 17 00:00:00 2001 From: Jesse Leite Date: Tue, 26 Nov 2024 16:27:31 -0500 Subject: [PATCH 7/7] Adjust these tests too. --- tests/StarterKits/InstallTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/StarterKits/InstallTest.php b/tests/StarterKits/InstallTest.php index 62946eff8b1..9d772235e64 100644 --- a/tests/StarterKits/InstallTest.php +++ b/tests/StarterKits/InstallTest.php @@ -754,8 +754,8 @@ public function it_parses_branch_from_package_param_when_installing() ]); // Ensure `Composer::requireDev()` gets called with `package:branch` - $this->assertEquals(Blink::get('composer-require-dev-package'), 'statamic/cool-runnings'); - $this->assertEquals(Blink::get('composer-require-dev-branch'), 'dev-custom-branch'); + $this->assertEquals(Blink::get('composer-require-package'), 'statamic/cool-runnings'); + $this->assertEquals(Blink::get('composer-require-branch'), 'dev-custom-branch'); // But ensure the rest of the installer handles parsed `package` without branch messing things up $this->assertFalse(Blink::has('starter-kit-repository-added')); @@ -777,8 +777,8 @@ public function it_installs_branch_with_slash_without_failing_package_validation ]); // Ensure `Composer::requireDev()` gets called with `package:branch` - $this->assertEquals(Blink::get('composer-require-dev-package'), 'statamic/cool-runnings'); - $this->assertEquals(Blink::get('composer-require-dev-branch'), 'dev-feature/custom-branch'); + $this->assertEquals(Blink::get('composer-require-package'), 'statamic/cool-runnings'); + $this->assertEquals(Blink::get('composer-require-branch'), 'dev-feature/custom-branch'); // But ensure the rest of the installer handles parsed `package` without branch messing things up $this->assertFalse(Blink::has('starter-kit-repository-added'));