Problem/Motivation

drupal_get_path() allows you to find the location of a module via the module name - this accounts for the different possible locations where a module could be installed ( contrib/, sites/SITENAME/modules/contrib etc.)

However, there are multiple uses on core where it's not necessary, because the call is inside the module being referred to.

In these cases, just __DIR__ returns the same thing (or sometimes __DIR__ . '../' if the call is in a subdirectory of the module) without having to go through all the Drupal logic.

When we eventually deprecate drupal_get_path(), this will reduce the number of replacements that need to happen. It should also make it clearer what use-cases the replacement actually needs to solve.

An obvious example is this one in statistics.module:

$settings = ['data' => ['nid' => $node->id()], 'url' => \Drupal::request()->getBasePath() . '/' . drupal_get_path('module', 'statistics') . '/statistics.php'];

A previous attempt was made in #2351919: Replace uses of drupal_get_path() with __DIR__ where possible, but either it didn't catch everything, or new uses have crept in.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#8 Sample_Test_Result.txt5.57 KBcodersukanta
#4 Search_Result.jpg110.36 KBcodersukanta

Issue fork drupal-3151676

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

catch created an issue. See original summary.

catch’s picture

catch
he/him
Primary language English
commented
Issue summary: View changes
codersukanta’s picture

codersukanta commented
Assigned: Unassigned » codersukanta
codersukanta’s picture

codersukanta commented
StatusFileSize
new Search_Result.jpg110.36 KB

I have searched drupal_get_path in drupal core and found approx 149 results in 90 files.

It would be a very big patch if we replace the drupal_get_path function in a single patch. So according to me, we should spit this issue in multiple issues or we can split this module wise.

Search Result

codersukanta’s picture

codersukanta commented
Assigned: codersukanta » Unassigned
catch’s picture

catch
he/him
Primary language English
commented

Most of the drupal_get_path() calls can't be replaced in this issue - especially where $module or $theme is a variable, and cases where the call is referring to a different module. So it should be fine to do the ones that can in one patch.

In general we try to avoid doing per-module clean-ups for issues like this since reviewing 20-30 small patches with the same kinds of straightforward changes takes longer than reviewing one.

codersukanta’s picture

codersukanta commented
Assigned: Unassigned » codersukanta
codersukanta’s picture

codersukanta commented
Assigned: codersukanta » Unassigned
StatusFileSize
new Sample_Test_Result.txt5.57 KB

It seems like simply replacing drupal_get_path with __DIR__ will not work. Sometimes it's breaking the file paths. Attached test cases of such scenarios.

siddhant.bhosale’s picture

siddhant.bhosale commented
Assigned: Unassigned » siddhant.bhosale

drumm made their first commit to this issue’s fork.

drumm opened merge request !1

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Active » Needs work

I’m using this issue to test out merge requests as part of #3152637: Opt-in to the Drupal.org Issue Forks and Merge Requests beta.

The commit on that merge request only covers core/modules/system/tests/modules/batch_test/batch_test.module so far. The replaceable drupal_get_path() instances I found are:

