[RFC] Add support for importing remote functions and tasks

[pyrech]

Replace #136 (too much conflicts and I wanted to keep a trace of the previous implementation).

This new implementation use Composer as described in #136 (comment). composer.json, composer.lock files and dependencies are stored in .castor/vendor directory at the root of the Castor project. A .gitignore is automatically created in it to ignore everything from git so it's invisible from users.

See the documentation to understand how imports work: https://github.com/jolicode/castor/blob/import-with-composer/doc/going-further/extending-castor/remote-imports.md

The third commit add a "cache" layer to avoid calling Composer when nothing have changed in the imports.

Todo:

• Add a flag to update/remove dependencies
• Embed Composer in the static binary ?

[TheoD02]

👏 Great job! Excited to see the results!

I'm currently using a custom method involving a git clone on my side 😃

Maybe interesting:

• In my custom method, I've implemented an "auto-update" feature that is cached for a day. This ensures the automatic synchronization of the dev team, updating dependencies at a specific interval. This eliminates the need to manually instruct everyone or deal with issues like "why isn't it working?" and responding with "just update your dependencies" 😄
  But certainly doeable from events

• I tested it for now, as a work in progress, and here are some feedback:

  • When I copy the example for a git import, if the repository doesn't exist, it is added to the composer.json and creates some empty directories. However, in reality, the clone has failed. Maybe consider adding the package to composer.json and all created files/dirs only if the clone is successful?

  • Adding a fetching icon to indicate when something is happening in the background when fetching vendors, with a progress indicator. (In case of calling castor, and nothing appears for some seconds until all ready)

  • castor/vendor/** is initiated automatically. Wouldn't it be better to do it only if at least one import uses remote? This can prevent generating useless files for those who don't use this feature.

  • Handle the case when no connection is available to prevent castor to be unusable ?

[tigitz]

Feature seems interesting however I fail to see what are the pros and cons over regular composer usage?

What is it trying to solve or why should I use it? Current doc is not helping in this regard.

Will it loose static analysis abilities and autocompletion on IDE? Does it need imported packages to be prefixed to avoid collision with castor own classes?

[joelwurtz]

Feature seems interesting however I fail to see what are the pros and cons over regular composer usage?

Mainly DX, some people that use castor don't know PHP and its ecosystem, tell them to use composer and do a specific setup for that is not wanted, where we could provide extensions developer to only add a single line in their README to copy/paste for the end user.

What is it trying to solve or why should I use it? Current doc is not helping in this regard.

We want to provide optional features that some project need and some doesn't without bloating current castor with too many deps and features

Also some people want specific features for their needs in castor, as they used it in many projects, but on our side we do not want to maintain those features, having this would allow those people to create their own extension maintain it according to their needs for their multiples projects (and even other users if published on composer)

Will it loose static analysis abilities and autocompletion on IDE?

Files will be download / cached in a directory relative to the castor.php file (.castor/vendor) so normally IDE would have access to it without problem

Does it need imported packages to be prefixed to avoid collision with castor own classes?

I don't think so, PHP / Composer in this regard will load the first class available so it should be safe.

However there may be problem when current castor and extension loaded have a different version that need specific features. But this is definitely something that we want to work on to avoid as less pain as possible

[joelwurtz]

Embed Composer in the static binary ?

It should be embed, otherwise people will need php and composer in their system to use extension which defeat the purpose of having a static binary

[pyrech]

Thanks @TheoD02 for your quick feedback, I took some time this morning to implement your suggestions:

• A spinner is displayed while downloading
  

• .castor/vendor is now removed if no remote package were required

[tigitz]

Trying to pack castor into a .phar seems to unecessarily import packages in examples, could it be avoided ?

[TheoD02]

Trying to pack castor into a .phar seems to unecessarily import packages in examples, could it be avoided ?

With --no-remote maybe ?

[tigitz]

I don't think so, PHP / Composer in this regard will load the first class available so it should be safe.

My guess was that it would try to redeclare and fail so it would need to be dynamically prefixed while importing. The same way Rector have to do it.

I've tried and it indeed failed running php castor.linux-amd64.phar

With castor.php set to

<?php

namespace greetings;

use Castor\Attribute\AsTask;
use function Castor\import;

import('composer://symfony/string');

#[AsTask]
function hello(): void
{

}

PHP Fatal error:  Cannot declare class Symfony\Component\String\AbstractUnicodeString, because the name is already in use in /home/psegatori/www/castor-app-as-binary/.castor/vendor/symfony/string/AbstractUnicodeString.php on line 29
PHP Stack trace:
PHP   1. {main}() /home/psegatori/www/castor-app-as-binary/castor.linux-amd64.phar:0
PHP   2. require() /home/psegatori/www/castor-app-as-binary/castor.linux-amd64.phar:12
PHP   3. Symfony\Component\Console\Application->run($input = *uninitialized*, $output = *uninitialized*) phar:///home/psegatori/www/castor-app-as-binary/castor.linux-amd64.phar/bin/castor:13
PHP   4. Castor\Console\Application->doRun($input = class Symfony\Component\Console\Input\ArgvInput { protected $definition = class Symfony\Component\Console\Input\InputDefinition { private array $arguments = [...]; private int $requiredCount = 0; private ?Symfony\Component\Console\Input\InputArgument $lastArrayArgument = NULL; private ?Symfony\Component\Console\Input\InputArgument $lastOptionalArgument = NULL; private array $options = [...]; private array $negations = [...]; private array $shortcuts = [...] }; protected $stream = NULL; protected $options = []; protected $arguments = []; protected $interactive = TRUE; private array $tokens = []; private array $parsed = *uninitialized* }, $output = class Symfony\Component\Console\Output\ConsoleOutput { private int ${Symfony\Component\Console\Output\Output}verbosity = 32; private Symfony\Component\Console\Formatter\OutputFormatterInterface ${Symfony\Component\Console\Output\Output}formatter = class Symfony\Component\Console\Formatter\OutputFormatter { private bool $decorated = TRUE; private array $styles = [...]; private Symfony\Component\Console\Formatter\OutputFormatterStyleStack $styleStack = class Symfony\Component\Console\Formatter\OutputFormatterStyleStack { ... } }; private ${Symfony\Component\Console\Output\StreamOutput}stream = resource(2) of type (stream); private Symfony\Component\Console\Output\OutputInterface $stderr = class Symfony\Component\Console\Output\StreamOutput { private int ${Symfony\Component\Console\Output\Output}verbosity = 32; private Symfony\Component\Console\Formatter\OutputFormatterInterface ${Symfony\Component\Console\Output\Output}formatter = class Symfony\Component\Console\Formatter\OutputFormatter { ... }; private $stream = resource(3) of type (stream) }; private array $consoleSectionOutputs = [] }) phar:///home/psegatori/www/castor-app-as-binary/castor.linux-amd64.phar/vendor/symfony/console/Application.php:175
PHP   5. Castor\Console\Application->initializeApplication($input = class Symfony\Component\Console\Input\ArgvInput { protected $definition = class Symfony\Component\Console\Input\InputDefinition { private array $arguments = [...]; private int $requiredCount = 0; private ?Symfony\Component\Console\Input\InputArgument $lastArrayArgument = NULL; private ?Symfony\Component\Console\Input\InputArgument $lastOptionalArgument = NULL; private array $options = [...]; private array $negations = [...]; private array $shortcuts = [...] }; protected $stream = NULL; protected $options = []; protected $arguments = []; protected $interactive = TRUE; private array $tokens = []; private array $parsed = *uninitialized* }) phar:///home/psegatori/www/castor-app-as-binary/castor.linux-amd64.phar/src/Console/Application.php:140
PHP   6. Castor\FunctionFinder->doFindFunctions($files = [0 => class SplFileInfo { private $pathName = '/home/psegatori/www/castor-app-as-binary/castor.php'; private $fileName = 'castor.php' }]) phar:///home/psegatori/www/castor-app-as-binary/castor.linux-amd64.phar/src/Console/Application.php:187
PHP   7. Castor\castor_require($file = '/home/psegatori/www/castor-app-as-binary/castor.php') phar:///home/psegatori/www/castor-app-as-binary/castor.linux-amd64.phar/src/FunctionFinder.php:61
PHP   8. require_once() phar:///home/psegatori/www/castor-app-as-binary/castor.linux-amd64.phar/src/FunctionFinder.php:259
PHP   9. Castor\import($path = 'composer://symfony/string', $file = *uninitialized*, $version = *uninitialized*, $vcs = *uninitialized*, $source = *uninitialized*) /home/psegatori/www/castor-app-as-binary/castor.php:9
PHP  10. Castor\castor_require($file = '/home/psegatori/www/castor-app-as-binary/.castor/vendor/symfony/string/AbstractUnicodeString.php') phar:///home/psegatori/www/castor-app-as-binary/castor.linux-amd64.phar/src/functions.php:786
PHP  11. require_once() phar:///home/psegatori/www/castor-app-as-binary/castor.linux-amd64.phar/src/FunctionFinder.php:259
13:03:41 ERROR     [castor] Fatal Compile Error: Cannot declare class Symfony\Component\String\AbstractUnicodeString, because the name is already in use ["exception" => Symfony\Component\ErrorHandler\Error\FatalError^ { …}]
Symfony\Component\ErrorHandler\Error\FatalError^ {#169
  #message: "Compile Error: Cannot declare class Symfony\Component\String\AbstractUnicodeString, because the name is already in use"
  #code: 0
  #file: "./.castor/vendor/symfony/string/AbstractUnicodeString.php"
  #line: 29
  -error: array:4 [
    "type" => 64
    "message" => "Cannot declare class Symfony\Component\String\AbstractUnicodeString, because the name is already in use"
    "file" => "/home/psegatori/www/castor-app-as-binary/.castor/vendor/symfony/string/AbstractUnicodeString.php"
    "line" => 29
  ]
}

[tigitz]

Thinking out loud, I'm wondering if there could be an apporach to dynamically download and autoload on demand the missing functions by hooking into the spl_autoload_register function.

Something like:

use AcmeCorp\HelloWorld\HelloWorld;

spl_autoload_register(function() {
    // Transform FQCN into a packagist package name (e.g AcmeCorp\HelloWorld\HelloWorld into acme-corp/hello-world)

    // composer require package-name

    require_once __DIR__ . '/.castor/vendor/autoload.php';
});

HelloWorld::hello('test');

That would require a clear convention on Namespace -> Package Name mapping for castor packages and calling extra package helpers through classes but usage would feel frictionless and implementation less complex. No more custom import overhead.

I've tinkered around supporting autoimport of undefined function calls too but logic has to reside in the exception handler (which could have side effects) and there's no way to retrieved the passed undefined fuction call arguments in the Error type:

use function Hello\World\hello; // in __DIR__ . '/hello.php'

set_exception_handler(function (Error $error) {
    if (preg_match('/Call to undefined function (.+)\(\)/', $error->getMessage(), $matches)) {
        // Transform Hello\World\hello to package name hello/world

        // composer require hello/world

        call_user_func($functionFQN, ...$args);
    }
});

hello('test');

I wish there would be a spl_autoload_register equivalent for functions.

[lyrixx]

Very very good ! I like it a lot 💛
thankkkkkkkks!

Few niptick, but nothing hard.

[lyrixx]

Could you add some words about the autoloader?

[lyrixx]

⬆️ No a blocker for now

[pyrech]

PR rebased, with a new commit fixing almost all feedback from lyrixx + a refactor of the import layer:

• all the logic in import() function has been moved to another Castor\Import\Importer class so the function is no longer bloated;
• move the internal functions in another functions-internal.php file
• the new Importer class will also now throw exceptions and warnings containing the exact file and line causing an import issue, for a better DX (see all the tests covering invalid import).

For example, before, when trying to importing a simple castor file with a wrong path:

and now:

[lyrixx]

👍🏼 Thanks. Only 2/3 little nitpick

[lyrixx]

⬆️ No a blocker for now

[lyrixx]

👍🏼 Let's merge it 🎉🎉🎉
And iterate

[lyrixx]

Thanks

[joelwurtz]

Thanks @pyrech can wait to test this 🚀🚀🚀