Replace lib/Zend with shardj/zf1-future 🚀 #2827

sreichel · 2022-12-20T00:57:45Z

Description (*)

This PR replaces lib/Zend with shardj/zf1-future.

All changes made to lib/Zend or app/code/core/Zend have been replaced with patches. There are only two files left in app/code/core/Zend that needs reviews.

/app/code/core/Zend/Db/Select.php
/app/code/core/Zend/Db/Statement.php

Applied patches: #2787 (comment) & #2787 (comment)

Related Pull Requests

Closes Replaced bundled Zend Framework with zf1-future #2787

Comments

update README ... how add patches, special help for mac aliens 👽

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All automated tests passed successfully (all builds are green)
Add yourself to contributors list

fballiano · 2022-12-20T09:35:22Z

I'd have loved to modify my PR instead of a new one since I held this topic very dear.

Anyway it is an amazing job, I only have a question, do you like the "patches" folder in the root folder? wouldn't it be possible to hide it somewhere else? like in a ".patches" folder or something like that? It's a very minor thing but now files/folders in root make me itchy :-D

ADDISON74 · 2022-12-20T10:01:47Z

Thank you Sven for your work, an important step in OM future.

Please let me know what files will remain in the /app/code/core (I did not implement locally this PR). In my test only a few files created trouble after deleting, especially those related to database operations.

fballiano · 2022-12-20T15:46:27Z

conflict with the baseline (isn't this a new thing)

fballiano · 2022-12-20T15:53:43Z

applying the patches doesn't seem to work correctly, using -vvv I was able to see this


Executing command (CWD): patch '-p1' --no-backup-if-mismatch -d '/Users/fab/Projects/openmage/vendor/shardj/zf1-future' < '/Users/fab/Projects/openmage/patches/MAG-1.1.1.patch'
patching file 'library/Zend/Date.php'
Patch creates file that already exists!  Assume -R? [y]

fballiano · 2022-12-20T15:54:05Z

I would still move the patches directory to another place if possible

sreichel · 2022-12-20T18:08:38Z

l have a question, do you like the "patches" folder in the root folder?

~~".patches" would be better.~~ I'll test it later.

Please let me know what files will remain in the /app/code/core

Its /app/code/core/Zend/Db/Select.php and Statement.php

applying the patches doesn't seem to work correctly, using -vvv I was able to see this

This is strange. It works for me ...

patch '-p1' --no-backup-if-mismatch -d '/var/www/html/vendor/shardj/zf1-future' < '/var/www/html/patches/MAG-1.1.1.patch'
patching file library/Zend/Date.php

Hunk #1 succeeded at 1827 (offset -10 lines).
patching file library/Zend/Mime.php

conflict with the baseline (isn't this a new thing)

Guess it comes from #2823. And we made some changest to Zend files too.

fballiano · 2022-12-20T18:11:38Z

This is strange. It works for me ...

I'm using php7.3 and composer 2.4.4, removed vendor completely before running the install

sreichel · 2022-12-20T18:17:02Z

PHP7.3 and composer 2.4.2.

I think the problem is that Date.php is patched three times causing Hunk #1 succeeded at 1827 (offset -10 lines). for me and for some reason it fails for you. I made the ZF-299 before MAG-1.1., so lines numbers could have changed .... i'll test it later.

sreichel · 2022-12-20T18:23:07Z

@fballiano can you do a quick test please ...

MAG-1.1.1 change lines to @@ -1827,6 +1827,16 @@
ZF-299: change lines (2nd part) to @@ -4796,5 +4806,46 @@

fballiano · 2022-12-20T18:26:16Z

I modified patches/MAG-1.1.1.patch to

--- /dev/null
+++ ../library/Zend/Date.php
@@ -1827,6 +1827,16 @@

                 require_once 'Zend/Date/Exception.php';
                 throw new Zend_Date_Exception("invalid date ($date) operand, hour expected", 0, null, $date);
+                break;
+
+            case self::HOUR_SHORT:
+                if (is_numeric($date)) {
+                    return $this->_assign($calc, $this->mktime((int)$date, 0, 0, 1, 1, 1970, true),
+                                                 $this->mktime($hour,         0, 0, 1, 1, 1970, true), false);
+                }
+
+                require_once 'Zend/Date/Exception.php';
+                throw new Zend_Date_Exception("invalid date ($date) operand, hour expected", 0, null, $date);

             case self::HOUR_AM:
                 if (is_numeric($date)) {

--- /dev/null
+++ ../library/Zend/Mime.php
@@ -38,7 +38,7 @@
     const ENCODING_BASE64          = 'base64';
     const DISPOSITION_ATTACHMENT   = 'attachment';
     const DISPOSITION_INLINE       = 'inline';
-    const LINELENGTH               = 72;
+    const LINELENGTH               = 200;
     const LINEEND                  = "\n";
     const MULTIPART_ALTERNATIVE    = 'multipart/alternative';
     const MULTIPART_MIXED          = 'multipart/mixed';

but it seems not to work:

  - Applying patches for shardj/zf1-future
    patches/MAG-1.1.1.patch (MAG-1.1.1)
patch '-p1' --no-backup-if-mismatch -d '/Users/fab/Projects/openmage/vendor/shardj/zf1-future' < '/Users/fab/Projects/openmage/patches/MAG-1.1.1.patch'
Executing command (CWD): patch '-p1' --no-backup-if-mismatch -d '/Users/fab/Projects/openmage/vendor/shardj/zf1-future' < '/Users/fab/Projects/openmage/patches/MAG-1.1.1.patch'
patching file 'library/Zend/Date.php'
Patch creates file that already exists!  Assume -R? [y]

fballiano · 2022-12-22T09:19:57Z

Y

😂

ADDISON74

Let's take this important step. There may still be issues but we can solve them as soon as they appear.

fballiano · 2022-12-22T09:42:23Z

ouuu nouuuu we've a conflict with the freaking baseline, I was that excited to hit the "merge" button!

ADDISON74 · 2022-12-22T09:49:12Z

There is another one that I approved in the last hour with the same problem.

# Conflicts: # phpstan.dist.baseline.neon

fballiano · 2022-12-22T20:15:02Z

OMG I can't believe we did it 😍

ADDISON74 · 2022-12-22T20:21:39Z

Unfortunately, OM 20 is still waiting behind version 19, with a gap of many commits. We should reverse the priority from 2023 and this version 20 should receive all the attention. OM 19 also goes ahead, but it will only collect certain commits from the default version. When I clone this repository to have the possibility to get an OM 20 up to date, not to wait for it to be updated from time to time. I think that it would make the work easier for the maintainers as well.

fballiano · 2022-12-22T20:26:14Z

as I said many times, we should stop working on v19 and work only on v20. have a single default branch which is v20 and that's it.

v19 should receive only security updates if extremely necessary.

we do not have the manpower to maintain 2 branches and develop something that's also interesting and fun for us to do (which is also very important).

ADDISON74 · 2022-12-22T20:33:40Z

I support your point of view. v19 must pick from v20 not the other way around. the decision must be taken as soon as possible.

Related to this PR all that remains is to intensively test the new ZF-1 version with OM. I did not get any issues after 2 weeks. It is incredible to see now only two files in /app/code/core/Zend and a brand new /lib/Zend with all updates inside. If I remember in my tests I had to keep those two files because OM stopped working if I renamed them. Indeed it's really a huge update for this project.

There is nothing to say more. Many thanks to everyone involved and happy holidays. This year there have been many changes like PHP 8.1, ZF1-Future and many others. We still have problems with the management of reported issues and PRs, there are too many, but if we compare with what it was at the beginning of the year I can say that we are on the right track.

sreichel · 2022-12-22T20:43:00Z

I support your point of view. v19 must pick from v20 not the other way around.

I still prefer v19 over v20. What are the major changes?

removing frontend templates ... okay lets add them as optional composer repo (in progress)
dropping IE support ... YES. (not if that can be done in one branch)
BC breaking changes ... it would be possible add a config switch for some feature
changes to core methods, like formatDate() ... something i'd revert ..

However ... let not dicuss this here.

ADDISON74 · 2022-12-22T20:59:34Z

The directory /lib/Zend no longer exists in our repository in any branch. Do you think that for those who do not use Composer we should inform them in the Readme that they can download the archive from ZF1-Future and add it themselves to /lib/Zend? If they don't do this they won't run OM anymore.

fballiano · 2022-12-22T21:01:18Z

The directory /lib/Zend no longer exists in our repository in any branch. Do you think that for those who do not use Composer we should inform them in the Readme that they can download the archive from ZF1-Future and add it themselves to /lib/Zend? If they don't do this they won't run OM anymore.

we've to inform them that they have to remove lib/Zend, the release builder will already have the right components in the vendor folder ;-)

fballiano · 2022-12-22T21:03:43Z

and I'd also bump the version to 20.1.0 and 19.5.0 at least, since this change is pretty massive

sreichel · 2022-12-22T21:13:16Z

README should be updated.

Have have to tell how to make changes to vendor/zf1f (adding patches) and about change requirements. E.g. patch command is installed nearly everywhere, but on an outdated host it can be missing or be not up to date, but its requierd to apply patches.

fballiano · 2022-12-22T21:17:26Z

I'll do that ASAP

sreichel · 2023-01-09T22:05:32Z

Nice. My PRs got merged, so we can removes some patch files with next release :)

sreichel added 5 commits December 20, 2022 00:42

Removed files from lib/Zend & code/core/Zend

f4193f4

Added shardj/zf1-future 1.21.4

caf630d

Added patches

eda00c7

Removed diffs to zf1

6934225

Updated phpstan.dist.baseline.neon

a9ab12c

github-actions bot added Component: lib/Zend Component: lib/* Relates to lib/* composer Relates to composer.json labels Dec 20, 2022

Updated composer.lock

96004ba

sreichel requested review from fballiano, tmotyl, elidrissidev, ADDISON74, colinmollenhour, Flyingmana and luigifab and removed request for fballiano and tmotyl December 20, 2022 02:04

fballiano previously approved these changes Dec 22, 2022

View reviewed changes

ADDISON74 previously approved these changes Dec 22, 2022

View reviewed changes

Merge branch '1.9.4.x' into remove-lib-zend

b9e2828

# Conflicts: # phpstan.dist.baseline.neon

sreichel dismissed stale reviews from ADDISON74 and fballiano via b9e2828 December 22, 2022 17:50

ADDISON74 approved these changes Dec 22, 2022

View reviewed changes

fballiano approved these changes Dec 22, 2022

View reviewed changes

fballiano merged commit 622681a into OpenMage:1.9.4.x Dec 22, 2022

sreichel deleted the remove-lib-zend branch December 22, 2022 20:25

sreichel mentioned this pull request Dec 22, 2022

Sonar: fixed path to lib/Zend #2834

Merged

4 tasks

fballiano mentioned this pull request Dec 23, 2022

Version bump #2835

Merged

This was referenced Dec 24, 2022

Remove unused Zend libraries #374

Closed

Remove files from lib/Zend that exist in Magentos core/Zend #1162

Closed

fballiano mentioned this pull request Dec 25, 2022

Added notes about composer library/modules to README (for 19.5.x and 20.1.x) #2851

Merged

elidrissidev mentioned this pull request Mar 30, 2023

Invalid report date #3122

Closed

sreichel mentioned this pull request Mar 31, 2023

Fixed monthly report dates (date was 1970) #3126

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace lib/Zend with shardj/zf1-future 🚀 #2827

Replace lib/Zend with shardj/zf1-future 🚀 #2827

sreichel commented Dec 20, 2022 •

edited

Loading

fballiano commented Dec 20, 2022

ADDISON74 commented Dec 20, 2022

fballiano commented Dec 20, 2022

fballiano commented Dec 20, 2022

fballiano commented Dec 20, 2022

sreichel commented Dec 20, 2022 •

edited

Loading

fballiano commented Dec 20, 2022

sreichel commented Dec 20, 2022

sreichel commented Dec 20, 2022

fballiano commented Dec 20, 2022

fballiano commented Dec 22, 2022

ADDISON74 left a comment

fballiano commented Dec 22, 2022

ADDISON74 commented Dec 22, 2022

fballiano commented Dec 22, 2022

ADDISON74 commented Dec 22, 2022

fballiano commented Dec 22, 2022 •

edited

Loading

ADDISON74 commented Dec 22, 2022

sreichel commented Dec 22, 2022

ADDISON74 commented Dec 22, 2022

fballiano commented Dec 22, 2022

fballiano commented Dec 22, 2022

sreichel commented Dec 22, 2022

fballiano commented Dec 22, 2022

sreichel commented Jan 9, 2023

Replace lib/Zend with shardj/zf1-future 🚀 #2827

Replace lib/Zend with shardj/zf1-future 🚀 #2827

Conversation

sreichel commented Dec 20, 2022 • edited Loading

Description (*)

Related Pull Requests

Comments

Contribution checklist (*)

fballiano commented Dec 20, 2022

ADDISON74 commented Dec 20, 2022

fballiano commented Dec 20, 2022

fballiano commented Dec 20, 2022

fballiano commented Dec 20, 2022

sreichel commented Dec 20, 2022 • edited Loading

fballiano commented Dec 20, 2022

sreichel commented Dec 20, 2022

sreichel commented Dec 20, 2022

fballiano commented Dec 20, 2022

fballiano commented Dec 22, 2022

ADDISON74 left a comment

Choose a reason for hiding this comment

fballiano commented Dec 22, 2022

ADDISON74 commented Dec 22, 2022

fballiano commented Dec 22, 2022

ADDISON74 commented Dec 22, 2022

fballiano commented Dec 22, 2022 • edited Loading

ADDISON74 commented Dec 22, 2022

sreichel commented Dec 22, 2022

ADDISON74 commented Dec 22, 2022

fballiano commented Dec 22, 2022

fballiano commented Dec 22, 2022

sreichel commented Dec 22, 2022

fballiano commented Dec 22, 2022

sreichel commented Jan 9, 2023

sreichel commented Dec 20, 2022 •

edited

Loading

sreichel commented Dec 20, 2022 •

edited

Loading

fballiano commented Dec 22, 2022 •

edited

Loading