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

disable require once call #2

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

disable require once call #2

wants to merge 2 commits into from

Conversation

glensc
Copy link

@glensc glensc commented May 4, 2015

as zend classes are not in include path, any require once call will cause fatal error.

and enabling Zend autoloader causes it to load things from include path (which may be outside vendor/ dir)

@deadbeef84
Copy link
Contributor

Hi, sorry for not getting back to you. When removing require-statements from the code, this guide was followed.

I'm guessing the reasoning for not stripping the require-statement for the autoloader, is that this guide assumes no autoloader is registered at that moment, while in our case we already have composer doing the autoloading for us.

However, for the zend autoloader to work, I think the include-path must be set properly, so even if we would remove the require statement, the zend loader wouldn't work. Though, I'm guessing you could use Zend_Application without the loader, so it might still make sense to remove this require-statement.

Following this reasoning, I'm agreeing with you, the require-statement probably does more harm than it does good. I will remove it, however I cannot simply accept pull requests as this repository is automatically updated from the official zf1 repository (so this pull-request would get overwritten on the next official release). I will update the scripts that removes the require statements, so that the next version will have the require statements removed. I hope this is sufficient!

@glensc
Copy link
Author

glensc commented May 16, 2015

if you want to keep therequire statement, you must fill include-path in json so the require statement would not give fatal error

@glensc
Copy link
Author

glensc commented May 16, 2015

would you mind publishing your scripts, i would make PR there (maybe not this one, maybe next one)

@deadbeef84
Copy link
Contributor

The scripts are a bit messy, but I will see what I can do!

@formigone
Copy link

Would it be easier to push a version of the code without the require statements in a different branch or tag? That way, those that want the requires gone can just use that instead.

@holtkamp
Copy link

As suggested by @glensc, adding this to the composer.json would do the trick for now:

  "include-path": [
    "vendor/zf1/zend-loader/library"
  ]

Note: the use of include-path is deprecated though!
Note2: for certain adapters that are loaded dynamically, additional paths might be required:

  "include-path": [
    "vendor/zf1/zend-cache/library",
    "vendor/zf1/zend-translate/library",
  ]

Since this initiative is specific to the use of Composer, I would vote for stripping all require_once statements and depend on the (optimized) Composer autoloading mechanism.

@holtkamp
Copy link

@deadbeef84 ZF-1.12.12 has been released, which includes a fix for preventing warnings to be displayed.

When will your scripts / CRON job run to incorporate this? Indeed, publishing these scripts might be a good idea to get some workload off your chest ;)

@holtkamp
Copy link

holtkamp commented Jun 4, 2015

2 week bump, the community would like to help you with maintaining this...

@glensc glensc mentioned this pull request Jun 15, 2015
@glensc
Copy link
Author

glensc commented Jun 15, 2015

i found i needed to disable these two lines in Zend_Application to prevent Zend autoloader kick in and do lots of damage:

//        require_once 'Zend/Loader/Autoloader.php';
//        $this->_autoloader = Zend_Loader_Autoloader::getInstance();

http://stackoverflow.com/a/2080984 suggest to disable autoloader:

Zend_Loader::registerAutoload('Zend_Loader', false);

but that method invokes deprecation message:

PHP Notice:  Zend_Loader::Zend_Loader::registerAutoload is deprecated as of 1.8.0 and will be removed with 2.0.0; use Zend_Loader_Autoloader instead in /home/glen/scm/delfi/elvis-admin/vendor/zf1/zend-loader/library/Zend/Loader.php on line 254

imho the deprecation should be removed as 2.0.0 is different product

@glensc
Copy link
Author

glensc commented Jun 15, 2015

as i looked Zend_Application and Zend_Autoloader classes, and there really is no way to disable the builtin loader without modifying the code, I made such fake class, which overrides constructor doing exactly the same as original class but without the two lines, and not calling parent constructor:

class Fake_Zend_Application extends Zend_Application {
    /**
     * @inheritdoc
     */
    public function __construct($environment, $options = null) {
        $this->_environment = (string)$environment;

        if (null !== $options) {
            if (is_string($options)) {
                $options = $this->_loadConfig($options);
            } elseif ($options instanceof Zend_Config) {
                $options = $options->toArray();
            } elseif (!is_array($options)) {
                throw new Zend_Application_Exception(
                    'Invalid options provided; must be location of config file,'
                    . ' a config object, or an array'
                );
            }

            $this->setOptions($options);
        }
    }
}

$application = new Fake_Zend_Application('myapp', require __DIR__ . '/application/configs/config.php');

the same conclusions are in this stackoverflow thread as well

@glensc glensc force-pushed the patch-1 branch 3 times, most recently from 1184f1d to 6f5979e Compare June 15, 2015 15:38
@holtkamp
Copy link

nice catch @glensc. @deadbeef84, any way to get the newest ZF1 versions in this GitHub Organization, or should it be marked as abandoned? It really helped us to determine which packages of ZF1 our applications depend an, so a migration path could be set. Some way we can help?

@holtkamp
Copy link

@deadbeef84 2 week bump...

@holtkamp
Copy link

@deadbeef84 little bit off-topic, but since I don't know where to ask it otherwise, will the recent version updates in https://github.com/bombayworks/zendframework1 to also bubble down to the separate packages in https://github.com/zf1 ?

@holtkamp
Copy link

@deadbeef84, would you consider bump the isolated ZF1 components to 1.12.16?

@glensc
Copy link
Author

glensc commented Nov 19, 2015

@deadbeef84 ping!

@holtkamp
Copy link

@deadbeef84, would you consider bump the isolated ZF1 components in this GitHub organisation to the release you just tagged 1.12.18?

PS, would it be an idea to share the build script somewhere in a dedicated repository in this GitHub organisation so others can have a look and maybe contribute?

@glensc
Copy link
Author

glensc commented Apr 14, 2016

@deadbeef84 and do not be shamed how ugly your scripts are, community is there to help and improve them!

i had idea that i will redo the scripts from scratch based on description, but never got around of actually doing that :(

glensc referenced this pull request in bombayworks/zendframework1 Dec 19, 2016
@glensc
Copy link
Author

glensc commented Dec 19, 2016

1.12.20 got released meanwhile

@glensc
Copy link
Author

glensc commented Jul 24, 2017

rebased this branch against current master (release-1.12.20)

@holtkamp
Copy link

@glensc @deadbeef84 it seems @unhawkable has forked this project and got a working build / separation process running, see https://github.com/zf1s, maybe the efforts can be combined and keep it an up-to-date approach centrally at https://github.com/zf1 ??

@glensc
Copy link
Author

glensc commented Apr 30, 2019

@deadbeef84 said on May 15, 2018

Sorry guys, I'm no longer involved in the PHP community; also I never intended to maintain this fork apart from mirroring the official library. Since zf1 reached its EOL more than 1½ year ago, I think it's about time to look for other frameworks 😁.

If there's anyone willing to take over and maintain this repo, let me know and I'll consider it.

seems the current maintained version seems to be @zf1s
sad that he didn't answer here, would I had helped for the community...

@glensc
Copy link
Author

glensc commented Apr 30, 2019

however, @zf1s doesn't solve problem described, i've started to maintain own fork:

@falkenhawk
Copy link

@glensc if you submitted a PR to https://github.com/zf1s/zend-application I would be more than glad to merge it...

as zend classes are not in include path, any require once call will cause fatal error
@glensc
Copy link
Author

glensc commented May 30, 2019

wtf. @falkenhawk any ideas how your release notes commit appeared to this pull request?

image

and as the edit is also off, I must have pushed myself?

image

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.

5 participants