TBH I never expected anything other than that from @SharkyKZ LMAO... so predictable now...

This is ready for testing now. Thanks.

Sorry to be a pain but can you rename invalidateOpcache to invalidateFileCache or something. Just so we have options in the future if we want to call other cache invalidates

s/invalidateOpcache/invalidateFileCache

Done.

Thankyou!

Done on the Joomla 3 version, and the Joomla-framework version PRs too.

Not sure why this hasn't been closed yet. As maintainers will explain to you, this PR is wrong.

Set to "closed" on behalf of @SharkyKZ by The JTracker Application at issues.joomla.org/joomla-cms/32915

Maybe you could actually tell me?

well its just been merged into Joomla-framework/filesystem so you best go all their maintainers they were wrong too @n9iels hahaha

this PR is wrong.

But yet you dont have the decency to actually explain ... Its time you were removed as a maintainer @SharkyKZ

If @SharkyKZ doesn't want to explain what's wrong, at least one of our maintainers should explain what's wrong in this PR. Closing a PR without explaining will hurt our contributors :(

Well @wilsonge seemed happy enough to ask for changes without rejecting it, and he's a maintainer.

@PhilETaylor Maybe @SharkyKZ has explained what's wrong with our maintainers. So please wait for an explanation.

Ive waited 4 days since his thumbs down.

Its time his rights were removed.

Its time his rights were removed.

I agree. In fact, all of my rights should have been revoked in November 2020 when I left JBS. But, clearly, someone royally screwed up with my offboarding. Or perhaps didn't even bother at all. I bet if I hadn't manually removed myself from the organization on Github, I'd still have permissions here too. Unauthorized users managing the issue tracker? Sure, no problem! ? And this alone perfectly sums the state of the entire project. Too much ignorance on the part of people who run things here. Frankly, Phil, based on your issue history you should understand this better than anyone else.

and still you have refused to give a reason for your drive by closing of PR's.

That is disrespectful to contributors WHOEVER they are.

Hiding behind a alias "SharkyKZ", that even your name is not published in official OSM meeting minutes, is it even legal for OSM to grant administrative permissions to a ghost?

Are you going to provide feedback on why you have rejected the hard work of contributors or are you just going to personally attack me again?

Sorry, that this happened.

Sorry, that this happened.

Im sorry too. I think I deserve and explanation, but I know I won't get one.

If there is a genuine technical reason why this PR (and the others like it are "wrong" then Im open to feedback, as all my recent PR's show, Im happy to take constructive comments on board and improve them.

I think I deserve and explanation, but I know I won't get one.

I wish I had some, but I know as little as you do about why he does that.

I have no explaination, it was a missuse of permission (it seems we forgot to revoke all permissions there) and should not have happened.
I really appreciate all of your contributions (wherever I'm not always agreeing, they still should be valued). So I can only say again, I'm sorry, that it escalated and we try to be more careful in the future.

i ask excuse to you all
My fault cause i was the JBS team leader at that time
and i've only moved you @SharkyKZ in the honor list

i've wrongly assumed that was enough

Thanks for the comments.

Does a maintainer actually have a problem with these proposed opcache PRs or not then?

Does a maintainer actually have a problem with these proposed opcache PRs or not then?

@PhilETaylor I don't have a problem with them, but as you maybe know I'm not really the super expert. I have some knowledge on this or that, and I know a bit about opcache and use it for my home page, but I might be wrong or missing something. I don't belong to those people who think about themselves that they know everything. But maybe that's Sharky's problem, that people like me are maintainers ... I don't really know.

@PhilETaylor Maybe there is something wrong with this PR, I'm not sure yet. System tests are failing when removing the installation folder after the installation. I've tried a few times. Have started drone again to see if it still fails at that place.

Hmm, again same place, when removing installation folder at the end of a new installation:

https://ci.joomla.org/joomla/joomla-cms/41482/1/22

Not sure yet if that could be related to this PR. I have to test later or tomorrow.

I'm out driving my kid to her sleepover. I'll test the tests later tonight.

Might need to test if the file exists before clearing the opcache ? But then that would defeat the changes as we want to force the opcache to notice the file is removed

Thanks for testing

It seems that the Refactoring (renaming) of the static method George requested did not rename all instances (very rare for phpStorm to not do this correctly!) so there were some renames missed in f281cf6 a few hours ago. (Before the drama)

I have corrected those in 02c03ca and fingers crossed the tests now pass.

Hmm, I should have seen that in code, too, but I was so focused on the drama that I had no eyes for code anymore.

I have also checked if running opcache_invalidate() on a non-existing file (for example, when opcache preloading is used, or when opcache.revalidate_timestamps is not used and you delete a file and need to remove it from the opcache) causes an issue, and it doesn't

Eg https://3v4l.org/9J3sb

EDIT: Although there is a UserLand comment on https://www.php.net/opcache_invalidate saying to remove a deleted file you have to invalidate it first then delete it... Im checking that, but I need to rebuild a docker image for it.

and thats why we have tests ;)

@PhilETaylor I was trying to find out what could be the reasons for Sharky's thumbs down. Not sure if that explains all, but I've found something interesting from another well known product: https://developer.wordpress.org/reference/functions/wp_opcache_invalidate/ . They check not only if the "opcache_invalidate" function exists but also if there is a "opcache.restrict_api" ini parameter set and if the currently handled PHP file is restricted by that parameter. In the comment to their function:
For more details, see:

• https://www.php.net/manual/en/opcache.configuration.php
• https://www.php.net/manual/en/reserved.variables.server.php
• https://core.trac.wordpress.org/ticket/36455

The last link is the most interesting ?

I'm not sure if that was the reason for the thumbs down, I don't really think so.

But maybe we should do something like that anyway.

I do not believe there was any code reason for the thumbs down at all. I doubt he even looked at the code.

So you mean WordPress fixed this 4 years ago and still Joomla refuse to accept it was/is an issue /facepalm :)

