Force alphabetical ordering of app installation to fix on osx 10.13#29142
Conversation
|
Now what did I |
| while( false !== ( $filename = readdir( $dir ))) { | ||
| $nodes = scandir($dir); | ||
| foreach($nodes as $filename) { | ||
| if( substr( $filename, 0, 1 ) != '.' and is_dir($app_dir['path']."/$filename") ) { |
There was a problem hiding this comment.
This is also moderately dodgy. There is no check for the parent dir entry .. - I think it will pass this "if" test.
The test on the next line will (thankfully) be false, because some file like apps/../appinfo/info.xml is not going to exist.
But not directly relevant to the reason for this PR.
There was a problem hiding this comment.
.. is caught by the first condition, since it is checking only the first character of the filename
There was a problem hiding this comment.
Oh yes! I skimmed over the detail too quickly. And I guess in this context we do not care if there is a folder ".hidden-stuff" that is skipped over - that should not have an app in it.
|
just grepping for other areas this may effect |
|
diff --git a/tests/enable_all.php b/tests/enable_all.php
index 11b983eb78..67de8c04ff 100644
--- a/tests/enable_all.php
+++ b/tests/enable_all.php
@@ -16,6 +16,7 @@ function enableApp($app) {
}
}
+enableApp('dav');
enableApp('files_sharing');
enableApp('files_trashbin');
enableApp('encryption');fixes - but why is this now a problem |
because the load order of apps has changed? we might simply got lucky in the past because dav sorts up pretty hight .... no idea |
my brain was avoiding this theory 😒 digging... |
|
On my Ubuntu 16.04 with PHP 7.0.22 the output is: so the old way is "random order" on *nix also. |
|
Just be lucky that your |
tomneedham
force-pushed
the
fix-install-app-order
branch
from
f455b35 to
7f45658
Compare
|
Could this explain the wrong migration orders in some setups or is this unrelated ? Ref #28285 |
I thought so - but they are explicitly sorted by their numbers before being applied. But could still be related - as apps are installing in different orders which could cause weird effects when they are trying to apply schema changes based on what is present |
|
@tomneedham your attempt at making the universe deterministic has failed: |
tomneedham
force-pushed
the
fix-install-app-order
branch
from
0488a7e to
d54b938
Compare
|
Explicitly setting files, dav as the first two apps seems to work - otherwise somehow dav gets installed without the migrations running 😟 |
PVince81
left a comment
PVince81
left a comment
There was a problem hiding this comment.
👍, assuming there is no adjustable unit test due to staticness.
Not happy about the hard-coded bits but there is no better way. At least app loading will be deterministic.
Likewise - but at least it is stable |
|
@tom please backport to stable10 |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
5 participants
Description
Well this was an interesting one to find. PHP
readdirreturns in order that the filesystem stores a directory - which is not necessarily alphabetical ordering. However, apparently we require this ordering, since apps with namesfiles_typically rely onfilesto already be installed.Since updating to OSX High Sierra 10.13, they now use a new 'optimised filesystem' which appears to affect the listing order. This caused the install to fail because trashbin was installed before files and there was an autoloading exception.
The fix forces the installer to use an alphabetical ordering when listing the contents of app folders.
Motivation and Context
Install of core fails since OSX High Sierra upgrade.
How Has This Been Tested?
UI and OCC installs on OSX High Sierra.
Types of changes
Checklist: