From 9bc00c28fdfc796a7934850c40ba23be8cd25930 Mon Sep 17 00:00:00 2001 From: Alex Skrypnyk Date: Mon, 7 Sep 2026 12:03:56 +1000 Subject: [PATCH 1/3] [#3094] Derived the install manifest at run time and recorded replaced project changes. --- .vortex/docs/content/updating-vortex.mdx | 2 + .../installer/src/Command/InstallCommand.php | 10 +- .../src/Prompts/InstallerPresenter.php | 16 +- .../installer/src/Prompts/PromptManager.php | 20 ++ .vortex/installer/src/Utils/FileManager.php | 158 ++++++++----- .../installer/src/Utils/UpdateRegistry.php | 172 ++++++++++++++ .../handler_process/_baseline/.ignorecontent | 1 - .../Command/InstallExcludedPathsTest.php | 212 +++++++++--------- .../tests/Unit/Utils/FileManagerTest.php | 154 +++++++++++-- .../tests/Unit/Utils/UpdateRegistryTest.php | 121 ++++++++++ .../phpunit/Functional/InstallerTest.php | 2 +- 11 files changed, 683 insertions(+), 185 deletions(-) create mode 100644 .vortex/installer/src/Utils/UpdateRegistry.php create mode 100644 .vortex/installer/tests/Unit/Utils/UpdateRegistryTest.php diff --git a/.vortex/docs/content/updating-vortex.mdx b/.vortex/docs/content/updating-vortex.mdx index 81b65366d4..f2658704d7 100644 --- a/.vortex/docs/content/updating-vortex.mdx +++ b/.vortex/docs/content/updating-vortex.mdx @@ -65,6 +65,8 @@ Specifically check if any environment variables were added or changed. It may be a good idea to commit changes in smaller chunks to make it easier to review and revert if necessary. + Where the update replaced a file your project had changed, `.logs/vortex-update.md` records the replaced change alongside the change the new version brings. Work through it to re-apply your changes, then delete the entries you have resolved. The file is not tracked in Git. + Your `composer.json` has most likely deviated from the **Vortex** version, so review the package version changes carefully. We recommend temporarily reverting the changes to `composer.json` and `composer.lock` to preserve diff --git a/.vortex/installer/src/Command/InstallCommand.php b/.vortex/installer/src/Command/InstallCommand.php index 9e3b3dba52..555529b59e 100644 --- a/.vortex/installer/src/Command/InstallCommand.php +++ b/.vortex/installer/src/Command/InstallCommand.php @@ -224,7 +224,13 @@ protected function execute(InputInterface $input, OutputInterface $output): int $version = $this->getRepositoryDownloader()->download($this->artifact, $this->config->get(Config::TMP), $release_prefix); $this->config->set(Config::VERSION, $version); $this->fileManager->snapshotTemplate(); - $this->fileManager->snapshotPreviousTemplate($this->getRepositoryDownloader(), $this->artifact); + $this->fileManager->snapshotPreviousTemplate( + $this->getRepositoryDownloader(), + $this->artifact, + function (string $dir, string $ref): void { + $this->promptManager->renderTemplate($dir, $ref); + }, + ); return $version; }, hint: fn(): string => sprintf('Downloading from "%s" repository at ref "%s"', $this->artifact->getRepo(), $this->artifact->getRef()), @@ -262,7 +268,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int return Command::FAILURE; } - $this->presenter->footer(); + $this->presenter->footer($this->fileManager->getRegistryFile()); $should_build = TRUE; $requested_build = (bool) $this->config->get(Config::BUILD_NOW); diff --git a/.vortex/installer/src/Prompts/InstallerPresenter.php b/.vortex/installer/src/Prompts/InstallerPresenter.php index deb4649410..39b84a43f8 100644 --- a/.vortex/installer/src/Prompts/InstallerPresenter.php +++ b/.vortex/installer/src/Prompts/InstallerPresenter.php @@ -7,6 +7,7 @@ use DrevOps\VortexInstaller\Downloader\Artifact; use DrevOps\VortexInstaller\Prompts\Handlers\Starter; use DrevOps\VortexInstaller\Utils\Config; +use DrevOps\VortexInstaller\Utils\File; use DrevOps\VortexInstaller\Utils\Strings; use DrevOps\VortexInstaller\Utils\Tui; use Symfony\Component\Process\ExecutableFinder; @@ -122,13 +123,26 @@ public function header(Artifact $artifact, string $version): void { Tui::box($content, $title); } - public function footer(): void { + /** + * Display the footer after the installation finished. + * + * @param string|null $registry_file + * Path of the registry of project changes the update replaced, or NULL + * when the update replaced none. + */ + public function footer(?string $registry_file = NULL): void { $output = ''; $prefix = ' '; if ($this->config->isVortexProject()) { $title = 'Finished updating Vortex'; $output .= 'Please review the changes and commit the required files.'; + + if ($registry_file !== NULL) { + $output .= PHP_EOL . PHP_EOL; + $output .= 'Project changes replaced by this update are recorded in:' . PHP_EOL; + $output .= $prefix . File::toRelative($registry_file, (string) $this->config->getDestination()) . PHP_EOL; + } } else { $title = 'Finished installing Vortex'; diff --git a/.vortex/installer/src/Prompts/PromptManager.php b/.vortex/installer/src/Prompts/PromptManager.php index 14782febfc..93e23a40d9 100644 --- a/.vortex/installer/src/Prompts/PromptManager.php +++ b/.vortex/installer/src/Prompts/PromptManager.php @@ -348,6 +348,26 @@ public function runProcessors(): void { File::runDirectoryTasks($this->config->get(Config::TMP)); } + /** + * Render a template download using the responses this run collected. + * + * @param string $dir + * Directory holding an unprocessed template download. + * @param string $version + * Version to stamp into the rendered content. + */ + public function renderTemplate(string $dir, string $version): void { + $config = clone $this->config; + $config->set(Config::TMP, $dir, TRUE); + $config->set(Config::VERSION, $version, TRUE); + + // Handlers bind to the directory they are constructed with, so rendering + // into a directory other than this run's staging copy needs its own set. + $manager = new self($config); + $manager->responses = $this->responses; + $manager->runProcessors(); + } + /** * Run all post-build processors. * diff --git a/.vortex/installer/src/Utils/FileManager.php b/.vortex/installer/src/Utils/FileManager.php index ee28684bc2..e48445b39c 100644 --- a/.vortex/installer/src/Utils/FileManager.php +++ b/.vortex/installer/src/Utils/FileManager.php @@ -25,23 +25,33 @@ class FileManager { */ protected array $templatePaths = []; - /** - * Name of the file recording what the installer wrote into the project. - */ - const MANIFEST_FILE = '.vortex-manifest.json'; - /** * Algorithm used to detect project edits to shipped files. */ const HASH_ALGO = 'sha256'; /** - * Content hashes shipped by the version the project currently runs. + * Content hashes the version the project currently runs could have written. * - * @var array + * @var array> */ protected array $previousTemplateHashes = []; + /** + * Directory holding the rendered version the project currently runs. + */ + protected ?string $previousDir = NULL; + + /** + * Reference of the version the project currently runs. + */ + protected ?string $previousRef = NULL; + + /** + * Path of the registry of project changes this update replaced. + */ + protected ?string $registryFile = NULL; + public function __construct( protected Config $config, ) {} @@ -67,12 +77,18 @@ public function snapshotTemplate(): void { } /** - * Record the paths shipped by the version the project currently runs. + * Record what the version the project currently runs installed. * * A path the template has stopped shipping altogether is absent from the - * incoming download, so the selection diff alone cannot see it. Listing the - * project's own version restores it as a candidate, which is what makes a - * file dropped between releases removable rather than permanent. + * incoming download, so the selection diff alone cannot see it. Rendering + * the project's own version restores it as a candidate, which is what makes + * a file dropped between releases removable rather than permanent. + * + * The download is hashed both as it arrives and once rendered. Rendering + * resolves token replacements and directory renames, which the download's + * own files cannot match; rendering also applies this run's answers, which + * strips whatever the current selection drops. Either hash therefore stands + * for content the project could hold, so both are kept as candidates. * * Failure is not fatal: the recorded reference may no longer resolve, in * which case only the selection diff applies. @@ -81,8 +97,11 @@ public function snapshotTemplate(): void { * The repository downloader. * @param \DrevOps\VortexInstaller\Downloader\Artifact $artifact * The artifact identifying the repository to read the reference from. + * @param callable|null $render + * Callback turning the download into installable content, receiving the + * directory and the reference. */ - public function snapshotPreviousTemplate(RepositoryDownloader $downloader, Artifact $artifact): void { + public function snapshotPreviousTemplate(RepositoryDownloader $downloader, Artifact $artifact, ?callable $render = NULL): void { if (!$this->config->isVortexProject()) { return; } @@ -100,12 +119,24 @@ public function snapshotPreviousTemplate(RepositoryDownloader $downloader, Artif File::remove($dir); File::mkdir($dir); $downloader->download(Artifact::create($artifact->getRepo(), $ref), $dir); - $this->previousTemplateHashes = $this->hashDirectory($dir); + + $hashes = array_map(fn(string $hash): array => [$hash], $this->hashDirectory($dir)); + + if ($render !== NULL) { + $render($dir, $ref); + + foreach ($this->hashDirectory($dir) as $path => $hash) { + $hashes[$path][] = $hash; + } + } + + $this->previousTemplateHashes = array_map(fn(array $candidates): array => array_values(array_unique($candidates)), $hashes); + $this->previousDir = $dir; + $this->previousRef = $ref; } catch (\Exception) { $this->previousTemplateHashes = []; - } - finally { + $this->previousDir = NULL; File::remove($dir); } } @@ -148,15 +179,10 @@ public function copyFiles(): void { $destination = $this->config->getDestination(); // What the project should hold for a path this install no longer ships. - // The manifest is authoritative because it records the processed content - // that was actually written; the previous version's own files stand in for - // projects installed before manifests existed, and match only where the - // installer copied the file through unchanged. $expected = $this->previousTemplateHashes; - $expected = $this->readManifest() + $expected; - // Anything either version of the template ships, or the last install - // wrote, but the staged copy no longer holds. + // Anything the version the project runs installed, or either version of + // the template ships, but the staged copy no longer holds. $shipped = array_merge(array_keys($expected), $this->templatePaths); $excluded = array_diff($shipped, $this->relativePaths($src)); @@ -188,6 +214,8 @@ public function copyFiles(): void { File::rmdirIfEmpty($dir); } + $this->recordReplacedChanges($src); + if (is_dir($src) && !File::dirIsEmpty($src)) { File::copy($src, $destination); } @@ -199,7 +227,7 @@ public function copyFiles(): void { $this->removeExcludedPaths($excluded, $expected); $this->removeObsoletePaths(); - $this->writeManifest($src); + $this->cleanupPreviousTemplate(); } /** @@ -214,8 +242,8 @@ public function copyFiles(): void { * * @param array $paths * Template-relative paths absent from the staged copy. - * @param array $expected - * Content hashes the template last wrote, keyed by path. + * @param array> $expected + * Content hashes the template could have written, keyed by path. */ protected function removeExcludedPaths(array $paths, array $expected): void { if (!$this->config->isVortexProject()) { @@ -240,7 +268,7 @@ protected function removeExcludedPaths(array $paths, array $expected): void { // Without a recorded hash there is nothing to compare the project's copy // against, so ownership cannot be established. - if (!isset($expected[$path]) || hash_file(self::HASH_ALGO, $target) !== $expected[$path]) { + if (!isset($expected[$path]) || !in_array(hash_file(self::HASH_ALGO, $target), $expected[$path], TRUE)) { continue; } @@ -260,47 +288,68 @@ protected function removeExcludedPaths(array $paths, array $expected): void { } /** - * Record what this install wrote, so the next one can detect project edits. + * Record the project changes that the copy is about to replace. * - * The staged copy at this point holds exactly the processed content that was - * copied into the destination, which is what a later run has to compare the - * project's files against. + * The copy overlays the staged content without regard for what the project + * put there, so a change the project made to a shipped file is lost. The + * content of all three sides is only available before the overlay, which is + * where the registry has to be built. * * @param string $src * The staged template directory. */ - protected function writeManifest(string $src): void { - $hashes = $this->hashDirectory($src); - - if ($hashes === []) { + protected function recordReplacedChanges(string $src): void { + if ($this->previousDir === NULL) { return; } - ksort($hashes); + $destination = (string) $this->config->getDestination(); + $registry = new UpdateRegistry($destination); - File::dump($this->config->getDestination() . '/' . self::MANIFEST_FILE, json_encode($hashes, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES) . PHP_EOL); - } + foreach ($this->relativePaths($src) as $path) { + $project = $destination . '/' . $path; - /** - * Read the hashes recorded by the previous install. - * - * @return array - * Content hashes keyed by path, empty when the project has no manifest. - */ - protected function readManifest(): array { - $file = $this->config->getDestination() . '/' . self::MANIFEST_FILE; + if (!is_file($project)) { + continue; + } - if (!is_file($file)) { - return []; + $next = $src . '/' . $path; + $project_hash = hash_file(self::HASH_ALGO, $project); + + // The file still holds what the template put there, or already holds + // what the copy would put there, so the copy replaces nothing. + if (in_array($project_hash, $this->previousTemplateHashes[$path] ?? [], TRUE) || $project_hash === hash_file(self::HASH_ALGO, $next)) { + continue; + } + + // A path the version the project runs never shipped has no content to + // diff the project's copy against. + $previous = $this->previousDir . '/' . $path; + + $registry->add($path, is_file($previous) ? File::read($previous) : '', File::read($project), File::read($next)); } - $data = json_decode((string) file_get_contents($file), TRUE); + $this->registryFile = $registry->write((string) $this->previousRef, (string) $this->config->get(Config::VERSION), date('Y-m-d H:i:s')); + } - if (!is_array($data)) { - return []; + /** + * Remove the rendered copy of the version the project currently runs. + */ + protected function cleanupPreviousTemplate(): void { + if ($this->previousDir !== NULL) { + File::remove($this->previousDir); + $this->previousDir = NULL; } + } - return array_filter($data, fn(mixed $hash, mixed $path): bool => is_string($path) && is_string($hash), ARRAY_FILTER_USE_BOTH); + /** + * Get the registry of project changes this update replaced. + * + * @return string|null + * Path of the registry, or NULL when no project change was replaced. + */ + public function getRegistryFile(): ?string { + return $this->registryFile; } /** @@ -356,11 +405,12 @@ protected function relativePaths(string $directory): array { public function removeObsoletePaths(): void { $destination = $this->config->getDestination(); - // 'scripts/vortex/' was the location of shipped Vortex scripts before - // they were extracted into the 'drevops/vortex-tooling' Composer package. - // Consumer projects updated from older Vortex versions still have it. $obsolete = [ + // The location of shipped Vortex scripts before they were extracted + // into the 'drevops/vortex-tooling' Composer package. 'scripts/vortex', + // Install-time bookkeeping, derived at run time instead. + '.vortex-manifest.json', ]; foreach ($obsolete as $relative) { diff --git a/.vortex/installer/src/Utils/UpdateRegistry.php b/.vortex/installer/src/Utils/UpdateRegistry.php new file mode 100644 index 0000000000..a466e4e105 --- /dev/null +++ b/.vortex/installer/src/Utils/UpdateRegistry.php @@ -0,0 +1,172 @@ + + */ + protected array $entries = []; + + public function __construct( + protected string $destination, + ) {} + + /** + * Record a path the update is about to replace. + * + * @param string $path + * Template-relative path. + * @param string $previous + * Content the version the project runs installed, empty when it shipped + * no such path. + * @param string $project + * Content the project holds. + * @param string $next + * Content the update installs. + */ + public function add(string $path, string $previous, string $project, string $next): void { + $this->entries[$path] = ['previous' => $previous, 'project' => $project, 'next' => $next]; + } + + /** + * Check whether anything was recorded. + */ + public function isEmpty(): bool { + return $this->entries === []; + } + + /** + * Append the recorded entries to the registry. + * + * @param string $from + * Version the project runs. + * @param string $to + * Version the update installs. + * @param string $time + * Timestamp of the update. + * + * @return string|null + * Absolute path of the registry, or NULL when nothing was recorded. + */ + public function write(string $from, string $to, string $time): ?string { + if ($this->isEmpty()) { + return NULL; + } + + ksort($this->entries); + + $content = sprintf('## %s to %s, %s', $from, $to, $time) . PHP_EOL . PHP_EOL; + + foreach ($this->entries as $path => $contents) { + $content .= $this->renderEntry($path, $contents); + } + + $file = $this->destination . '/' . self::FILE; + $existing = is_file($file) ? File::read($file) : self::HEADING . PHP_EOL; + + File::dump($file, $existing . $content); + + return $file; + } + + /** + * Render a single entry as Markdown. + * + * @param string $path + * Template-relative path. + * @param array{previous: string, project: string, next: string} $contents + * The three versions of the file's content. + * + * @return string + * The rendered entry. + */ + protected function renderEntry(string $path, array $contents): string { + $content = sprintf('### %s', $path) . PHP_EOL . PHP_EOL; + + foreach ($contents as $side) { + if (str_contains($side, "\0")) { + return $content . 'Binary file. Recover the project copy from version control.' . PHP_EOL . PHP_EOL; + } + + if (strlen($side) > self::MAX_DIFF_BYTES) { + return $content . 'File is too large to diff. Recover the project copy from version control.' . PHP_EOL . PHP_EOL; + } + } + + if ($contents['previous'] === '') { + $content .= 'The version the project runs did not ship this file. Project content that the update replaced:' . PHP_EOL . PHP_EOL; + + return $content . $this->renderDiff($contents['project'], $contents['next'], 'project', 'update'); + } + + $content .= 'Project change that the update replaced:' . PHP_EOL . PHP_EOL; + $content .= $this->renderDiff($contents['previous'], $contents['project'], 'installed', 'project'); + + if ($contents['previous'] === $contents['next']) { + return $content . 'The update ships this file unchanged.' . PHP_EOL . PHP_EOL; + } + + $content .= 'Change that the update brings:' . PHP_EOL . PHP_EOL; + + return $content . $this->renderDiff($contents['previous'], $contents['next'], 'installed', 'update'); + } + + /** + * Render a unified diff inside a fenced code block. + * + * @param string $from + * Content to diff from. + * @param string $to + * Content to diff to. + * @param string $from_label + * Label for the left side. + * @param string $to_label + * Label for the right side. + * + * @return string + * The fenced diff. + */ + protected function renderDiff(string $from, string $to, string $from_label, string $to_label): string { + $header = sprintf('--- %s', $from_label) . PHP_EOL . sprintf('+++ %s', $to_label) . PHP_EOL; + $differ = new Differ(new UnifiedDiffOutputBuilder($header)); + + return '```diff' . PHP_EOL . rtrim($differ->diff($from, $to)) . PHP_EOL . '```' . PHP_EOL . PHP_EOL; + } + +} diff --git a/.vortex/installer/tests/Fixtures/handler_process/_baseline/.ignorecontent b/.vortex/installer/tests/Fixtures/handler_process/_baseline/.ignorecontent index cb3f378abb..90c987a162 100644 --- a/.vortex/installer/tests/Fixtures/handler_process/_baseline/.ignorecontent +++ b/.vortex/installer/tests/Fixtures/handler_process/_baseline/.ignorecontent @@ -5,4 +5,3 @@ package-lock.json node_modules vendor .env.local -.vortex-manifest.json diff --git a/.vortex/installer/tests/Functional/Command/InstallExcludedPathsTest.php b/.vortex/installer/tests/Functional/Command/InstallExcludedPathsTest.php index 9fd804deb7..a05fa99e95 100644 --- a/.vortex/installer/tests/Functional/Command/InstallExcludedPathsTest.php +++ b/.vortex/installer/tests/Functional/Command/InstallExcludedPathsTest.php @@ -10,8 +10,9 @@ use DrevOps\VortexInstaller\Utils\Env; use DrevOps\VortexInstaller\Utils\File; use DrevOps\VortexInstaller\Utils\FileManager; +use DrevOps\VortexInstaller\Utils\Git; +use DrevOps\VortexInstaller\Utils\UpdateRegistry; use PHPUnit\Framework\Attributes\CoversClass; -use PHPUnit\Framework\Attributes\DataProvider; use Symfony\Component\Process\ExecutableFinder; /** @@ -19,143 +20,132 @@ */ #[CoversClass(FileManager::class)] #[CoversClass(InstallCommand::class)] +#[CoversClass(UpdateRegistry::class)] class InstallExcludedPathsTest extends FunctionalTestCase { + /** + * Selection that keeps every tool. + */ + const PROMPTS_ALL_TOOLS = '{"tools":["behat","dclint","eslint","hadolint","jest","phpcs","phpstan","phpunit","rector","stylelint","twig_cs_fixer"]}'; + /** * Selection that drops Jest, PHPStan and PHPUnit but keeps PHPCS and Behat. */ const PROMPTS_WITHOUT_TEST_TOOLS = '{"tools":["behat","dclint","eslint","hadolint","phpcs","rector","stylelint","twig_cs_fixer"]}'; - #[DataProvider('dataProviderExcludedPaths')] - public function testExcludedPaths(bool $is_vortex_project, array $existing, array $recorded, string $prompts, array $absent, array $present): void { - foreach ($existing as $path => $contents) { + public function testUpdateRemovesUnmodifiedExcludedPaths(): void { + $ref = $this->templateRef(); + + $this->runInstall(self::PROMPTS_ALL_TOOLS, $ref); + $this->assertFileExists(static::$sut . '/jest.config.js', 'A selected tool ships its configuration.'); + $this->assertFileExists(static::$sut . '/phpstan.neon', 'A selected tool ships its configuration.'); + + $this->runInstall(self::PROMPTS_WITHOUT_TEST_TOOLS, $ref); + + $this->assertFileDoesNotExist(static::$sut . '/jest.config.js', 'A deselected tool loses its unmodified configuration.'); + $this->assertFileDoesNotExist(static::$sut . '/phpstan.neon', 'A deselected tool loses its unmodified configuration.'); + $this->assertFileExists(static::$sut . '/phpcs.xml', 'A tool that stayed selected keeps its configuration.'); + $this->assertFileExists(static::$sut . '/behat.yml', 'A tool that stayed selected keeps its configuration.'); + } + + public function testUpdateKeepsModifiedExcludedPaths(): void { + $ref = $this->templateRef(); + + $this->runInstall(self::PROMPTS_ALL_TOOLS, $ref); + + $modified = "parameters:\n level: 8\n"; + File::dump(static::$sut . '/phpstan.neon', $modified); + + $this->runInstall(self::PROMPTS_WITHOUT_TEST_TOOLS, $ref); + + $this->assertFileExists(static::$sut . '/phpstan.neon', 'A file the project edited is never removed.'); + $this->assertStringEqualsFile(static::$sut . '/phpstan.neon', $modified, 'The project edit is left untouched.'); + $this->assertFileDoesNotExist(static::$sut . '/jest.config.js', 'Unmodified siblings are still removed.'); + } + + public function testUpdateKeepsProjectAuthoredPaths(): void { + $ref = $this->templateRef(); + + $this->runInstall(self::PROMPTS_ALL_TOOLS, $ref); + + $project_files = [ + 'custom-notes.md' => "Project notes.\n", + 'scripts/custom-deploy.sh' => "echo deploy\n", + 'web/modules/custom/mymodule/mymodule.info.yml' => "name: My module\n", + // Matched only by a glob over project content, not by a shipped path. + 'web/modules/custom/mymodule/js/mymodule.test.js' => "test('kept', () => {});\n", + ]; + foreach ($project_files as $path => $contents) { File::dump(static::$sut . '/' . $path, $contents); } - if ($recorded !== []) { - $hashes = array_map(fn(string $contents): string => hash('sha256', $contents), $recorded); - File::dump(static::$sut . '/' . FileManager::MANIFEST_FILE, (string) json_encode($hashes, JSON_PRETTY_PRINT)); - } + $this->runInstall(self::PROMPTS_WITHOUT_TEST_TOOLS, $ref); - if ($is_vortex_project) { - File::dump(static::$sut . '/README.md', '[![Vortex](https://img.shields.io/badge/Vortex-1.40.0-65ACBC.svg)](https://github.com/drevops/vortex)'); + foreach ($project_files as $path => $contents) { + $this->assertFileExists(static::$sut . '/' . $path, sprintf('Project-authored "%s" kept in the destination.', $path)); + $this->assertStringEqualsFile(static::$sut . '/' . $path, $contents, sprintf('Project-authored "%s" kept its contents.', $path)); } + } - $this->runInstall($prompts); + public function testUpdateRecordsReplacedProjectChanges(): void { + $ref = $this->templateRef(); - foreach ($absent as $path) { - $this->assertFileDoesNotExist(static::$sut . '/' . $path, sprintf('Path "%s" removed from the destination.', $path)); - } + $this->runInstall(self::PROMPTS_ALL_TOOLS, $ref); - foreach ($present as $path => $contents) { - $this->assertFileExists(static::$sut . '/' . $path, sprintf('Path "%s" kept in the destination.', $path)); + // A file the template keeps shipping, so the update replaces it. + $shipped = File::read(static::$sut . '/.ahoy.yml'); + File::dump(static::$sut . '/.ahoy.yml', $shipped . PHP_EOL . '# Project addition.' . PHP_EOL); - if ($contents !== NULL) { - $this->assertStringEqualsFile(static::$sut . '/' . $path, $contents, sprintf('Path "%s" kept its contents.', $path)); - } - } + $this->runInstall(self::PROMPTS_WITHOUT_TEST_TOOLS, $ref); + + $registry = static::$sut . '/' . UpdateRegistry::FILE; + + $this->assertFileExists($registry, 'A replaced project change is recorded.'); + $this->assertFileContainsString($registry, '### .ahoy.yml'); + $this->assertFileContainsString($registry, '+# Project addition.'); + $this->assertFileNotContainsString(static::$sut . '/.ahoy.yml', '# Project addition.', 'The update still replaces the project file.'); } - public static function dataProviderExcludedPaths(): \Iterator { + public function testUpdateRemovesCommittedManifest(): void { + $ref = $this->templateRef(); + + $this->runInstall(self::PROMPTS_ALL_TOOLS, $ref); + $this->assertFileDoesNotExist(static::$sut . '/.vortex-manifest.json', 'Install records nothing in the project.'); + + File::dump(static::$sut . '/.vortex-manifest.json', '{"composer.json":"abc"}'); + + $this->runInstall(self::PROMPTS_WITHOUT_TEST_TOOLS, $ref); + + $this->assertFileDoesNotExist(static::$sut . '/.vortex-manifest.json', 'A manifest an earlier install left behind is removed.'); + } + + public function testNothingRemovedFromDestinationThatIsNotVortexProject(): void { $shipped = [ 'phpstan.neon' => 'parameters: []', - 'phpunit.xml' => '', 'jest.config.js' => 'module.exports = {};', - 'tests/phpunit/bootstrap.php' => ' $contents) { + File::dump(static::$sut . '/' . $path, $contents); + } - yield 'unmodified excluded paths removed' => [ - TRUE, - $shipped, - $shipped, - self::PROMPTS_WITHOUT_TEST_TOOLS, - [ - 'phpstan.neon', - 'phpunit.xml', - 'jest.config.js', - 'tests/phpunit/bootstrap.php', - ], - [ - // A tool that stayed selected keeps its shipped configuration. - 'phpcs.xml' => NULL, - 'behat.yml' => NULL, - ], - ]; - yield 'modified excluded paths kept with their contents' => [ - TRUE, - [ - 'phpstan.neon' => "parameters:\n level: 8", - 'phpunit.xml' => '', - ], - $shipped, - self::PROMPTS_WITHOUT_TEST_TOOLS, - [ - // Unmodified, so still removed. - 'phpunit.xml', - ], - [ - 'phpstan.neon' => "parameters:\n level: 8", - ], - ]; - yield 'excluded paths kept when nothing was recorded' => [ - TRUE, - $shipped, - [], - self::PROMPTS_WITHOUT_TEST_TOOLS, - [], - [ - 'phpstan.neon' => 'parameters: []', - 'jest.config.js' => 'module.exports = {};', - ], - ]; - yield 'project-authored paths kept' => [ - TRUE, - [ - 'custom-notes.md' => 'Project notes.', - 'scripts/custom-deploy.sh' => 'echo deploy', - 'web/modules/custom/mymodule/mymodule.info.yml' => 'name: My module', - 'web/modules/custom/mymodule/js/mymodule.test.js' => "test('kept', () => {});", - ], - $shipped, - self::PROMPTS_WITHOUT_TEST_TOOLS, - [], - [ - // Never shipped by the template, so never a candidate for removal. - 'custom-notes.md' => 'Project notes.', - 'scripts/custom-deploy.sh' => 'echo deploy', - 'web/modules/custom/mymodule/mymodule.info.yml' => 'name: My module', - // Matched only by a glob over project content, not by a shipped path. - 'web/modules/custom/mymodule/js/mymodule.test.js' => "test('kept', () => {});", - ], - ]; - yield 'harness paths kept' => [ - TRUE, - ['.vortex/CLAUDE.md' => 'Project owned.'], - ['.vortex/CLAUDE.md' => 'Project owned.'], - self::PROMPTS_WITHOUT_TEST_TOOLS, - [], - [ - // The harness is stripped unconditionally rather than by selection. - '.vortex/CLAUDE.md' => 'Project owned.', - ], - ]; - yield 'nothing removed from a destination that is not a Vortex project' => [ - FALSE, - $shipped, - $shipped, - self::PROMPTS_WITHOUT_TEST_TOOLS, - [], - [ - 'phpstan.neon' => 'parameters: []', - 'jest.config.js' => 'module.exports = {};', - ], - ]; + $this->runInstall(self::PROMPTS_WITHOUT_TEST_TOOLS, $this->templateRef()); + + foreach ($shipped as $path => $contents) { + $this->assertStringEqualsFile(static::$sut . '/' . $path, $contents, sprintf('Path "%s" kept its contents.', $path)); + } + } + + /** + * Get the reference the template is installed from. + */ + protected function templateRef(): string { + return (new Git(File::dir(static::$root)))->getLastShortCommitId(); } /** * Run a non-interactive install into the system under test. */ - protected function runInstall(string $prompts): void { + protected function runInstall(string $prompts, string $ref): void { $executable_finder = $this->createMock(ExecutableFinder::class); $executable_finder->method('find')->willReturnCallback(fn(string $command): string => '/usr/bin/' . $command); @@ -168,7 +158,7 @@ protected function runInstall(string $prompts): void { $this->applicationRun([ '--' . InstallCommand::OPTION_NO_INTERACTION => TRUE, - '--' . InstallCommand::OPTION_URI => File::dir(static::$root), + '--' . InstallCommand::OPTION_URI => sprintf('%s#%s', File::dir(static::$root), $ref), '--' . InstallCommand::OPTION_DESTINATION => static::$sut, '--' . InstallCommand::OPTION_PROMPTS => $prompts, ]); diff --git a/.vortex/installer/tests/Unit/Utils/FileManagerTest.php b/.vortex/installer/tests/Unit/Utils/FileManagerTest.php index 4f5cb034f8..650f660d87 100644 --- a/.vortex/installer/tests/Unit/Utils/FileManagerTest.php +++ b/.vortex/installer/tests/Unit/Utils/FileManagerTest.php @@ -4,11 +4,14 @@ namespace DrevOps\VortexInstaller\Tests\Unit\Utils; +use DrevOps\VortexInstaller\Downloader\Artifact; use DrevOps\VortexInstaller\Downloader\Downloader; +use DrevOps\VortexInstaller\Downloader\RepositoryDownloader; use DrevOps\VortexInstaller\Tests\Unit\UnitTestCase; use DrevOps\VortexInstaller\Utils\Config; use DrevOps\VortexInstaller\Utils\File; use DrevOps\VortexInstaller\Utils\FileManager; +use DrevOps\VortexInstaller\Utils\UpdateRegistry; use PHPUnit\Framework\Attributes\CoversClass; /** @@ -175,7 +178,7 @@ public function testCopyFilesRemovesUnmodifiedExcludedPaths(): void { // A previous install wrote both, unmodified since. file_put_contents(File::mkdir($destination) . '/phpstan.neon', 'parameters: []'); file_put_contents(File::mkdir($destination . '/.circleci') . '/config.yml', 'version: 2.1'); - $this->stubManifest($destination, [ + $this->stubPreviousTemplate($fm, $destination, [ 'phpstan.neon' => 'parameters: []', '.circleci/config.yml' => 'version: 2.1', ]); @@ -206,7 +209,7 @@ public function testCopyFilesKeepsModifiedExcludedPaths(): void { // The project edited the file after the previous install wrote it. file_put_contents(File::mkdir($destination) . '/phpstan.neon', "parameters:\n level: 8"); - $this->stubManifest($destination, ['phpstan.neon' => 'parameters: []']); + $this->stubPreviousTemplate($fm, $destination, ['phpstan.neon' => 'parameters: []']); $fm->snapshotTemplate(); File::remove($src . '/phpstan.neon'); @@ -228,7 +231,7 @@ public function testCopyFilesKeepsExcludedPathsWithoutRecordedHash(): void { $fm = new FileManager($config); $fm->snapshotTemplate(); - // No manifest and no previous version, so ownership cannot be established. + // No previous version, so ownership cannot be established. file_put_contents(File::mkdir($destination) . '/phpstan.neon', 'parameters: []'); File::remove($src . '/phpstan.neon'); @@ -237,9 +240,9 @@ public function testCopyFilesKeepsExcludedPathsWithoutRecordedHash(): void { $this->assertFileExists($destination . '/phpstan.neon', 'Without a recorded hash the file is left alone.'); } - public function testCopyFilesWritesTheManifest(): void { - $src = self::$sut . '/src_manifest'; - $destination = self::$sut . '/dst_manifest'; + public function testCopyFilesWritesNoManifest(): void { + $src = self::$sut . '/src_no_manifest'; + $destination = self::$sut . '/dst_no_manifest'; file_put_contents(File::mkdir($src) . '/composer.json', '{}'); file_put_contents(File::mkdir($src . '/scripts') . '/provision.sh', 'echo 1'); @@ -249,11 +252,119 @@ public function testCopyFilesWritesTheManifest(): void { $fm->copyFiles(); - $manifest = json_decode((string) file_get_contents($destination . '/' . FileManager::MANIFEST_FILE), TRUE); + $this->assertFileDoesNotExist($destination . '/.vortex-manifest.json', 'Install records nothing in the project.'); + } + + public function testCopyFilesRemovesExcludedPathsMatchedOnlyAfterRendering(): void { + $src = self::$sut . '/src_rendered'; + $destination = self::$sut . '/dst_rendered'; + file_put_contents(File::mkdir($src) . '/composer.json', '{}'); + file_put_contents($src . '/rector.php', 'paths: your_site'); + + $config = new Config('/tmp/root', $destination, $src); + $config->set(Config::IS_VORTEX_PROJECT, TRUE, TRUE); + $fm = new FileManager($config); + + // The project holds the rendered content, which the download never has. + file_put_contents(File::mkdir($destination) . '/rector.php', 'paths: star_wars'); + $this->stubPreviousTemplate($fm, $destination, ['rector.php' => 'paths: your_site'], function (string $dir): void { + File::dump($dir . '/rector.php', 'paths: star_wars'); + }); + + $fm->snapshotTemplate(); + File::remove($src . '/rector.php'); + + $fm->copyFiles(); + + $this->assertFileDoesNotExist($destination . '/rector.php', 'Rendering resolves tokens that the download itself cannot match.'); + } + + public function testCopyFilesRemovesExcludedPathsDroppedByRendering(): void { + $src = self::$sut . '/src_deselected'; + $destination = self::$sut . '/dst_deselected'; + file_put_contents(File::mkdir($src) . '/composer.json', '{}'); + file_put_contents($src . '/jest.config.js', 'module.exports = {};'); + + $config = new Config('/tmp/root', $destination, $src); + $config->set(Config::IS_VORTEX_PROJECT, TRUE, TRUE); + $fm = new FileManager($config); + + file_put_contents(File::mkdir($destination) . '/jest.config.js', 'module.exports = {};'); + + // This run deselected the tool, so rendering the previous version with + // these answers drops the file the project still holds. + $this->stubPreviousTemplate($fm, $destination, ['jest.config.js' => 'module.exports = {};'], function (string $dir): void { + File::remove($dir . '/jest.config.js'); + }); + + $fm->snapshotTemplate(); + File::remove($src . '/jest.config.js'); + + $fm->copyFiles(); + + $this->assertFileDoesNotExist($destination . '/jest.config.js', "The download's own hash establishes ownership when rendering drops the file."); + } + + public function testCopyFilesRecordsReplacedProjectChanges(): void { + $src = self::$sut . '/src_registry'; + $destination = self::$sut . '/dst_registry'; + file_put_contents(File::mkdir($src) . '/phpstan.neon', "parameters:\n level: 9\n"); + + $config = new Config('/tmp/root', $destination, $src); + $config->set(Config::IS_VORTEX_PROJECT, TRUE, TRUE); + $config->set(Config::VERSION, '1.41.0', TRUE); + $fm = new FileManager($config); + + // The project edited the file the previous version installed. + file_put_contents(File::mkdir($destination) . '/phpstan.neon', "parameters:\n level: 8\n"); + $this->stubPreviousTemplate($fm, $destination, ['phpstan.neon' => "parameters:\n level: 5\n"]); + + $fm->snapshotTemplate(); + $fm->copyFiles(); - $this->assertIsArray($manifest); - $this->assertArrayHasKey('scripts/provision.sh', $manifest, 'Manifest records every shipped path.'); - $this->assertEquals(hash('sha256', 'echo 1'), $manifest['scripts/provision.sh'], 'Manifest records the content that was written.'); + $registry = $destination . '/' . UpdateRegistry::FILE; + + $this->assertEquals($registry, $fm->getRegistryFile()); + $this->assertStringEqualsFile($destination . '/phpstan.neon', "parameters:\n level: 9\n", 'The update still replaces the project file.'); + $this->assertFileContainsString($registry, '## 1.40.0 to 1.41.0'); + $this->assertFileContainsString($registry, '### phpstan.neon'); + $this->assertFileContainsString($registry, '- level: 5'); + $this->assertFileContainsString($registry, '+ level: 8'); + $this->assertFileContainsString($registry, '+ level: 9'); + } + + public function testCopyFilesRecordsNothingWithoutProjectChanges(): void { + $src = self::$sut . '/src_no_registry'; + $destination = self::$sut . '/dst_no_registry'; + file_put_contents(File::mkdir($src) . '/phpstan.neon', "parameters:\n level: 9\n"); + + $config = new Config('/tmp/root', $destination, $src); + $config->set(Config::IS_VORTEX_PROJECT, TRUE, TRUE); + $fm = new FileManager($config); + + file_put_contents(File::mkdir($destination) . '/phpstan.neon', "parameters:\n level: 5\n"); + $this->stubPreviousTemplate($fm, $destination, ['phpstan.neon' => "parameters:\n level: 5\n"]); + + $fm->snapshotTemplate(); + $fm->copyFiles(); + + $this->assertNull($fm->getRegistryFile()); + $this->assertFileDoesNotExist($destination . '/' . UpdateRegistry::FILE, 'An untouched file leaves nothing to reconcile.'); + } + + public function testCopyFilesRemovesCommittedManifest(): void { + $src = self::$sut . '/src_stale_manifest'; + $destination = self::$sut . '/dst_stale_manifest'; + file_put_contents(File::mkdir($src) . '/composer.json', '{}'); + + $config = new Config('/tmp/root', $destination, $src); + $fm = new FileManager($config); + + file_put_contents(File::mkdir($destination) . '/.vortex-manifest.json', '{"composer.json":"abc"}'); + + $fm->copyFiles(); + + $this->assertFileDoesNotExist($destination . '/.vortex-manifest.json', 'A manifest an earlier install left behind is removed.'); } public function testCopyFilesKeepsPathsTheTemplateNeverShipped(): void { @@ -349,17 +460,30 @@ public function testRemoveObsoletePathsSilentOnMissing(): void { } /** - * Write a manifest recording what a previous install wrote. + * Snapshot a stubbed download of the version the project runs. * + * @param \DrevOps\VortexInstaller\Utils\FileManager $fm + * The file manager to snapshot into. * @param string $destination * The project directory. * @param array $files - * Content the previous install wrote, keyed by relative path. + * Content the previous version installed, keyed by relative path. + * @param callable|null $render + * Callback turning the download into installable content. */ - protected function stubManifest(string $destination, array $files): void { - $hashes = array_map(fn(string $contents): string => hash('sha256', $contents), $files); + protected function stubPreviousTemplate(FileManager $fm, string $destination, array $files, ?callable $render = NULL): void { + File::dump($destination . '/README.md', '[![Vortex](https://img.shields.io/badge/Vortex-1.40.0-65ACBC.svg)](https://github.com/drevops/vortex)'); + + $downloader = $this->createStub(RepositoryDownloader::class); + $downloader->method('download')->willReturnCallback(function (Artifact $artifact, ?string $dir = NULL) use ($files): string { + foreach ($files as $path => $contents) { + File::dump($dir . '/' . $path, $contents); + } + + return $artifact->getRef(); + }); - File::dump($destination . '/' . FileManager::MANIFEST_FILE, (string) json_encode($hashes, JSON_PRETTY_PRINT)); + $fm->snapshotPreviousTemplate($downloader, Artifact::create('https://github.com/drevops/vortex.git', '1.40.0'), $render); } /** diff --git a/.vortex/installer/tests/Unit/Utils/UpdateRegistryTest.php b/.vortex/installer/tests/Unit/Utils/UpdateRegistryTest.php new file mode 100644 index 0000000000..2fbd9d64c3 --- /dev/null +++ b/.vortex/installer/tests/Unit/Utils/UpdateRegistryTest.php @@ -0,0 +1,121 @@ +assertTrue($registry->isEmpty()); + $this->assertNull($registry->write('1.40.0', '1.41.0', '2026-09-07 09:31:22')); + $this->assertFileDoesNotExist(self::$sut . '/' . UpdateRegistry::FILE); + } + + public function testWriteRendersBothDiffs(): void { + $registry = new UpdateRegistry(self::$sut); + $registry->add('phpstan.neon', "level: 5\n", "level: 8\n", "level: 9\n"); + + $file = $registry->write('1.40.0', '1.41.0', '2026-09-07 09:31:22'); + + $this->assertFalse($registry->isEmpty()); + $this->assertEquals(self::$sut . '/' . UpdateRegistry::FILE, $file); + $this->assertFileContainsString((string) $file, '# Vortex update registry'); + $this->assertFileContainsString((string) $file, '## 1.40.0 to 1.41.0, 2026-09-07 09:31:22'); + $this->assertFileContainsString((string) $file, '### phpstan.neon'); + $this->assertFileContainsString((string) $file, 'Project change that the update replaced:'); + $this->assertFileContainsString((string) $file, 'Change that the update brings:'); + $this->assertFileContainsString((string) $file, '-level: 5'); + $this->assertFileContainsString((string) $file, '+level: 8'); + $this->assertFileContainsString((string) $file, '+level: 9'); + $this->assertFileContainsString((string) $file, '```diff'); + } + + public function testWriteOmitsUnchangedUpdateDiff(): void { + $registry = new UpdateRegistry(self::$sut); + $registry->add('phpstan.neon', "level: 5\n", "level: 8\n", "level: 5\n"); + + $file = (string) $registry->write('1.40.0', '1.41.0', '2026-09-07 09:31:22'); + + $this->assertFileContainsString($file, 'The update ships this file unchanged.'); + $this->assertFileNotContainsString($file, 'Change that the update brings:'); + } + + public function testWriteNotesBinaryContent(): void { + $registry = new UpdateRegistry(self::$sut); + $registry->add('logo.png', "\x89PNG\0old", "\x89PNG\0project", "\x89PNG\0new"); + + $file = (string) $registry->write('1.40.0', '1.41.0', '2026-09-07 09:31:22'); + + $this->assertFileContainsString($file, '### logo.png'); + $this->assertFileContainsString($file, 'Binary file. Recover the project copy from version control.'); + $this->assertFileNotContainsString($file, '```diff'); + } + + public function testWriteNotesOversizedContent(): void { + $registry = new UpdateRegistry(self::$sut); + $registry->add('package-lock.json', '{}', str_repeat('a', UpdateRegistry::MAX_DIFF_BYTES + 1), '{}'); + + $file = (string) $registry->write('1.40.0', '1.41.0', '2026-09-07 09:31:22'); + + $this->assertFileContainsString($file, '### package-lock.json'); + $this->assertFileContainsString($file, 'File is too large to diff. Recover the project copy from version control.'); + $this->assertFileNotContainsString($file, '```diff'); + } + + public function testWriteRendersProjectAuthoredPath(): void { + $registry = new UpdateRegistry(self::$sut); + $registry->add('phpstan.neon', '', "level: 8\n", "level: 9\n"); + + $file = (string) $registry->write('1.40.0', '1.41.0', '2026-09-07 09:31:22'); + + $this->assertFileContainsString($file, 'The version the project runs did not ship this file.'); + $this->assertFileContainsString($file, '-level: 8'); + $this->assertFileContainsString($file, '+level: 9'); + $this->assertFileNotContainsString($file, 'Change that the update brings:'); + } + + public function testWriteSortsEntriesByPath(): void { + $registry = new UpdateRegistry(self::$sut); + $registry->add('phpstan.neon', "a\n", "b\n", "c\n"); + $registry->add('.circleci/config.yml', "a\n", "b\n", "c\n"); + + $file = (string) $registry->write('1.40.0', '1.41.0', '2026-09-07 09:31:22'); + + $this->assertLessThan( + (int) strpos(File::read($file), '### phpstan.neon'), + (int) strpos(File::read($file), '### .circleci/config.yml'), + 'Entries are rendered in path order.' + ); + } + + public function testWriteAppendsToExistingRegistry(): void { + $registry = new UpdateRegistry(self::$sut); + $registry->add('phpstan.neon', "level: 5\n", "level: 8\n", "level: 9\n"); + $registry->write('1.40.0', '1.41.0', '2026-09-07 09:31:22'); + + $next = new UpdateRegistry(self::$sut); + $next->add('behat.yml', "a\n", "b\n", "c\n"); + $file = (string) $next->write('1.41.0', '1.42.0', '2026-09-08 10:00:00'); + + $content = File::read($file); + + $this->assertEquals(1, substr_count($content, '# Vortex update registry'), 'The heading is written once.'); + $this->assertFileContainsString($file, '## 1.40.0 to 1.41.0, 2026-09-07 09:31:22'); + $this->assertFileContainsString($file, '## 1.41.0 to 1.42.0, 2026-09-08 10:00:00'); + $this->assertFileContainsString($file, '### phpstan.neon'); + $this->assertFileContainsString($file, '### behat.yml'); + } + +} diff --git a/.vortex/tests/phpunit/Functional/InstallerTest.php b/.vortex/tests/phpunit/Functional/InstallerTest.php index e2ca1e87f3..25e38dcf30 100644 --- a/.vortex/tests/phpunit/Functional/InstallerTest.php +++ b/.vortex/tests/phpunit/Functional/InstallerTest.php @@ -94,7 +94,7 @@ public function testUpdateRemovesUnmodifiedFilesDroppedByTemplate(): void { $this->logSubstep('Install the SUT from the version that ships the script'); $this->installSutFrom($commit_with_script); $this->assertFileExists('scripts/provision-50-legacy.sh', 'Template-owned script installed into the SUT'); - $this->assertFileExists('.vortex-manifest.json', 'Install records what it wrote'); + $this->assertFileDoesNotExist('.vortex-manifest.json', 'Install records nothing in the project'); $this->gitCommitAll(static::$sut, 'Init Vortex'); $commit_without_script = $this->dropLegacyScriptFromTemplate(); From 9bf7a21f34d2d407870c6c63e1c1259aa5d9b7a1 Mon Sep 17 00:00:00 2001 From: Alex Skrypnyk Date: Mon, 7 Sep 2026 12:41:10 +1000 Subject: [PATCH 2/3] [#3094] Rendered the version the project runs with discovered answers. --- .../installer/src/Command/InstallCommand.php | 2 +- .../installer/src/Prompts/PromptManager.php | 54 ++++++++++++++++--- .vortex/installer/src/Utils/FileManager.php | 30 +++++------ .../Command/InstallExcludedPathsTest.php | 4 ++ .../tests/Unit/Utils/FileManagerTest.php | 14 +++-- 5 files changed, 69 insertions(+), 35 deletions(-) diff --git a/.vortex/installer/src/Command/InstallCommand.php b/.vortex/installer/src/Command/InstallCommand.php index 555529b59e..6e49941415 100644 --- a/.vortex/installer/src/Command/InstallCommand.php +++ b/.vortex/installer/src/Command/InstallCommand.php @@ -228,7 +228,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $this->getRepositoryDownloader(), $this->artifact, function (string $dir, string $ref): void { - $this->promptManager->renderTemplate($dir, $ref); + $this->promptManager->renderAsInstalled($dir, $ref); }, ); return $version; diff --git a/.vortex/installer/src/Prompts/PromptManager.php b/.vortex/installer/src/Prompts/PromptManager.php index 93e23a40d9..a14be11c1e 100644 --- a/.vortex/installer/src/Prompts/PromptManager.php +++ b/.vortex/installer/src/Prompts/PromptManager.php @@ -72,6 +72,14 @@ class PromptManager { */ protected array $responses = []; + /** + * Responses describing the destination as the handlers found it. + * + * Collected while the prompts run, so each value is the one discovery + * produced with the same preceding responses in context. + */ + protected array $discoveredResponses = []; + /** * Current response index. * @@ -247,6 +255,27 @@ public function runPrompts(): void { // Filter out elements with numeric keys returned by intro() calls. $responses = array_filter($responses, fn($key): bool => !is_numeric($key), ARRAY_FILTER_USE_KEY); + if ($this->config->getNoInteraction()) { + Tui::output()->setVerbosity($original_verbosity); + } + + $this->responses = $this->normalizeResponses($responses); + + // Discovery covers only the handlers that read the destination, so the + // collected answers fill the rest. + $this->discoveredResponses = $this->normalizeResponses(array_replace($responses, $this->discoveredResponses)); + } + + /** + * Fold internal answers into the responses they qualify. + * + * @param array $responses + * Raw responses keyed by handler ID. + * + * @return array + * The responses with internal answers merged and removed. + */ + protected function normalizeResponses(array $responses): array { if (isset($responses[Profile::id()]) && $responses[Profile::id()] === Profile::CUSTOM && isset($responses[ProfileCustom::id()])) { $responses[Profile::id()] = $responses[ProfileCustom::id()]; } @@ -270,11 +299,7 @@ public function runPrompts(): void { $responses[Starter::id()] = Starter::LOAD_DATABASE_DEMO; } - if ($this->config->getNoInteraction()) { - Tui::output()->setVerbosity($original_verbosity); - } - - $this->responses = $responses; + return $responses; } /** @@ -349,14 +374,19 @@ public function runProcessors(): void { } /** - * Render a template download using the responses this run collected. + * Render a template download as the destination has it installed. + * + * The answers come from discovery against the destination rather than from + * the choices this run collected, so the render reproduces the project's + * current configuration even where this run changes it. That is what makes + * the result comparable to the project's own files. * * @param string $dir * Directory holding an unprocessed template download. * @param string $version * Version to stamp into the rendered content. */ - public function renderTemplate(string $dir, string $version): void { + public function renderAsInstalled(string $dir, string $version): void { $config = clone $this->config; $config->set(Config::TMP, $dir, TRUE); $config->set(Config::VERSION, $version, TRUE); @@ -364,7 +394,7 @@ public function renderTemplate(string $dir, string $version): void { // Handlers bind to the directory they are constructed with, so rendering // into a directory other than this run's staging copy needs its own set. $manager = new self($config); - $manager->responses = $this->responses; + $manager->responses = $this->discoveredResponses; $manager->runProcessors(); } @@ -679,6 +709,10 @@ protected function args(string $handler_class, mixed $default_override = NULL, a $default_from_prompts = $this->promptOverrides[$id] ?? NULL; $default_from_discovery = $handler->discover(); + if ($default_from_discovery !== NULL) { + $this->discoveredResponses[$id] = $default_from_discovery; + } + if ($default_from_prompts !== NULL) { $default = $default_from_prompts; } @@ -723,6 +757,10 @@ protected function resolveOrPrompt(string $handler_id, array $r, callable $promp Tui::success($message); } + // A resolved value is read from the destination, so it stands in for + // discovery for handlers that never reach a prompt. + $this->discoveredResponses[$handler_id] = $resolved; + return $resolved; } diff --git a/.vortex/installer/src/Utils/FileManager.php b/.vortex/installer/src/Utils/FileManager.php index e48445b39c..c9ac5b8149 100644 --- a/.vortex/installer/src/Utils/FileManager.php +++ b/.vortex/installer/src/Utils/FileManager.php @@ -31,9 +31,9 @@ class FileManager { const HASH_ALGO = 'sha256'; /** - * Content hashes the version the project currently runs could have written. + * Content hashes the version the project currently runs installed. * - * @var array> + * @var array */ protected array $previousTemplateHashes = []; @@ -84,11 +84,11 @@ public function snapshotTemplate(): void { * the project's own version restores it as a candidate, which is what makes * a file dropped between releases removable rather than permanent. * - * The download is hashed both as it arrives and once rendered. Rendering - * resolves token replacements and directory renames, which the download's - * own files cannot match; rendering also applies this run's answers, which - * strips whatever the current selection drops. Either hash therefore stands - * for content the project could hold, so both are kept as candidates. + * The download is rendered rather than hashed as it arrives, resolving the + * token replacements and directory renames that leave the template's own + * files matching nothing in the project. Rendering it as the destination + * has it installed, rather than as this run would install it, is what keeps + * a path this run drops recognisable as template-owned. * * Failure is not fatal: the recorded reference may no longer resolve, in * which case only the selection diff applies. @@ -120,17 +120,11 @@ public function snapshotPreviousTemplate(RepositoryDownloader $downloader, Artif File::mkdir($dir); $downloader->download(Artifact::create($artifact->getRepo(), $ref), $dir); - $hashes = array_map(fn(string $hash): array => [$hash], $this->hashDirectory($dir)); - if ($render !== NULL) { $render($dir, $ref); - - foreach ($this->hashDirectory($dir) as $path => $hash) { - $hashes[$path][] = $hash; - } } - $this->previousTemplateHashes = array_map(fn(array $candidates): array => array_values(array_unique($candidates)), $hashes); + $this->previousTemplateHashes = $this->hashDirectory($dir); $this->previousDir = $dir; $this->previousRef = $ref; } @@ -242,8 +236,8 @@ public function copyFiles(): void { * * @param array $paths * Template-relative paths absent from the staged copy. - * @param array> $expected - * Content hashes the template could have written, keyed by path. + * @param array $expected + * Content hashes the template last installed, keyed by path. */ protected function removeExcludedPaths(array $paths, array $expected): void { if (!$this->config->isVortexProject()) { @@ -268,7 +262,7 @@ protected function removeExcludedPaths(array $paths, array $expected): void { // Without a recorded hash there is nothing to compare the project's copy // against, so ownership cannot be established. - if (!isset($expected[$path]) || !in_array(hash_file(self::HASH_ALGO, $target), $expected[$path], TRUE)) { + if (!isset($expected[$path]) || hash_file(self::HASH_ALGO, $target) !== $expected[$path]) { continue; } @@ -318,7 +312,7 @@ protected function recordReplacedChanges(string $src): void { // The file still holds what the template put there, or already holds // what the copy would put there, so the copy replaces nothing. - if (in_array($project_hash, $this->previousTemplateHashes[$path] ?? [], TRUE) || $project_hash === hash_file(self::HASH_ALGO, $next)) { + if ($project_hash === ($this->previousTemplateHashes[$path] ?? NULL) || $project_hash === hash_file(self::HASH_ALGO, $next)) { continue; } diff --git a/.vortex/installer/tests/Functional/Command/InstallExcludedPathsTest.php b/.vortex/installer/tests/Functional/Command/InstallExcludedPathsTest.php index a05fa99e95..56e9aeef90 100644 --- a/.vortex/installer/tests/Functional/Command/InstallExcludedPathsTest.php +++ b/.vortex/installer/tests/Functional/Command/InstallExcludedPathsTest.php @@ -102,6 +102,10 @@ public function testUpdateRecordsReplacedProjectChanges(): void { $this->assertFileExists($registry, 'A replaced project change is recorded.'); $this->assertFileContainsString($registry, '### .ahoy.yml'); + // Rendering the installed version reproduces token replacements and the + // theme directory rename, so files the project left alone match it. + $this->assertFileNotContainsString($registry, '### composer.json', 'A token-processed file the project did not change is not recorded.'); + $this->assertFileNotContainsString($registry, '### web/themes/custom/star_wars/package.json', 'A file under a renamed directory is not recorded.'); $this->assertFileContainsString($registry, '+# Project addition.'); $this->assertFileNotContainsString(static::$sut . '/.ahoy.yml', '# Project addition.', 'The update still replaces the project file.'); } diff --git a/.vortex/installer/tests/Unit/Utils/FileManagerTest.php b/.vortex/installer/tests/Unit/Utils/FileManagerTest.php index 650f660d87..8c4bcb4490 100644 --- a/.vortex/installer/tests/Unit/Utils/FileManagerTest.php +++ b/.vortex/installer/tests/Unit/Utils/FileManagerTest.php @@ -276,10 +276,10 @@ public function testCopyFilesRemovesExcludedPathsMatchedOnlyAfterRendering(): vo $fm->copyFiles(); - $this->assertFileDoesNotExist($destination . '/rector.php', 'Rendering resolves tokens that the download itself cannot match.'); + $this->assertFileDoesNotExist($destination . '/rector.php', 'Rendering resolves tokens that the download itself does not match.'); } - public function testCopyFilesRemovesExcludedPathsDroppedByRendering(): void { + public function testCopyFilesRemovesExcludedPathsDeselectedByThisRun(): void { $src = self::$sut . '/src_deselected'; $destination = self::$sut . '/dst_deselected'; file_put_contents(File::mkdir($src) . '/composer.json', '{}'); @@ -291,18 +291,16 @@ public function testCopyFilesRemovesExcludedPathsDroppedByRendering(): void { file_put_contents(File::mkdir($destination) . '/jest.config.js', 'module.exports = {};'); - // This run deselected the tool, so rendering the previous version with - // these answers drops the file the project still holds. - $this->stubPreviousTemplate($fm, $destination, ['jest.config.js' => 'module.exports = {};'], function (string $dir): void { - File::remove($dir . '/jest.config.js'); - }); + // Discovery answers describe the project, which still has the tool, so + // the render keeps the file even though this run deselects it. + $this->stubPreviousTemplate($fm, $destination, ['jest.config.js' => 'module.exports = {};']); $fm->snapshotTemplate(); File::remove($src . '/jest.config.js'); $fm->copyFiles(); - $this->assertFileDoesNotExist($destination . '/jest.config.js', "The download's own hash establishes ownership when rendering drops the file."); + $this->assertFileDoesNotExist($destination . '/jest.config.js', 'A tool the project has is still removable when this run deselects it.'); } public function testCopyFilesRecordsReplacedProjectChanges(): void { From 16e702b261b61ce192e94f0fa449bcb96d73b953 Mon Sep 17 00:00:00 2001 From: Alex Skrypnyk Date: Mon, 7 Sep 2026 13:20:54 +1000 Subject: [PATCH 3/3] Addressed code review: discovered conditional answers, gated manifest removal, widened diff fences. --- .../installer/src/Prompts/PromptManager.php | 13 ++++++++++ .vortex/installer/src/Utils/FileManager.php | 10 +++++--- .../installer/src/Utils/UpdateRegistry.php | 23 +++++++++++------- .../tests/Unit/Utils/FileManagerTest.php | 16 +++++++++++++ .../tests/Unit/Utils/UpdateRegistryTest.php | 24 ++++++++++++++++++- 5 files changed, 74 insertions(+), 12 deletions(-) diff --git a/.vortex/installer/src/Prompts/PromptManager.php b/.vortex/installer/src/Prompts/PromptManager.php index a14be11c1e..8e3774bfa3 100644 --- a/.vortex/installer/src/Prompts/PromptManager.php +++ b/.vortex/installer/src/Prompts/PromptManager.php @@ -261,6 +261,19 @@ public function runPrompts(): void { $this->responses = $this->normalizeResponses($responses); + // A conditional prompt this run skips never reaches args(), so its handler + // is asked directly. Otherwise the answer describing the destination would + // be replaced by the one describing this run. + foreach ($this->handlers as $id => $handler) { + if (!isset($this->discoveredResponses[$id])) { + $discovered = $handler->discover(); + + if ($discovered !== NULL) { + $this->discoveredResponses[$id] = $discovered; + } + } + } + // Discovery covers only the handlers that read the destination, so the // collected answers fill the rest. $this->discoveredResponses = $this->normalizeResponses(array_replace($responses, $this->discoveredResponses)); diff --git a/.vortex/installer/src/Utils/FileManager.php b/.vortex/installer/src/Utils/FileManager.php index c9ac5b8149..b3c3bcdad6 100644 --- a/.vortex/installer/src/Utils/FileManager.php +++ b/.vortex/installer/src/Utils/FileManager.php @@ -320,7 +320,7 @@ protected function recordReplacedChanges(string $src): void { // diff the project's copy against. $previous = $this->previousDir . '/' . $path; - $registry->add($path, is_file($previous) ? File::read($previous) : '', File::read($project), File::read($next)); + $registry->add($path, is_file($previous) ? File::read($previous) : NULL, File::read($project), File::read($next)); } $this->registryFile = $registry->write((string) $this->previousRef, (string) $this->config->get(Config::VERSION), date('Y-m-d H:i:s')); @@ -403,10 +403,14 @@ public function removeObsoletePaths(): void { // The location of shipped Vortex scripts before they were extracted // into the 'drevops/vortex-tooling' Composer package. 'scripts/vortex', - // Install-time bookkeeping, derived at run time instead. - '.vortex-manifest.json', ]; + // Install-time bookkeeping, derived at run time instead. Only a project + // that already runs Vortex can hold one the installer wrote. + if ($this->config->isVortexProject()) { + $obsolete[] = '.vortex-manifest.json'; + } + foreach ($obsolete as $relative) { $path = $destination . '/' . $relative; if (file_exists($path)) { diff --git a/.vortex/installer/src/Utils/UpdateRegistry.php b/.vortex/installer/src/Utils/UpdateRegistry.php index a466e4e105..442fed1448 100644 --- a/.vortex/installer/src/Utils/UpdateRegistry.php +++ b/.vortex/installer/src/Utils/UpdateRegistry.php @@ -39,7 +39,7 @@ class UpdateRegistry { /** * Replaced content, keyed by template-relative path. * - * @var array + * @var array */ protected array $entries = []; @@ -52,15 +52,15 @@ public function __construct( * * @param string $path * Template-relative path. - * @param string $previous - * Content the version the project runs installed, empty when it shipped + * @param string|null $previous + * Content the version the project runs installed, or NULL when it shipped * no such path. * @param string $project * Content the project holds. * @param string $next * Content the update installs. */ - public function add(string $path, string $previous, string $project, string $next): void { + public function add(string $path, ?string $previous, string $project, string $next): void { $this->entries[$path] = ['previous' => $previous, 'project' => $project, 'next' => $next]; } @@ -110,7 +110,7 @@ public function write(string $from, string $to, string $time): ?string { * * @param string $path * Template-relative path. - * @param array{previous: string, project: string, next: string} $contents + * @param array{previous: string|null, project: string, next: string} $contents * The three versions of the file's content. * * @return string @@ -119,7 +119,7 @@ public function write(string $from, string $to, string $time): ?string { protected function renderEntry(string $path, array $contents): string { $content = sprintf('### %s', $path) . PHP_EOL . PHP_EOL; - foreach ($contents as $side) { + foreach (array_filter($contents, is_string(...)) as $side) { if (str_contains($side, "\0")) { return $content . 'Binary file. Recover the project copy from version control.' . PHP_EOL . PHP_EOL; } @@ -129,7 +129,7 @@ protected function renderEntry(string $path, array $contents): string { } } - if ($contents['previous'] === '') { + if ($contents['previous'] === NULL) { $content .= 'The version the project runs did not ship this file. Project content that the update replaced:' . PHP_EOL . PHP_EOL; return $content . $this->renderDiff($contents['project'], $contents['next'], 'project', 'update'); @@ -165,8 +165,15 @@ protected function renderEntry(string $path, array $contents): string { protected function renderDiff(string $from, string $to, string $from_label, string $to_label): string { $header = sprintf('--- %s', $from_label) . PHP_EOL . sprintf('+++ %s', $to_label) . PHP_EOL; $differ = new Differ(new UnifiedDiffOutputBuilder($header)); + $diff = rtrim($differ->diff($from, $to)); - return '```diff' . PHP_EOL . rtrim($differ->diff($from, $to)) . PHP_EOL . '```' . PHP_EOL . PHP_EOL; + // The template ships Markdown that itself contains fences, and a diff + // renders an unchanged line with a single leading space, which Markdown + // still reads as a closing fence. + preg_match_all('/`{3,}/', $diff, $matches); + $fence = str_repeat('`', $matches[0] === [] ? 3 : max(3, max(array_map(strlen(...), $matches[0])) + 1)); + + return $fence . 'diff' . PHP_EOL . $diff . PHP_EOL . $fence . PHP_EOL . PHP_EOL; } } diff --git a/.vortex/installer/tests/Unit/Utils/FileManagerTest.php b/.vortex/installer/tests/Unit/Utils/FileManagerTest.php index 8c4bcb4490..db2cce2074 100644 --- a/.vortex/installer/tests/Unit/Utils/FileManagerTest.php +++ b/.vortex/installer/tests/Unit/Utils/FileManagerTest.php @@ -356,6 +356,7 @@ public function testCopyFilesRemovesCommittedManifest(): void { file_put_contents(File::mkdir($src) . '/composer.json', '{}'); $config = new Config('/tmp/root', $destination, $src); + $config->set(Config::IS_VORTEX_PROJECT, TRUE, TRUE); $fm = new FileManager($config); file_put_contents(File::mkdir($destination) . '/.vortex-manifest.json', '{"composer.json":"abc"}'); @@ -365,6 +366,21 @@ public function testCopyFilesRemovesCommittedManifest(): void { $this->assertFileDoesNotExist($destination . '/.vortex-manifest.json', 'A manifest an earlier install left behind is removed.'); } + public function testCopyFilesKeepsManifestInDestinationThatIsNotVortexProject(): void { + $src = self::$sut . '/src_foreign_manifest'; + $destination = self::$sut . '/dst_foreign_manifest'; + file_put_contents(File::mkdir($src) . '/composer.json', '{}'); + + $config = new Config('/tmp/root', $destination, $src); + $fm = new FileManager($config); + + file_put_contents(File::mkdir($destination) . '/.vortex-manifest.json', '{"owned":"by the project"}'); + + $fm->copyFiles(); + + $this->assertFileExists($destination . '/.vortex-manifest.json', 'A destination that never ran Vortex keeps its own file.'); + } + public function testCopyFilesKeepsPathsTheTemplateNeverShipped(): void { $src = self::$sut . '/src_unknown'; $destination = self::$sut . '/dst_unknown'; diff --git a/.vortex/installer/tests/Unit/Utils/UpdateRegistryTest.php b/.vortex/installer/tests/Unit/Utils/UpdateRegistryTest.php index 2fbd9d64c3..543a475add 100644 --- a/.vortex/installer/tests/Unit/Utils/UpdateRegistryTest.php +++ b/.vortex/installer/tests/Unit/Utils/UpdateRegistryTest.php @@ -76,7 +76,7 @@ public function testWriteNotesOversizedContent(): void { public function testWriteRendersProjectAuthoredPath(): void { $registry = new UpdateRegistry(self::$sut); - $registry->add('phpstan.neon', '', "level: 8\n", "level: 9\n"); + $registry->add('phpstan.neon', NULL, "level: 8\n", "level: 9\n"); $file = (string) $registry->write('1.40.0', '1.41.0', '2026-09-07 09:31:22'); @@ -86,6 +86,28 @@ public function testWriteRendersProjectAuthoredPath(): void { $this->assertFileNotContainsString($file, 'Change that the update brings:'); } + public function testWriteDiffsAgainstAnEmptyInstalledFile(): void { + $registry = new UpdateRegistry(self::$sut); + $registry->add('.env.local.example', '', "ADDED=1\n", "SHIPPED=1\n"); + + $file = (string) $registry->write('1.40.0', '1.41.0', '2026-09-07 09:31:22'); + + $this->assertFileNotContainsString($file, 'The version the project runs did not ship this file.', 'A file shipped empty is not reported as never shipped.'); + $this->assertFileContainsString($file, 'Project change that the update replaced:'); + $this->assertFileContainsString($file, 'Change that the update brings:'); + } + + public function testWriteWidensFenceAroundContentWithFences(): void { + $registry = new UpdateRegistry(self::$sut); + $registry->add('README.md', "# Title\n```php\n\$a = 1;\n```\n", "# Title\n```php\n\$a = 2;\n```\n", "# Title\n```php\n\$a = 3;\n```\n"); + + $file = (string) $registry->write('1.40.0', '1.41.0', '2026-09-07 09:31:22'); + + $this->assertFileContainsString($file, '````diff', 'The fence outgrows the longest backtick run in the diff.'); + $this->assertFileContainsString($file, PHP_EOL . '````' . PHP_EOL, 'The closing fence matches the opening one.'); + $this->assertStringNotContainsString('```' . PHP_EOL . '```diff', File::read($file), 'No entry opens with a fence the diff can close.'); + } + public function testWriteSortsEntriesByPath(): void { $registry = new UpdateRegistry(self::$sut); $registry->add('phpstan.neon', "a\n", "b\n", "c\n");