Lets copy WordPress then.. if its good enough for 30% of the internet its good enough for 2.1% of the internet...

I'll make the further refinements later today

So you mean WordPress fixed this 4 years ago and still Joomla refuse to accept it was/is an issue /facepalm :)

@PhilETaylor Who is Joomla? Aren't we all Joomla? As you can see, I don't refuse to accept it is an issue. I just was not aware of it before it was raised again in recent discussions, and then I was not able to provide quickly a fix, and so I am very happy that you do. I'm just one of the maintainers, and I'm only a human being, so my time and energy is limited, and my focus maybe too. The same might apply to other maintainers. So you should not say we still refuse to accept it is an issue. Whatever went wrong in past, let's not stick to that. Let's fix it (what you do, and again thanks for that) and collaborate in the best way so that at the end all will be good.

I was referring to #32592 - nothing personal. Tongue in cheek :)

Working on it now.

Working on it now.

@PhilETaylor Maybe you can check their code base first if there have been made any changes on that function after it was implemented? Maybe there were bug fixes on it meanwhile.

I have refactored the static function now to learn the lessons from other projects that have gone before us.

I have added additional documentation.

I have also noticed that Joomla Cache doesn't invalidate using the filesystem class, so I have forked that to #32961 as something else to fix.

Once we are happy here, I will back port these additional changes to the Joomla 3 and Framework merges.

Maybe you can check their code base first

I have looked, their code not much different from what Phil made ?
UPD: Placing "invalidate" in File class much better than WP solution.

In general, to me it like workaround of PHP bug. Maybe there already something on PHP bug tracker?
I think the cache should be invalidated automaticly by the opcache itself, for every file manipulation with: fwrite, file_put_content, unlink etc. But this is question to opcache developers.

Other thing when the files copied manually by user , via ftp etc.

I have also noticed that Joomla Cache doesn't invalidate using the filesystem class, so I have forked that to #32961 as something else to fix.

because the file is removed it does not make sense to "invalidate" the cache or?
there another comment https://www.php.net/manual/en/function.opcache-invalidate.php#119026 :

Note that invalidation doesn't actually evict anything from the cache, it just forces a recompile....

so maybe it makes sense to call invalidate only for write/copy action. But I have no idea how to test :)

In general, to me it like workaround of PHP bug.

This is NOT a workaround for a PHP "bug". There is no PHP bug. The OPcache is doing exactly as designed.

I think the cache should be invalidated automaticly by the opcache itself, for every file manipulation

You can think that, and for "some" web hosts that is exactly what will happen. For "some" other web hosts this will NOT happen.

OPcache can be run with many different configurations. The most popular is "just enable it and leave it alone" and what you describe is how that will work.

However some web hosts "optimise" their PHP configuration and attempt to run PHP code exclusively from memory (the OPCache) and try to avoid file system calls.

Other artisan hosting will use PHP Preload features, and TOTALLY serve all PHP from the cache and NEVER check the file on the hard drive every again after the initial compile. This is how mySites.guru runs, this is how a lot of dockerised PHP apps run.

There are web hosting services throughout the spectrum above.

It all depends on the exact PHP OPcache configuration. Mainly

opcache.validate_timestamps
opcache.revalidate_path
opcache.revalidate_freq
opcache.enable_file_override

As Joomla is mass-distributed, we need to ensure that we are compatible with a huge range of configurations.

We do already see some web hosts (Siteground) causing issues with over aggressive caching.

because the file is removed it does not make sense to "invalidate" the cache

it totally makes sense. Because under certain circumstances described above (for example if opcache.revalidate_freq is say 60 seconds then it can take PHP a whole minute to realise the file was deleted! The default is 2 seconds, which is still a HUGE amount of time if the script you are running then goes on to do other things), (Moodle as another example, has a configuration documentation with a opcache.revalidate_freq of 60!!!) PHP will not even notice the file has been deleted unless you tell it to check by invalidating that files opcache. PHP will just carry on loading the compiled version of the deleted file. This is one of the issues reported by Akeeba backup developers and has caused a lot of support issues.

Note that invalidation doesn't actually evict anything from the cache, it just forces a recompile

I know this. However what I propose is still correct.

But I have no idea how to test :)

Like I said in the testing instructions "Hard to test unless you really know what you are doing" :-) I was specifically vague as to only get tests from people that actually understand the PHP OPcache and how to manually configure PHP. Some PRs just need a little more experience and knowledge to test and simply following a developers provided steps to test something just doesn't cut it.

However, if you set your PHP configuration with the following settings, your Joomla site will run exclusively from opcache, and thus be at the extreme case end of the test scenario.

opcache.validate_timestamps=0
opcache.max_accelerated_files=20000
opcache.interned_strings_buffer=16
opcache.save_comments = 0
opcache.enable_file_override = 1
opcache.fast_shutdown=1
opcache.log_verbosity_level=4
opcache.error_log=

Install and use a opcache viewer to view your opcache and its contents and settings.
https://github.com/amnuts/opcache-gui
https://github.com/rlerdorf/opcache-status

