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

Fixes #648: Allow Symfony 4 components in composer.json. #651

Merged
merged 4 commits into from
Dec 13, 2017

Conversation

greg-1-anderson
Copy link
Member

Overview

This pull request:

  • Fixes a bug
  • Adds a feature
  • Breaks backwards compatibility
  • Has tests that cover changes

Summary

Allow Symfony 4 components in composer.json. Note that Codeception is still pinning us to Symfony 3. This should allow folks using Robo as a Framework to use Symfony 4, albeit without test coverage on Robo with Symfony 4.

After composer install --no-dev:

$ composer why symfony/console
consolidation/annotated-command  2.8.2       requires  symfony/console (^2.8|^3|^4)  
consolidation/log                1.0.5       requires  symfony/console (^2.8|^3|^4)  
consolidation/output-formatters  3.1.13      requires  symfony/console (^2.8|^3|^4)  
consolidation/robo               dev-master  requires  symfony/console (^2.8|^3|^4)

@greg-1-anderson
Copy link
Member Author

greg-1-anderson commented Nov 30, 2017

This PR, as of c2f6d1e, will now resolve Symfony 4 components if desired. The composer.json is currently pinning to Symfony 3 by way of the extra.platform.php configuration item in the composer.json file. Delete this value (or set it to 7.1.3 or later) and run composer update, and you will get Symfony 4 components.

Ad-hoc testing with Symfony 4 shows Robo core to be working. However, the dev versions of Codeception are not working. Running the tests results in:

$ ./robo test
 [Testing\Codecept] Executing /Users/ganderson/local/consolidation/Robo/vendor/bin/codecept run 
 [Testing\Codecept] Running /Users/ganderson/local/consolidation/Robo/vendor/bin/codecept run 
 [error]  The provided cwd does not exist. 

The tests here are failing because the "lowest" tests require PHP 5.5, but we have set our platform php version to 5.6, which breaks this requirement. I'll improve the highest/lowest testing once our dependencies are ready to merge.

Prerequisites:

Optional:

@greg-1-anderson
Copy link
Member Author

It turns out that henrikbjorn/lurker is a fork of https://github.com/flint/Lurker.

$this->process->setWorkingDirectory($this->workingDirectory);

if ($this->workingDirectory) {
$this->process->setWorkingDirectory($this->workingDirectory);

Choose a reason for hiding this comment

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

Line indented incorrectly; expected at least 12 spaces, found 10

…x Symfony/Process 4.x API convention change: working directory must be explicitly provided. Make our own copy of Symfony ProcessUtils, since escapeArgument() is removed in Symfony 4.
@greg-1-anderson
Copy link
Member Author

It turns out that the error message above was not caused by Codeception at all, but rather by a new requirement in Symfony/Process 4.x that the cwd be explicitly provided. This PR causes Robo to subvert this behavior by explicitly providing getcwd() to symfony/process if the caller does not provide a working directory. We'll keep this in version 1.x of Robo to provide support for Symfony 4 while maintaining backwards compatibility. Presumably, Robo 2 will be Symfony 4 and later only (ergo PHP 7.1 and later). There's no timeline for starting a Robo 2 branch, though.

At the time of this writing, the test failures are expected, as this branch is set up to use Symfony 4 components, ergo anything earlier than php 7.1 will fail. I expect we'll want the Robo phar to pin to Symfony 3, and allow compatibility with Symfony 4 only for those who use Robo as a framework.

We now have:

  • grasmash/yaml-expander now has a stable release
  • php-coveralls/php-coveralls now has a stable release, but it's no longer necessary, as guzzle/guzzle is now compatible with Symfony 4.

We still need:

  • Stable tag for codeception/aspect-mock dev-master
  • codeception/base component updated for Symfony 4

@DavertMik
Copy link
Member

DavertMik commented Dec 12, 2017

At the time of this writing, the test failures are expected, as this branch is set up to use Symfony 4 components, ergo anything earlier than php 7.1 will fail. I expect we'll want the Robo phar to pin to Symfony 3, and allow compatibility with Symfony 4 only for those who use Robo as a framework.

I just faced an issue while executing my Robo script. Due to different symfony/process versions I received this error

Uncaught Error: Call to undefined method Symfony\Component\Process\ProcessUtils::escapeArgument()

This makes Robo unusable with projects containing Symfony 4 components.
Looks like Symfony 4 should be implemented either as a new version (2.0?) or by adding a compatibility layer.


Stable tag for codeception/aspect-mock dev-master

Here it is: 2.1.1. https://github.com/Codeception/AspectMock/releases/tag/2.1.1

codeception/base component updated for Symfony 4

Yes, that's what I'm trying to do but my Robo script is broken 😫

@DavertMik
Copy link
Member

Ok, codeception/base 4.0 supports Symfony 4.0 so you can go on 👍

@greg-1-anderson
Copy link
Member Author

I'll give codeception/base a spin and see how it goes.

Note that this PR already solves the ProcessUtils::escapeArgument undefined problem.

@greg-1-anderson
Copy link
Member Author

@DavertMik codeception/aspect-mock is pinning the version of symfony/finder to 3.x:

$ composer why symfony/finder
codeception/aspect-mock          2.1.1       requires  symfony/finder (~2.4|~3.0)   
codeception/base                 2.3.7       requires  symfony/finder (>=2.7 <5.0)  
consolidation/annotated-command  2.8.2       requires  symfony/finder (^2.5|^3|^4)  
consolidation/output-formatters  3.1.13      requires  symfony/finder (^2.5|^3|^4)  
consolidation/robo               dev-master  requires  symfony/finder (^2.5|^3|^4)

Not a big deal, probably - I'll charge forward ignoring this. Fix it up in a new aspect-mock release when you have a chance.

@greg-1-anderson
Copy link
Member Author

Tests are once again passing for php 7.1, so we're just about done here: just need to do highest/lowest testing with Symfony 2, 3 and 4 and make all the tests green.

@greg-1-anderson
Copy link
Member Author

Tested all the things! Symfony 2, 3 and 4 all passing.

@greg-1-anderson greg-1-anderson merged commit 32e57f8 into master Dec 13, 2017
@greg-1-anderson greg-1-anderson deleted the symfony-4 branch May 27, 2020 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants