Skip to content
This repository has been archived by the owner on Jun 10, 2019. It is now read-only.

Patch contribution #20

Closed
DimaSoroka opened this issue Feb 15, 2016 · 22 comments
Closed

Patch contribution #20

DimaSoroka opened this issue Feb 15, 2016 · 22 comments

Comments

@DimaSoroka
Copy link

Hello

I'm just wondering if you guys tried to contribute patch to Magento to get official support of PHP7 for M1?

Thanks

@icurdinj
Copy link
Contributor

Not yet, but that is a good idea. It would be quite small diff, if they are interested.

@daanggc
Copy link

daanggc commented Feb 16, 2016

Or make a module/package for it. (You may even add it to Magento Connect)
It is a bit early in the live cycle of PHP7 and a maybe a bit late in the live cycle of Magento 1, but at this moment, PHP7 support (using this awesome module) lives on a developer level.

When PHP7 becomes widely available (cPanel), there is going to be a need for a more friendly way to add this to an installation.
On the other hand, I completely understand if you would like to keep this on a developers level ;)

@icurdinj
Copy link
Contributor

All options are open. Our only motive here is to have M1 reliably running on PHP7.
Magento Connect module would probably require user level support, so, yeah, it is more comfortable to stay on developers level, but it's not out of the question.

@DimaSoroka
Copy link
Author

@piotrekkaminski will you be interested in such contribution?

@durzel
Copy link
Contributor

durzel commented Feb 16, 2016

It would be fairly trivial to contribute a package.xml file to allow a master zip file to be installed as a Magento Connect module. That being said this isn't the sort of thing you'd ever want to switch off (since the site will stop working), so the utility of a module is largely redundant. If you ever did switch it off inside the backend the site would cease to function.

I ended up using modman to deploy this module, having not previously used it, but to be honest I would've been just as happy directly copying the files in - given how essential this module is to Magento 1.x working at all on PHP 7.x

@wienczny
Copy link

Keep in mind that the Magento module install/updates don't work when running on PHP7 because they have version constraints.

@WesMage
Copy link

WesMage commented Feb 29, 2016

@wienczny Do you mean that if I upgrade my M1 to PHP7, I will no longer be able to use the Magento Connect Manager to install and update free extensions?

@daanggc
Copy link

daanggc commented Feb 29, 2016

@WesMage He means this required option when packaging an extention:
screenshot 2016-02-29 08 04 51

So basically, all modules that have not something like "PHP 7 support" in the changelog or any module that isn't updated in the last couple of months, will have this Maximum [PHP Version] field set to PHP 5.6.
Therefore a module install/update will not run in most cases. (When in PHP 7 mode)

@durzel
Copy link
Contributor

durzel commented Feb 29, 2016

It's worth noting that already installed modules will survive an upgrade to PHP 7.x, but as said attempting to upgrade modules through Magento Connect might fail due to the above version check.

I would strongly recommend attempting module updates on a dev site that is also running PHP 7.x first, because if the module you try and upgrade has the min/max PHP version constraint you will be left without the module installed at all (the upgrade process removes the old version first).

Magento Connect otherwise works exactly the same with PHP 7.x as it does with PHP 5.x

@shirtsofholl
Copy link

@piotrekkaminski +1 for core update

@jdchmiel
Copy link

conversely, what about an actual diff that you use a composer script to apply?

@Sam-PUMP
Copy link

Is there a way to remove the php constraint in Magento Connect Manager?
M1 works perfectly with PHP7 with your patch and it solved all the errors I was getting but yeah, updating or installing anything via Magento Connect Manager is the one thing the extension didn't make compatible with PHP 7. Thanks!

@icurdinj
Copy link
Contributor

icurdinj commented May 3, 2016

I guess connect manager could be hacked not to check the constraints, but that is not a solution - constraints are there for a reason. What's needed is more awareness of PHP 7 from extension makers - they should check their code and update the constraints.

@durzel
Copy link
Contributor

durzel commented May 3, 2016

Each extension's package.xml will define the PHP constraints (or not - they aren't required), and there will be good reasons to preserve this sanity check. As @icurdinj said above - the extension makers will need to become better aware that people are using PHP 7.

The safest course of action is to install an extension/update on a dev environment also running PHP 7, that way if it fails you won't be adversely affected.

@bjoern-tantau
Copy link

I'm currently checking the changes in Magento 1.9.3 and so far it looks like all of the changes from here were implemented. You might want to update the Readme accordingly.

@durzel
Copy link
Contributor

durzel commented Oct 13, 2016

Can you clarify.. do you mean that Magento incorporated the fixes in this extension, thereby making Magento 1.x PHP 7 compatible? If so this is a pretty significant change...

@bjoern-tantau
Copy link

No, not all fixes. The Model rewrites are all fixed. But I'm currently looking through the overwrites in Mage and at least Mage_Connect_Packager still is not fully PHP 7 compatible.

@icurdinj
Copy link
Contributor

I haven't had time yet to check it, but if that's the case, we'll probably make a new branch for this extension where we'll leave only the fixes still needed for 1.9.3.

@bjoern-tantau
Copy link

All right, I'm done with my check. The only changes remaining are

  • Inchoo_PHP7_Helper_Data (though I don't quite get what the change has to do with PHP 7 compatibility)
  • Mage_Connect_Packager
  • Mage_Core_Model_Resource_Session

All the model rewrites and the sales/quote/totals settings are not needed anymore.

@icurdinj
Copy link
Contributor

I've just checked a site with 1.9.2.4 core and SUPEE-8788 patch applied. Not a thing was fixed by patch, everything is still needed from this extension. Next thing I'll check - actual 1.9.3 core.

@icurdinj
Copy link
Contributor

1.9.3 core does fix some of the compatibility, but not all. I've just released 2.0 version of this extension, which leaves only the fixes needed for M CE 1.9.3.

@icurdinj
Copy link
Contributor

Because this thread went off the theme, and I feel like closing issues today, I'll just conclude with this - we contacted Magento through different channels and offered to contribute PHP 7 compatibility patches. The answers were along the line "definitely maybe once in the future". Anyway, the extension is here, free, and working fine.

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

No branches or pull requests

10 participants