modify file_scan_directory to include a regex for the nomask.
tayknight - July 20, 2006 - 14:20
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | file system |
| Category: | feature request |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed |
Description
I need a way for file_scan_directory's nomask parameter to accept a regex. The mask parameter accepts a regex, so it made sense to me that the nomask should as well.
This patch patches install.php includes/file.inc, includes/install.inc, and modules/system/system.module. Before the patch file_scan_directory is called with the defualt:
array('.', '..', 'CVS')
The same idea can be passed to the modified file_scan_directory with:
'/(\.\.?|CVS)$/'
| Attachment | Size |
|---|---|
| fsd.patch | 4.51 KB |
#1
I realized after I made the original patch that I was having functions paratemers that were mixing ereg and preg_match.
This is the patch re-rolled so the functino only uses ereg(). preg_match might be faster, but ereg'd regular expressions were being passed before the patch. I could patch core to pass preg_match'ed regular expressions, but I bet that breaks more contrib modules.
#2
like the idea, but it's do for a re-roll
#3
#4
Rerolled patch for 7.x which I tested by installing drupal7 with this patch applied.
Installation was successfull.
#5
I cannot seem to be able to upload a patch file. I browser my file (called something.patch) and click on "Attach" button, wait that the file is uploaded. I leave the mark in the "List" checkbox and then click on "Post Comment" button but still nothing!
Here is the patch (I'll try again to upload)
Index: install.php
===================================================================
RCS file: /cvs/drupal/drupal/install.php,v
retrieving revision 1.119
diff -u -p -r1.119 install.php
--- install.php 26 May 2008 17:12:54 -0000 1.119
+++ install.php 15 Jun 2008 03:40:16 -0000
@@ -416,7 +416,7 @@ function install_settings_form_submit($f
* Find all .profile files.
*/
function install_find_profiles() {
- return file_scan_directory('./profiles', '\.profile$', array('.', '..', 'CVS'), 0, TRUE, 'name', 0);
+ return file_scan_directory('./profiles', '\.profile$', '(\.\.?|CVS)$', 0, TRUE, 'name', 0);
}
/**
@@ -483,7 +483,7 @@ function install_select_profile_form(&$f
* Find all .po files for the current profile.
*/
function install_find_locales($profilename) {
- $locales = file_scan_directory('./profiles/' . $profilename . '/translations', '\.po$', array('.', '..', 'CVS'), 0, FALSE);
+ $locales = file_scan_directory('./profiles/' . $profilename . '/translations', '\.po$', '(\.\.?|CVS)$', 0, FALSE);
array_unshift($locales, (object) array('name' => 'en'));
return $locales;
}
Index: includes/common.inc
===================================================================
RCS file: /cvs/drupal/drupal/includes/common.inc,v
retrieving revision 1.771
diff -u -p -r1.771 common.inc
--- includes/common.inc 9 Jun 2008 08:11:44 -0000 1.771
+++ includes/common.inc 15 Jun 2008 03:40:17 -0000
@@ -1887,7 +1887,7 @@ function _drupal_load_stylesheet($matche
* Delete all cached CSS files.
*/
function drupal_clear_css_cache() {
- file_scan_directory(file_create_path('css'), '.*', array('.', '..', 'CVS'), 'file_delete', TRUE);
+ file_scan_directory(file_create_path('css'), '.*', '(\.\.?|CVS)$', 'file_delete', TRUE);
}
/**
@@ -2255,7 +2255,7 @@ function drupal_build_js_cache($files, $
* Delete all cached JS files.
*/
function drupal_clear_js_cache() {
- file_scan_directory(file_create_path('js'), '.*', array('.', '..', 'CVS'), 'file_delete', TRUE);
+ file_scan_directory(file_create_path('js'), '.*', '(\.\.?|CVS)$', 'file_delete', TRUE);
variable_set('javascript_parsed', array());
}
@@ -2623,7 +2623,7 @@ function drupal_system_listing($mask, $d
// Get current list of items
foreach ($searchdir as $dir) {
- $files = array_merge($files, file_scan_directory($dir, $mask, array('.', '..', 'CVS'), 0, TRUE, $key, $min_depth));
+ $files = array_merge($files, file_scan_directory($dir, $mask, '(\.\.?|CVS)$', 0, TRUE, $key, $min_depth));
}
return $files;
Index: includes/file.inc
===================================================================
RCS file: /cvs/drupal/drupal/includes/file.inc,v
retrieving revision 1.125
diff -u -p -r1.125 file.inc
--- includes/file.inc 26 May 2008 17:12:54 -0000 1.125
+++ includes/file.inc 15 Jun 2008 03:40:17 -0000
@@ -896,7 +896,7 @@ function file_download() {
* @param $mask
* The regular expression of the files to find.
* @param $nomask
- * An array of files/directories to ignore.
+ * The regular expression of the files to ignore.
* @param $callback
* The callback function to call for each match.
* @param $recurse
@@ -917,13 +917,13 @@ function file_download() {
* "path", "basename", and "name" members corresponding to the
* matching files.
*/
-function file_scan_directory($dir, $mask, $nomask = array('.', '..', 'CVS'), $callback = 0, $recurse = TRUE, $key = 'filename', $min_depth = 0, $depth = 0) {
+function file_scan_directory($dir, $mask, $nomask = '(\.\.?|CVS)$', $callback = 0, $recurse = TRUE, $key = 'filename', $min_depth = 0, $depth = 0) {
$key = (in_array($key, array('filename', 'basename', 'name')) ? $key : 'filename');
$files = array();
if (is_dir($dir) && $handle = opendir($dir)) {
while (false !== ($file = readdir($handle))) {
- if (!in_array($file, $nomask) && $file[0] != '.') {
+ if (!ereg($nomask, $file) && $file[0] != '.') {
if (is_dir("$dir/$file") && $recurse) {
// Give priority to files in this folder by merging them in after any subdirectory files.
$files = array_merge(file_scan_directory("$dir/$file", $mask, $nomask, $callback, $recurse, $key, $min_depth, $depth + 1), $files);
Index: includes/locale.inc
===================================================================
RCS file: /cvs/drupal/drupal/includes/locale.inc,v
retrieving revision 1.176
diff -u -p -r1.176 locale.inc
--- includes/locale.inc 26 May 2008 17:12:54 -0000 1.176
+++ includes/locale.inc 15 Jun 2008 03:40:17 -0000
@@ -2453,7 +2453,7 @@ function locale_batch_by_language($langc
// with names ending with $langcode.po. This allows for filenames
// like node-module.de.po to let translators use small files and
// be able to import in smaller chunks.
- $files = array_merge($files, file_scan_directory(dirname($component->filename) . '/translations', '(^|\.)' . $langcode . '\.po$', array('.', '..', 'CVS'), 0, FALSE));
+ $files = array_merge($files, file_scan_directory(dirname($component->filename) . '/translations', '(^|\.)' . $langcode . '\.po$', '(\.\.?|CVS)$', 0, FALSE));
$components[] = $component->name;
}
@@ -2485,7 +2485,7 @@ function locale_batch_by_component($comp
// as $langcode.po or with names ending with $langcode.po. This allows
// for filenames like node-module.de.po to let translators use small
// files and be able to import in smaller chunks.
- $files = array_merge($files, file_scan_directory(dirname($component->filename) . '/translations', '(^|\.)(' . $language_list . ')\.po$', array('.', '..', 'CVS'), 0, FALSE));
+ $files = array_merge($files, file_scan_directory(dirname($component->filename) . '/translations', '(^|\.)(' . $language_list . ')\.po$','(\.\.?|CVS)$' , 0, FALSE));
}
}
return _locale_batch_build($files, $finished);
#6
Patch still applies with some offset (the patch below is identical to #5, except with the offset fixed), tested and work. RTBC.
Testing
System: 257 passes, 0 fails, 0 exceptions
Bootstrap: 8 passes, 0 fails, 0 exceptions
Robin
#7
We should probably be using preg_match: #64967: Replace ereg with preg
#8
#64967: Replace ereg with preg got committed. so this should definitely use preg_match.
#9
using a preg_match() style regular expression. added a test.
#10
#11
#12
re-rolled.
#13
I fixed one typo and committed it to CVS HEAD. I'm marking this 'code needs work' until the module upgrade instructions have been updated. Thanks!
#14
#15
I had to rollback this patch because it breaks the installer. It didn't break any of the simpletests though. More proof that we should have a simpletest for the installer. Make this patch could be a driver for that.
#16
Big +1, yes. This wouldn't have slipped through with an installation test.
Problem appears to be that the installer still tries to pass an array to the function, which treats it as an expression.
Question would be: Do we use is_array() to allow both regular expressions and arrays, or switch to regular expressions entirely in the calling functions?
#17
boombatower is working on abstracting the testing.drupal.org install script into a core patch - it's hard to test the installer at the moment, because we don't let simpletest run it for security reasons.
However, the patch broke testing.drupal.org - and boombatower had to switch off the first test of reporting back as CNW due to it. So when that's actually switched on, it'd catch issues like this for now.
#18
Arancaytar, i'm generally opposed to overloaded parameters. and having one parameter be only regular expressions and the other array or regular expression seems really annoying.
looks like the tests didn't get rolled back so here's a re-roll that fixes the install.php file.
#19
I just backed out the tests since they were breaking SimpleTest. So this one needs to be re-rolled with the tests again.
#20
okay, here's a re-roll that adds the tests back in and the fix for the install.php file.
#21
The last submitted patch failed testing.
#22
re-rolling.
#23
I think you might have been mixing patches here.
// changelog+ * Drupal core no longer requires CREATE TEMPORARY TABLES or LOCK TABLES
+ database rights.
Shouldn't belong in the file.inc patch, unless they are related in a way I don't understand. ;)
#24
not sure how that got in there. re-rolled without it.
#25
The last submitted patch failed testing.
#26
Committed to CVS HEAD along with a code clean-up. Thanks!
#27
The patch that went in missed an instance of file_scan_directory() in install.inc, which still contains an array instead of an regexp.
It breaks installation entirely. Attached patch to fix this.
#28
warnings on install read "preg_match() expects parameter 1 to be string, array given"
This patch brings me through installation.
#29
#30
Thanks, committed. We'll need to re-submit a bunch of other patches for re-testing.
#31
Oh, also, docs please! :D
#32
Isn't it hilarious that after all the work that went into automated unit tests, it only means that if we do break HEAD, we get to spend hours submitting re-tests for hundreds of patches?
Okay, I suppose it's not that hilarious when you're the one who has to do that.
#33
For documentation purposes:
This bug in HEAD has caused the testing bot to report a large number of failed to install HEAD.
#34
http://drupal.org/node/224333#file_scan_directory_nomatch
sorry for all the fun with the test bot... seems like it might be a good thing to have it watch for commits and run the tests against a clean copy of the code before testing patches.
#35
Yep. See #335122: Test clean HEAD after every commit. Jimmy tells me he has some code written for this, ready to go as early as tomorrow, which is great. :)
#36
Automatically closed -- issue fixed for two weeks with no activity.