Activity

Starting from 2.3, the new amazing DnD has added new files to the long chain of files loaded per request; among these new ones there is repository/lib.php:course/dnduploadlib.php

* Library to handle drag and drop course uploads

...

require_once($CFG->dirroot.'/repository/lib.php');

...

Under some configurations - tipically running under $CFG->reverseproxy - I'm having HTTP 500 issues due to the incorrect resolving of the supposed path for the config.php requested by repository/lib.php.# strace -ttfp 3132

and while Moodle seems to run happily, my code is working too .
Moodle seems to work fine without that line 'cause that library is always included when config.php i.e. $CFG has been already loaded:$ find /var/www/moodle-master/ -type f -name lib.php |xargs grep repository/lib\.php

I'm not sure about such a deletion - that's the reason why this issue has been filed as an improvement a not as a bug - BUT looking at the overall libraries code design this is mostly a small but required cleanup/improvement, if confirmed, - please note - regardless it will resolve an issue for my specific use case.

Frédéric Massart
added a comment - 16/Oct/12 2:08 PM Thanks for your patch and feedback Matteo. I think it is safe to remove the inclusion but only in master. There could be some plugins which rely on this inclusion and we do not want to break them.
I have kept your patch and added a commit on top of it to remove the inclusion from locallib.php (nice spotting!) and to add the `defined('MOODLE_INTERNAL') || die();` statement.
Many thanks!
Fred

Hi Fred,
The changes look good.
However if we are cleaning this up, there are a whole bunch of such usage which needs to be reviewed. A simple grep will show you we have widespread use of this. This needs to be taken care of as well, you can update this issues description and do it in this one or feel free to create a new one.
Thanks

Ankit Agarwal
added a comment - 16/Oct/12 2:35 PM Hi Fred,
The changes look good.
However if we are cleaning this up, there are a whole bunch of such usage which needs to be reviewed. A simple grep will show you we have widespread use of this. This needs to be taken care of as well, you can update this issues description and do it in this one or feel free to create a new one.
Thanks

Thanks Ankit! I'd rather not take care of those in the scope of this issue, the main reason being that I know this area a little bit and really the other ones. I will raise an issue to take care of it. Thanks!

Edit: Actually decided not to create another issue about it because I don't have any specific file to point to, and each file resulting in the following command should be manually checked:

Frédéric Massart
added a comment - 17/Oct/12 3:13 PM - edited Thanks Ankit! I'd rather not take care of those in the scope of this issue, the main reason being that I know this area a little bit and really the other ones. I will raise an issue to take care of it. Thanks!
Edit: Actually decided not to create another issue about it because I don't have any specific file to point to, and each file resulting in the following command should be manually checked:
find . -iname "*.php" -print0 | xargs -0 grep -E '(require|include).*config\.php'

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

Aparup Banerjee
added a comment - 19/Oct/12 9:50 AM The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.
TIA and ciao