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

Require only needed dependencies #438

Merged
merged 7 commits into from
Sep 15, 2015
Merged

Require only needed dependencies #438

merged 7 commits into from
Sep 15, 2015

Conversation

javiereguiluz
Copy link
Collaborator

No description provided.

@Pierstoval
Copy link
Contributor

Must add symfony/css-selector too

@javiereguiluz
Copy link
Collaborator Author

Closing it in favor of #432.

@ogizanagi
Copy link
Contributor

The installed twig version for the failing build is v1.12.2. Kind of strange to me as the TwigBridge requirement says "twig/twig": "~1.20|~2.0" in 2.3.
I think you might try to fix this ? It might be related given the stacktrace.

Hmmm, btw, the symfony/twig-bridge installed version is wrong (2.2) too.

Edit: I looked at the branches instead of the tagged versions. This seems actually correct.

@ogizanagi
Copy link
Contributor

Adding the symfony/twig-bridge: ~2.3 requirement seems to solve most of the issues:

...Uncaught PHP Exception JavierEguiluz\Bundle\EasyAdminBundle\Exception\UndefinedEntityException: "The InexistentEntity entity is not defined in the configuration of your backend." at /EasyAdminBundle/Controller/AdminController.php line 108
............................................................  63 / 255 ( 24%)
..........................................Uncaught PHP Exception JavierEguiluz\Bundle\EasyAdminBundle\Exception\NoEntitiesConfiguredException: "Your backend is empty because you haven't configured any Doctrine entity to manage." at /EasyAdminBundle/Controller/AdminController.php line 98
....Uncaught PHP Exception JavierEguiluz\Bundle\EasyAdminBundle\Exception\ForbiddenActionException: "The requested edit action is not allowed." at /EasyAdminBundle/Controller/AdminController.php line 74
.Uncaught PHP Exception JavierEguiluz\Bundle\EasyAdminBundle\Exception\ForbiddenActionException: "The requested new action is not allowed." at /EasyAdminBundle/Controller/AdminController.php line 74
..........SS.... 126 / 255 ( 49%)
SSS....SS....S................................................. 189 / 255 ( 74%)
............................................................... 252 / 255 ( 98%)
...

Time: 7.24 seconds, Memory: 333.75Mb

OK, but incomplete, skipped, or risky tests!
Tests: 255, Assertions: 440, Skipped: 8.

@Pierstoval
Copy link
Contributor

Hope it'll solve every problem :)

@ogizanagi
Copy link
Contributor

Actually I don't know why exceptions are not converted to proper exception pages instead of being thrown like this, and I didn't investigate a lot, but it seems to be the only ones :)

@Pierstoval
Copy link
Contributor

I had this problem on another repo, I didn't remember how I solved it. I don't even remember having solved it 😛

@ogizanagi
Copy link
Contributor

... That helps a lot 😸

@ogizanagi
Copy link
Contributor

For the record: https://travis-ci.org/ogizanagi/EasyAdminBundle/builds/79192251 (no need for an umpteenth PR I guess)

The 2.3 requirement for the TwigBridge wasn't enough (but it works on my computer. PHP 5.6 however).
I fell on symfony/symfony#8721 which seems to solve the issue we got here.
(There are still the uncaught exceptions however)

# composer.json
+ "symfony/twig-bridge": "^2.3.4"

Should solve the issue (at least for the Warning: Unexpected character in input: [...] thing on the low deps build)

@javiereguiluz
Copy link
Collaborator Author

Lots of new errors appear after the latest changes:

PHP 5.3 / deps = low

PHP Fatal error: Class 'Doctrine\Common\Inflector\Inflector' not found in /home/travis/build/javiereguiluz/EasyAdminBundle/Reflection/ClassPropertyReflector.php on line 31

PHP 5.6 / Symfony 2.3.x

This error is repeated lots of times:

