Skip to content
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

[make:*] interactivly install composer dependencies #1466

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions phpunit.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
<env name="SYMFONY_PHPUNIT_VERSION" value="9.6" />
<env name="MAKER_SKIP_MERCURE_TEST" value="false"/>
<env name="MAKER_SKIP_PANTHER_TEST" value="false" />
<env name="MAKER_INTERACTIVE_DEPENDENCIES" value="false" />
<!-- Overrides process timeout when step debugging -->
<!-- <env name="MAKER_PROCESS_TIMEOUT" value="null" /> -->
</php>
Expand Down
8 changes: 8 additions & 0 deletions src/Command/MakerCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use Symfony\Bundle\MakerBundle\ApplicationAwareMakerInterface;
use Symfony\Bundle\MakerBundle\ConsoleStyle;
use Symfony\Bundle\MakerBundle\Dependency\DependencyManager;
use Symfony\Bundle\MakerBundle\DependencyBuilder;
use Symfony\Bundle\MakerBundle\Exception\RuntimeCommandException;
use Symfony\Bundle\MakerBundle\FileManager;
Expand Down Expand Up @@ -62,6 +63,13 @@ protected function initialize(InputInterface $input, OutputInterface $output): v
$dependencies = new DependencyBuilder();
$this->maker->configureDependencies($dependencies, $input);

// env is for tests - this way we don't have to add a `y` to every test when a dependency is needed.
$dependencyManager = new DependencyManager($this->io, getenv('MAKER_INTERACTIVE_DEPENDS') ?? true);

$this->maker->configureComposerDependencies($dependencyManager);

$dependencyManager->installRequiredDependencies();

if ($missingPackagesMessage = $dependencies->getMissingPackagesMessage($this->getName())) {
throw new RuntimeCommandException($missingPackagesMessage);
}
Expand Down
118 changes: 118 additions & 0 deletions src/Dependency/DependencyManager.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
<?php

/*
* This file is part of the Symfony MakerBundle package.
*
* (c) Fabien Potencier <[email protected]>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bundle\MakerBundle\Dependency;

use Symfony\Bundle\MakerBundle\ConsoleStyle;
use Symfony\Bundle\MakerBundle\Dependency\Model\OptionalClassDependency;
use Symfony\Bundle\MakerBundle\Dependency\Model\RequiredClassDependency;
use Symfony\Bundle\MakerBundle\Exception\RuntimeCommandException;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Process\Process;

/**
* @author Jesse Rushlow<[email protected]>
*
* @internal
*/
final class DependencyManager
{
/** @var RequiredClassDependency[] */
private array $requiredClassDependencies = [];

/** @var OptionalClassDependency[] */
private array $optionalClassDependencies = [];

public function __construct(
private ConsoleStyle $io,
private bool $interactiveMode = true,
) {
}

public function addDependency(RequiredClassDependency|OptionalClassDependency|array $dependency): self
{
$dependencies = [];

if (!\is_array($dependency)) {
$dependencies[] = $dependency;
}

foreach ($dependencies as $dependency) {
if ($dependency instanceof RequiredClassDependency) {
$this->requiredClassDependencies[] = $dependency;

continue;
}

$this->optionalClassDependencies[] = $dependency;
}

return $this;
}

public function installRequiredDependencies(): self
{
foreach ($this->requiredClassDependencies as $dependency) {
if (class_exists($dependency->className) || !$this->askToInstallDependency($dependency)) {
continue;
}

$dependency->preInstallMessage ?: $this->io->caution($dependency->preInstallMessage);

$this->runComposer($dependency);
}

return $this;
}

public function installOptionalDependencies(): self
{
foreach ($this->optionalClassDependencies as $dependency) {
if (class_exists($dependency->className) || !$this->askToInstallDependency($dependency)) {
continue;
}

$dependency->preInstallMessage ?: $this->io->caution($dependency->preInstallMessage);

$this->runComposer($dependency);
}

return $this;
}

public function installInteractively(): bool
{
return $this->interactiveMode;
}

private function askToInstallDependency(RequiredClassDependency|OptionalClassDependency $dependency): bool
{
return $this->io->confirm(
question: sprintf('Do you want us to run <fg=yellow>composer require %s</> for you?', $dependency->composerPackage),
default: true // @TODO - Should we default to yes or no on this...
);
}

private function runComposer(RequiredClassDependency|OptionalClassDependency $dependency): void
{
$process = Process::fromShellCommandline(
sprintf('composer require%s %s', $dependency->installAsRequireDev ?: ' --dev', $dependency->composerPackage)
);

if (Command::SUCCESS === $process->run()) {
return;
}

$this->io->block($process->getErrorOutput());

throw new RuntimeCommandException(sprintf('Oops! There was a problem installing "%s". You\'ll need to install the package manually.', $dependency->className));
}
}
28 changes: 28 additions & 0 deletions src/Dependency/Model/AbstractClassDependency.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

