Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Since it checks in the db after installation and we consider profiles as modules in the db, drupal_get_filename('profile', 'standard') returns nothing (this also affect drupal_get_path(), which wraps drupal_get_filename().
This patch fixes this for the case when the DB is used for lookups, and makes the documentation for both functions correct (it is currently partially incorrect).
This is a pretty big issue if you're doing custom profile development and trying to use drupal_get_path with your profile.
Comments
Comment #1
pillarsdotnet CreditAttribution: pillarsdotnet commentedWondering if this also fixes #1149580: Prevent url() from expanding aliases while the installer is running.
Comment #2
catchIt's very sad that we have so much special casing of profiles all over the place, when turning them into modules was supposed to remove that, but the patch looks good.
However it also looks like this could be added as a test very quickly too.
Comment #3
catchShould have one fail.
Comment #4
catchShould pass.
Comment #5
sunLooks ready to fly for me.
Comment #6
Chi CreditAttribution: Chi commented1.
file_exists(DRUPAL_ROOT . '/' . $file)
The code above returns TRUE even if $file = FALSE. I think we should fix it.
2. In drupal 6 this path also doesn't come from db. Do we need backport to D6?
3. After applying the previous patch drupal_get_filename works only for current installed profile because other profiles aren't present in the system table.
Comment #7
Chi CreditAttribution: Chi commentedTitle edited by mistake.
Comment #8
Chi CreditAttribution: Chi commentedComment #9
girishmuraly CreditAttribution: girishmuraly commented@chi, if you look at the schema for the system table, 'filename' is set to be the primary key and cannot be NULL. So I think the extra check to see if $file is empty is not needed. Marking this as RTBC again.
Patch #4 works well for me.
Comment #10
pillarsdotnet CreditAttribution: pillarsdotnet commentedEDIT: Ignore this (How did that extra stuff get into the patch???)
Comment #12
pillarsdotnet CreditAttribution: pillarsdotnet commentedRe-uploading the correct patch, for clarification.
drupal_get_filename-installed_profile-testonly-1136820-12.patch
drupal_get_filename-installed_profile-test+fix-1136820-12.patch
Comment #13
Chi CreditAttribution: Chi commented@girishmuraly
I think it isn't clean code.
Example for drupal_get_filename('module', 'nonexist');
Comment #14
girishmuraly CreditAttribution: girishmuraly commented@Chi, actually I have to agree with you on that. Didn't think about that case. Yeah, checking:
if ($file && file_exists(DRUPAL_ROOT . '/' . $file))
makes sense. A new test case needs also to be added.Comment #15
girishmuraly CreditAttribution: girishmuraly commentedWork on the D7 version of this bug is in a different bug - http://drupal.org/node/1006714. That looks more comprehensive and tests are better. Suggest we use that for D7 atleast. Removing the 'needs backport' tag here, if that makes sense.
Comment #16
catchI'm closing this as duplicate of #1006714: drupal_get_path doesn't work for profiles.
Comment #17
pillarsdotnet CreditAttribution: pillarsdotnet commentedSee also #1311774: Prevent path functions from using database tables that have not yet been created.