sonata-project / SonataAdminBundle

The missing Symfony Admin Generator
https://docs.sonata-project.org/projects/SonataAdminBundle
MIT License
2.11k stars 1.26k forks source link

Symfony 5 compatibility #5788

Closed mouad-mahfoud closed 4 years ago

mouad-mahfoud commented 4 years ago

I am trying to install sonata admin bundle in a fresh Symfony 5 installation but I get this error : Problem 1

core23 commented 4 years ago

Feel free to help us supporting symfony 5 @mouad-mahfoud

michaelKaefer commented 4 years ago

@core23 How can we help? Would it be correct to clone the latest version, then upgrade all symfony packages via composer to 5.*, then run the tests and fix all errors? Is that enough? I want to do this in the holidays.

core23 commented 4 years ago

Would it be correct to clone the latest version, then upgrade all symfony packages via composer to 5.*, then run the tests and fix all errors?

That's the way you go.

But keep in mind, that we have version lock because of required (dev-)dependencies: https://github.com/sonata-project/SonataAdminBundle/pull/5797#issuecomment-567165651. There are also more third party dependencies that may need an update to work with symfony 5

I want to do this in the holidays.

:100: !

michaelKaefer commented 4 years ago

I think this is too much for me, sorry, somebody else has to do it..

Bodarev commented 4 years ago

https://github.com/sonata-project/SonataAdminBundle/issues/5786#issuecomment-567935452

help please

core23 commented 4 years ago

Feel free to help with this by contributing @Bodarev

MrPoliteGrizlyM commented 4 years ago

Have same problem, but with assets:

Problem 1
    - sonata-project/admin-bundle 3.56.1 requires symfony/asset ^3.4.30 || ^4.2 -> no matching package found.
    - sonata-project/admin-bundle 3.56.0 requires symfony/asset ^3.4.30 || ^4.2 -> no matching package found.
    - Installation request for sonata-project/admin-bundle ^3.56 -> satisfiable by sonata-project/admin-bundle[3.56.0, 3.56.1]. 
VincentLanglet commented 4 years ago

I am trying to install sonata admin bundle in a fresh Symfony 5 installation but I get this error : Problem 1

  • Conclusion: don't install sonata-project/admin-bundle 3.56.1
  • Conclusion: remove twig/twig v3.0.0
  • Installation request for sonata-project/admin-bundle ^3.56 -> satisfiable by sonata-project/admin-bundle[3.56.0, 3.56.1].
  • Conclusion: don't install twig/twig v3.0.0
  • sonata-project/admin-bundle 3.56.0 requires twig/twig ^2.10 -> satisfiable by twig/twig[v2.10.0, v2.11.0, v2.11.1, v2.11.2, v2.11.3, v2.12.0, v2.12.1, v2.12.2].
  • Can only install one of: twig/twig[v2.10.0, v3.0.0].
  • Can only install one of: twig/twig[v2.11.0, v3.0.0].
  • Can only install one of: twig/twig[v2.11.1, v3.0.0].
  • Can only install one of: twig/twig[v2.11.2, v3.0.0].
  • Can only install one of: twig/twig[v2.11.3, v3.0.0].
  • Can only install one of: twig/twig[v2.12.0, v3.0.0].
  • Can only install one of: twig/twig[v2.12.1, v3.0.0].
  • Can only install one of: twig/twig[v2.12.2, v3.0.0].
  • Installation request for twig/twig (locked at v3.0.0) -> satisfiable by twig/twig[v3.0.0].

This is not an issue with Symfony 5 but only Twig 3. I think two different issues should be created. One for Sf5 and one for Twig 3.

greg0ire commented 4 years ago

Yeah plus, starting with just Twig 3 might be easier.

jorrit commented 4 years ago

It is clear that Sonata Admin doesn't work with Symfony 5. Until support is added, perhaps a warning could be added to the installation instructions? I can submit a PR for this, but first I'd like to know if this sounds like a good idea.

core23 commented 4 years ago

It is clear that Sonata Admin doesn't work with Symfony 5. Until support is added, perhaps a warning could be added to the installation instructions? I can submit a PR for this, but first I'd like to know if this sounds like a good idea.

If I got you right: You want a warning, that we don't support symfony 5? You have composer for that

jorrit commented 4 years ago

What I mean is that it could be helpful to list somewhere in the docs, let's say at https://sonata-project.org/bundles/admin/3-x/doc/getting_started/installation.html, which Symfony version(s) are supported.

Composer does give warnings when installing on Symfony 5, but the warnings given refer to symfony/asset not being the right version, it does not clearly say what the root problem is: Sonata Admin is compatible with Symfony up to 4, not 5.

Perhaps the message currently given by composer is sufficient for most users, but a small addition to the manual would not hurt, I think.

core23 commented 4 years ago

IMHO it's overhead to update two places for every dependency. If we start to list supported symfony versions, we must also list supported versions of php, doctrine, twig, phpunit, ...

VincentLanglet commented 4 years ago

IMHO it's overhead to update two places for every depedency. If we start to list supported symfony versions, we must also list supported versions of php, doctrine, twig, phpunit, ...

I agree

In the initial issue, composer return the following message.

sonata-project/admin-bundle 3.56.0 requires twig/twig ^2.10

This seems pretty clear.

greg0ire commented 4 years ago

If anyone wants to help, here is where to start:

composer why-not twig/twig 3   
jms/translation-bundle       1.4.4   requires  twig/twig (^1.27 || ^2.0)  
sonata-project/block-bundle  3.18.3  requires  twig/twig (^1.34 || ^2.0)  
sonata-project/core-bundle   3.18.0  requires  twig/twig (^1.34 || ^2.0)  
sonata-project/intl-bundle   2.7.0   requires  twig/twig (^2.9)           
twig/extensions              v1.5.4  requires  twig/twig (^1.27|^2.0)
jaikdean commented 4 years ago

As an update to @greg0ire's last comment…

VincentLanglet commented 4 years ago

And you created the issue https://github.com/sonata-project/SonataAdminBundle/issues/5909

greg0ire commented 4 years ago

See also:

goetas commented 4 years ago

schmittjoh/JMSTranslationBundle#519 schmittjoh/JMSTranslationBundle#520

Both PR have been merged

greg0ire commented 4 years ago

@goetas here is one more : https://github.com/schmittjoh/JMSTranslationBundle/pull/522 :)

VincentLanglet commented 4 years ago
  • sonata-project/block-bundle 4.0.0 added support for Symfony 5 and Twig 3

The AdminBundle will need to drop support of 3.x blockbundle to be Symfony is 5 compatible or we'll need to fix some deprecation on the 3.x version.

For example:

The "Sonata\BlockBundle\Event\BlockEvent" class extends "Symfony\Component\EventDispatcher\Event" that is deprecated since Symfony 4.3, use "Symfony\Contracts\EventDispatcher\Event" instead.
    1x in CRUDControllerTest::testList from Sonata\AdminBundle\Tests\Functional\Controller

Does updating the major version of a dependency is a BC-break @greg0ire ?

jaikdean commented 4 years ago

The AdminBundle will need to drop support of 3.x blockbundle to be Symfony is 5 compatible or we'll need to fix some deprecation on the 3.x version.

AdminBundle will need to add support for 4.x of BlockBundle. It wouldn't necessarily need to drop support for 3.x.

VincentLanglet commented 4 years ago

Oh yeah indeed. It will use both Symfony 5 and BlockBundle 4 or both Symfony 4 and BlockBundle 3.

bhyassine commented 4 years ago

So it is not possible yet to install Sonata on a Symfony 5 that uses Twig 3?

VincentLanglet commented 4 years ago

So it is not possible yet to install Sonata on a Symfony 5 that uses Twig 3?

No

greg0ire commented 4 years ago

Does updating the major version of a dependency is a BC-break @greg0ire ?

No it is not.

parijke commented 4 years ago

@greg0ire how can I help?

VincentLanglet commented 4 years ago

@parijke You can run composer why-not image

And try to fix one by one the dependency.

For example, the admin-bundle should support block-bundle@5 and stop using the core-bundle. And then, we'll have to fix all the deprecation in the admin-bundle.

parijke commented 4 years ago

@greg0ire dont know exactly what you mean. Can you delegate me some simple tasks or explain what we'll have to do?

greg0ire commented 4 years ago

@parijke I've had a look at the situation on the admin bundle, and it's complicated, because it depends on other bundles… maybe you could start with https://github.com/sonata-project/exporter ? It has no dependencies with sonata bundles.

If I run composer show symfony/* on that project, I get the following list:

symfony/config                     v4.4.5  Symfony Config Component
symfony/console                    v4.4.5  Symfony Console Component
symfony/debug                      v4.4.5  Symfony Debug Component
symfony/dependency-injection       v4.4.5  Symfony DependencyInjection Component
symfony/error-handler              v4.4.5  Symfony ErrorHandler Component
symfony/event-dispatcher           v4.4.5  Symfony EventDispatcher Component
symfony/event-dispatcher-contracts v1.1.7  Generic abstractions related to dispatching event
symfony/filesystem                 v5.0.5  Symfony Filesystem Component
symfony/http-foundation            v4.4.5  Symfony HttpFoundation Component
symfony/http-kernel                v4.4.5  Symfony HttpKernel Component
symfony/inflector                  v5.0.5  Symfony Inflector Component
symfony/mime                       v5.0.5  A library to manipulate MIME messages
symfony/phpunit-bridge             v4.4.5  Symfony PHPUnit Bridge
symfony/polyfill-ctype             v1.14.0 Symfony polyfill for ctype functions
symfony/polyfill-intl-idn          v1.14.0 Symfony polyfill for intl's idn_to_ascii and idn_to_utf8 functions
symfony/polyfill-mbstring          v1.14.0 Symfony polyfill for the Mbstring extension
symfony/polyfill-php72             v1.14.0 Symfony polyfill backporting some PHP 7.2+ features to lower PHP versions
symfony/polyfill-php73             v1.14.0 Symfony polyfill backporting some PHP 7.3+ features to lower PHP versions
symfony/property-access            v4.4.5  Symfony PropertyAccess Component
symfony/routing                    v4.4.5  Symfony Routing Component
symfony/service-contracts          v2.0.1  Generic abstractions related to writing services
symfony/var-dumper                 v5.0.5  Symfony mechanism for exploring and dumping PHP variables
symfony/yaml                       v5.0.5  Symfony Yaml Component

If I ask composer why the config component is not in version 5, I can see that it's only because we don't allow it in composer.json: https://github.com/sonata-project/exporter/blob/2.x/composer.json#L30

Simple task you can do:

  1. clone the exporter project
  2. make sure you successfully run the tests
  3. allow symfony/config 5 and run the test again
  4. fix the tests that are broken
  5. repeat steps above with symfony/console, and so on.
greg0ire commented 4 years ago

Regarding Twig 3: issues with JMSTranslationBundle are fixed :)

composer why-not twig/twig 3
sonata-project/admin-bundle  3.x-dev  requires  twig/twig (^2.12)          
sonata-project/block-bundle  3.18.4   requires  twig/twig (^1.34 || ^2.0)  
sonata-project/core-bundle   3.18.0   requires  twig/twig (^1.34 || ^2.0)  
twig/extensions              v1.5.4   requires  twig/twig (^1.27|^2.0)
parijke commented 4 years ago

Ok, so I cloned the repo... but then step 2. how do I run the tests?

VincentLanglet commented 4 years ago

Ok, so I cloned the repo... but then step 2. how do I run the tests?

make test works if you have phpunit installed.

But you can use vendor/bin/simple-phpunit too I think

parijke commented 4 years ago

ok, I had to do a composer install before step 2? Otherwise I do not have a vendor directory. Now it outputs: ../vendor/bin/simple-phpunit ExporterTest.php PHPUnit 8.3.5 by Sebastian Bergmann and contributors.

Testing Sonata\Exporter\Tests\ExporterTest FFWEEEEE 8 / 8 (100%)

Time: 26 ms, Memory: 6.00 MB

There were 5 errors:

1) Sonata\Exporter\Tests\ExporterTest::testGetResponse with data set #0 ('json', 'foo.json', 'application/json', '#foo#') Error: Class 'Sonata\Exporter\Source\ArraySourceIterator' not found

/home/paul/www/exporter/tests/ExporterTest.php:63

2) Sonata\Exporter\Tests\ExporterTest::testGetResponse with data set #1 ('xml', 'foo.xml', 'text/xml', '#foo#') Error: Class 'Sonata\Exporter\Source\ArraySourceIterator' not found

/home/paul/www/exporter/tests/ExporterTest.php:63

3) Sonata\Exporter\Tests\ExporterTest::testGetResponse with data set #2 ('xls', 'foo.xls', 'application/vnd.ms-excel', '#foo#') Error: Class 'Sonata\Exporter\Source\ArraySourceIterator' not found

/home/paul/www/exporter/tests/ExporterTest.php:63

4) Sonata\Exporter\Tests\ExporterTest::testGetResponse with data set #3 ('csv', 'foo.csv', 'text/csv', '#foo#') Error: Class 'Sonata\Exporter\Source\ArraySourceIterator' not found

/home/paul/www/exporter/tests/ExporterTest.php:63

5) Sonata\Exporter\Tests\ExporterTest::testGetResponse with data set #4 ('made-up', 'foo.made-up', 'application/made-up', '##') Error: Class 'Sonata\Exporter\Source\ArraySourceIterator' not found

/home/paul/www/exporter/tests/ExporterTest.php:63

--

There was 1 warning:

1) Sonata\Exporter\Tests\ExporterTest::testGetAvailableFormats Cannot stub or mock class or interface "Sonata\Exporter\Writer\TypedWriterInterface" which does not exist

--

There were 2 failures:

1) Sonata\Exporter\Tests\ExporterTest::testFilter Failed asserting that exception message 'Cannot stub or mock class or interface "Sonata\Exporter\Source\SourceIteratorInterface" which does not exist' contains 'Invalid "foo" format'.

2) Sonata\Exporter\Tests\ExporterTest::testConstructorRejectsNonTypedWriters Failed asserting that exception of type "Error" matches expected exception "TypeError". Message was: "Class 'Sonata\Exporter\Exporter' not found" at /home/paul/www/exporter/tests/ExporterTest.php:45 .

ERRORS! Tests: 8, Assertions: 3, Errors: 5, Failures: 2, Warnings: 1.

VincentLanglet commented 4 years ago

ok, I had to do a composer install before step 2? Otherwise I do not have a vendor directory.

Yes. You always have to run composer install if you want to correctly work on a repository ;)

When I

This is what I get image

Just run vendor/bin/simple-phpunit in the root of the project ; not in the /tests/ folder @parijke

parijke commented 4 years ago

Ahhhh ok, that worked, same result :-) already learned a few things

parijke commented 4 years ago

So I changed the line "symfony/config": "^3.4 || ^4.0 || ^5.0", to allow it.... run the tests again, they run fine....

but, is it tested against 5.x then? I don't think so or I do not understand it. composer show: symfony/config v4.4.5

sorry all, it is a one time investment to get me going I promise :-)

VincentLanglet commented 4 years ago

So I changed the line "symfony/config": "^3.4 || ^4.0 || ^5.0", to allow it.... run the tests again, they run fine....

but, is it tested against 5.x then? I don't think so or I do not understand it. composer show: symfony/config v4.4.5

I think you have to run composer update after allowing the v5 in order to install it.

parijke commented 4 years ago

ok, I got all the symfony/* allowed to be 5 and all tests pass. What now?

{
    "name": "sonata-project/exporter",
    "type": "library",
    "description": "Lightweight Exporter library",
    "keywords": [
        "export",
        "csv",
        "xls",
        "data",
        "client"
    ],
    "homepage": "https://github.com/sonata-project/Exporter",
    "license": "MIT",
    "authors": [
        {
            "name": "Thomas Rabaix",
            "email": "thomas.rabaix@gmail.com",
            "homepage": "https://sonata-project.org/"
        }
    ],
    "require": {
        "php": "^7.2"
    },
    "require-dev": {
        "doctrine/dbal": "^2.5",
        "doctrine/orm": "^2.4.5",
        "matthiasnoback/symfony-config-test": "^4.0",
        "matthiasnoback/symfony-dependency-injection-test": "^4.0",
        "propel/propel1": "^1.6",
        "symfony/config": "^3.4 || ^4.0 || ^5.0",
        "symfony/dependency-injection": "^3.4 || ^4.0 || ^5.0",
        "symfony/http-foundation": "^3.4 || ^4.0 || ^5.0",
        "symfony/http-kernel": "^3.4 || ^4.0 || ^5.0",
        "symfony/phpunit-bridge": "^4.3.8 || ^5.0",
        "symfony/property-access": "^3.4 || ^4.0 || ^5.0",
        "symfony/routing": "^3.4 || ^4.0 || ^5.0"
    },
    "suggest": {
        "propel/propel1": "To export propel collections",
        "symfony/property-access": "To be able to export from database entities",
        "symfony/routing": "To be able to export the routes of a Symfony app"
    },
    "config": {
        "sort-packages": true
    },
    "extra": {
        "branch-alias": {
            "dev-master": "2.x-dev"
        }
    },
    "autoload": {
        "psr-4": {
            "Sonata\\Exporter\\": "src/"
        }
    },
    "autoload-dev": {
        "psr-4": {
            "Sonata\\Exporter\\Tests\\": "tests/"
        }
    }
}
PHPUnit 8.5.0 by Sebastian Bergmann and contributors.

Runtime:       PHP 7.3.15-3+ubuntu19.10.1+deb.sury.org+1 with Xdebug 2.9.2
Configuration: /home/paul/www/exporter/phpunit.xml.dist

Testing Sonata Exporter Test Suite
................................................................. 65 / 98 ( 66%)
.................................                                 98 / 98 (100%)

Time: 1.03 seconds, Memory: 42.00 MB

OK (98 tests, 306 assertions)
VincentLanglet commented 4 years ago

Create a PR, seems like there is no breaking change for us

parijke commented 4 years ago

Can you help me with that?

VincentLanglet commented 4 years ago
git checkhout -b myNewBranch
git add theFileIChanged
git commit -m TheMessageOfMyCommit
git push

Then https://help.github.com/en/desktop/contributing-to-projects/creating-a-pull-request

parijke commented 4 years ago

I think I did.... https://github.com/sonata-project/exporter/pull/330

parijke commented 4 years ago

So I think the steps are

  1. Fork the repo
  2. clone the exporter project
  3. composer install
  4. make sure you successfully run the tests
  5. allow symfony/config 5
  6. composer update
  7. run the test again
  8. fix the tests that are broken
  9. repeat steps above with symfony/console, and so on.
  10. commit the changes
  11. create pull request
greg0ire commented 4 years ago

Thanks for your contribution @parijke ! If you want to try your hand on other repositories, please check out the graph on this issue: https://github.com/sonata-project/dev-kit/issues/216 exporter was easy because it's a leaf node of the dependency graph. You might find other leaf node libraries that are not yet compatible with Symfony 5, and do your thing, that will help a lot!

parijke commented 4 years ago

Yes I want really to help get it ready for symfony 5 (got time as well as I am locked down in Spain for at least two weeks). I looked at the issue/graph but it doesn't make sense to me (yet). Maybe it is easier to delegate some (easy) tasks to me that I can finish with a little help sometimes, like the exporter thing. I learned a few things along the way about runnings tests and the github workflow.
Maybe it is better to discuss this further in slack as well?

greg0ire commented 4 years ago

What I was proposing is that you try exactly what you did for the exporter, but for other libs. You can find the exporter on the graph, it's a leaf node (arrows come to it, but do not come out of it). There are some other leaf nodes on this graph, like for instance https://github.com/sonata-project/GoogleAuthenticator or https://github.com/sonata-project/SonataFormatterBundle. Can you look if those are sf5 compatible, and make them compatible if that's not the case? Don't hesitate to post on our channel in the Symfony Slack if you are stuck.

the graph

adriendarki commented 4 years ago

it is still impossible to install it on symfony 5

"Restricting packages listed in "symfony/symfony" to "5.0.*" Your requirements could not be resolved to an installable set of packages."

" Your equirements could not be resolved to an installable set of packages.

Problem 1

VincentLanglet commented 4 years ago

it is still impossible to install it on symfony 5

Nobody said it was.

adriendarki commented 4 years ago

it is still impossible to install it on symfony 5

Nobody said it was.

sorry my english is not perfect. I think addition of support symfony 5.0 will result in a compatible