Then use Joomla to manipulate files - like installing an extension or removing an extension and ensure no error happen, and that the opcache contents change based on your actions (remembering that with the above settings the PHP is running ONLY from the opcache and its NEVER reading the PHP files on the hard disk, unless it needs to recompile the file for the opcache.

Another good test is to configure as per my last comment, ensure you have the latest copy of this PR, and then set your site offline, and you should see the frontend offline. This means configuration.php has been recompiled into the opcache, which is what Joomla 4 always used to do, but now we do it at a Filesystem level.

@PhilETaylor There is one thing which bothers me a bit after I have read the complete https://core.trac.wordpress.org/ticket/36455 : As far as I remember, they said they invalidate the cache for a file before they modify (write) or delete it, but here and in the corresponding staging PR and in the merged PR in the framework it seems to be done after having written or deleted the file.

Don't believe everything you read. Trust no one. People believing everything they read is why we have opcache_reset in our code. :-)

Yep, seems I was reading something wrong. They do it also after it, like here in this PR.

For writing a file they are wrong.

You can prove this by saving joomla global configuration with the test config of opcache I provided and you will see it works perfectly.

Once I have finished the ironing I'm going to set up test cases to prove the deletion comments are wrong too.

PHP Source code to read: https://github.com/php/php-src/blob/a6dd92f8201f1de8a1b95f4054c10f7ecc7a4fd8/ext/opcache/ZendAccelerator.c#L1293

Looks like passing force = true (which I do in this PR) will FORCE a deleted file to be removed from opcache, even if a file doesn't exist (which would make do_validate_timestamps(persistent_script, &file_handle) == FAILURE also true, as the file_handle would be false as the file would not be able to be opened.

https://github.com/php/php-src/blob/a6dd92f8201f1de8a1b95f4054c10f7ecc7a4fd8/ext/opcache/ZendAccelerator.c#L1318

Thats the way I read the source code of PHP-SRC.

But Im no C programming expert :)

Maybe the not existing file is handled before the place you've mentioned: https://github.com/php/php-src/blob/a6dd92f8201f1de8a1b95f4054c10f7ecc7a4fd8/ext/opcache/ZendAccelerator.c#L1304

It seems you might be right:

https://bugs.php.net/bug.php?id=75939

Gulp ... "This can be problematic in general and for esoteric in particular." ... nice said there ;-)

Do we have a kind of task force for esoteric problems?

Do we have a kind of task force for esoteric problems?

Thats you and me buddy on easter weekend!

Ok I have completed ACTUAL testing now.

I can confirm that to expire the opcache for a deleted file, you DO need to invalidate it BEFORE deleting it. The rumours are right, but its always good to CHECK and not blindly follow comments on the PHP functions documentation :-)

I have proved this by having two files deleteme.php and deleteit.php

After setting up PHP exactly as per the test configuration I provided (which runs ALL PHP from opcache and NEVER looks back at the PHP files unless the opcache is empty for that file) ... I start PHP, visit deleteme.php to create an compiled opcache for that file

TEST A: If I first unlink the file and then invalidate the opcache, the opcache for that file is NOT removed.

unlink('deleteme.php');
var_dump(opcache_invalidate('deleteme.php', true)); // = FALSE
var_dump(opcache_is_script_cached('deleteme.php')); //  = FALSE

TEST B: If I first invalidate opcache and then unlink the file, the opcache for that file is correctly removed.

var_dump(opcache_invalidate('deleteme.php', true)); // = TRUE
var_dump(opcache_is_script_cached('deleteme.php')); //  = FALSE
unlink('deleteme.php');

There is the TINIEST SLITHER of time between invalidating and unlinking where a script COULD POTENTIALLY be recompiled (by another process, another visit to the website by another visitor for example). However there is not much we can do about that as Joomla developers.

var_dump(opcache_invalidate('deleteme.php', true)); // = TRUE
// new visitor, in a different PHP process, visits deleteme.php here in parallel to this script running, which recompiles deleteme.php into opcache
unlink('deleteme.php');
var_dump(opcache_is_script_cached('deleteme.php')); //  = TRUE

Thats you and me buddy on easter weekend!

I was afraid you'd say that.

delete now changed in 1c893ce - Thanks.

Hmm, there comes something into my mind. I remember that in past, before my webspace provider has started to enable mod_deflate, I had used an ugly hack for my private website to get js and css compressed: I processed these files with PHP and so could compress the output. Because this was a dangerous hack (there could be something in a js file which is a short tag for PHP), I later changed to statically gzipped files, before I later could use mod_deflate.

Am asking myself now if it can happen that we have opcache working on files which don't have the ".php" extension.

@richard67 opcache is a cache of the compiled PHP source code, not a cache of the output the PHP code generates. So you can still use opcache and php files to output gz compressed js/css with no issues

@PhilETaylor You are right, and I was maybe a bit paranoid. Just wanted to be sure we don't forget anything.

@PhilETaylor I should have remembered it earlier, but now I do.

We have a function for renaming files with incorrect case in file names on core updates here:

https://github.com/joomla/joomla-cms/blob/4.0-dev/administrator/components/com_admin/script.php#L7175

This uses rename and unlink PHP functions directly.

The same applies to J3:

https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_admin/script.php#L2668

That function was once implemented by Sharky and later we took over and finished his work.

What shall we do there? Make it use functions from our File (or on J3 JFile) class? Or add opcache invalidation to that function in script.php?

And maybe we have other places where PHP functions are used instead of using our File class?

Another thing comes into my mind for J3, but I discuss it here because we discussed all the other stuff here, too, and not in the J3 PR: On J3 we might still have older PHP versions which don't have opcache but apc cache, whatever that is. Could that be? Do we have to handle that, too, on J3?

Im far from finished with opcache cases. Searching for file_put_contents, unlink, rename are still on my to do list.

This PR was just the start, and specifically to address extension installations (which happen more often than Joomla updates) and to address Nicolas's valid concerns that other projects had already addressed.

The hope is to at least get the Joomla 3 version of this PR into Joomla 3.9.26.

I dont care about 3rd party cache providers. PHP Opcache is shipped with PHP and is provided and supported by the PHP Team. Im not going to be investigating and invalidating all 3rd party caches, that's a rabbit hole you dont want to go down.

@PhilETaylor Maybe it could make sense to do the removal of the opcache_reset calls here in this PR, too? These calls might complicate testing a bit if still present.

already working on that, and some risky ones in the update process :scared:

/note to self... TURN OFF AGGRESSIVE OPCACHING WHEN TRYING TO EDIT PHP FILES - DOH! Time lost: 37mins...

@PhilETaylor Should we port back all recent changes to the staging PR? Or should we ask release leads for opinions? Me personally I'd like it to be ported back, but I might be wrong.

Im not too fussed with Joomla 3 anymore :) lets make Joomla 4 amazing :)

