Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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)$/'
Comment | File | Size | Author |
---|---|---|---|
#27 | 74645_missed_file_scan_directory.patch | 899 bytes | maartenvg |
#24 | file_74645.patch | 7.47 KB | drewish |
#22 | file_74645.patch | 8.95 KB | drewish |
#20 | file_74645.patch | 8.53 KB | drewish |
#18 | file_74645.patch | 7 KB | drewish |
Comments
Comment #1
tayknight CreditAttribution: tayknight commentedI 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.
Comment #2
dmitrig01 CreditAttribution: dmitrig01 commentedlike the idea, but it's do for a re-roll
Comment #3
drewish CreditAttribution: drewish commentedComment #4
roychri CreditAttribution: roychri commentedRerolled patch for 7.x which I tested by installing drupal7 with this patch applied.
Installation was successfull.
Comment #5
roychri CreditAttribution: roychri commentedI 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)
Comment #6
Robin Monks CreditAttribution: Robin Monks commentedPatch 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
Comment #7
drewish CreditAttribution: drewish commentedWe should probably be using preg_match: #64967: Replace ereg with preg
Comment #8
drewish CreditAttribution: drewish commented#64967: Replace ereg with preg got committed. so this should definitely use preg_match.
Comment #9
drewish CreditAttribution: drewish commentedusing a preg_match() style regular expression. added a test.
Comment #10
drewish CreditAttribution: drewish commentedComment #11
drewish CreditAttribution: drewish commentedComment #12
drewish CreditAttribution: drewish commentedre-rolled.
Comment #13
Dries CreditAttribution: Dries commentedI 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!
Comment #14
Dries CreditAttribution: Dries commentedComment #15
Dries CreditAttribution: Dries commentedI 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.
Comment #16
cburschkaBig +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?
Comment #17
catchboombatower 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.
Comment #18
drewish CreditAttribution: drewish commentedArancaytar, 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.
Comment #19
webchickI just backed out the tests since they were breaking SimpleTest. So this one needs to be re-rolled with the tests again.
Comment #20
drewish CreditAttribution: drewish commentedokay, here's a re-roll that adds the tests back in and the fix for the install.php file.
Comment #21
Anonymous (not verified) CreditAttribution: Anonymous commentedThe last submitted patch failed testing.
Comment #22
drewish CreditAttribution: drewish commentedre-rolling.
Comment #23
cburschkaI think you might have been mixing patches here.
Shouldn't belong in the file.inc patch, unless they are related in a way I don't understand. ;)
Comment #24
drewish CreditAttribution: drewish commentednot sure how that got in there. re-rolled without it.
Comment #26
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD along with a code clean-up. Thanks!
Comment #27
maartenvg CreditAttribution: maartenvg commentedThe 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.
Comment #28
alex_b CreditAttribution: alex_b commentedwarnings on install read "preg_match() expects parameter 1 to be string, array given"
This patch brings me through installation.
Comment #29
catchComment #30
webchickThanks, committed. We'll need to re-submit a bunch of other patches for re-testing.
Comment #31
webchickOh, also, docs please! :D
Comment #32
cburschkaIsn'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.
Comment #33
boombatower CreditAttribution: boombatower commentedFor documentation purposes:
This bug in HEAD has caused the testing bot to report a large number of failed to install HEAD.
Comment #34
drewish CreditAttribution: drewish commentedhttp://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.
Comment #35
webchickYep. 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. :)