-
Notifications
You must be signed in to change notification settings - Fork 28
feat(filesystem): add ContextAwareFilesystem for relative path handling #747
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| <?php | ||
|
|
||
| use Castor\Attribute\AsTask; | ||
|
|
||
| use function Castor\context; | ||
| use function Castor\fs; | ||
| use function Castor\io; | ||
|
|
||
| #[AsTask(description: 'Demonstrates context-aware filesystem operations')] | ||
| function contextAware(): void | ||
| { | ||
| $fs = fs(); | ||
|
|
||
| // Create a temporary directory for the test | ||
| $baseDir = '/tmp/castor-fs-context-test'; | ||
| $fs->remove($baseDir); | ||
| $fs->mkdir($baseDir); | ||
|
|
||
| // Create a subdirectory | ||
| $fs->mkdir($baseDir . '/subdir'); | ||
|
|
||
| // Test 1: Using fs() with default context (current working directory) | ||
| io()->section('Test 1: Default context'); | ||
| io()->writeln('Current working directory: ' . context()->workingDirectory); | ||
|
|
||
| // Create a file using an absolute path - should work regardless of context | ||
| $fs->dumpFile($baseDir . '/absolute-path.txt', 'Created with absolute path'); | ||
| io()->writeln('Created file with absolute path: ' . $baseDir . '/absolute-path.txt'); | ||
| io()->writeln('File exists: ' . ($fs->exists($baseDir . '/absolute-path.txt') ? 'yes' : 'no')); | ||
|
|
||
| // Test 2: Using fs() with a different working directory context | ||
| io()->section('Test 2: Context with different working directory'); | ||
| $customContext = context()->withWorkingDirectory($baseDir . '/subdir'); | ||
| $contextualFs = fs($customContext); | ||
|
|
||
| io()->writeln('Context working directory: ' . $customContext->workingDirectory); | ||
|
|
||
| // Create a file using a relative path - should be created in context's working directory | ||
| $contextualFs->dumpFile('relative-path.txt', 'Created with relative path in context'); | ||
| io()->writeln('Created file with relative path: relative-path.txt'); | ||
|
|
||
| // Check if the file was created in the correct location | ||
| $expectedPath = $baseDir . '/subdir/relative-path.txt'; | ||
| io()->writeln('Actual file location: ' . $expectedPath); | ||
| io()->writeln('File exists at expected location: ' . ($fs->exists($expectedPath) ? 'yes' : 'no')); | ||
|
|
||
| // Verify content | ||
| if ($fs->exists($expectedPath)) { | ||
| $content = file_get_contents($expectedPath); | ||
| io()->writeln('File content: ' . $content); | ||
| } | ||
|
|
||
| // Test 3: Demonstrating that absolute paths work the same way in any context | ||
| io()->section('Test 3: Absolute paths are context-independent'); | ||
| $contextualFs->dumpFile($baseDir . '/another-absolute.txt', 'Created with absolute path from contextualized fs'); | ||
| io()->writeln('Created file with absolute path from contextualized fs'); | ||
| io()->writeln('File exists: ' . ($fs->exists($baseDir . '/another-absolute.txt') ? 'yes' : 'no')); | ||
|
|
||
| // Cleanup | ||
| $fs->remove($baseDir); | ||
| io()->success('Context-aware filesystem test completed successfully!'); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,195 @@ | ||
| <?php | ||
|
|
||
| namespace Castor\Filesystem; | ||
|
|
||
| use Symfony\Component\Filesystem\Filesystem; | ||
| use Symfony\Component\Filesystem\Path; | ||
|
|
||
| readonly class ContextAwareFilesystem | ||
| { | ||
| public function __construct( | ||
| private Filesystem $filesystem, | ||
| private string $workingDirectory, | ||
| ) { | ||
| } | ||
|
|
||
| public function copy(string $originFile, string $targetFile, bool $overwriteNewerFiles = false): void | ||
| { | ||
| $this->filesystem->copy( | ||
| $this->resolvePath($originFile), | ||
| $this->resolvePath($targetFile), | ||
| $overwriteNewerFiles | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * @param string|iterable<string> $dirs | ||
| */ | ||
| public function mkdir(string|iterable $dirs, int $mode = 0o777): void | ||
| { | ||
| $this->filesystem->mkdir($this->resolvePaths($dirs), $mode); | ||
| } | ||
|
|
||
| /** | ||
| * @param string|iterable<string> $files | ||
| */ | ||
| public function exists(string|iterable $files): bool | ||
| { | ||
| return $this->filesystem->exists($this->resolvePaths($files)); | ||
| } | ||
|
|
||
| /** | ||
| * @param string|iterable<string> $files | ||
| */ | ||
| public function touch(string|iterable $files, ?int $time = null, ?int $atime = null): void | ||
| { | ||
| $this->filesystem->touch($this->resolvePaths($files), $time, $atime); | ||
| } | ||
|
|
||
| /** | ||
| * @param string|iterable<string> $files | ||
| */ | ||
| public function remove(string|iterable $files): void | ||
| { | ||
| $this->filesystem->remove($this->resolvePaths($files)); | ||
| } | ||
|
|
||
| /** | ||
| * @param string|iterable<string> $files | ||
| */ | ||
| public function chmod(string|iterable $files, int $mode, int $umask = 0o000, bool $recursive = false): void | ||
| { | ||
| $this->filesystem->chmod($this->resolvePaths($files), $mode, $umask, $recursive); | ||
| } | ||
|
|
||
| /** | ||
| * @param string|iterable<string> $files | ||
| */ | ||
| public function chown(string|iterable $files, string|int $user, bool $recursive = false): void | ||
| { | ||
| $this->filesystem->chown($this->resolvePaths($files), $user, $recursive); | ||
| } | ||
|
|
||
| /** | ||
| * @param string|iterable<string> $files | ||
| */ | ||
| public function chgrp(string|iterable $files, string|int $group, bool $recursive = false): void | ||
| { | ||
| $this->filesystem->chgrp($this->resolvePaths($files), $group, $recursive); | ||
| } | ||
|
|
||
| public function rename(string $origin, string $target, bool $overwrite = false): void | ||
| { | ||
| $this->filesystem->rename( | ||
| $this->resolvePath($origin), | ||
| $this->resolvePath($target), | ||
| $overwrite | ||
| ); | ||
| } | ||
|
|
||
| public function symlink(string $originDir, string $targetDir, bool $copyOnWindows = false): void | ||
| { | ||
| $this->filesystem->symlink( | ||
| $this->resolvePath($originDir), | ||
| $this->resolvePath($targetDir), | ||
| $copyOnWindows | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * @param string|iterable<string> $targetFiles | ||
| */ | ||
| public function hardlink(string $originFile, string|iterable $targetFiles): void | ||
| { | ||
| $this->filesystem->hardlink( | ||
| $this->resolvePath($originFile), | ||
| $this->resolvePaths($targetFiles) | ||
| ); | ||
| } | ||
|
|
||
| public function readlink(string $path, bool $canonicalize = false): ?string | ||
| { | ||
| return $this->filesystem->readlink($this->resolvePath($path), $canonicalize); | ||
| } | ||
|
|
||
| public function makePathRelative(string $endPath, string $startPath): string | ||
| { | ||
| return $this->filesystem->makePathRelative( | ||
| $this->resolvePath($endPath), | ||
| $this->resolvePath($startPath) | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * @param \Traversable<mixed>|null $iterator | ||
| * @param array<mixed> $options | ||
| */ | ||
| public function mirror(string $originDir, string $targetDir, ?\Traversable $iterator = null, array $options = []): void | ||
| { | ||
| $this->filesystem->mirror( | ||
| $this->resolvePath($originDir), | ||
| $this->resolvePath($targetDir), | ||
| $iterator, | ||
| $options | ||
| ); | ||
| } | ||
|
|
||
| public function isAbsolutePath(string $file): bool | ||
| { | ||
| return $this->filesystem->isAbsolutePath($file); | ||
| } | ||
|
|
||
| public function tempnam(string $dir, string $prefix, string $suffix = ''): string | ||
| { | ||
| return $this->filesystem->tempnam($this->resolvePath($dir), $prefix, $suffix); | ||
| } | ||
|
|
||
| /** | ||
| * @param string|resource $content | ||
| */ | ||
| public function dumpFile(string $filename, $content): void | ||
| { | ||
| $this->filesystem->dumpFile($this->resolvePath($filename), $content); | ||
| } | ||
|
|
||
| /** | ||
| * @param string|resource $content | ||
| */ | ||
| public function appendToFile(string $filename, $content, bool $lock = false): void | ||
| { | ||
| $this->filesystem->appendToFile($this->resolvePath($filename), $content, $lock); | ||
| } | ||
|
|
||
| public function readFile(string $filename): string | ||
| { | ||
| return $this->filesystem->readFile($this->resolvePath($filename)); | ||
| } | ||
|
|
||
| private function resolvePath(string $path): string | ||
| { | ||
| if (Path::isAbsolute($path)) { | ||
| return $path; | ||
| } | ||
|
|
||
| return Path::makeAbsolute($path, $this->workingDirectory); | ||
| } | ||
|
|
||
| /** | ||
| * @param string|iterable<string> $files | ||
| * | ||
| * @return string|array<string> | ||
| */ | ||
| private function resolvePaths(string|iterable $files): string|array | ||
| { | ||
| if (\is_string($files)) { | ||
| return $this->resolvePath($files); | ||
| } | ||
|
|
||
| $resolved = []; | ||
| foreach ($files as $file) { | ||
| $resolved[] = $this->resolvePath($file); | ||
| } | ||
|
|
||
| return $resolved; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ | |
| use Castor\Exception\ProblemException; | ||
| use Castor\Exception\WaitFor\ExitedBeforeTimeoutException; | ||
| use Castor\Exception\WaitFor\TimeoutReachedException; | ||
| use Castor\Filesystem\ContextAwareFilesystem; | ||
| use Castor\Helper\CompressionMethod; | ||
| use Castor\Helper\HasherHelper; | ||
| use Castor\Helper\PathHelper; | ||
|
|
@@ -23,7 +24,6 @@ | |
| use Symfony\Component\Console\Output\OutputInterface; | ||
| use Symfony\Component\Console\Style\SymfonyStyle; | ||
| use Symfony\Component\Dotenv\Dotenv; | ||
| use Symfony\Component\Filesystem\Filesystem; | ||
| use Symfony\Component\Finder\Finder; | ||
| use Symfony\Component\Process\ExecutableFinder; | ||
| use Symfony\Component\Process\Process; | ||
|
|
@@ -265,9 +265,15 @@ function task(bool $allowNull = false): ?Command | |
| return Container::get()->getCommand($allowNull); | ||
| } | ||
|
|
||
| function fs(): Filesystem | ||
| function fs(?Context $context = null): ContextAwareFilesystem | ||
| { | ||
| return Container::get()->fs; | ||
| $container = Container::get(); | ||
| $context ??= $container->contextRegistry->getCurrentContext(); | ||
|
|
||
| return new ContextAwareFilesystem( | ||
| $container->fs, | ||
| $context->workingDirectory, | ||
| ); | ||
| } | ||
|
Comment on lines
+268
to
277
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO, this is a BC-break because it will change behaviour for people already using the fs() function and having defined a workingDirectory in their context. We must either:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we introduce something like that (in new PR) : #[AsConfig]
function config(): CastorConfig
{
return new CastorConfig(
enabledFeatures:
FeatureFlag::ContextAwareFilesystem,
FeatureFlag::AnotherFeatureWithBC,
);
}And do something like : function fs(?Context $context = null): Filesystem|ContextAwareFilesystem
{
$container = Container::get();
$context ??= $container->contextRegistry->getCurrentContext();
$config = $container->config;
// Type-safe feature check with enum
if ($config->isEnabled(FeatureFlag::ContextAwareFilesystem)) {
return new ContextAwareFilesystem(
$container->fs,
$context->workingDirectory,
);
}
// Trigger deprecate here
return $container->fs;
}
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not against a config object, but IMO it doesn't need a special attribute, just put it in the context : And to update it, use the default context / context attribute with something like Also i don't think it should be a feature flag, because in this case we only have 2 choice :
Or we need 3 choices to handle BC break if we want this to be the default behavior in the future :
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm fine with that, waiting for feedback from other core team members, I opened a topic here #748, to not pollute this PR |
||
|
|
||
| function finder(): Finder | ||
|
|
@@ -603,19 +609,19 @@ function wait_for_docker_container( | |
| } | ||
|
|
||
| /** | ||
| * @see Yaml::parse() | ||
| * | ||
| * @param int-mask-of<Yaml::PARSE_*> $flags A bit field of DUMP_* constants to customize the dumped YAML string | ||
| * | ||
| * @see Yaml::parse() | ||
| */ | ||
| function yaml_parse(string $content, int $flags = 0): mixed | ||
| { | ||
| return Yaml::parse($content, $flags); | ||
| } | ||
|
|
||
| /** | ||
| * @see Yaml::dump() | ||
| * | ||
| * @param int-mask-of<Yaml::DUMP_*> $flags A bit field of DUMP_* constants to customize the dumped YAML string | ||
| * | ||
| * @see Yaml::dump() | ||
| */ | ||
| function yaml_dump(mixed $input, int $inline = 2, int $indent = 4, int $flags = 0): string | ||
| { | ||
|
|
@@ -765,6 +771,5 @@ function run_php(string $pharPath, array $arguments = [], ?Context $context = nu | |
| { | ||
| return Container::get() | ||
| ->phpRunner | ||
| ->run($pharPath, $arguments, $context, $callback) | ||
| ; | ||
| ->run($pharPath, $arguments, $context, $callback); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| <?php | ||
|
|
||
| namespace Castor\Tests\Generated; | ||
|
|
||
| use Castor\Tests\TaskTestCase; | ||
| use Symfony\Component\Process\Exception\ProcessFailedException; | ||
|
|
||
| class ContextAwareTest extends TaskTestCase | ||
| { | ||
| // context-aware | ||
| public function test(): void | ||
| { | ||
| $process = $this->runTask(['context-aware']); | ||
|
|
||
| if (0 !== $process->getExitCode()) { | ||
| throw new ProcessFailedException($process); | ||
| } | ||
|
|
||
| $this->assertStringEqualsFile(__FILE__ . '.output.txt', $process->getOutput()); | ||
| $this->assertSame('', $process->getErrorOutput()); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| Test 1: Default context | ||
| ----------------------- | ||
|
|
||
| Current working directory: ... | ||
| Created file with absolute path: /tmp/castor-fs-context-test/absolute-path.txt | ||
| File exists: yes | ||
|
|
||
| Test 2: Context with different working directory | ||
| ------------------------------------------------ | ||
|
|
||
| Context working directory: /tmp/castor-fs-context-test/subdir | ||
| Created file with relative path: relative-path.txt | ||
| Actual file location: /tmp/castor-fs-context-test/subdir/relative-path.txt | ||
| File exists at expected location: yes | ||
| File content: Created with relative path in context | ||
|
|
||
| Test 3: Absolute paths are context-independent | ||
| ---------------------------------------------- | ||
|
|
||
| Created file with absolute path from contextualized fs | ||
| File exists: yes | ||
|
|
||
| [OK] Context-aware filesystem test completed successfully! | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I renamed the file because, during the rebase, I encountered conflicts caused by having both
ls.php(for functions) andLs.php(for the class). Is that okay?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 👍