Im not sure (because no one answers me) how much time we have for Joomla 3.9.26 to be released, and as that's 2 versions after the advertised "last version" in the Joomla 3.9 series...

I guess some of the recent changes could be added to the Joomla 3 PR, but I dont want to add anything that will hamper it being merged without a fight.

Also I would want the changes in the Akeeba provided update (restore.php restore_filnilization.php) tested EXTENSIVELY before release :)

@PhilETaylor Do we even need to invalidate anything in the copy function? We just create a new file for the opcache and not change the old one. Ah, ok, got the answer already.

Ah, ok, got the answer already.

:-) #32915 (comment)

Will test later today or tomorrow or both if it takes that long.

I think we need to test at least following szenarios for the Joomla Update stuff:

• Updating a 4.0 Beta 7 without the patch of this PR applied to the update package built by drone for this PR.
• Updating a 4.0 Beta 7 with the patch of this PR applied to the update package built by drone for this PR.
• Updating from 3.10-dev to the update package built by drone for this PR.

4.0 Beta 7 for the first 2 because there will be some files and folders to be deleted.

It might also make sense to prepare some files to be renamed by that function here so that it should do something:

https://github.com/joomla/joomla-cms/blob/4.0-dev/administrator/components/com_admin/script.php#L7175

And of course we have to test extension updates and saving changes in configuration.php.

We should test all that with realistic and with extreme opcache settings.

Appreciated. Thanks.

If we wanna be extremely safe with testing, we do all test on PHP 7.x and PHP 8.

I'll ask my boss in my paid job if she can give me 1 week holiday ;-)

Ah and of course to be safe it also needs some test if all continues to work when opcache is not available or switched off.

when opcache is not available or switched off.

Insane web hosts ;-)

Hmm I just see testing with using 4.0 Beta 7 and the patch applied will not work. Have to use current 4.0-dev or latest nightly instead.

define "will not work" ?

My 2nd scenario "Updating a 4.0 Beta 7 with the patch of this PR applied to the update package built by drone for this PR.": The patch of this PR can't really be applied to beta 7 because the modified function in script.php did not exist yet.

ah ok, well that's not this PR's fault :)

ah ok, well that's not this PR's fault :)

Right, it was my scenario being wrong.

Phew :)

@PhilETaylor Silly me. I've looked at the wrong place. My scenario is valid, i.e. we can use Beta 7 for testing.

@PhilETaylor Now after some stuff has been merged up from staging/3.10-dev into 4.0-dev, there is a conflict in script.php. Let me know if I shall help with solving it. It is my PR #32176 coming up from staging which causes this now.

Looks like you took slashes out of the paths, the only bit I changed was the rename/unlink function calls and replaced them with Filesystem API calls.

@PhilETaylor The slashes I have taken out because I have added them before to the array:

https://github.com/joomla/joomla-cms/pull/32176/files#diff-db7eb77540ff419bd7e6557d2779f4cf1e31158c20cb4b63dbbe8d6c426385adR2671-R2673

You can't see that when checking for conflicts only because there was no conflict.

So here where the conflicts are we have to take both changes together, yours and mine.

If you want I can do that for you here.

Im back to the day job tomorrow, and still need to drive the other side of the island in an hour to collect the kid. So I'll not get to this, or any other PR's probably until 24 hours or more - sorry. Easter is over :)

Im back to the day job tomorrow, and still need to drive the other side of the island in an hour to collect the kid. So I'll not get to this, or any other PR's probably until 24 hours or more - sorry. Easter is over :)

@PhilETaylor No problem. I solve the conflict here and you just pull later from remote to local before continuing work.

Independently from the conflict I need to do more tests here, too, stay tuned, it may take some time.

I also need to see how many of the 115 comments here have been, and have not yet been, merged to the Joomla 3 version of this PR. #32918

Current failures in drone are not caused by this PR here but by the 4.0-dev branch not being ok, and I've just updated this PR by that. I will update it again if the 4.0-dev branch is ok.

Drone has been very unstable recently.

@PhilETaylor Regarding integrate changes from here back to the staging PR: I discovered some weird problems when testing, of which I am not sure it they come from the opcache_reset calls being removed here, or if I do something wrong with my opcache configuration. Maybe we should wait with that until we know if we are ok here.

@PhilETaylor Regarding drone: Right now we have a js cs error in the 4.0-dev branch which will be fixed by #33026 , and a merge conflict here in addition: 321def1#r49127384 ... so not the usual instability of drone only.

But as said, I will have an eye on it so you don't need to care.

some weird problems

I need more than that context haha :)

You could be like me, and spend an hour wasting time writing new code in my "other project", refreshing and trying to work out why the hell nothing was working on your "other project", and then realise that you still had opcode caching set to very aggressive and never revalidating the PHP files... that was a wasted hour...

You could be like me, and spend an hour wasting time writing new code in my "other project", refreshing and trying to work out why the hell nothing was working on your "other project", and then realise that you still had opcode caching set to very aggressive and never revalidating the PHP files... that was a wasted hour...

No, I was testing this PR here, with and without the one from the framework in addition, and with the very aggressive opcache settings you have proposed above in comments.

I've noticed that when I update a 4.0-dev to the update package for this PR, using the custom URL provided by drone package step, then after the update the usual database error due not not matching CMS version was not shown (like it is expected when using these packages for update). When I then "manually" triggered an opcache_reset, the CMS version was updated, and that database error was shown.

When I started before such an update without the patch of this PR applied, I got after the update to the package with this PR an alert with a warning that the restoration.php file could not be deleted:

But in fact the file has been deleted.

When starting with the patch of this PR applied, that alert was not shown.

