[#35168] - Installation cleanup
- Fixed in Code Base
- 17 Aug 2021
- Medium
- Build: 4.0-dev
- # 35168
- Diff
- wilsonge:installation-cleanup
User tests: Successful: Unsuccessful:
Summary of Changes
Completing installation now deletes the installation directory "automatically" before continuing when not in development mode. When in development mode you continue to get the existing button.
This allows the user to delete installation rather than currently being stuck as the red button is already currently only shown in development mode. Massive shoutout to @zero-24 for last minute finding this one. Would have made for a very fast 4.0.1 otherwise :)
Testing Instructions
Actual result BEFORE applying this Pull Request
You can't delete the installation directory when the build is "stable"
Expected result AFTER applying this Pull Request
In development mode the installation directory is not deleted when clicking complete. Only when pressing the big red button. In stable mode it is only when you click either continue button the installation directory is deleted and you are allowed to proceed
- Go to the installer and change the
Joomla\Version\Version::DEV_STATUSto beStable. Ensure you can't see the red button but when clicking on complete the installation directory is removed before you are allowed to proceed - Go to the installer leaving yourself in dev mode. Ensure the red button is available and on clicking it you have the installation folder deleted. But when clicking on complete the redirect happens with no installation folder removal.
Documentation Changes Required
None.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | JavaScript Installation |
| Labels |
Added:
?
?
|
||
| Category | JavaScript Installation | ⇒ | External Library Composer Change JavaScript Installation |
| Labels |
Added:
Composer Dependency Changed
|
||
| Labels |
Added:
?
?
Removed: ? ? |
||
| Status | Pending | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-08-17 08:20:29 |
| Closed_By | ⇒ | wilsonge |
We'll fix drone this evening once work is done.
Well maybe @HLeithner https://github.com/HLeithner should have tested #32915 before release day!? The "race condition" is not a race condition, its the default timeout of 2 seconds for opcache and a developer that doesnt understand opcache.
Throwing in opcache_reset seconds before a major release is a major step back!
On 17 Aug 2021, at 09:20, George Wilson @.***> wrote:
@wilsonge commented on this pull request.
In installation/src/Model/CleanupModel.php #35168 (comment):
@@ -38,6 +41,12 @@ public function deleteInstallationFolder()
$return = File::move(JPATH_ROOT . '/robots.txt.dist', JPATH_ROOT . '/robots.txt');
}
if (function_exists('opcache_reset')) {\opcache_reset();OK So concretely @HLeithner https://github.com/HLeithner on his server gets race conditions without this line. And opcache invalidating that line also doesn't seem to solve it. Unclear right now if opcache just takes a bit longer so solves the race condition or there's some concrete cache being solved. But we'll go with this for 4.0.0 and then we can spend some time and figure out if we're just masking the issue or not for 4.0.1. Again fully agree that we need #32915 #32915 - just it doesn't apply to this specific case.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub #35168 (comment), or unsubscribe https://github.com/notifications/unsubscribe-auth/AADBVXB35XDLEZD46KBPHGTT5ILT5ANCNFSM5CJIAOPQ.
@wilsonge I think @PhilETaylor is right here. Should he make a PR to change from "opcache_reset" to something like here for the "installation/index.php" file?
I have tested this item✅ successfully on cbdd6d0
I downloaded from here: https://ci.joomla.org/artifacts/joomla/joomla-cms/4.0-dev/35168/downloads/46699/
Tested folder in dev.
Then changed the following to act like stable (thanks @zero-24):
libraries\src\Version.php
const EXTRA_VERSION = '';
const DEV_STATUS = 'stable';
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35168.