From e087fbb9688e04093e0cec60770f6c85aa90a9a0 Mon Sep 17 00:00:00 2001 From: Serghei Iakovlev Date: Fri, 27 Dec 2019 00:21:42 +0200 Subject: [PATCH 1/5] Added test to verify cache issue --- unit-tests/sharness/t0004-generate-success.sh | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100755 unit-tests/sharness/t0004-generate-success.sh diff --git a/unit-tests/sharness/t0004-generate-success.sh b/unit-tests/sharness/t0004-generate-success.sh new file mode 100755 index 0000000000..98263c4941 --- /dev/null +++ b/unit-tests/sharness/t0004-generate-success.sh @@ -0,0 +1,31 @@ +#!/usr/bin/env bash + +# shellcheck disable=SC2034 +test_description="Test generate command for success" + +# shellcheck disable=SC1091 +source ./setup.sh + +test_expect_success "Should not use the same cache when working with different projects" " + cd $OUTPUTDIR && + rm -rf ./gen1 && + zephirc init gen1 && + cd ./gen1 && + echo 'namespace Gen1;' > ./gen1/test.zep && + echo 'class Test { }' >> ./gen1/test.zep && + zephirc generate && + test -d ./.zephir && + rm -r ./.zephir && + + cd $OUTPUTDIR && + rm -rf ./gen2 && + zephirc init gen2 && + cd ./gen2 && + echo 'namespace Gen2;' > ./gen2/test.zep && + echo 'class Test { }' >> ./gen2/test.zep && + zephirc generate && + test -d ./.zephir && + test ! -d $OUTPUTDIR/gen1/.zephir +" + +test_done From 673da4649b765b8d9496154acd88e65889fd83f3 Mon Sep 17 00:00:00 2001 From: Serghei Iakovlev Date: Fri, 27 Dec 2019 00:34:39 +0200 Subject: [PATCH 2/5] Use a different path for the Kernel cache if possible This patch fixes a cache collision issue. The issue is after creating the cache and filling it with a project-specific configuration, there is no way to invalidate it. Any next project will use the same cache and the same Kernel configuration (if any). --- Library/DependencyInjection/ZephirKernel.php | 55 ++++++++++++++------ Library/FileSystem/FileSystemInterface.php | 24 ++++----- Library/FileSystem/HardDisk.php | 41 ++++++++------- 3 files changed, 75 insertions(+), 45 deletions(-) diff --git a/Library/DependencyInjection/ZephirKernel.php b/Library/DependencyInjection/ZephirKernel.php index 07d5122719..9b9390d5eb 100644 --- a/Library/DependencyInjection/ZephirKernel.php +++ b/Library/DependencyInjection/ZephirKernel.php @@ -12,12 +12,15 @@ namespace Zephir\DependencyInjection; use const DIRECTORY_SEPARATOR; +use Exception; use Oneup\FlysystemBundle\OneupFlysystemBundle; +use RuntimeException; use Symfony\Bundle\MonologBundle\MonologBundle; use Symfony\Component\Config\Loader\LoaderInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\HttpKernel\Bundle\BundleInterface; use Symfony\Component\HttpKernel\Kernel; +use Throwable; use Zephir\DependencyInjection\CompilerPass\CollectCommandsToApplicationCompilerPass; use function Zephir\is_macos; use function Zephir\is_windows; @@ -61,7 +64,7 @@ public function registerBundles() * * @param LoaderInterface $loader * - * @throws \Exception|\Throwable + * @throws Exception|Throwable */ public function registerContainerConfiguration(LoaderInterface $loader) { @@ -77,10 +80,10 @@ public function registerContainerConfiguration(LoaderInterface $loader) * * @return string */ - public function getCacheDir() + public function getCacheDir(): string { // allows container rebuild when config or version changes - $hash = Zephir::VERSION.$this->environment.serialize($this->extraConfigFiles); + $home = getenv('HOME') ?: (getenv('HOMEDRIVE').DIRECTORY_SEPARATOR.getenv('HOMEPATH')); if (is_macos()) { @@ -94,20 +97,40 @@ public function getCacheDir() $cacheDir .= '/zephir'; } - $path = $cacheDir.DIRECTORY_SEPARATOR.substr(md5($hash), 0, 16); - if (!is_dir($path) && !mkdir($path, 0777, true) && !is_dir($path)) { - throw new \RuntimeException(sprintf('Directory "%s" was not created', $path)); + $path = $cacheDir.DIRECTORY_SEPARATOR.$this->getPathSalt(); + if (!is_dir($path) && !mkdir($path, 0755, true) && !is_dir($path)) { + throw new RuntimeException(sprintf('Unable to create cache directory: "%s"', $path)); } return $path; } + /** + * Allows container rebuild when config or version changes. + * This also used to get unique path to kernel logs. + * + * @return string + */ + private function getPathSalt(): string + { + $hash = Zephir::VERSION.$this->environment.serialize($this->extraConfigFiles); + + $localConfig = $this->startedDir.DIRECTORY_SEPARATOR.'config.json'; + if (file_exists($localConfig) && is_readable($localConfig)) { + $salt = md5_file($localConfig); + } else { + $salt = $this->startedDir; + } + + return substr(md5("{$hash}{$salt}"), 0, 16); + } + /** * {@inheritdoc} * * @return string */ - public function getLogDir() + public function getLogDir(): string { $home = getenv('HOME') ?: (getenv('HOMEDRIVE').DIRECTORY_SEPARATOR.getenv('HOMEPATH')); @@ -123,9 +146,9 @@ public function getLogDir() $stateDir .= '/zephir'; } - $path = $stateDir.DIRECTORY_SEPARATOR.Zephir::VERSION; - if (!is_dir($path) && !mkdir($path, 0777, true) && !is_dir($path)) { - throw new \RuntimeException(sprintf('Directory "%s" was not created', $path)); + $path = $stateDir.DIRECTORY_SEPARATOR.$this->getPathSalt(); + if (!is_dir($path) && !mkdir($path, 0755, true) && !is_dir($path)) { + throw new RuntimeException(sprintf('Unable to create logs directory: "%s"', $path)); } return $path; @@ -136,15 +159,17 @@ public function getLogDir() * * @return string The project root dir */ - public function getProjectDir() + public function getProjectDir(): string { - return \dirname(\dirname(__DIR__)); + return \dirname(__DIR__, 2); } /** * {@inheritdoc} + * + * @return string */ - public function getRootDir() + public function getRootDir(): string { return \dirname(__DIR__); } @@ -154,7 +179,7 @@ public function getRootDir() * * @return string The local cache dir */ - public function getStartedDir() + public function getStartedDir(): string { return $this->startedDir.'/.zephir'; } @@ -174,7 +199,7 @@ protected function build(ContainerBuilder $containerBuilder) * * @return array An array of kernel parameters */ - protected function getKernelParameters() + protected function getKernelParameters(): array { $parameters = parent::getKernelParameters(); $parameters['kernel.local_cache_dir'] = $this->getStartedDir(); diff --git a/Library/FileSystem/FileSystemInterface.php b/Library/FileSystem/FileSystemInterface.php index d27910e998..fb648ceee5 100644 --- a/Library/FileSystem/FileSystemInterface.php +++ b/Library/FileSystem/FileSystemInterface.php @@ -18,7 +18,7 @@ interface FileSystemInterface * * @return bool */ - public function isInitialized(); + public function isInitialized(): bool; /** * Initialize the filesystem. @@ -32,7 +32,7 @@ public function initialize(); * * @return bool */ - public function exists($path); + public function exists(string $path): bool; /** * Creates a directory inside the temporary container. @@ -41,7 +41,7 @@ public function exists($path); * * @return bool */ - public function makeDirectory($path); + public function makeDirectory(string $path): bool; /** * Returns a temporary entry as an array. @@ -50,7 +50,7 @@ public function makeDirectory($path); * * @return array */ - public function file($path); + public function file(string $path): array; /** * Requires a file from the temporary directory. @@ -59,7 +59,7 @@ public function file($path); * * @return mixed */ - public function requireFile($path); + public function requireFile(string $path); /** * Attempts to remove recursively the temporary directory with all subdirectories and files. @@ -72,7 +72,7 @@ public function clean(); * @param string $path * @param string $data */ - public function write($path, $data); + public function write(string $path, string $data); /** * Writes data from a temporary entry. @@ -81,14 +81,14 @@ public function write($path, $data); * * @return string */ - public function read($path); + public function read(string $path): string; /** * Deletes a temporary entry. * * @param string $path */ - public function delete($path); + public function delete(string $path); /** * Generate a hash value using the contents of a given file. @@ -99,7 +99,7 @@ public function delete($path); * * @return string */ - public function getHashFile($algorithm, $sourceFile, $useCache = false); + public function getHashFile(string $algorithm, string $sourceFile, $useCache = false): string; /** * Returns the modification time of a temporary entry. @@ -108,7 +108,7 @@ public function getHashFile($algorithm, $sourceFile, $useCache = false); * * @return int */ - public function modificationTime($path); + public function modificationTime(string $path): int; /** * Executes a command and saves the result into a temporary entry. @@ -117,7 +117,7 @@ public function modificationTime($path); * @param string $descriptor * @param string $destination */ - public function system($command, $descriptor, $destination); + public function system(string $command, string $descriptor, string $destination); /** * Normalizes path to be used as a temporary entry. @@ -126,5 +126,5 @@ public function system($command, $descriptor, $destination); * * @return string */ - public function normalizePath($path); + public function normalizePath(string $path): string; } diff --git a/Library/FileSystem/HardDisk.php b/Library/FileSystem/HardDisk.php index 81d997f054..7b12833228 100644 --- a/Library/FileSystem/HardDisk.php +++ b/Library/FileSystem/HardDisk.php @@ -11,6 +11,7 @@ namespace Zephir\FileSystem; +use ErrorException; use League\Flysystem; use Zephir\Exception\CompilerException; use Zephir\Exception\FileSystemException; @@ -70,7 +71,7 @@ public function setBasePath($basePath) * * @return bool */ - public function isInitialized() + public function isInitialized(): bool { return $this->initialized; } @@ -98,7 +99,7 @@ public function initialize() * * @return bool */ - public function exists($path) + public function exists(string $path): bool { if ('.' === $path || empty($path)) { $path = $this->localPath; @@ -116,7 +117,7 @@ public function exists($path) * * @return bool */ - public function makeDirectory($path) + public function makeDirectory(string $path): bool { if ('.' === $path || empty($path)) { $path = $this->localPath; @@ -136,7 +137,7 @@ public function makeDirectory($path) * * @return array */ - public function file($path) + public function file(string $path): array { try { $contents = $this->filesystem->read($this->localPath."/{$path}"); @@ -152,9 +153,11 @@ public function file($path) * * @param string $path * + * @return int + * * @throws Flysystem\FileNotFoundException */ - public function modificationTime($path) + public function modificationTime(string $path): int { return (int) $this->filesystem->getTimestamp($this->localPath."/{$path}"); } @@ -168,7 +171,7 @@ public function modificationTime($path) * * @return string */ - public function read($path) + public function read(string $path): string { return (string) $this->filesystem->read($this->localPath."/{$path}"); } @@ -180,7 +183,7 @@ public function read($path) * * @throws Flysystem\FileNotFoundException */ - public function delete($path) + public function delete(string $path) { $this->filesystem->delete($this->localPath."/{$path}"); } @@ -193,7 +196,7 @@ public function delete($path) * * @throws Flysystem\FileExistsException */ - public function write($path, $contents) + public function write(string $path, string $contents) { $this->filesystem->write($this->localPath."/{$path}", $contents); } @@ -205,11 +208,11 @@ public function write($path, $contents) * @param string $descriptor * @param string $destination */ - public function system($command, $descriptor, $destination) + public function system(string $command, string $descriptor, string $destination) { // fallback $redirect = "{$this->localPath}/{$destination}"; - if (false == empty($this->basePath)) { + if (!empty($this->basePath)) { $redirect = "{$this->basePath}/{$this->localPath}/{$destination}"; } @@ -233,9 +236,9 @@ public function system($command, $descriptor, $destination) * * @throws Flysystem\FileNotFoundException */ - public function requireFile($path) + public function requireFile(string $path) { - if (false == empty($this->basePath)) { + if (!empty($this->basePath)) { return require "{$this->basePath}/{$this->localPath}/{$path}"; } @@ -254,7 +257,7 @@ public function clean() { try { $this->filesystem->deleteDir($this->localPath); - } catch (\ErrorException $e) { + } catch (ErrorException $e) { // For reasons beyond our control, the actual owner of the directory // contents may not be the same as the current user. Therefore we need // to catch ErrorException and throw an expected exception with informing @@ -278,7 +281,7 @@ public function clean() * * @return string */ - public function getHashFile($algorithm, $sourceFile, $useCache = false) + public function getHashFile(string $algorithm, string $sourceFile, $useCache = false): string { if (false === $useCache) { return hash_file($algorithm, $sourceFile); @@ -296,14 +299,16 @@ public function getHashFile($algorithm, $sourceFile, $useCache = false) $this->filesystem->write($cacheFile, $contents); return $contents; - } elseif (filemtime($sourceFile) > $this->filesystem->getTimestamp($cacheFile)) { + } + + if (filemtime($sourceFile) > $this->filesystem->getTimestamp($cacheFile)) { $contents = hash_file($algorithm, $sourceFile); $this->filesystem->update($cacheFile, $contents); return $contents; - } else { - return $this->filesystem->read($cacheFile); } + + return $this->filesystem->read($cacheFile); } /** @@ -313,7 +318,7 @@ public function getHashFile($algorithm, $sourceFile, $useCache = false) * * @return string */ - public function normalizePath($path) + public function normalizePath(string $path): string { return str_replace(['\\', ':', '/'], '_', $path); } From 9b1c70588f6bc85bb992d9c793b2aefa26999577 Mon Sep 17 00:00:00 2001 From: Serghei Iakovlev Date: Fri, 27 Dec 2019 00:42:28 +0200 Subject: [PATCH 3/5] Small CS fix --- Library/DependencyInjection/ZephirKernel.php | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/Library/DependencyInjection/ZephirKernel.php b/Library/DependencyInjection/ZephirKernel.php index 9b9390d5eb..ce2f847980 100644 --- a/Library/DependencyInjection/ZephirKernel.php +++ b/Library/DependencyInjection/ZephirKernel.php @@ -82,8 +82,6 @@ public function registerContainerConfiguration(LoaderInterface $loader) */ public function getCacheDir(): string { - // allows container rebuild when config or version changes - $home = getenv('HOME') ?: (getenv('HOMEDRIVE').DIRECTORY_SEPARATOR.getenv('HOMEPATH')); if (is_macos()) { @@ -99,7 +97,9 @@ public function getCacheDir(): string $path = $cacheDir.DIRECTORY_SEPARATOR.$this->getPathSalt(); if (!is_dir($path) && !mkdir($path, 0755, true) && !is_dir($path)) { - throw new RuntimeException(sprintf('Unable to create cache directory: "%s"', $path)); + throw new RuntimeException( + sprintf('Unable to create cache directory: "%s"', $path) + ); } return $path; @@ -113,7 +113,10 @@ public function getCacheDir(): string */ private function getPathSalt(): string { - $hash = Zephir::VERSION.$this->environment.serialize($this->extraConfigFiles); + $hash = + Zephir::VERSION. + $this->environment. + serialize($this->extraConfigFiles); $localConfig = $this->startedDir.DIRECTORY_SEPARATOR.'config.json'; if (file_exists($localConfig) && is_readable($localConfig)) { @@ -148,7 +151,9 @@ public function getLogDir(): string $path = $stateDir.DIRECTORY_SEPARATOR.$this->getPathSalt(); if (!is_dir($path) && !mkdir($path, 0755, true) && !is_dir($path)) { - throw new RuntimeException(sprintf('Unable to create logs directory: "%s"', $path)); + throw new RuntimeException( + sprintf('Unable to create logs directory: "%s"', $path) + ); } return $path; From 30ec46664eabc5e29b373919eaf1f643850459d0 Mon Sep 17 00:00:00 2001 From: Serghei Iakovlev Date: Fri, 27 Dec 2019 00:44:50 +0200 Subject: [PATCH 4/5] Update change log --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3fde65842f..195277aaf7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/). There are no needs to dump it for every config change. Also, this patch removes `Config::$changed` variable that is no longer needed [#2035](https://github.com/phalcon/zephir/pull/2035) +- Use a different path for the Kernel cache if possible. + This patch fixes a cache collision issue. The issue is after creating the + cache and filling it with a project-specific configuration, there is no + way to invalidate it. Any next project will use the same Kernel cache and + the same Kernel configuration (if any). ### Changed - Improved type hint for arrays when generating stubs From 964eaf05f469cf00fb22c9d006e1066bbaf16cc9 Mon Sep 17 00:00:00 2001 From: Serghei Iakovlev Date: Fri, 27 Dec 2019 00:55:55 +0200 Subject: [PATCH 5/5] Minor cleanup and update change log --- CHANGELOG.md | 1 + Library/DependencyInjection/ZephirKernel.php | 11 +++++------ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 195277aaf7..5cfcd75f08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). cache and filling it with a project-specific configuration, there is no way to invalidate it. Any next project will use the same Kernel cache and the same Kernel configuration (if any). + [#2036](https://github.com/phalcon/zephir/pull/2036) ### Changed - Improved type hint for arrays when generating stubs diff --git a/Library/DependencyInjection/ZephirKernel.php b/Library/DependencyInjection/ZephirKernel.php index ce2f847980..38935db2f0 100644 --- a/Library/DependencyInjection/ZephirKernel.php +++ b/Library/DependencyInjection/ZephirKernel.php @@ -113,19 +113,18 @@ public function getCacheDir(): string */ private function getPathSalt(): string { - $hash = + $prefix = Zephir::VERSION. $this->environment. serialize($this->extraConfigFiles); $localConfig = $this->startedDir.DIRECTORY_SEPARATOR.'config.json'; - if (file_exists($localConfig) && is_readable($localConfig)) { - $salt = md5_file($localConfig); - } else { - $salt = $this->startedDir; + $suffix = $this->startedDir; + if (file_exists($localConfig)) { + $suffix = md5_file($localConfig); } - return substr(md5("{$hash}{$salt}"), 0, 16); + return substr(md5("{$prefix}{$suffix}"), 0, 16); } /**