/*
* This file is part of the Symfony MakerBundle package.
*
* (c) Fabien Potencier <[email protected]>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bundle\MakerBundle\Dependency\Model;

/**
* @author Jesse Rushlow <[email protected]>
*
* @internal
*/
abstract class AbstractClassDependency
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally went with a single class that had bool $isRequired to determine if the dependency was required or optional. I went with an abstract base model and a RequiredClassDependency && OptionalClassDependency for better DX / code review readability... Atleast that was the intent..

{
public function __construct(
public string $className,
public string $composerPackage,
public bool $installAsRequireDev = false,
public ?string $preInstallMessage = null,
) {
}
}
21 changes: 21 additions & 0 deletions src/Dependency/Model/OptionalClassDependency.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php

/*
* This file is part of the Symfony MakerBundle package.
*
* (c) Fabien Potencier <[email protected]>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bundle\MakerBundle\Dependency\Model;

/**
* @author Jesse Rushlow <[email protected]>
*
* @internal
*/
final class OptionalClassDependency extends AbstractClassDependency
{
}
21 changes: 21 additions & 0 deletions src/Dependency/Model/RequiredClassDependency.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php

/*
* This file is part of the Symfony MakerBundle package.
*
* (c) Fabien Potencier <[email protected]>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bundle\MakerBundle\Dependency\Model;

/**
* @author Jesse Rushlow <[email protected]>
*
* @internal
*/
final class RequiredClassDependency extends AbstractClassDependency
{
}
12 changes: 12 additions & 0 deletions src/Maker/AbstractMaker.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Bundle\MakerBundle\Maker;

use Symfony\Bundle\MakerBundle\ConsoleStyle;
use Symfony\Bundle\MakerBundle\Dependency\DependencyManager;
use Symfony\Bundle\MakerBundle\DependencyBuilder;
use Symfony\Bundle\MakerBundle\MakerInterface;
use Symfony\Component\Console\Command\Command;
Expand Down Expand Up @@ -54,4 +55,15 @@ protected function addDependencies(array $dependencies, ?string $message = null)
$message
);
}

public function configureComposerDependencies(DependencyManager $dependencyManager): void
{
// @TODO - method here in abstract prevents BC with signature added to `MakerInterface::class`
}

public function configureDependencies(DependencyBuilder $dependencies)
{
// @TODO - do we deprecate this method in favor of the one above. then remove in 2.x
// @TODO - still have plenty of work todo to determine if thats possible or a good idea...
}
Copy link
Collaborator Author

@jrushlow jrushlow Feb 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a fan of touching the abstraction - but it keeps BC in check w/ the interface change. (see note on that below)

}
49 changes: 35 additions & 14 deletions src/Maker/MakeTwigComponent.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@
namespace Symfony\Bundle\MakerBundle\Maker;

use Symfony\Bundle\MakerBundle\ConsoleStyle;
use Symfony\Bundle\MakerBundle\DependencyBuilder;
use Symfony\Bundle\MakerBundle\Dependency\DependencyManager;
use Symfony\Bundle\MakerBundle\Dependency\Model\OptionalClassDependency;
use Symfony\Bundle\MakerBundle\Dependency\Model\RequiredClassDependency;
use Symfony\Bundle\MakerBundle\Generator;
use Symfony\Bundle\MakerBundle\InputConfiguration;
use Symfony\Bundle\MakerBundle\Str;
Expand All @@ -28,6 +30,8 @@
*/
final class MakeTwigComponent extends AbstractMaker
{
private DependencyManager $dependencyManager;

public static function getCommandName(): string
{
return 'make:twig-component';
Expand All @@ -47,20 +51,44 @@ public function configureCommand(Command $command, InputConfiguration $inputConf
;
}

public function configureDependencies(DependencyBuilder $dependencies): void
public function configureComposerDependencies(DependencyManager $dependencyManager): void
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we can simplify. A few things:

  • Could we keep DependencyBuilder? It already records the required dependencies, which allows us to know if we need to throw an error. Now we're just going to use this to run composer require instead of throwing an error.
  • In DependencyBuilder, we have this idea of an "optional dependency". I'm wondering if we can scrap that. It seems rarely used and the usages look questionable. For example, in MakeForm, we mark validator as optional. We should be more opinionated: validation is required. We also mark orm as optional in MakeForm. That probably should be optional. But why record that as an optional dependency at all? I'd just not mention it or handle it at all.

The tl;dr is

A) Can we keep using DependencyBuilder but not worry about this optional dependencies idea anymore?
B) For packages that we discover that we need only during the interactive phase (e.g. ux-live-component after answering the "live" question), could we just make some new service available (that maker commands could DI into their constructor) that allows you to trigger the install? e.g.

if ($input->getOption('live')) {
    $this->dependencyDownloader->installPackage('symfony/ux-live-component', $io);
}

This could interactively ask them if they want to install and do it. Else throw an exception to exit the command. Or maybe we go further and just composer require packages that are needed and just notify the user as we're doing it.

{
$dependencies->addClassDependency(AsTwigComponent::class, 'symfony/ux-twig-component');
// $this is a hack - we need the manager later in `interact()`
$this->dependencyManager = $dependencyManager;

$dependencyManager
->addDependency(new RequiredClassDependency(
className: AsTwigComponent::class,
composerPackage: 'symfony/ux-twig-component',
preInstallMessage: 'This command requires the Symfony UX Twig Component Package.'
))
->addDependency(new OptionalClassDependency(
className: AsLiveComponent::class,
composerPackage: 'symfony/ux-live-component',
preInstallMessage: 'The Symfony UX Live Component is needed to make this a live component.'
))
;
}

public function interact(InputInterface $input, ConsoleStyle $io, Command $command): void
{
if (!$input->getOption('live')) {
$input->setOption('live', $io->confirm('Make this a live component?', false));
}

if (!$input->getOption('live')) {
return;
}

// @TODO - with the dependencyManager in `Command` -> we can't use it outside of configure dependencies.....
$this->dependencyManager->installOptionalDependencies();
Comment on lines +83 to +84
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how to go about using the manager in other maker methods (aside from configureDepends...) Gonna play around with this a bit more. but other ideas are welcome.. Thought about some sort of setter in AbstractMaker or a constructor param in the individual makers that needs this...

}

public function generate(InputInterface $input, ConsoleStyle $io, Generator $generator): void
{
$name = $input->getArgument('name');
$live = $input->getOption('live');

if ($live && !class_exists(AsLiveComponent::class)) {
throw new \RuntimeException('You must install symfony/ux-live-component to create a live component (composer require symfony/ux-live-component)');
}

$factory = $generator->createClassNameDetails(
$name,
'Twig\\Components',
Expand All @@ -87,11 +115,4 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen
$io->writeln(" To render the component, use <fg=yellow><twig:{$shortName} /></>.");
$io->newLine();
}

public function interact(InputInterface $input, ConsoleStyle $io, Command $command): void
{
if (!$input->getOption('live')) {
$input->setOption('live', $io->confirm('Make this a live component?', false));
}
}
}
26 changes: 10 additions & 16 deletions src/Maker/Security/MakeFormLogin.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
use Doctrine\Bundle\DoctrineBundle\DoctrineBundle;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Bundle\MakerBundle\ConsoleStyle;
use Symfony\Bundle\MakerBundle\DependencyBuilder;
use Symfony\Bundle\MakerBundle\Dependency\DependencyManager;
use Symfony\Bundle\MakerBundle\Dependency\Model\RequiredClassDependency;
use Symfony\Bundle\MakerBundle\Exception\RuntimeCommandException;
use Symfony\Bundle\MakerBundle\FileManager;
use Symfony\Bundle\MakerBundle\Generator;
Expand Down Expand Up @@ -77,22 +78,15 @@ public static function getCommandDescription(): string
return 'Generate the code needed for the form_login authenticator';
}

public function configureDependencies(DependencyBuilder $dependencies): void
public function configureComposerDependencies(DependencyManager $dependencyManager): void
{
$dependencies->addClassDependency(
SecurityBundle::class,
'security'
);

$dependencies->addClassDependency(TwigBundle::class, 'twig');

// needed to update the YAML files
$dependencies->addClassDependency(
Yaml::class,
'yaml'
);

$dependencies->addClassDependency(DoctrineBundle::class, 'orm');
$dependencyManager
->addDependency([
new RequiredClassDependency(SecurityBundle::class, 'security'),
new RequiredClassDependency(TwigBundle::class, 'twig'),
new RequiredClassDependency(Yaml::class, 'yaml'),
new RequiredClassDependency(DoctrineBundle::class, 'orm'),
]);
}

public function interact(InputInterface $input, ConsoleStyle $io, Command $command): void
Expand Down
2 changes: 2 additions & 0 deletions src/MakerInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,15 @@

namespace Symfony\Bundle\MakerBundle;

use Symfony\Bundle\MakerBundle\Dependency\DependencyManager;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputInterface;

/**
* Interface that all maker commands must implement.
*
* @method static string getCommandDescription()
* @method void configureComposerDependencies(DependencyManager $dependencyManager)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a fan of touching the interface... better ideas are welcome..

*
* @author Ryan Weaver <[email protected]>
*/
Expand Down
Loading