In both cases I had the issue with the CMS version not being up to date in backend (but on filesystem it was, so backend was using the old opcached info still.

Another issue I've observed when installing patchtester v 4.0.0, which doesn't show the badges in the pulls list due to missing changes for bs5, and then updating that to v 4.0.1 were this issue was fixed, and then checking again the pulls list, and it was not ok. Only when "manually" resetting opcache, I was the changes.

For manual opcache reset I use a PHP script in my Joomla root which calls opcache_reset() and which I've navigated to in my browser.

With opcache lopg severity 4 I could see the corresponding debug messages.

I couldn't drag down yet to where goes what wrong for me.

I think for today I'm done, will continue testing in the next days.

I have a different Mac at the (physical, not home) office so will rebuild from scratch there tomorrow and test like you, as that Mac has not been used in the development of this pr at all.

It's possible there are more file operations to take into account - remembering that joomla updates are extracted differently (standalone with restore.php) to extension updates (extracted by joomla)

@PhilETaylor Now as the 4.0-dev branch has been fixed so CI tests are passing again, it could make sense to update this branch to current 4.0-dev. If you want I can do this for you to save you some work. But I'm asking you for permission first because it may disturb your workflow. As far as I can see, I have the necessary permission on your GitHub repo.

Go for it - I can always pull again.

Done. That will help us with testing core updates using the update package or custom URL built by drone because those will be not too old.

@PhilETaylor Having your PR #33040 in mind now, I'm asking myself if it could make sense to add come cache clearing to those places where we remove the opcache_reset calls here. I don't think it explains my testing results so far because I had caching switched off in global configuration. But I think it might be necessary since we don't invalidate the cached files, only the "originals", while an opcache reset also will cause a revalidation of the cached files later, right?

The cache files in the cache folder will be invalidated by my other PR .. #32993

Maybe I should merged the 2 PRs as there have been no other interest from anyone on the other one.

The cache files in the cache folder will be invalidated by my other PR .. #32993

@PhilETaylor Yes, that PR makes sure we invalidate them when we purge the cache. But what I mean is that we have removed calls to opcache_reset here, but we might not have calls to reset cache at all these places. As far as I know we clear cache after core updates, and I guess we do that also after extension updates or deletions or installations, but about the latter I'm not sure. And who knows where else we might need to clear cache now where we didn't need it before.

And who knows where else we might need to clear cache now where we didn't need it before.

that's the whole point. we should NEVER call opcache_reset ever. We need to trawl through everything and ensure that its using opcache_invalidate - luckily Joomla uses the Filesystem API mainly and so we are good in those places.

. We need to trawl through everything

well, searching for unlink move copy rename file_put_contents etc...

@PhilETaylor I have also seen some places where there are options for stream and not stream, don't remember now where. Does this use anything from the phar stream wrapper which still has the opcache_reset in its belly?

The phar stuff we cannot do anything about, that's a 3pd library.

The phar stuff we cannot do anything about, that's a 3pd library.

Do we use that somewhere, and if so, could we just stop to use it?

IIRC it was a security fix and that's why we have the Phar Wrapper
https://developer.joomla.org/security-centre/781-%2020190502-core-by-passing-protection-of-phar-stream-wrapper-interceptor.html

I honestly dont believe anyone is using phar's with Joomla though :)

We support so much stuff which nobody really uses ;-)

This PR seems to have stalled - to be clear for those reading, you can test and provide your test results here - its ready for testing - this PR is needed to future proof Joomla on decent web hosts that use decent opcache configurations (and those with crazy opcache configurations)

@PhilETaylor Not stalled. I have it on my list for deeper testing. My previous tests were partly not ok, but maybe due to my opcache settings were not right. Stay tuned.

This shit has to stop!

This shit has to stop!

Agree.

@SharkyKZ I have given you a 7 days break, your negative behaviour is not acceptable. The only thing you contribute are thumbs down without providing more information.

Thank you. To be clear this was continued negative behaviour was reported to Brian Mitchel and Marco Dings weeks ago and both have refused to even reply to my email or acknowledge it, or follow ups but Luca acknowledged the email.

Joomla Leadership doesn't take contributor safety seriously at all and the CoC process is just a joke!

Thank you for having the guts to take action.

Merged in the FTP Layer removal stuff that conflicted, resolved conflicts and force pushed again.

Is there any reason why this is not RTC yet? OPcache not being cleared for .php files being written during an update DOES cause issues in any non-trivial extension's post-installation code. See https://www.akeeba.com/news/1744-advice-for-install-probs-with-march-2021-software-releases.html it is under “PHP opcache can cause weird issues”. I can still reproduce this issue after rewriting my extensions using core Joomla 4 MVC and Bootstrap 5 so any notion that the problem was ‘magically’ caused by my frameworks not only flies against common sense but is categorically disproven.

This problem also affects Global Configuration (com_config) writing the configuration.php file. Look in the forums how many times people are told their configuration.php must be unwriteable even though there is no error message telling them this is the case and you will see just how often Joomla not handling OPcache scores an own goal and loses users.

This not only needs to be set RTC and merged a.s.a.p. in Joomla 4 but also back ported to Joomla 3. It's not 2011 anymore when opcode caching was something exotic. It's 2021 and you'd be hard pressed to find a server which doesn't have OPcache turned on by default.

The only reason this has not been a big issue yet is that the default revalidate time is 2 seconds, Joomla 3 is slow, people run it on PHP 5.6 or earlier which is dead slow and stuff it with a million plugins which are make it even slower. This made operations take substantially more than the default 2 seconds of OPcache revalidation frequency. Joomla 4 is much faster, it requires modern PHP versions which are faster and even the plugins (Events) architecture mostly guards against plugins bogging down the site. We will see a heck of a lot more operations going into the few hundreds milliseconds to two seconds which will expose the OPcache issues.

I wrote a Joomla 3 PR at the same time as this one and it got little to no attention and I closed it - it's here on Gh somewhere, probably needs dusting off...

@nikosdion I had tested that PR and it failed for me when I had used the aggressive opcache settings which Phil had proposed, see my comment above #32915 (comment) . With normal opcache settings it worked for me, but I also was not able to reproduce any issue.

If you CAN reproduce an issue: Why don't you test this PR? It would for sure help to come forward with it.

Joomla 3 version #32918