$ for module in $(find core/modules -name '*.module'); do name=$(basename -s '.module' "${module}"); git --no-pager grep --line-number "drupal_get_path('module', '${name}')" $(dirname "${module}"); done
core/modules/statistics/statistics.module:43:    $settings = ['data' => ['nid' => $node->id()], 'url' => \Drupal::request()->getBasePath() . '/' . drupal_get_path('module', 'statistics') . '/statistics.php'];
core/modules/statistics/tests/src/Functional/StatisticsAdminTest.php:90:    $stats_path = $base_url . '/' . drupal_get_path('module', 'statistics') . '/statistics.php';
core/modules/statistics/tests/src/Functional/StatisticsAdminTest.php:125:    $stats_path = $base_url . '/' . drupal_get_path('module', 'statistics') . '/statistics.php';
core/modules/statistics/tests/src/Functional/StatisticsAdminTest.php:160:    $stats_path = $base_url . '/' . drupal_get_path('module', 'statistics') . '/statistics.php';
core/modules/statistics/tests/src/Functional/StatisticsLoggingTest.php:100:    $module_path = drupal_get_path('module', 'statistics');
core/modules/statistics/tests/src/Functional/StatisticsReportsTest.php:38:    $stats_path = $base_url . '/' . drupal_get_path('module', 'statistics') . '/statistics.php';
core/modules/statistics/tests/src/Functional/StatisticsTokenReplaceTest.php:38:    $stats_path = $base_url . '/' . drupal_get_path('module', 'statistics') . '/statistics.php';
core/modules/statistics/tests/src/Functional/Views/IntegrationTest.php:84:    $stats_path = $base_url . '/' . drupal_get_path('module', 'statistics') . '/statistics.php';
core/modules/locale/locale.bulk.inc:162:      'file'          => drupal_get_path('module', 'locale') . '/locale.bulk.inc',
core/modules/locale/locale.bulk.inc:595:    'file'          => drupal_get_path('module', 'locale') . '/locale.bulk.inc',
core/modules/locale/locale.compare.inc:247:    'file' => drupal_get_path('module', 'locale') . '/locale.batch.inc',
core/modules/locale/locale.fetch.inc:47:    'file' => drupal_get_path('module', 'locale') . '/locale.batch.inc',
core/modules/locale/locale.fetch.inc:76:    'file' => drupal_get_path('module', 'locale') . '/locale.batch.inc',
core/modules/update/src/Form/UpdateManagerUpdate.php:387:      'file' => drupal_get_path('module', 'update') . '/update.manager.inc',
core/modules/update/update.authorize.inc:56:    'file' => drupal_get_path('module', 'update') . '/update.authorize.inc',
core/modules/update/update.authorize.inc:110:    'file' => drupal_get_path('module', 'update') . '/update.authorize.inc',
core/modules/comment/tests/src/Functional/CommentCSSTest.php:118:        $this->assertRaw(drupal_get_path('module', 'comment') . '/js/comment-by-viewer.js', 'drupal.comment-by-viewer library is present.');
core/modules/color/color.module:289:  $preview_html_path = \Drupal::root() . '/' . (isset($info['preview_html']) ? drupal_get_path('theme', $theme) . '/' . $info['preview_html'] : drupal_get_path('module', 'color') . '/preview.html');
core/modules/aggregator/tests/src/Functional/AggregatorTestBase.php:343:    return $GLOBALS['base_url'] . '/' . drupal_get_path('module', 'aggregator') . '/tests/modules/aggregator_test/aggregator_test_rss091.xml';
core/modules/aggregator/tests/src/Functional/AggregatorTestBase.php:355:    return $GLOBALS['base_url'] . '/' . drupal_get_path('module', 'aggregator') . '/tests/modules/aggregator_test/aggregator_test_atom.xml';
core/modules/aggregator/tests/src/Functional/AggregatorTestBase.php:365:    return $GLOBALS['base_url'] . '/' . drupal_get_path('module', 'aggregator') . '/tests/modules/aggregator_test/aggregator_test_title_entities.xml';
core/modules/ckeditor/src/Plugin/CKEditorPlugin/DrupalImage.php:25:    return drupal_get_path('module', 'ckeditor') . '/js/plugins/drupalimage/plugin.js';
core/modules/ckeditor/src/Plugin/CKEditorPlugin/DrupalImage.php:54:        'image' => drupal_get_path('module', 'ckeditor') . '/js/plugins/drupalimage/icons/drupalimage.png',
core/modules/ckeditor/src/Plugin/CKEditorPlugin/DrupalImageCaption.php:49:    return drupal_get_path('module', 'ckeditor') . '/js/plugins/drupalimagecaption/plugin.js';
core/modules/ckeditor/src/Plugin/CKEditorPlugin/DrupalImageCaption.php:73:      drupal_get_path('module', 'ckeditor') . '/css/plugins/drupalimagecaption/ckeditor.drupalimagecaption.css',
core/modules/ckeditor/src/Plugin/CKEditorPlugin/DrupalLink.php:23:    return drupal_get_path('module', 'ckeditor') . '/js/plugins/drupallink/plugin.js';
core/modules/ckeditor/src/Plugin/CKEditorPlugin/DrupalLink.php:49:    $path = drupal_get_path('module', 'ckeditor') . '/js/plugins/drupallink';
core/modules/ckeditor/src/Plugin/CKEditorPlugin/Language.php:132:        drupal_get_path('module', 'ckeditor') . '/css/plugins/language/ckeditor.language.css',
core/modules/ckeditor/src/Plugin/Editor/CKEditor.php:432:      drupal_get_path('module', 'ckeditor') . '/css/ckeditor-iframe.css',
core/modules/ckeditor/tests/src/Functional/CKEditorLoadingTest.php:103:    $this->assertNoRaw(drupal_get_path('module', 'ckeditor') . '/js/ckeditor.js', 'CKEditor glue JS is absent.');
core/modules/ckeditor/tests/src/Functional/CKEditorLoadingTest.php:107:    $this->assertNoRaw(drupal_get_path('module', 'ckeditor') . '/js/ckeditor.js', 'CKEditor glue JS is absent.');
core/modules/ckeditor/tests/src/Kernel/CKEditorPluginManagerTest.php:68:      'drupalimage' => drupal_get_path('module', 'ckeditor') . '/js/plugins/drupalimage/plugin.js',
core/modules/ckeditor/tests/src/Kernel/CKEditorPluginManagerTest.php:69:      'drupallink' => drupal_get_path('module', 'ckeditor') . '/js/plugins/drupallink/plugin.js',
core/modules/ckeditor/tests/modules/ckeditor_test.module:14:  $css[] = drupal_get_path('module', 'ckeditor_test') . '/ckeditor_test.css';
core/modules/ckeditor/tests/modules/src/Form/AjaxCssForm.php:84:    $url = drupal_get_path('module', 'ckeditor_test') . '/css/test.css';
core/modules/ckeditor/tests/modules/src/Plugin/CKEditorPlugin/Llama.php:54:    return drupal_get_path('module', 'ckeditor_test') . '/js/llama.js';
core/modules/ckeditor/tests/modules/src/Plugin/CKEditorPlugin/LlamaButton.php:32:    return drupal_get_path('module', 'ckeditor_test') . '/js/llama_button.js';
core/modules/ckeditor/tests/modules/src/Plugin/CKEditorPlugin/LlamaContextual.php:38:    return drupal_get_path('module', 'ckeditor_test') . '/js/llama_contextual.js';
core/modules/ckeditor/tests/modules/src/Plugin/CKEditorPlugin/LlamaContextualAndButton.php:53:    return drupal_get_path('module', 'ckeditor_test') . '/js/llama_contextual_and_button.js';
core/modules/ckeditor/tests/modules/src/Plugin/CKEditorPlugin/LlamaCss.php:35:      drupal_get_path('module', 'ckeditor_test') . '/css/llama.css',
core/modules/ckeditor/tests/modules/src/Plugin/CKEditorPlugin/LlamaCss.php:43:    return drupal_get_path('module', 'ckeditor_test') . '/js/llama_css.js';
core/modules/system/tests/modules/batch_test/batch_test.module:26:    'file' => drupal_get_path('module', 'batch_test') . '/batch_test.callbacks.inc',
core/modules/system/tests/modules/batch_test/batch_test.module:49:    'file' => drupal_get_path('module', 'batch_test') . '/batch_test.callbacks.inc',
core/modules/system/tests/modules/batch_test/batch_test.module:71:    'file' => drupal_get_path('module', 'batch_test') . '/batch_test.callbacks.inc',
core/modules/system/tests/modules/batch_test/batch_test.module:103:    'file' => drupal_get_path('module', 'batch_test') . '/batch_test.callbacks.inc',
core/modules/system/tests/modules/batch_test/batch_test.module:133:    'file' => drupal_get_path('module', 'batch_test') . '/batch_test.callbacks.inc',
core/modules/system/tests/modules/batch_test/batch_test.module:156:    'file' => drupal_get_path('module', 'batch_test') . '/batch_test.callbacks.inc',
core/modules/system/tests/modules/batch_test/batch_test.module:179:    'file' => drupal_get_path('module', 'batch_test') . '/batch_test.callbacks.inc',
core/modules/system/tests/modules/batch_test/batch_test.module:210:    'file' => drupal_get_path('module', 'batch_test') . '/batch_test.callbacks.inc',
core/modules/system/tests/modules/common_test/common_test.module:255:  $alterjs = drupal_get_path('module', 'common_test') . '/alter.js';
core/modules/system/system.admin.inc:290:          '#uri' => drupal_get_path('module', 'system') . '/images/no_screenshot.png',
core/modules/system/tests/src/Functional/System/HtaccessTest.php:34:    $path = drupal_get_path('module', 'system') . '/tests/fixtures/HtaccessTest';
core/modules/settings_tray/settings_tray.module:183:  $path = drupal_get_path('module', 'settings_tray') . '/css/settings_tray.theme.css';
core/modules/node/node.admin.inc:49:      'file' => drupal_get_path('module', 'node') . '/node.admin.inc',
core/modules/views/tests/src/Kernel/ViewsConfigUpdaterTest.php:51:    $config_dir = drupal_get_path('module', 'views') . '/tests/fixtures/update';
core/modules/editor/tests/src/Functional/EditorLoadingTest.php:305:      strpos($this->getSession()->getPage()->getContent(), drupal_get_path('module', 'editor') . '/js/editor.js') !== FALSE,
core/modules/media/media.install:22:  $source = drupal_get_path('module', 'media') . '/images/icons';
core/modules/media/tests/src/Traits/OEmbedTestTrait.php:20:    return drupal_get_path('module', 'media') . '/tests/fixtures/oembed';

drumm opened merge request !2

drumm’s picture

drumm
he/him
Location NY, US
commented

Looks like __DIR__ is not the exact same thing as drupal_get_path(), it is a full path, not relative to index.php. So not all of these replacements are straightforward, as shown by the 3151676-remove-unnecessary-drupal_get_path branch. For now I’ve abandoned that and am looking for other types of uses.

drumm closed merge request !1

siddhant.bhosale’s picture

siddhant.bhosale commented
Assigned: siddhant.bhosale » Unassigned

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone closed merge request !2

quietone reopened merge request !2

dimitriskr’s picture

dimitriskr commented
Status: Needs work » Postponed (maintainer needs more info)

There are no more uses of drupal_get_path() to 11.x
Some old usages have been replaced by \Drupal::service('extension.list.module');. Do we want to replace the service with __DIR__?

quietone’s picture

quietone commented
Status: Postponed (maintainer needs more info) » Closed (outdated)

@dimitriskr, no were should use the service.

drupal_get_path was removed in [#3269154]making this outdated.