I do NOT have PHP 5.3 to 7.1 inclusive installed on any of my test servers. I need people to test on those versions.

What needs to be tested for these versions is that updating from a 3.10.x which includes this PR to a later 3.10.y version which also includes this PR will work.

Updates to 4.x can't be tested with these versions because 4 requires 7.2.5 as minimum.

Updates to 4.1 have to be tested with PHP 7.2 to 8.1. For these versions it also would need tests if updating from a 3.10.x which includes this PR to a later 3.10.y version which also includes this PR will work.

I wish we had system tests for automated testing of updating. (not related to this PR but in general)

I wish we had system tests for automated testing of updating. (not related to this PR but in general)

There was a PR from me for exactly that but it did not work out as i planed it so i closed it. Maybe it worth to start another try on this with Hannes and Harald. But that is far out of scope for this PR for sure.

What needs to be tested for these versions is that updating from a 3.10.x which includes this PR to a later 3.10.y version which also includes this PR will work.

You are right. I wrote this before my first cup of coffee for the day.

I wish we had system tests for automated testing of updating. (not related to this PR but in general)

It is theoretically possible with integration tests. However, possible and practical live on different planets and speak different languages. Even if you disregard how sensitive these test are to changes in the testing framework or even the DOM of the output you still have to contend with their dependence on the system environment. If you try to address that with VMs, containers or whatever you have a monumental maintenance task. In the end of the day it is cheaper and more productive risking the occasional issue and quick follow up fix. Multi-trillion dollar companies do that, judging from the rollout of every major OS version the past 7 or 8 years...

I wish we had system tests for automated testing of updating. (not related to this PR but in general)

It is theoretically possible with integration tests. However, possible and practical live on different planets and speak different languages. Even if you disregard how sensitive these test are to changes in the testing framework or even the DOM of the output you still have to contend with their dependence on the system environment. If you try to address that with VMs, containers or whatever you have a monumental maintenance task. In the end of the day it is cheaper and more productive risking the occasional issue and quick follow up fix. Multi-trillion dollar companies do that, judging from the rollout of every major OS version the past 7 or 8 years...

Yes, I know ... was only dreaming the "what if we had unlimited resources" dream for a moment.

Ok I have done now the following tests without any issues:

First Test

• PHP 7.4.7
• Windows xampp
• 3.10.4 -> 3.10.5 (using the default update server)
• Before the update I have applied the patch attached here to 3.10.4.

Second Test

• PHP 5.3.8 (with changed php mimimum as it looks like thats the last 5.3.x xampp :D)
• Windows xampp
• 3.10.4 -> 3.10.5 (using the package upload of an prepared package with 5.3.8 support + the restore.php)
• Before the update I have applied the patch attached here to 3.10.4.

Third Test

• PHP 7.4.7
• Windows xampp
• 3.10.5 -> 4.1.0-dev (using the nighly update server)
• Before the update I have applied the patch attached here to 3.10.5.

All setups are with default settings of xampp and without explicite opache settings set.

Closing notes

Once this PR can be moved to the ready to review status and we will get the patched 3.10 packages and I will repeat the tests on 7.x with said packages. @nikosdion Is there anything left to do or can we set this PR here "ready for review"?

Ah looks like even on draft PRs we get the packages, didnt noticed :D But anyway once this here is ready I think we can move it to be ready for review.

I have just tested successul with the said package too:

• PHP 7.4.7
• Windows xampp
• 3.10.4 -> 3.10.6-dev (using the pre build package update server)
• Before the update I have NOT applied the patch attached here to test the "real world" case too.
• After the package is installed I have run a "reinstall core files" which also tests two times with the same restore.php file.

I have converted the draft PR to a regular PR since it looks like we've all got it working.

Let me say this here instead of individual comments.

DO NOT BLOODY NITPICK THE CODE FOR CODE STYLE, COMMENTS AND STUFF LIKE THAT. This is restore.php, the Akeeba Restore file which is third party as far as Joomla is concerned. The version you see here is the slightly modified one we had in the early Joomla 4.0 versions. The only REAL difference is that it supports OPcache.

I changed comments FOR CLARITY. Because you are all still making changes to an obsolete version of Akeeba Restore from 7 years ago and I don't want you to do something stupid.

Well, you know what, Brian? If we're going to nitpick I'm closing this PR. Good luck fixing your issue. I've got better things to do with my time.

@nikosdion I would like to keep this PR as it is can you please re-open this PR here.

@zero-24 As long as you're happy making any edits to the code without me, sure. I enabled the “Allow edits by maintainers” feature on GitHub.

Its not MY issue its OUR issue

/me out of here

Thanks. yes will keep this as it is as you correctly noted Akeeba Restore file which is third party as far as Joomla is concerned.
We also do not apply codestlye or typo fixes to other third party code.

When we would need to fix a bug or the thing that richard found and confirmed by you yes but all other cosmetic changes will be kept out of this PR.

@brianteeman "You" = Joomla. Not you as in Brian. The misunderstanding comes from the fact that "you" singular and "you" plural is the same word in English. If that was German I'd be using “Sie”, not “du”, if it was French I'd be using “vous”, not “tu”, in Greek I'd be using «εσείς», not «εσύ». That's all the languages I speak but hopefully you get the idea as IIRC Hebrew also uses different words.

What I meant is that if you don't like my approach of using the same file used in Joomla 4.0 with a. changes to make sure it still runs on PHP 5.3 (instead of 7.2+) and b. some changed comments on the code which contains the changes pertaining to OPcache handling for better clarity then any of you guys and gals who contribute code to Joomla could propose a different solution and make a PR.

Anyway, the point is kinda moot now.

This PR is one of a very few which I have on my task list for testing, but I was and still am very busy with private stuff and my paid job. I will try my best to test it on Saturday, or before when I find time.

I have tested this item ✅ successfully on 37c9043

I've tested following scenarios on 2 different environments (PHP 7.4. and 8.0) with opcache enabled. On one of them I could reproduce the issue, on the other one not.

1. Update 3.10-dev (or 3.10.5) without the patch of this PR to the patched package of this PR
2. Update 3.10-dev (or 3.10.5) with the patch of this PR to the patched package of this PR
3. Update 3.10-dev (or 3.10.5) with the patch of this PR to the latest 4.10-dev nightly build

   ---

   _{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36878.}

Merging here thanks!

@zero-24 @nikosdion I don't know what to say, but today I've tested updating from 3.10-dev (which includes this merged PR) to latest 4.1 nightly, and in some case the issue #36833 was back. When I had tested this PR here yesterday that did not happen.

I haven't changed anything on my environment since yesterday, and I haven't found yet any systematic in when it fails and when not.

I'm asking myself now if I just was "lucky" when testing yesterday (which in fact would be unlucky because the issue not having appeared).

What shall we do? Shall I re-open issue #36833 ? Can anybody re-produce the issue still?

And shall we re-activate @dgrammatiko 's PR #36841 ?

P.S. I don't think we should revert this PR here if my finding turns out to be right, because I think the PR made the issue at least appear less likely. But maybe we missed something, or maybe we need #36841 in addition.

I don't have an option to reopen #36841 so either Tobias can resurrect it or someone should recreate it (I could do that, it's a simple copy paste).

We can always add the nuclear option, opcache_reset at the very end of the extraction. As a result, when calling finalisation.php the OPcache will be completely reset for the entire server. This sucks for shared hosting but if there is even a remote chance that invalidating a file doesn't clear it from the OPcache I'd say screw this, let's do this.

Adding a delay of an arbitrary length, e.g. 6 seconds, won't really fix anything. What if the server does not revalidate files? What if the revalidate frequency is, dunno, 600 seconds? Sure, it'd fix your environment because the delay of 6 seconds is longer than the default revalidate frequency of 2 seconds and would a 3 second delay: it would only fix your environment or any environment matching the defaults. Surely, we can't have an infinite delay to take into account the worst case scenario, right?

@dgrammatiko I was able to re-open your PR, and the branch is also still there. But after my experience with testing this PR here, how can I be sure that it will not be the same with your PR? Today it works for me and tomorrow not? We need more people who can reproduce the issue so we can exclude problems with environments.

If the nuclear option fixes it then let’s do it. But we need more testers who can replicate the issue.

I will try to test both tomorrow ( I just need to remember to pick my old laptop to the office)

At the moment my self-confidence as a tester is close to zero.

Don't feel bad: cache invalidation is one of the hardest things in programming

Thanks @dgrammatiko and @richard67

@nikosdion when we would go with the nuclear option where would you suggest to put the opcache_reset call?

I share your thougths on the adding the delay to the JS.

Another additional thing could be take down what has been done in J4: https://github.com/joomla/joomla-cms/pull/32915/files
Phil suggested to use this method within restore.php, what do you think about that? #36841 (comment)

My problem is I have never been able to reproduce this issue myself so its hard for me to validate a possible solution.

A short term thing could be to go with opcache_reset for 3.10.6 so 3.10.6->4.1 upgrades do not fail and do a more balanced solution based on the PRs done for J4 with J3.10.7?

So, I am looking at what we were doing in restore.php in older Joomla 4 versions and it looks like we would only invalidate a file in the OPcache when it's deleted. Huh. Same thing for Joomla 3. It looks like the previous PRs to address this issue — before I wrote extract.php specifically for Joomla 4 — were incomplete.

However, there's a gotcha with PHP 7 and 8. OPcache offers both in-memory caching and file caching, the latter controlled by the opcache.file_cache PHP configuration option. Both opcache_invalidate and opcache_reset operate on the in-memory cache only. There is zero information in PHP's manual as to whether the file cache will or can be reset (the implication seems to be it's not possible). So that's one case where the update will fail and there's nothing we can do about it... except add a short delay and hope for the best.

In any case, I have updated this PR's branch to invalidate the in-memory cache for files being written. I would say that after all we will need BOTH this change AND the delay ☹️

Do we need these 2 changes (invalidate the in-memory cache for files being written and the delay in the JS) also for 4.x (i.e. the 4.10-dev branch, e.g. for later updates to 5)?

@richard67 Nope, that's already taken care of in extract.php except for the delay. About that, I wouldn't even bother. We don't make this kind of massive architectural changes in minor releases and when it comes to real world servers there is enough delay from the network that I have not seen a real world issue across millions of backup restorations in the 8 years since PHP 7 (with file cache) came out. I only said let's add the delay in the 3 to 4 upgrade because it's too late in the game to warn people of the opcache pitfalls.

For 4 to 5 I would put a warning that opcache should be DISABLED during the upgrade since there's sod all we can reasonably do to catch all use cases. I mean, it should stand to reason that you only cache compiled code when you don't expect it to change. And yet...

In any case, I have updated this PR's branch to invalidate the in-memory cache for files being written. I would say that after all we will need BOTH this change AND the delay ☹️

Can you do a new PR based on that branch? As I merged this PR here already into 3.10-dev your newst changes do not show up ;)