[Feature] Change to use Composer for autoload, drop PEAR/EZC #1340

andrerom · 2018-01-10T16:04:25Z

Mainly in order to solve the autloading issue with eZ Platform v2, but also:

Allows standalone usage in scripts using standard composer autoload
Automatically generates autoload on composer install / update (when legacy is root package. In legacy bridge setup, legacy bridge handles this and more)
Removes the dead code around PEAR and bundled ez Components which is not relevant anymore, and actually risky since they don't received (security) updates anymore

Result is more clean autoloading code which just have three cases to care about in regards to making sure things like Zeta Components are loaded.

emodric · 2018-01-10T17:43:33Z

autoload.php

@@ -19,6 +19,12 @@
    require_once __DIR__ . '/config.php';
 }

+// If composer autoloaded context such as eZ Platform & legacy bridge, skip trying to autoload Zeta Components and such
+if ( class_exists( 'Composer\Autoload\ClassLoader', false ) )


How is this different from what legacy bridge bundle does?

https://github.com/ezsystems/LegacyBridge/blob/master/bundle/EzPublishLegacyBundle.php#L43-L46

Had forgot about that one.

It's more or less the same, so not needed then. Only difference is that this also detects usage in plain legacy. As in:

require __DIR__ . '/vendor/autoload.php'; $ini = eZINI::instance(); echo $ini->variable( 'SiteAccessSettings', 'ForceVirtualHost' );

That said I'm unsure if allowing yet another way to do things is good. So I'll gladly remove it.

But main reason I added it is because I need it in order to be able to do the section in composer:

"autoload": { "files": ["autoload.php"] },

Okay. I'll test the patch tomorrow to see how it behaves.

Btw, is there a point in using Composer autoloading in context of pure legacy, seeing how you just can do require autoload.php, instead of require vendor/autoload.php and achieve the exact same thing?

@emodric I imagine this allows using composer-installed dependencies in a pure-legacy context... An interesting concept - albeit probably not many legacy extensions are currently available that take advantage of this

albeit probably not many legacy extensions are currently available that take advantage of this

@gggeek If any, hence, my question :) But I suppose there's no harm done in allowing Composer autoloading too.

Btw, is there a point in using Co

As said, this is not the main point* of this, just a side benefit/downside however you look at it. I know many legacy parts don't use compoer and never will, exhibit A: package system as used by setup wizard.

* Main point: To auto register legacy autoload in composer in a Platform + Legacy Bridge setup to get rid of dependency on app/autoload.php which was removed in Symfony 3 and hence also eZ Platform v2 as there is no need for it. Somewhat pure composer autoloading for the win 🎉

emodric · 2018-01-11T09:04:15Z

The error about CLIHandler (https://github.com/ezsystems/LegacyBridge/issues/132) is gone, but there's still an error when running the script to generate autoloads from eZ Platform root.

I think that eZ Publish Legacy still expects app/autoload.php to exist (https://github.com/ezsystems/ezpublish-legacy/blob/master/autoload.php#L48, https://github.com/ezsystems/ezpublish-legacy/blob/master/bin/php/ezpgenerateautoloads.php#L42), because when I create an empty app/autoload.php, the error is gone.

> eZ\Bundle\EzPublishLegacyBundle\Composer\ScriptHandler::generateAutoloads
Running script 'bin/php/ezpgenerateautoloads.php' in eZ Publish legacy context
PHP Warning:  require(Base/src/base.php): failed to open stream: No such file or directory in /media/eddie/data/html/netgensite/ezpublish_legacy/bin/php/ezpgenerateautoloads.php on line 58
PHP Fatal error:  require(): Failed opening required 'Base/src/base.php' (include_path='.:/usr/share/php') in /media/eddie/data/html/netgensite/ezpublish_legacy/bin/php/ezpgenerateautoloads.php on line 58
09:56:40 CRITICAL  [php] Fatal Compile Error: require(): Failed opening required 'Base/src/base.php' (include_path='.:/usr/share/php') ["exception" => Symfony\Component\Debug\Exception\FatalErrorException { …}] []

In ezpgenerateautoloads.php line 58:
                                                                               
  Compile Error: require(): Failed opening required 'Base/src/base.php' (incl  
  ude_path='.:/usr/share/php')                                                 
                                                                               

ezpublish:legacy:script [--legacy-help] [-h|--help] [-q|--quiet] [-v|vv|vvv|--verbose] [-V|--version] [--ansi] [--no-ansi] [-n|--no-interaction] [-e|--env ENV] [--no-debug] [--siteaccess [SITEACCESS]] [--] <command> <script>

Script eZ\Bundle\EzPublishLegacyBundle\Composer\ScriptHandler::generateAutoloads handling the symfony-scripts event terminated with an exception

                                                                                                                                                                                                                                    
  [RuntimeException]                                                                                                                                                                                                                
  An error occurred when executing the "'ezpublish:legacy:script bin/php/ezpgenerateautoloads.php'" command:                                                                                                                        
                                                                                                                                                                                                                                    
  Running script 'bin/php/ezpgenerateautoloads.php' in eZ Publish legacy context                                                                                                                                                    
                                                                                                                                                                                                                                    
                                                                                                                                                                                                                                    
  PHP Warning:  require(Base/src/base.php): failed to open stream: No such file or directory in /media/eddie/data/html/netgensite/ezpublish_legacy/bin/php/ezpgenerateautoloads.php on line 58                                  
  PHP Fatal error:  require(): Failed opening required 'Base/src/base.php' (include_path='.:/usr/share/php') in /media/eddie/data/html/netgensite/ezpublish_legacy/bin/php/ezpgenerateautoloads.php on line 58                  
  09:56:40 CRITICAL  [php] Fatal Compile Error: require(): Failed opening required 'Base/src/base.php' (include_path='.:/usr/share/php') ["exception" => Symfony\Component\Debug\Exception\FatalErrorException { …}] []             
                                                                                                                                                                                                                                    
  In ezpgenerateautoloads.php line 58:                                                                                                                                                                                              
                                                                                                                                                                                                                                    
    Compile Error: require(): Failed opening required 'Base/src/base.php' (incl                                                                                                                                                     
    ude_path='.:/usr/share/php')                                                                                                                                                                                                    
                                                                                                                                                                                                                                    
                                                                                                                                                                                                                                    
  ezpublish:legacy:script [--legacy-help] [-h|--help] [-q|--quiet] [-v|vv|vvv|--verbose] [-V|--version] [--ansi] [--no-ansi] [-n|--no-interaction] [-e|--env ENV] [--no-debug] [--siteaccess [SITEACCESS]] [--] <command> <script>

emodric

Everything works now

alongosz · 2018-01-11T10:07:59Z

Hmm, trying to test that with v2 + LegacyBridge but in meta-repository (ezplatform v2) I still need to modify autoload section of composer.json to have:

"autoload": {
        "psr-4": {
            "AppBundle\\": "src/AppBundle/"
        },
        "files": ["ezpublish_legacy/autoload.php"],
        "classmap": [ "app/AppKernel.php", "app/AppCache.php" ]
    },

Does v2 + LegacyBridge setup works for you without it?

Also now when clearing v2 Sf cache I'm getting:

In ezini.php line 1437:
                                                                       
  injected-merge-settings can only reference and contain array values

but this seems to be unrelated to changes, so just a side note.

emodric · 2018-01-11T10:21:13Z

@alongosz Works fine without any changes to root composer.json. Did you actually install this branch through Composer with dev-composer_autoload_support as 2017.12.0 as a version in require section?

I've tested with https://github.com/emodric/ezplatform-legacy with emodric/ezplatform-legacy@ab8d3e0 reverted.

Also, no mention of ezini.php error.

andrerom · 2018-01-11T10:55:16Z

Worked for me her now too.

> eZ\Bundle\EzPublishLegacyBundle\Composer\ScriptHandler::generateAutoloads
Running script 'bin/php/ezpgenerateautoloads.php' in eZ Publish legacy context
Scanning for PHP-files.

Scan complete. Found 266 PHP files.

Searching for classes (tokenizing).

Found 108 classes, added 108 of them to the autoload array.

> eZ\Bundle\EzPublishLegacyBundle\Composer\ScriptHandler::symlinkLegacyFiles
Aborting: The src directory "src/legacy_files" does not exist.

emodric · 2018-01-11T13:43:17Z

@andrerom Will this be tagged with 2017.12.1 ?

emodric · 2018-01-11T13:50:45Z

P.S. Just discovered this, when running legacy scripts in pure legacy context:

eddie@abyss [~/www/ezpublish_legacy] ( ± composer_autoload_support ● ) $ php bin/php/flatten.php all   
PHP Notice:  Constant EZCBASE_ENABLED already defined in /media/eddie/data/html/ezpublish_legacy/autoload.php on line 44

alongosz · 2018-01-11T14:14:15Z

@alongosz Works fine without any changes to root composer.json. Did you actually install this branch through Composer with dev-composer_autoload_support as 2017.12.0 as a version in require section?

@emodric I didn't think of that because I had dependency on LegacyBridge only, so just symlinked ezpublish_legacy directory to repo with these changes. I guess some composer workflow is missing in this case 😅

Anyway, when done properly as you've suggested - it works :)

P.S. Just discovered this, when running legacy scripts in pure legacy context

eddie@abyss [~/www/ezpublish_legacy] ( ± composer_autoload_support ● ) $ php bin/php/flatten.php all   
PHP Notice:  Constant EZCBASE_ENABLED already defined in ?>/media/eddie/data/html/ezpublish_legacy/autoload.php on line 44

It also is visible in unit tests on Travis, seems like autoload.php is included at least twice somewehere.

cc @andrerom

emodric · 2018-01-11T14:23:04Z

seems like autoload.php is included at least twice somewehere.

Yep, looks like it. For example, if I change require_once 'autoload.php'; to require_once 'vendor/autoload.php'; in bin/php/flatten.php, no more notice.

andrerom · 2018-01-12T11:34:58Z

Maybe it's too close to the weekend already, but like before it's surrounded by if ( !defined( 'EZCBASE_ENABLED' ) ), so have a hard time seeing why it would be defined twice actually 😕

emodric · 2018-01-12T12:01:14Z

@andrerom I'm not so sure myself. I can confirm that the file is ran twice, but I can't figure out why or how it goes into that if clause.

BTW, with these changes, this too can be removed now: https://github.com/ezsystems/ezpublish-legacy/blob/master/config.php-RECOMMENDED#L23-L53, as those three constants are not used anymore.

emodric · 2018-01-12T12:08:54Z

Debugging further, it seems that class_exists call triggers the autoloading mechanism, despite the second false argument.

andrerom · 2018-01-12T12:24:55Z

Hmm, I'd like to try to double check that once I have the time. But if that is the case, I'll probably have to resort to using spl_autoload_functions() and check for instance of composer. Or change the logic. Suggestions welcome.

emodric · 2018-01-12T13:07:25Z

It seems that I was wrong after all. The problem is not class_exists, but the fact that we're requiring vendor/autoload.php from autoload.php BEFORE EZCBASE_ENABLED is defined (lines 34 & 40), which again requires autoload.php due to it being declared in composer.json autoload section.

Hacking on vendor/composer/autoload_real.php to use require_once instead of require gets rid of the notice.

andrerom · 2018-01-12T16:46:44Z

That make complete sense, sorry for sending you down the recursion rabbit hole 🙈
Anyway, hope that is fixed now with latests changes, also removed the not relevant parts in config.php as well 😃

emodric · 2018-01-12T23:29:20Z

Thanks! I'll test soon 👍

emodric · 2018-01-15T09:30:27Z

@andrerom Everything works fine now, so 👍 for merging.

andrerom · 2018-01-15T09:43:49Z

@alongosz / @gggeek looks good now?

[Feature] Add support for autloading via Composer

9ce7963

andrerom requested review from emodric and alongosz January 10, 2018 16:04

emodric reviewed Jan 10, 2018

View reviewed changes

Cleanup all pear and bundeld ezc code, not relevant anymore

6b7f7f1

emodric approved these changes Jan 11, 2018

View reviewed changes

andrerom changed the title ~~[Feature] Add support for autoloading via Composer~~ [Feature] Change to use Composer for autoload, drop PEAR/EZC Jan 11, 2018

andrerom added 2 commits January 12, 2018 17:40

Update autoload.php

58ca305

Update config.php-RECOMMENDED

ae734e9

alongosz approved these changes Jan 15, 2018

View reviewed changes

andrerom merged commit ffaab4a into master Jan 15, 2018

andrerom deleted the composer_autoload_support branch January 24, 2018 13:29

pkamps mentioned this pull request Feb 6, 2018

[Feature] Change to use Composer for autoload, drop PEAR/EZC mugoweb/ezpublish-legacy#93

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] Change to use Composer for autoload, drop PEAR/EZC #1340

[Feature] Change to use Composer for autoload, drop PEAR/EZC #1340

andrerom commented Jan 10, 2018 •

edited

Loading

emodric Jan 10, 2018

andrerom Jan 10, 2018

andrerom Jan 10, 2018 •

edited

Loading

emodric Jan 10, 2018

emodric Jan 10, 2018

gggeek Jan 10, 2018

emodric Jan 10, 2018

andrerom Jan 11, 2018 •

edited

Loading

emodric commented Jan 11, 2018

emodric left a comment

alongosz commented Jan 11, 2018

emodric commented Jan 11, 2018 •

edited

Loading

andrerom commented Jan 11, 2018 •

edited

Loading

emodric commented Jan 11, 2018

emodric commented Jan 11, 2018

alongosz commented Jan 11, 2018

emodric commented Jan 11, 2018 •

edited

Loading

andrerom commented Jan 12, 2018

emodric commented Jan 12, 2018 •

edited

Loading

emodric commented Jan 12, 2018

andrerom commented Jan 12, 2018 •

edited

Loading

emodric commented Jan 12, 2018

andrerom commented Jan 12, 2018 •

edited

Loading

emodric commented Jan 12, 2018

emodric commented Jan 15, 2018

andrerom commented Jan 15, 2018

[Feature] Change to use Composer for autoload, drop PEAR/EZC #1340

[Feature] Change to use Composer for autoload, drop PEAR/EZC #1340

Conversation

andrerom commented Jan 10, 2018 • edited Loading

emodric Jan 10, 2018

Choose a reason for hiding this comment

andrerom Jan 10, 2018

Choose a reason for hiding this comment

andrerom Jan 10, 2018 • edited Loading

Choose a reason for hiding this comment

emodric Jan 10, 2018

Choose a reason for hiding this comment

emodric Jan 10, 2018

Choose a reason for hiding this comment

gggeek Jan 10, 2018

Choose a reason for hiding this comment

emodric Jan 10, 2018

Choose a reason for hiding this comment

andrerom Jan 11, 2018 • edited Loading

Choose a reason for hiding this comment

emodric commented Jan 11, 2018

emodric left a comment

Choose a reason for hiding this comment

alongosz commented Jan 11, 2018

emodric commented Jan 11, 2018 • edited Loading

andrerom commented Jan 11, 2018 • edited Loading

emodric commented Jan 11, 2018

emodric commented Jan 11, 2018

alongosz commented Jan 11, 2018

emodric commented Jan 11, 2018 • edited Loading

andrerom commented Jan 12, 2018

emodric commented Jan 12, 2018 • edited Loading

emodric commented Jan 12, 2018

andrerom commented Jan 12, 2018 • edited Loading

emodric commented Jan 12, 2018

andrerom commented Jan 12, 2018 • edited Loading

emodric commented Jan 12, 2018

emodric commented Jan 15, 2018

andrerom commented Jan 15, 2018

andrerom commented Jan 10, 2018 •

edited

Loading

andrerom Jan 10, 2018 •

edited

Loading

andrerom Jan 11, 2018 •

edited

Loading

emodric commented Jan 11, 2018 •

edited

Loading

andrerom commented Jan 11, 2018 •

edited

Loading

emodric commented Jan 11, 2018 •

edited

Loading

emodric commented Jan 12, 2018 •

edited

Loading

andrerom commented Jan 12, 2018 •

edited

Loading

andrerom commented Jan 12, 2018 •

edited

Loading