Opened 10 years ago
Last modified 8 years ago
#33741 reopened enhancement
Remove references to my-hacks.php and the hack_file option
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | 4.3 |
Component: | Plugins | Keywords: | has-patch |
Focuses: | Cc: |
Description
According to the documentation https://codex.wordpress.org/Hacking_WordPress my-hacks.php was deprecated in v1.5 and removed in v2.8
So the code in wp_get_active_and_valid_plugins() is no longer necessary
and can be deleted along with mentions of my-hacks and hack_file anywhere else in the code.
Attachments (2)
Change History (34)
#3
@
10 years ago
- Keywords dev-feedback added
According to the documentation
my-hacks.php
was deprecated in v1.5 and removed in v2.8
It's possible that someone's still using this file, but I doubt it. Anyway, some dev feedback on this would be great.
#4
@
10 years ago
If someone is still using the file, and it contains the standard comment for a plugin then wp_get_active_and_valid_plugins() will list it in the list of plugins or mu-plugins regardless of the setting of the hack_file option.
If it doesn't have the comments then it won't get listed in plugins, but will get shown in mu-plugins.
I can't see that as a problem
This ticket was mentioned in Slack in #core by sergey. View the logs.
10 years ago
#7
@
10 years ago
This will need a dev-notes post on make/core when it goes in. It's old and hopefully not used, but we should still caution people since it is a breaking change.
This ticket was mentioned in Slack in #core by jorbin. View the logs.
10 years ago
#9
@
10 years ago
- Owner set to jorbin
- Resolution set to fixed
- Status changed from new to closed
In 34291:
#10
@
10 years ago
From https://make.wordpress.org/core/2015/09/18/my-hacks-php-no-longer-supported/
What about creating a plugin that will look for my-hacks.php and then load it? :-)
Then it could email the site admin about this situation and prompt about doing the proper plugin.
This way there will be no broken sites.
Just my $0.02.
Cheers,
Gabriel
#11
@
10 years ago
- Keywords has-patch dev-feedback commit removed
- Resolution fixed deleted
- Status changed from closed to reopened
If an option is going to be removed, it needs to be added to $unusedoptions in schema.php.
Separately: @josephscott previously attempted to remove my-hacks.php. There are almost no CPU cycles gained by removing it, as hack_file is an autoloaded option. It's nice to clean it up, of course, but I'll reveal that he scanned all VaultPress sites for the existence of my-hacks.php and found 6 that still used it. That's a very low number, except that two of them were the sites of WordPress co-founders. So we ended up just dropping it.
I doubt that this will break a site, and I don't care enough to revert this, but we should do a make/core post about it.
#12
follow-up:
↓ 13
@
10 years ago
@nacin a make post was published minutes after this change went in. https://make.wordpress.org/core/2015/09/18/my-hacks-php-no-longer-supported/
I don't think it's the CPU cycles gain that is important here, but instead it is the removal of code and thus resulting simplification of the plugins loading code. my-hacks.php has been an edge case for a number of years and the elimination of that edge case helps the code be clean, lean and mean.
10 years of deprecation seems like enough warning. Do you happen to know the previous ticket when removal was attempted? I couldn't find it when searching through trac. I would also be interested in the VaultPress stats to see if that number has changed. Additionally, I asked @DH-Shredder to look into the stats on his end. Perhaps @mikehansenme would also want to take a look or any of the other hosts as well.
One proposal on the make blog was to attempt to move my-hacks.php to mu-plugins on upgrade. I would be support a patch that did that.
#13
in reply to:
↑ 12
@
10 years ago
Replying to nacin:
If an option is going to be removed, it needs to be added to $unusedoptions in schema.php.
This is in 33741.2.diff
Replying to jorbin:
10 years of deprecation seems like enough warning. Do you happen to know the previous ticket when removal was attempted? I couldn't find it when searching through trac.
My trac foo didn't surface anything when I went a looking either...
#14
follow-up:
↓ 15
@
10 years ago
- Keywords has-patch added
I'd just like to point out that I didn't actually read the code properly.
In my mind I'd read the array_unshift line as something completely different.
I thought it was removing the my-hacks plugin from active plugins rather than adding it to the list.
But after due consideration I've decided that my patch to remove the redundant code is still valid.
@nacin
hack_file is only an autoloaded option if it exists.
When it does not exist another query is run after the first one to load the autoload options
1,0.030197,SELECT option_name, option_value FROM wp_options WHERE autoload = 'yes' 2,0.001128,SELECT option_value FROM wp_options WHERE option_name = 'hack_file' LIMIT 1
The TRAC that removed the my-hacks checkbox was #9551.
@nacin @netweb
I can't see the point in adding an options field that should have been unused for 6 years into the $unusedoptions array.
In actual fact, you could actually break some code that was written since WordPress 2.8 and has nothing to do with wp-hacks but inadvertently used the option name 'hack_file'.
In my opinion the $unusedoptions array should be pruned every now and then.
There should at least be some documentation to list the options names that you shouldn't use!
#15
in reply to:
↑ 14
;
follow-up:
↓ 17
@
9 years ago
Replying to bobbingwide:
hack_file is only an autoloaded option if it exists.
When it does not exist another query is run after the first one to load the autoload options
hack_file continued to be created during install until [34291]. It was always autoloaded.
#16
@
9 years ago
my-hacks.php was mentioned by @josephscott in IRC on August 30, 2013: https://irclogs.wordpress.org/chanlog.php?channel=wordpress-dev&day=2013-08-30&sort=asc#m678600.
He followed up with me over Skype where he shared with me the extremely low usage of the file on VaultPress sites. It never made it to a ticket. In our conversation, moving it to mu-plugins did come up.
#17
in reply to:
↑ 15
@
9 years ago
Replying to nacin:
Replying to bobbingwide:
hack_file is only an autoloaded option if it exists.
When it does not exist another query is run after the first one to load the autoload options
hack_file continued to be created during install until [34291]. It was always autoloaded.
Ah yes. I missed that.
#18
@
9 years ago
So where do we stand on this?
As I said previously. I don't support adding hack_file into the $unusedoptions array since it could break someone else's code.
hack_file=0 my-hacks=N -> OK
hack_file=0 my-hacks=Y -> dormant file as far as core is concerned, but could be loaded some other way
Deleting the file could break someone's code.
hack_file=other my-hacks=N -> Deleting the option could break someone's code.
hack_file=other my-hacks=Y -> How long have they been ignoring the deprecated message?
Deleting the code could break someone's code... but that was the whole point of the deprecated file message.
#19
@
9 years ago
FYI earlier trac ticket I just found https://core.trac.wordpress.org/ticket/12996#comment:20
This ticket was mentioned in Slack in #core by helen. View the logs.
9 years ago
This ticket was mentioned in Slack in #core by jorbin. View the logs.
9 years ago
#23
@
9 years ago
- Resolution changed from fixed to wontfix
I really want to kill my-hacks. I really want to show that WordPress does actually revert or remove deprecated things. Even more so, I don't want to break sites. The commit message above and comment 18 outlines this.
#25
@
9 years ago
I don't understand the last few updates in comment:22 and comment:23.
33741.2.diff was the problem that I mentioned in comment:18.
If you don't delete the option then the only sites that get broken are the one's that are supposed to be broken!
Just use the first patch.
#26
@
9 years ago
- Resolution wontfix deleted
- Status changed from closed to reopened
After due consideration I have re-opened this ticket. If we cannot finally remove code that's been deprecated for 6 years then what chance have we dropping support for PHP 5.2.
The first patch ( 33741.patch ) simply removes the references to my-hacks and hack_file.
It finally implements what it was set out to do.
The second patch ( 33741.2.diff ) which implements "If an option is going to be removed, it needs to be added to $unusedoptions in schema.php" is, IMHO, the flawed logic that could lead to a site being unintentionally broken.
So, as stated before. Don't add hack_file into $unusedoptions.
One day, perhaps when 5.2 support is dropped, the same argument can be applied to all the other "unused options". They're only unused as far as WordPress core is concerned.
#28
@
9 years ago
- Type changed from defect (bug) to enhancement
- Version changed from trunk to 4.3
I'm not sure I understand why the revert happened either, or why the wontfix. I see the determined reason, but no discussion as to whether not bothering really is the right approach overall.
#29
@
9 years ago
The revert was entirely unrelated to $unusedoptions
. It was done because @jorbin was concerned we'd break a few sites without a good reason. The was essentially nixing that we should remove my-hacks.php
, and thus the ticket was properly closed as wontfix based on that decision.
My comment about $unusedoptions
was *only* that if we were going to go through with [34291], then it was useless to keep around the option in the DB, and it should also be removed. "Deleting the option could break someone's code" is not my concern here — we've removed 'dead' options many times before. This is especially OK to do (in a backwards-compatible context) when the "default" value (in this case "0") evaluates to false (equivalent of it not being in the DB).
#30
@
9 years ago
OK, I still don't understand. This is how I read it.
By adding hack_file to the $unusedoptions we could potentially break someone's code
... and because we think the value's 0, we're happy to do this.
But we're not happy to remove the code that has been telling people for 6 years that their site is going to break one day.
BTW: The title of the enhancement is still "Remove references to my-hacks.php and the hack_file option"
Should I add "ALL"?
#31
@
9 years ago
@bobbingwide Re: https://twitter.com/herb_miller/status/674631292101767168
On one site the file still exists and the option is still set to 1. However, the functions defined there are no longer used in the site's theme as the theme has changed.
Interestingly, if I preview my old theme, I can see that the my-hacks file is loaded but has errors now. I will happily remove it from my installation.
However I agree that no site should be broken by any changes proposed here.
Remove references to my-hacks.php and hack_file