Thanks for all the great work on this module, and the appserver as well. Really looking forward to using these to their full potential.
My only complaint/issue is with the install method(s) and how they affect working on local installs and/or on servers without FTP or SSH being needed. Without the SSH module being installed, leaving only FTP option with a great majority of sites not running over https, so sending insecure data. It is also (obviously) horrible to have to chmod 777 the entire sites directory. This issue (#1479164: Method for checking writability of server) is at least better, but it'd be great if we set this to check for a "special" directory inside sites/all/modules (like sites/all/modules/apps) and push all apps in there much like drush looks for the contrib folder when downloading projects and puts them there if it exists, otherwise, falls back to the default sites/all/modules.
I've ran a few tests, and have a working model that is (as far as I can see) getting closer to making the file move easier after an app(s) is downloaded to the temp directory without modifying permissions on the directory. This sample was taken from the Omega Tools module (http://drupal.org/project/omega_tools) where via the UI, a user can create a new Omega subtheme based on any provided starterkits. The process allows them to customize their starterkit a bit, then it will (upon settings during creation) automatically install and enable the theme, and optionally set it as default. What happens is all this gets written to the tmp directory, and at the end, it attempts to write the file to the appropriate location (sites/all/themes) and if issues arise that don't allow it to be written, the user is prompted to download the package, and told where to place it so it will be able to be enabled for their site.
If this can be proven to be a valid step towards something useful, I'll work on/help with a patch to be reviewed. I'm not 100% sure this is the best solution that we've used in Omega Tools, however, relying on the Drupal update manager, which has a whole bunch of issues (IMHO) I think should be the fallback (much like it is now if the sites directory isn't writable) but the check for writability is proving a bit out of whack.
The following code was able to at a VERY basic level get the install process working without any immediate issues.
In apps.installer.inc copied a couple functions from Omega Tools:
function apps_copy_recursive($source, $destination) {
if (is_dir($source)) {
if (!file_prepare_directory($destination, FILE_CREATE_DIRECTORY)) {
return FALSE;
}
$directory = dir($source);
while (FALSE !== ($read = $directory->read())) {
if ($read != '.' && $read != '..' ) {
if (!apps_copy_recursive($source . '/' . $read, $destination . '/' . $read)) {
return FALSE;
}
}
}
$directory->close();
}
else {
file_unmanaged_copy($source, $destination);
}
return TRUE;
}
/**
* @todo
*/
function apps_move($source, $destination) {
if (apps_copy_recursive($source, $destination)) {
file_unmanaged_delete_recursive($source);
return $destination;
}
return FALSE;
}
Now, my current "test" takes out the batch functionality as I'm only testing with a single app at the moment, and wanted to deal with as simplistic of a test as I could. So instead of:
if (is_writeable(conf_path())) {
module_load_include('inc', 'update', 'update.authorize');
$filetransfer = new FileTransferLocal(DRUPAL_ROOT);
apps_run_install($filetransfer, $updates); //this is our change
unset($_SESSION['update_manager_update_projects']);
}
else {
// Set the $_SESSION variables so that authorize form knows what to do after authorization.
system_authorized_init('apps_run_install', drupal_get_path('module', 'apps') . '/apps.installer.inc', array($updates), t('Update manager'));
require_once DRUPAL_ROOT . '/includes/authorize.inc';
// Get the authorize form.
$form = drupal_get_form('authorize_filetransfer_form');
return $form;
}
I've commented all that out (completely disabling the fallback for now) and created the following (please note I also added in a destination that would put any apps in sites/all/modules/apps
$destination = DRUPAL_ROOT . '/sites/all/modules/apps/';
foreach($updates AS $update) {
$result = apps_move($update['local_url'], $destination . $update['project']);
}
This seems to work... it doesn't (yet) provide the fallback to either possibly download the source, or fall back to the FTP/SSH options provided by updater.
So, the current code is_writeable(conf_path()) fails and says it's unwritable on my system(s) whereas file_prepare_directory and file_unmanaged_copy work just fine on the same system with the same permissions.
I hope someone will take some time to look at this and enlighten me if this is a 100% horrible approach, or if we can move forward with a better way to make this work as the "default" option... I know Omega Tools implementation potentially has some issues, but it has worked in a HUGE number of installs, and is used on a daily basis. As it is now, the limitations in the apps download system make using this a huge barrier for those setting up sites/installs that are the "end user" and way less technical than those of us developers.
Comments
Comment #1
jec006 commentedThis looks very promising, and I would love to see some work done on a patch - using the functions provided by drupal for managing and testing the writability of directories would be preferable especially since it appears more robust. I think providing a download option is also an interesting idea, though might be a bit outside of the goals for apps - if the user will have to place the module(s) anyway, apps seems to lose some of its appeal (imho).
Comment #2
nedjoYes, file access issues are a significant barrier and it's worth exploring all options here.
We need to keep in mind though that we need to handle libraries as well as modules, which raise similar write permission issues. Libraries won't work if nested in a 'sites/all/libraries/apps' subdirectory.
Comment #3
febbraro commentedThe app manifest does have libraries categorized separately so there is a good opportunity to handle them differently.
So in the ideal world all modules would be downloaded into sites/all/modules/apps? (or maybe in the future sites/[sitename]/modules/apps) while all libraries are downloaded to sites/all/libraries (or maybe in the future sites/[sitename]/libraries. It would use the unmanaged mechanism from above to allow for more flexible permissions, but still fall back to the core Updater for FTP and SSH installation?
Currently themes are slated to be handled as being part of an app (within the module) so no need to special theme handling? or do we need the ability to include external themes in Apps as well. An example might be having an Admin Tools app include the Rubik module.
Comment #4
hefox commentedDownload location is now configurable, so I thinkkk that covers this?