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

Remove unused Zend libraries #374

Closed
colinmollenhour opened this issue Nov 16, 2017 · 15 comments
Closed

Remove unused Zend libraries #374

colinmollenhour opened this issue Nov 16, 2017 · 15 comments

Comments

@colinmollenhour
Copy link
Member

As ZF1 is basically a dead project and there is a lot of code in lib/Zend that is not used by Magento and highly unlikely to ever be used by anyone using Magento, and that the code is easily available to those that want it I propose we remove Zend libraries that are not used by Magento core code from the repository. The presence of this code encourages it's use and to some may imply that it is supported code. Since it is neither supported nor it's use encouraged I think it does much more harm than good. Lastly, it is just a bunch of cruft that slows down IDEs, CI, code searching, etc. For example, everything under Zend_Service. These are extremely buggy even when they were supported and largely obsolete.

I propose that each subdirectory directly under lib/Zend be checked to see if it is used in app/code or lib/Varien and if not it should be considered for removal (only not removed if there is a really good reason) and then of course all Magento functionality tested. In addition it would make sense to also remove some subdirectories of libraries that are in use, such as unused adapters in Zend/Db/Adapter.

Reducing the Zend code down to only what is actually used would make it much easier to audit security and search for bugs and such. As of now there is so much code in lib/Zend that really thoroughly inspecting it is impractical.

This does have a potential to break third-party extensions that are dependent on libs that get removed but this can easily be detected and fixed by packaging the needed libs directly with the extension.

@tomekjordan
Copy link

tomekjordan commented Nov 16, 2017

"This does have a potential to break third-party extensions that are dependent on libs that get removed but this can easily be detected and fixed by packaging the needed libs directly with the extension."
yes, by devlopers, not by normal merchant with some IT skills...

I use plenty of extensions and I would like to avoid any problems.... but even don't know "how to easyily detect Zend packages needed by 3rd party extensions"..

hope you understand my point of view

Tomek

@colinmollenhour
Copy link
Member Author

colinmollenhour commented Nov 16, 2017

Sure, I use one in a couple extensions myself so I understand your point of view. In my case it is Zend_Gdata and it has a massive bug that I reported years ago which was never fixed which causes the entire spreadsheet to be downloaded into memory when the spreadsheet metadata is needed (every time a single request is made). This can easily consume over a gig of ram. Another example is the S3 class which only supports the original US east region. It has bugs and uses the old API that will eventually be deprecated and there is an official AWS lib that is well-maintained that should be used instead but people use the Zend one because it is there.

By "easily detect" I meant you will get Fatal errors due to class names that can't be resolved. Restore the missing files and the issue is fixed.

If you really want to avoid the issue using the sledgehammer approach you could just restore an old copy of lib/Zend or copy it into app/code/local/Zend and be done with it, but I don't recommend this.

My point is in short that there is a lot of code to maintain and it is a liability. Removing code that is there only because it was packaged with ZF1 back when it was all (sort of) maintained is prudent, in my opinion. However, it is certainly open for discussion so everyone's opinion is valued.

@infabo
Copy link

infabo commented Nov 17, 2017

hm, I can at least remember having used Zend_Validate in one of my extensions - but replaced it with Respect/Validation at an early refactoring stage ;).

@colinmollenhour
Copy link
Member Author

Zend_Validate is already used by Magento core so would not be a candidate for removal.

@sreichel
Copy link
Contributor

sreichel commented Dec 1, 2017

I like the idea to remove dead code. First place to start code be code/core/Zend. All these classes are loaded instead of lib/Zend ...

@sreichel
Copy link
Contributor

sreichel commented Dec 1, 2017

Another thing that can be deleted is XmlConnect code. It's deactivated by default and depends on some Enterprise_ classes that are not availble in CE.

@akira28
Copy link

akira28 commented Jan 26, 2018

First thing I would do is update zf to the latest version available 1.12.20.
As zf1 is a eoled product, I would then merge the classes from code/core/Zend in lib/Zend. Finally, delete all the unused parts of z1

@Flyingmana
Copy link
Contributor

we could start this by moving all expected to be unused libraries into a legacy_lib directory, so we dont have them in the include path by default anymore, but people who face problems, can easily add them again

@tmotyl
Copy link
Contributor

tmotyl commented Aug 22, 2020

FYI, magento/adobe maintains a fork of zend 1 here:
https://github.com/magento/zf1

Did anybody checked what did they changed there and how does it relate to code/core/zend?

@joshua-bn
Copy link
Contributor

Since they are going to be heavily modified from their original and it really doesn't make any difference between app/code/core and lib, how about moving the entire lib/Zend dir to app/code/core/Zend? At the same time, remove the unused files.

@Flyingmana
Copy link
Contributor

there is a still maintained ZF1 fork, which we will possibly switch to somewhen in the future (when we have other things figured out)
not sure if removing/moving things will make it harder or easier to migrate then.

@sreichel
Copy link
Contributor

sreichel commented Sep 3, 2020

there is a still maintained ZF1 fork

Yep. And this fork removed most of the unused classes. WIP ... ;)

@Flyingmana
Copy link
Contributor

One for example is https://github.com/vicampo/zf1 where iam in contact with someone for over a year

@fballiano
Copy link
Contributor

We'll probably migrate to zf1-future sometimes soon, it's the most sensible choice to move to a well maintained zf1 fork. Probably removing the whole thing from our repo and have it only as a composer dependancy. There are multiple issues about that so I'm closing this one that didn't get any feedback in almost 2 years. Anybody welcome to reopen it in case new ideas come up, but please check the other existing issuer/prs

@sreichel sreichel reopened this Oct 8, 2022
@sreichel
Copy link
Contributor

Closed with #2827

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants