-
-
Notifications
You must be signed in to change notification settings - Fork 8
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
Removed obsolete install scripts #65
Removed obsolete install scripts #65
Conversation
Ah - let me add copyright lines to the modified files. There were a lot of incorrect |
wooooow this is amazing!! can I do a test and merge it or you need more time? |
When I started working on this I had no idea this many files were obsolete, I was shocked as well!
It's ready to be merged once you confirm everything. I tested the web installer, CLI installer, and sample data. |
I'm in the middle of a big javascript thingy, when I manage to close it I'll test this right away |
Great, and of course I didn’t mean you should replicate the whole process with the PHP scripts etc, just maybe test the pre / post sql dumps if you want to confirm. |
Also I was curious to see how much total code we've removed so far: $ sloccount maho/app
Totals grouped by language (dominant language first):
php: 304903 (84.29%)
xml: 56843 (15.71%)
$ sloccount magento-mirror/app
Totals grouped by language (dominant language first):
php: 393254 (85.56%)
xml: 66357 (14.44%) So nearly -100,000 "Physical Source Lines of Code" (SLOC) removed from |
so, I tested this PR comparing 2 dumps (generated by installing Maho from main and from this PR) and the only differences I see are in the "created_at" times |
I've also run the debug-install-scripts.php (on main branch), modified all the install files and check the results after installing. I couldn't find any of the files that are deleted by this PR. So I think we can merge this right @justinbeaty? |
@fballiano I tested tons too and everything was good. Let's merge it! |
amazing 🥹 |
AHHAHAH that's the perfect GIF |
Magento does database upgrades by first looking for the highest
install-X.X.X.php
file, then applies anyupgrade-X.X.X-Y.Y.Y.php
files. So for example given these files, none of the1.5.X.0
scripts are ever executed:With the Magento 1.6.0.0 release, the mage devs included a new
install-1.6.0.0.php
script for basically every module so all the old scripts are obsolete for anyone installing a version >= 1.6.0.0.This PR deletes 611 out of the 786 install scripts and over 61,000 lines of code! Only 175 install scripts remain.
To figure this out, I wrote a few quick scripts and did the following process:
debug-install-scripts.php
which adds the following line to the top of every install / upgrade script.Create a fresh database and run the install process. Any file that was executed will write to
/tmp/install-scripts.php
.Edit
/tmp/install-scripts.php
to just make it valid PHP syntax, i.e. add:Run
git checkout app
to undo all of thefile_put_contents
lines.Run
delete-install-scripts.php
which will again loop through all install / upgrade scripts and delete any not found in the array.Run
update-install-scripts.php
which will update all of the/** @var Class_Name $this */
docblocks with the correct class.Manually fix two of the files from step 6 where the find/replace broke.
Since all of the files that were deleted weren't executed during an install (else they would have written to
/tmp/install-scripts.php
), there should be no difference with them gone, but I still verified with the following steps:diff --color=always old-schema.sql new-schema.sql git diff -U0 --color-words=[^\']+ --no-index old-full.sql new-full.sql
The first diff command produces:
The second command produces a longer output, but you can confirm the only changes are timestamps and the password hash.
You can view all of the remaining scripts with the following command:
tree $(ls -d -1 -- app/code/core/Mage/*/{sql,data}/*)
Click to expand
Here's a list of the deleted files:
Click to expand
Of the remaining files, I manually deleted Mage/Paygate/sql/paygate_setup/mysql4-data-upgrade-0.7.0-0.7.1.php even though it is executed during install since with a fresh install there's nothing in those payment tables to update. And nobody is upgrading from Magento 0.7.0 -> Maho.
Mage_Customer still has a few < 1.6.0.0 files, but those actually have an effect so they remain. It was probably a bug from mage devs that the
install-1.6.0.0.php
script didn't create the table correctly from the start. It could be fixed later.There are probably other upgrade scripts that could be purged if they're not modifying the schema, but that would require manual inspection of each file. Removing 78% of the files is good enough for now. 😄