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

Support for zendframework/zf1-extras #128

Open
fredericgboutin-yapla opened this issue Sep 20, 2022 · 10 comments
Open

Support for zendframework/zf1-extras #128

fredericgboutin-yapla opened this issue Sep 20, 2022 · 10 comments

Comments

@fredericgboutin-yapla
Copy link
Contributor

Hi,
I'm digging my way around and I'm struggling to find any info in regards to support for "zf1-extras" or "ZendX" components like JQuery - https://framework.zend.com/manual/1.12/en/zendx.jquery.html

I found this fork - https://github.com/Shardj/zf1-extras-future - which seems "popular" so I suppose I could use it in conjunction with zf1s but I would like to hear your input on the whole matter.

Thanks!

@fredericgboutin-yapla
Copy link
Contributor Author

fredericgboutin-yapla commented Oct 3, 2022

Well, it's been 2 weeks and I've got no feedback.

Anyway, I went ahead and I tried to use zf1s with zf1-extras and there are 2 problems,

  1. There are a lot of require_once in zf1-extras codebase which don't resolve with zf1s
  2. zf1s is NOT a drop-in replacement for zf1, as it does NOT / cannot inherently specify in the composer.json a definition like,
    "replace": {
        "zendframework/zendframework1": ">=1.12.20"
    }

All this means that we would need to fork zf1-extras, remove the require_once statements, adjust the code to make it work including the unit tests, put those tests under CI for php compatibility review, etc.

Basically when someone uses zf1s it alienates itself from using other components which could provide features to zf1, for example,

@falkenhawk
Copy link
Member

falkenhawk commented Oct 5, 2022

@fredericgboutin-yapla now full zf1s/zf1 package is available. https://packagist.org/packages/zf1s/zf1 v1.15.0 - please try.

@fredericgboutin-yapla
Copy link
Contributor Author

fredericgboutin-yapla commented Oct 6, 2022

I'm trying right now and I stumbled on something interesting,

composer require zf1s/zf1:^1.15.0 shardj/zf1-extras-future

PHP 7.0.33 (cli) (built: Dec 29 2018 06:50:58) ( NTS )
                                                                                                              
  [InvalidArgumentException]                                                                                  
  Package zf1s/zf1 has requirements incompatible with your PHP version, PHP extensions and Composer version:  
    - zf1s/zf1 1.15.0 requires ext-ldap * but it is not present.                                              
                                                                   

So, there is a requirement for the LDAP extension in the monorepo composer.json file,

"ext-ldap": "*",

Which makes sense since there is an LDAP module with the same requirement 👍 - https://github.com/zf1s/zend-ldap/blob/6569d32d80e162424c33450e0d9135a74af5212a/composer.json#L9

The problem is, we don't use LDAP so, as a work around I must modify my composer.json by adding,

  "config": {
    "platform": {
      "ext-ldap": "1"
    }
  }

I could have also called composer with --ignore-platform-req=ext-ldap but that's cumbersome.

Anyway, I retried the initial composer require and this time I got a 2 warnings when generating the autoload files,

Class Zend_Tool_Framework_Provider_DocblockManifestInterface located in ./vendor/zf1s/zf1/packages/zend-tool/library/Zend/Tool/Framework/Provider/DocblockManifestable.php does not comply with psr-0 autoloading standard. Skipping.

Class Zend_Gdata_Analytics_Goal located in ./vendor/zf1s/zf1/packages/zend-gdata/library/Zend/Gdata/Analytics/Extension/Goal.php does not comply with psr-0 autoloading standard. Skipping.

Not a big deal but still probably something to iron out.

I confirm that the original zendframework/zendframework1 package was NOT downloaded nor installed in the vendor folder. The lock file also confirms that the new replace statement introduced in 1.15.0 works as expected.

From my perspective, I can say it works.

Now, like you predicted / as expected the require_once statements in Zend Extras don't work, for example

Warning: require_once(Zend/Json.php): failed to open stream: No such file or directory in /var/www/html/vendor/shardj/zf1-extras-future/library/ZendX/JQuery.php on line 26

@falkenhawk
Copy link
Member

Thank you for sharing your findings @fredericgboutin-yapla !

Those two classes Zend_Tool_Framework_Provider_DocblockManifestInterface and Zend_Gdata_Analytics_Goal deserve a fix to rename/move their files so that composer autoloader does not complain.

As for php extensions listed in require section.. you might be right, it is probably too restrictive at the moment. Due to the nature of zf1, you may never need some of the extensions, as long as you do not use components which require them. But when you start using them, those components themselves should throw errors that required extensions are not installed.

@glensc did great job in #6, filling the requirements for each component individually. Then right after that PR got merged, I went ahead and added them to the main composer.json in fdf41f3, without a PR, but pushing directly to master 🙈 Why, I don't remember anymore.

I suppose they could be added to suggest section instead, or dropped entirely from the main composer.json 🤔

@glensc
Copy link
Contributor

glensc commented Oct 7, 2022

if you remove extension require from root composer.json then our CI can't validate what extensions are required. otoh, the requirements will likely never change and they already are present in CI configs. but if the base image changes extensions included, our jobs have no way to validate what are required. and some tests are just silently skipped if extension is missing and we might not notice we are skipping some tests.

so, mixed feelings here again :)

but of course we could do some hacking like transform all suggests to require in our ci job. something similar done here:

@glensc
Copy link
Contributor

glensc commented Oct 7, 2022

ok. moving to require-dev is better, I didn't think of that:

@fredericgboutin-yapla
Copy link
Contributor Author

fredericgboutin-yapla commented Oct 7, 2022

As for php extensions listed in require section.. you might be right, it is probably too restrictive at the moment. Due to the nature of zf1, you may never need some of the extensions,

To be fair, I personally think it is alright. The workaround I came with is good because I had to explicitly acknowledge the requirement and deal with it somehow. I basically like being told of extension requirements.

But yeah, if I had to deal with half a dozen... and figuring transitive dependencies between different Zend modules to understand what I'm really entitled to... but still, I think it is probably a fair drawback of using the monorepo at the first place - i.e. by default you are required to fulfill all the extension requirements.

@fredericgboutin-yapla
Copy link
Contributor Author

fredericgboutin-yapla commented Oct 19, 2022

@falkenhawk I had to bootstrap zfdebug from https://github.com/jokkedk/ZFDebug with zf1s and I think I basically found something interesting. I truly had to understand what require_once is,

Files are included based on the file path given or, if none is given, the include_path specified. If the file isn't found in the include_path, include will finally check in the calling script's own directory and the current working directory before failing.

https://www.php.net/manual/en/function.include.php

Since I don't want / I'm unable to modify all the require_once 'Zend/[...]'; out there I decided to brute force things with a bunch of include-path in my composer.json - https://getcomposer.org/doc/04-schema.md#include-path

  "include-path": [
    "vendor/zf1s/zend-acl/library",
    "vendor/zf1s/zend-application/library",
    "vendor/zf1s/zend-auth/library",
    "vendor/zf1s/zend-cache/library",
    "vendor/zf1s/zend-config/library",
    "vendor/zf1s/zend-console-getopt/library",
    "vendor/zf1s/zend-controller/library",
    "vendor/zf1s/zend-crypt/library",
    "vendor/zf1s/zend-date/library",
    "vendor/zf1s/zend-db/library",
    "vendor/zf1s/zend-dom/library",
    "vendor/zf1s/zend-exception/library",
    "vendor/zf1s/zend-file-transfer/library",
    "vendor/zf1s/zend-filter/library",
    "vendor/zf1s/zend-form/library",
    "vendor/zf1s/zend-json/library",
    "vendor/zf1s/zend-layout/library",
    "vendor/zf1s/zend-loader/library",
    "vendor/zf1s/zend-locale/library",
    "vendor/zf1s/zend-log/library",
    "vendor/zf1s/zend-measure/library",
    "vendor/zf1s/zend-navigation/library",
    "vendor/zf1s/zend-paginator/library",
    "vendor/zf1s/zend-registry/library",
    "vendor/zf1s/zend-server/library",
    "vendor/zf1s/zend-session/library",
    "vendor/zf1s/zend-test/library",
    "vendor/zf1s/zend-translate/library",
    "vendor/zf1s/zend-uri/library",
    "vendor/zf1s/zend-validate/library",
    "vendor/zf1s/zend-version/library",
    "vendor/zf1s/zend-view/library",
    "vendor/zf1s/zend-xml/library"
  ],

And it works 🥳 but it's ugly 💩

In fact, the original Zend composer definition had an include-path statement - https://github.com/zendframework/zf1/blob/master/composer.json#L19

So, my hypothesis here is to simply provide an equivalent definition, both for the monorepo (which I don't use in my example) and every single modules.

UPDATE: I tried with a wildcard but Composer doesn't support it seemingly.

@falkenhawk
Copy link
Member

Yes @fredericgboutin-yapla that's exactly what you need to do if you need to combine zf1s with zf1 extensions which still utilize require_once... and yes it is ugly and potentially may affect general php performance regarding file lookups (although I am not sure about that, only guessing) so I would not want it to be added to the main composer.json of zf1s so that it affects everyone.

It's a matter of finding a compromise - either forking/patching old codebase for zf1 extensions, or adding include-path for all components in your project. Or switching to zf1-future, diablomedia, etc. forks :)

Again, thanks for sharing!

@glensc
Copy link
Contributor

glensc commented Oct 20, 2022

But perhaps worth adding this to readme, no performance impact there ;)

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

No branches or pull requests

3 participants