Argument 1 passed to Symfony\Bridge\Twig\Extension\SecurityExtension::__construct() must be an instance of Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface, instance of Symfony\Component\Security\Core\SecurityContext given, called in /home/travis/build/javiereguiluz/EasyAdminBundle/build/cache/default_backend/AppDefault_backendDebugProjectContainer.php on line 2496 and defined

@Pierstoval
Copy link
Contributor

For the Inflector problem, we require doctrine/common:^2.4.0.
Haven't investigated on the Security problem, but I guess it might be a problem coming from the fact that the security has been split in symfony 2.4 so there might be some strange regressions...

@ogizanagi
Copy link
Contributor

The issue for the PHP 5.6 / Symfony 2.3.x built about security is due to the fact the symfony/twig-bridge (v2.7.4) is installed where we asked for symfony components to be installed as the 2.3.x version:

if [ "$SYMFONY_VERSION" != "" ]; then composer require --dev --no-update symfony/symfony:$SYMFONY_VERSION; fi

The result in the vendor/symfony dir is:

├── symfony
│   ├── ...
│   └── src
│       └── Symfony
│           ├── Bridge
│           │   ├── ...
│           │   └── Twig
│           ├── Bundle
│           │   ├── FrameworkBundle
│           │   ├── SecurityBundle
│           │   ├── TwigBundle
│           │   └── ...
│           └── Component
│               ├── BrowserKit
│               └── ...
└── twig-bridge
    ├── AppVariable.php
    ├── CHANGELOG.md
    ├── ...
    ├── composer.json
    └── phpunit.xml.dist

and the vendor/composer/autoload_psr4.php file reference this mapping:

array(
    // ...
    'Symfony\\Bridge\\Twig\\' => array($vendorDir . '/symfony/twig-bridge'),
);

The symfony/twig-bridge installed in the 2.7.4 version is taken over the 2.3 one installed with symfony/symfony.

So we cannot work like this with Travis (requiring the whole framework as it is currently done) 😕

@Pierstoval
Copy link
Contributor

We could with a composer remove symfony/twig-bridge

@ogizanagi
Copy link
Contributor

We could do that indeed. But what about the identified 2.3.4 requirement for the symfony/twig-bridge (it should not fail though, as it only failed with the PHP 5.3 build) ?
Also, in a real application requiring symfony/symfony in the 2.3 version, will we encounter the same issue as in the tests (two twig-bridge cohabiting in different versions) ? I should have missed something...

@javiereguiluz
Copy link
Collaborator Author

@Pierstoval thanks to your suggested change, that error disappeared. But we still have the other problem and as mentioned by @ogizanagi, it looks not that easy to solve.

@javiereguiluz
Copy link
Collaborator Author

In fda0e3a I'm testing a new Travis configuration borrowed from the one used by our friends at FOSUserBundle: https://github.com/FriendsOfSymfony/FOSUserBundle/blob/master/.travis.yml

@javiereguiluz
Copy link
Collaborator Author

After the last changes, everything is green again. To be honest, I don't know if this is luck, or if the new config is wrong or if we finally fixed the problem.

@ogizanagi
Copy link
Contributor

So, the issue was only about the 2.3.x and 2.3.* difference ?

@javiereguiluz
Copy link
Collaborator Author

@xabbuh thanks for starting this PR. We finally solved all problems are tests are green again, so I'm merging it.

@javiereguiluz javiereguiluz merged commit 47e3ee7 into master Sep 15, 2015
javiereguiluz added a commit that referenced this pull request Sep 15, 2015
This PR was merged into the master branch.

Discussion
----------

Require only needed dependencies

Commits
-------

47e3ee7 Removed an unneded job
fda0e3a Tweaked the Travis CI configuration
7e4fd0d Required an specific version of doctrine/common
0624f6c Added "symfony/twig-bridge" and reordered some dependencies
ddaadb4 Added CssSelector component
79c49d2 Added BrowserKit component to the list of dev dependencies
7e2a632 require only needed dependencies
@javiereguiluz javiereguiluz deleted the pr/432 branch September 27, 2015 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants