search

zf1s / zf1

Monorepo of a fork of Zend Framework 1, with all components split into individual composer packages. PHP 5.3-8.3 compatible.
BSD 3-Clause "New" or "Revised" License
60 stars 22 forks source link

Support for zendframework/zf1-extras #128

Open fredericgboutin-yapla opened 2 years ago

fredericgboutin-yapla commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

fredericgboutin-yapla commented 2 years ago

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, https://github.com/zf1s/zf1/blob/be1785d1a1bcd65fb815ad0b48d5f2baca1a37c5/composer.json#L11

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 commented 2 years ago

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 https://github.com/zf1s/zf1/pull/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 https://github.com/zf1s/zf1/commit/fdf41f38e332f5879341d7312715785962b27ef3, 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 commented 2 years ago

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 commented 2 years ago

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

fredericgboutin-yapla commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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