Moved phpseclib, mcrypt_compat, Cm_RedisSession, Cm_Cache_Backend_Redis and Pelago_Emogrifier to composer

[fballiano]

This PR is the start of the migration of 3rd party modules to composer dependencies.
This PR continues from #2138 which I couldn't rebase from 20.0.

Notes:

• Zend Framework is left out because it needs a dedicated tasks
• Once we merge this I'll work on the release builder to create a release tarball file for each supported PHP version

justinbeaty

[kiatng]

There is always this check if (function_exists('idn_to_ascii')) { before Net_IDNA2 is used:

magento-lts/app/code/core/Mage/Core/Helper/Url.php

Lines 174 to 179 in e6f0851

	if (function_exists('idn_to_ascii')) {
	$host = idn_to_ascii($parsedUrl['host']);
	} else {
	$idn = new Net_IDNA2();
	$host = $idn->encode($parsedUrl['host']);
	}

also if (function_exists('idn_to_utf8')) {:

magento-lts/app/code/core/Mage/Core/Helper/Url.php

Lines 196 to 201 in e6f0851

	if (function_exists('idn_to_utf8')) {
	$host = idn_to_utf8($parsedUrl['host']);
	} else {
	$idn = new Net_IDNA2();
	$host = $idn->decode($parsedUrl['host']);
	}

When I checked the PHP doc, idn_to_ascii and 'idn_to_utf8 are functions from PHP intl extension. So, if we make intl as required, can we remove Net_IDNA2 entirely?

[fballiano]

@kiatng wow this is a great find, I'm checking it out more in detail now :-)

[fballiano]

I've removed Net_IDNA and switched to a proper polyfill which is allowing us to not have a "if funcion exists" in our code, I kinda prefer this solution, I hope you too :-)

[justinbeaty]

@fballiano nice work! The polyfill definitely seems too big to just add to functions.php so I like it being separate (and not maintained by us).

I think it will however make the release builder more complicated. But if we can figure out a way to bundle any composer package in lib we could unlock a lot of possibilities like the php8 polyfill package or also removing a lot of our own polyfills from this repo.

[fballiano]

mmm I think there's no way to move everything in lib because of dependancies, but if we have the vendor folder in the release it should be solved.

I know... it will make the release bigger but... i don't know any other way...

maybe set the composer install dir as "lib"? mmmmmmmmmmm seems dangerous

[justinbeaty]

@fballiano Bundling vendor would be okay IMO. I basically do the same thing for the sites I manage. I only use composer locally and then build it by copying vendor to htdocs. Then I just deploy that to live and it all works without composer being installed on the server.

There’s one issue I don’t like about it though, Magento source code is duplicated there as well as the code for all modules. I have it on my roadmap to eventually see if it’s possible to avoid that.

[justinbeaty]

Maybe this could help: https://github.com/liborm85/composer-vendor-cleaner

Not sure if it could remove the entire openmage/magento-lts dir in vendor, but maybe that can just be rm -rf’d

My plans were to always do something like loop through all packages and remove any that had magento-source or magento-module in their composer.json

[fballiano]

yes i totally agree that it's really not nice to have the duplicated magento code, i think the composer plugin (i'll check it out asap) is a really good idea, not only for the production servers but also for local environment (like when I try to search on phpstorm i get duplicated results because of the vendor folder).

I'll take a look at it asap, thanks so much!

[fballiano]

actually it seems OM is not in the vendor folder here

it would be only in a "final" project, but not here

[justinbeaty]

Ah, you're right since this repo doesn't require itself. But for example the colinmollenhour/magento-redis-session directory could go. Anything with "type":"magento-module", in its composer.json file should be safe to remove I think.

[fballiano]

Maybe this could help: https://github.com/liborm85/composer-vendor-cleaner

it seems this plugin runs BEFORE deploy, so we can't use it otherwise the deployment of modules brakes :-(

[sreichel]

As composer user i'm happy with it, but is release builder ready to merge this one for next release?

(Discussion? Composer as requirement?)

[fballiano]

As composer user i'm happy with it, but is release builder ready to merge this one for next release?

(Discussion? Composer as requirement?)

ah ok ok, I would have worked on that right after merging this one, resuming the other "release builder workflow" PR :-)

[fballiano]

Fixed conflict regenarating composer.lock

will we ever merge this?

[sreichel]

will we ever merge this?

I hope so. Maybe its better to split that PR and start with one replacement? (Cm_Cache?)

[sreichel]

@fballiano what is needed to get it merged? (I'd like to help, but how?)

[fballiano]

@sreichel I think it's completed, we just will need a release building which is here: #2165 but I'll work on that when this one will be merged ;-)

[fballiano]

wow, let's go, time has come 🤞🤞🤞