I'm interested in merging this. But this is way too scary to just merge on review given the level of things touched. I fully agree it's something that needs testing however :)

The problem is and has been historically getting developers of a calibre that actually understand PHP and how it really works in order to test complex things like this.

Anyone can test pixel pushing or accessibility...

But then this is Joomlas whole issue moving forward, attracting and retaining top quality developers to contribute

The problem is and has been historically getting developers of a calibre that actually understand PHP and how it really works in order to test complex things like this.

Anyone can test pixel pushing or accessibility...

But then this is Joomlas whole issue moving forward, attracting and retaining top quality developers to contribute

@PhilETaylor I am aware of the fact that I am not a PHP expert. I have knowledge in other things, like SQL. But to say that anyone who isn't a PHP expert just does pixel pushing or accessibility (and so in between the lines also say accessibility is nothing really important) .. that hurts.

Taking everything out of context and believing it's a personal attack is also a trait of this project.

I would hope you knew I was talking about the project in general and not you specifically

There are very few people at Nicolas's level of the php game, and those that are, tend to be in laravel or symfony and shun joomla for many reasons.

You could be the worlds best accessibility expert or graphic designer - those skills are also needed in this project generally - but to get this PR merged needs people who understand the depths of PHP. What I said doesn't diminish other skills, it just states they are not what is needed for this Pr

Subtlety in comments are often lost in translation

"Anyone can test pixel pushing or accessibility" .. what is subtile in this? What is lost in translation?

I was being generous

Serves me right for sitting on the loo and typing... let me finish the sentence

"Anyone can test pixel pushing or accessibility, and can follow a list of instructions on how to test and see if something that was broken, is now changed and is now fixed when it was not fixed before, but what this PR needs is someone who understands the internals of PHP and how opcaches work IN THE REAL WORLD (not just on your local environment with a single tenant test site but on a server with 1000 clients and websites all competing for resources) and that understands what each opcache setting does and how it effects the stack and how each setting interacts with a reset of the opcache - a technology that is ("normally" but there are exceptions) a server wide configuration and can often be badly/wrongly/screwed up by webhosts manipulating the settings and setting up crap servers too. Joomla should could and must be able to run in all kinds of different good bad and screwed up opcache configurations.

It took WordPress 5 years to perfect a PR, it's not rocket science but it takes understanding, testing and people who have experience in this area.

By your own admission @richard67 this is not you, but I'm grateful for the testing you have already done to get it this far

We are over 191 comments in and yet no further forward

@richard67 Phil wrote this PR after he read my post-mortem which I linked to in my previous comment. Between these two events a member of the Joomla leadership said publicly and in no uncertain terms that the problems I faced where of my own making, blaming Joomla — even with cold, hard facts — is deflecting my responsibility, called for banning my extensions and implementing them in core. In response, I published this which gives technical details of all the very real bugs Joomla has, the ones I have been reporting in private for eight years and nobody wants to even acknowledge. I know you were unaware so I won't begrudge you for telling me to test. I am just telling you why I can't and won't.

As long as the environment surrounding contributing to Joomla remains toxic I cannot be involved in reporting issues I encounter or file PRs to fix them unless I can also reproduce them with a third party extension. This is unlikely as I use very little extensions beyond mine. Since I am not aware of an extension other than mine that can be used to reproduce this issue I cannot in good conscience file a test, even though I can test it and find it to be working. Bummer.

@wilsonge I don't buy the “this thing touches too much” excuse. The BS4 to BS5 PR was touching FAR MORE code than this PR. The only difference is that HTML output can be understood by everyone and their dog whereas the OPcache requires people who understand PHP and can think beyond the HTML output.

I can only tell you the following in lieu of a test.

I have reproduced this issue on March 2nd, 2021 on my local server and three separate servers of my clients. I have confirmed that invalidating the OPcache for installed .php files solves this issue on my local server and three separate servers of my clients. FWIW the reproduction instructions for a local server is exactly what Phil told you.

Akeeba Backup 8 has for the last six (6) months been doing the same thing as this PR in its post-installation code. I go through all the files installed by Joomla and invalidate OPcache for them. I don't even check for restricted_api which is a reasonable check to add.

Phil's approach to restore.php is something I am going to use on Akeeba Kickstart.

As far as I am concerned, this is a valid issue and a valid fix. The issue has been reproduced and the fix confirmed on both local and live servers owned by different people. The PR uses the same technique I have been using in production for 6 months, meaning it's ran on tens of thousands of sites without a single ticket about it. This is extraordinary. With the installed base I have, extremely rare issues are guaranteed to get at least one to two tickets in any given 6 month period. Having none in six months means that, even though I am not checking for opcache.restricted_api, nothing broke on dozens of thousands of sites.

Take this as you like. I just can't and won't file an official test on this PR because politics would kill it. That's all.

@richard67 are you able to test specifically that the File::move in the script.php for the case sensitive file name works on the OS's you worked on please? I'll test OSX this evening too.

@richard67 are you able to test specifically that the File::move in the script.php for the case sensitive file name works on the OS's you worked on please? I'll test OSX this evening too.

I can only test if it works with normal opcache settings. Tesing with extreme settings like suggested by @PhilETaylor has failed for me in past.

But I can't promise I have the time today.

It's fine. I don't need extreme settings. Just some validation that across environments that the case sensitive rename doesn't break - because we had so many iterations of that script.

@wilsonge If it will not go into 4.0.0 today I assume it is for 4.0.1, right? At least that's my information here, so I won't hurry with tests.

@PhilETaylor I've allowed myself to update the branch of this PR to latest 4.0-dev to force a new package build by drone so people can test using the update package. The previous build for this PR was so long ago that the packages meanwhile have been deleted.

@wilsonge Test the files renaming on update on Linux (shared hosting with opcache enabled and working) for different cases (files with right and wrong case or only the ones with wrong case present). It worked. Windows I will test later. For testing exotic OS (Apple) it's your turn. You can use the update package built by drone. I've triggered new builds just before by a branch update.

I have tested this item ? unsuccessfully on 15ae8a2

On Linux all worked fine.

But on Windows, the filename casing fix fails with an internal server error and following in the PHP error log:

[21-Aug-2021 17:30:49 Europe/Berlin] PHP Fatal error:  Uncaught Error: Call to undefined method Joomla\CMS\Filesystem\File::move() in C:\xampp\htdocs\joomla-cms\administrator\components\com_admin\script.php:8163
Stack trace:
#0 C:\xampp\htdocs\joomla-cms\administrator\components\com_admin\script.php(7393): JoomlaInstallerScript->fixFilenameCasing()
#1 C:\xampp\htdocs\joomla-cms\administrator\components\com_joomlaupdate\restore_finalisation.php(75): JoomlaInstallerScript->deleteUnexistingFiles()
#2 C:\xampp\htdocs\joomla-cms\administrator\components\com_joomlaupdate\restore.php(5430): finalizeRestore('C:\\xampp\\htdocs...', 'C:/xampp/htdocs...')
#3 {main}
  thrown in C:\xampp\htdocs\joomla-cms\administrator\components\com_admin\script.php on line 8163

I have renamed the 2 files in 2 steps before updating current 3.10-dev to the patched package built by drone for this PR just today, so they looked as follows:

C:\xampp\htdocs\joomla-cms>dir /b libraries\src\Filesystem\Support\stringcontroller.php libraries\src\Form\Rule\subformrule.php
Stringcontroller.php
SubFormRule.php

C:\xampp\htdocs\joomla-cms>

I think (have to check) that it needs to add a function File::move() to restore_finalisation.php, too.

@PhilETaylor You have to add a move function to that class here: https://github.com/joomla/joomla-cms/blob/4.0-dev/administrator/components/com_joomlaupdate/restore_finalisation.php#L87-L128 . If you want I can do that and make a PR for your branch.

@PhilETaylor See PhilETaylor#10 for a fix.

Ta. Back home in 2 hrs

Hmm, now with the fix from my PR it works on the Windows host where it failed before, and it still works fine on my Linux shared hosting with and without opcache enabled.

But on my local Ubuntu Linux with PHP 7.4 and opcache disabled, updating from 3.10-dev to the patched update package from this PR fails.

I get a blank page soon after the update has started. In PHP error log I see:

[Sun Aug 22 01:07:46.583916 2021] [php7:error] [pid 74204] [client 192.168.98.1:63993] PHP Fatal error:  Uncaught RuntimeException: Library path /libraries/joomla cannot be found. in /home/richard/lamp/public_html/joomla-cms-3.10-dev/libraries/loader.php:344
Stack trace:
#0 /home/richard/lamp/public_html/joomla-cms-3.10-dev/libraries/loader.php(490): JLoader::registerPrefix()
#1 /home/richard/lamp/public_html/joomla-cms-3.10-dev/libraries/import.legacy.php(52): JLoader::setup()
#2 /home/richard/lamp/public_html/joomla-cms-3.10-dev/administrator/includes/framework.php(17): require_once('/home/richard/l...')
#3 /home/richard/lamp/public_html/joomla-cms-3.10-dev/administrator/index.php(40): require_once('/home/richard/l...')
#4 {main}
  thrown in /home/richard/lamp/public_html/joomla-cms-3.10-dev/libraries/loader.php on line 344, referer: https://www.joomla-310-dev.vmubu01.vmnet2.local/administrator/index.php?option=com_joomlaupdate&task=update.install&5bf0ac822cd9ac38a28f72a495a40408=1

Using a regular update to 4.0.0 works with the same update method (Live Update) on the same environment.

Will continue to investigate what the reason could be.

Update: Same environment, same kind of git clone just in another folder which has no dot in the path (i.e. Joomla root is /home/richard/lamp/public_html/test-1/ instead of /home/richard/lamp/public_html/joomla-cms-3.10-dev/: It works.

I wouldn't think it's related to this PR, but without this PR it works on the one with dot, too.

The PR is based on current 4.0-dev, the other test was using a 4.0.0 update package. Maybe something happened between 4.0.0 and now in the 4.0-dev branch which causes that problem? I don't think so, but I will check with an update package based on current 4.0-dev later, or tomorrow with a nightly.

Update: What I have forgotten to mention: When reloading the white page after that error occurred, the update runs and ends with success. The update log shows that all actions and all SQL statements of the update have been performed.

Update: The white page is shown after the first log entry in administrator/logs/joomla_update.php with message "Starting installation of new version." has been made, i.e. it happens at the very beginning of the update. After page reload the update continues. And as said, it doesn't happen with a regular nightly build update package from tonight, only with the package from this PR.

Hmm, right now it worked where I had the problem before ... so maybe my issue on that one environment is related to that environment ... possibly an issue with file or folder permissions or chmod privileges.

I have not tested this item.

I just see my previous, invalid test result is still counted despite of the later commits which fixed that error.

Setting to "not tested" as long as I'm investigating with the issue I have now with one of my testing environments.

Are you sure these issues are related to this PR, and not just problems with the upgrade process? ;-)

Are you sure these issues are related to this PR, and not just problems with the upgrade process? ;-)

@PhilETaylor If I was sure about anything yet, I would not investigate still ;-)

But on my local Ubuntu Linux with PHP 7.4 and opcache disabled, updating from 3.10-dev to the patched update package from this PR fails.

If opcache is disabled then that pretty much rules out this PR, as everything in this PR is pretty much reliant on opcache being enabled, and if not no harm should be caused :)

as everything in this PR is pretty much reliant on opcache being enabled, and if not no harm should be caused :)

@PhilETaylor Except of the changes where calls to native PHP functions have been replaced by calls to methods of the File and Folder classes of the Filesystem library and the latter include calls to Path::clean. Such changes may have an effect regardless of opcache.

Those methods have been used in Joomla for a long time though and I would be surprised if they had considerable bugs in them :)

Update: The white page is shown after the first log entry in administrator/logs/joomla_update.php with message "Starting installation of new version."

At this point the code running would be 3.10 right? without any of this PR contained in it? its not until after the restore/extraction that the Joomla 4 code is run containing my changes...

At this point the code running would be 3.10 right? without any of this PR contained in it? its not until after the restore/extraction that the Joomla 4 code is run containing my changes...

Im correct, this " "Starting installation of new version." log entry is before the restoration file is created, so way before my code is run for the first time.

try
		{
			Log::add(Text::_('COM_JOOMLAUPDATE_UPDATE_LOG_INSTALL'), Log::INFO, 'Update');
		}
		catch (\RuntimeException $exception)
		{
			// Informational log only
		}

		/** @var \Joomla\Component\Joomlaupdate\Administrator\Model\UpdateModel $model */
		$model = $this->getModel('Update');

		$file = $this->app->getUserState('com_joomlaupdate.file', null);
		$model->createRestorationFile($file);

@PhilETaylor Other question: The proxy function I have added with my PR PhilETaylor#10 here has only 2 parameters: https://github.com/joomla/joomla-cms/pull/32915/files#diff-d0a9b340bfb7e20bbd4f1c8603bc3efc4eeca71c8cd65b078a6ae4c166f3ad01R131

That's ok as long as in script.php it's used with only 2 parameters.

But the function which is proxied has 2 additional parameters: https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/Filesystem/File.php#L264

Shall we add them to the proxy? If so, what to do with the $useStreams? Shall we handle that somehow or just silently ignore it?

@PhilETaylor Regarding my issue: I think you are right and it's not related to your PR. I just want to solve it and finally check if everything works before I give your PR a good test.

Shall we add them to the proxy? If so, what to do with the $useStreams? Shall we handle that somehow or just silently ignore it?

Nics code doesnt use streams so its ok to ignore that. The restoration code, being standalone from Joomla on purpose, should be as compact, and featureless as possible. At the end of the day all it needs to be able to do is extract a zip file without erroring, and then return control to Joomla to cleanup.

We all know the restore.php is well overdue for a complete rewrite, but none of us are brave enough for that challenge, and maybe end up breaking Joomla update completely :)

Shall we add them to the proxy? If so, what to do with the $useStreams? Shall we handle that somehow or just silently ignore it?

Nics code doesnt use streams so its ok to ignore that. The restoration code, being standalone from Joomla on purpose, should be as compact, and featureless as possible. At the end of the day all it needs to be able to do is extract a zip file without erroring, and then return control to Joomla to cleanup.

@PhilETaylor So what do you finally suggest? Is https://github.com/joomla/joomla-cms/pull/32915/files#diff-d0a9b340bfb7e20bbd4f1c8603bc3efc4eeca71c8cd65b078a6ae4c166f3ad01R131 ok as it is now, or shall I add the other 2 parameters, too, handle the $path parameter right and ignore the $useStreams, in order to have the same signature and in case if someone in future wants call File::move in script.php with such parameters?

I would say leave it. Less we change less we can break. If we add the path then we also then need to clean the path and provide Path::clean. Its not needed that ever before.

if someone in future wants call File::move in script.php with such parameters

Then let them add them then at that point. File::move in the update process is not meant to be a fully featured API like the one in the Filesystem namespace. Its mean to be a minimised version that can run standalone to Joomla, for the purposes of extracting the zip file only.

@PhilETaylor My last issue was definitely caused by my testing environment. It could have been some sticky bit having been set by mistake on some folders, together with wrong group maybe. No idea how that could happen. Am just finishing a few tests for the installer. Looks good so far with opcache on and also with off.

Then let them add them then at that point. File::move in the update process is not meant to be a fully featured API like the one in the Filesystem namespace. Its mean to be a minimised version that can run standalone to Joomla, for the purposes of extracting the zip file only.

Agree.

There are also 20 copies of

/**
 * Akeeba Restore
 * A JSON-powered JPA, JPS and ZIP archive extraction library
 *
 * @copyright   2008-2017 Nicholas K. Dionysopoulos / Akeeba Ltd.
 * @license     GNU GPL v2 or - at your option - any later version
 * @package     akeebabackup
 * @subpackage  kickstart
 */

in this file... I think we could also trim that while still being compliant ;-)

I have tested this item ✅ successfully on 15ae8a2

Have tested on diverse environments:

• IONOS shared hosting on Linux with opcache enabled (file cache only)
• Linux without opcache enabled
• Windows without opcache enabled

The renaming of files with wrong case in script.php still works on Linux and Windows.

Installing, updating and uninstalling extensions works.

I'm still worried about certain thumbs down, maybe I've missed something. But as the author of the thumbs down will not explain the reason there is nothing I can do about that.

@wilsonge I've tested the rename stuff on Linux on Windows. Would be cool if you could check on IOS because that was the most weird one.

I'm still worried about certain thumbs down, maybe I've missed something. But as the author of the thumbs down will not explain the reason there is nothing I can do about that.

The thumbs down from my stalker were applied moments after this PR was first opened, it should be noted that this PR has been drastically improved since then though 44 revisions.

I'm still worried about certain thumbs down, maybe I've missed something. But as the author of the thumbs down will not explain the reason there is nothing I can do about that.

The thumbs down from my stalker were applied moments after this PR was first opened, it should be noted that this PR has been drastically improved since then though 44 revisions.

@PhilETaylor Yes, that's my hope, that the reasons for it were fixed on the way.

@PhilETaylor can we just now change the installation to use the Filesystem method in the PR please https://github.com/joomla/joomla-cms/blob/4.0-dev/installation/src/Model/CleanupModel.php#L33-L46

@wilsonge well ultimately that block of code just needs removing as Folder::delete in turn calls File::delete on all files in the folder, and the File::delete method is now "opcache aware" and invalidates the opcache before deleting the file.

Done.

Thanks!

@wilsonge wilsonge merged commit 268e492 into joomla:4.0-dev 23 seconds ago

/me runs and hides ....