Problem/Motivation

Certain Drupal core functions depend on the existence of core database tables, and thus should not be invoked, either directly or indirectly, from an install profile.

Most of these core functions are found in includes/path.inc, which may be replaced by setting $conf['path_inc'].

Proposed resolution

The install system should supply a includes/path.inc replacement that does not depend on the existence of core database tables, or check drupal_get_bootstrap_phase() before using the database.

Remaining tasks

The two alternative patches are both awaiting review:

  1. #6: Use a replacement includes/path.inc plugin during installation.
  2. #9: Check drupal_get_bootstrap_phase() before using the database.

User interface changes

Certain fatal errors during profile installation will no longer be possible.

API changes

Code in $module.install files will no longer have to check the result of drupal_installation_attempted() or drupal_get_bootstrap_phase() before calling includes/path.inc functions.

Comments

pillarsdotnet’s picture

Status: Active » Needs review
StatusFileSize
new2.25 KB

Initial patch for discussion and review.

chx’s picture

Note that I suggested this so flame me instead of pillarsdotnet who rolled it into a patch.

tstoeckler’s picture

Status: Needs review » Needs work

Over in #1082328: Provide a proper no-op cache.inc it was decided to use "DrupalNullCache". Not that I personally care, but I think this should be consistent, so maybe path.null.inc. Ideally, of course, the path system should be turned into an interface, but I don't think that needs to happen in this issue.

tstoeckler’s picture

Status: Needs work » Needs review
StatusFileSize
new2.04 KB

Well, not to hold up this issue on a triviality, I rolled it with path.null.inc :)

pillarsdotnet’s picture

Title: Use a path.fake.inc during installation to avoid trying to access database tables that have not yet been created. » Use a replacement path.inc plugin during installation to avoid trying to access database tables that have not yet been created.
pillarsdotnet’s picture

StatusFileSize
new8.88 KB

Copied the real path.inc and then eliminated only the functionality that depends on database queries.

David_Rothstein’s picture

Why would we prevent path aliases during installation?

By the time hook_install() implementations are called, Drupal is already fully bootstrapped, and the normal path.inc works fine. That is the vast majority of module/profile code that gets run during the installer, and it wouldn't be good to force a stub path.inc for that.

It seems to me that only stuff like hook_requirements() (described in #1149580: Prevent url() from expanding aliases while the installer is running.) and a few other edge cases have this problem - in other words, code that runs before Drupal is fully bootstrapped. And trying to use path-related functions before Drupal is fully bootstrapped can break your site even outside of the installer. For example, try calling url() from within hook_boot() even on an already-installed site...

So I think this requires a more general solution.

pillarsdotnet’s picture

So I think this requires a more general solution.

I look forward to reviewing your patches.

pillarsdotnet’s picture

Title: Use a replacement path.inc plugin during installation to avoid trying to access database tables that have not yet been created. » Prevent path functions from using database tables that have not yet been created.
StatusFileSize
new4.57 KB

Perhaps this was what you meant by a "more general solution" ?

pillarsdotnet’s picture

Issue summary: View changes

fix

David_Rothstein’s picture

Yeah, something like that makes sense to me. It's unfortunate to have to do that (especially since a couple of those functions are in the critical path and we're adding extra function calls to them), but at least as a stopgap fix it looks like it's on the right track, and short of rewriting all of path.inc I can't think of another way to do it.

pillarsdotnet’s picture

Is there some kind of canonical list of the functions that are "on the critical path" or is that phrase just a matter of opinion?

EDIT: According to chx, the critical path is the code ran to serve a cached page.

The drupal.org website really needs a glossary. So many terms have a specific meaning here that is different from the rest of the world.

EDIT(2): found it. Added critical path entry.

David_Rothstein’s picture

Ha, well, I thought the phrase was just a matter of opinion :)

I don't think these functions actually run during cached page requests, but on regular pages they can run an awful lot (e.g. every time a link displayed on the page is processed). Overall, it's probably not too big of a deal.

pillarsdotnet’s picture

Probably needs benchmarks to be sure. Unfortunately I have not yet acquired the relevant (xhprof) skillset.

xjm’s picture

StatusFileSize
new4.13 KB

Rerolled for core/.

kscheirer’s picture

#14: path-bootstrap-1311774-13.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, path-bootstrap-1311774-13.patch, failed testing.

xjm’s picture

Issue tags: +needs profiling
xjm’s picture

Issue summary: View changes

Two alternative solutions.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

catch’s picture

Status: Needs work » Closed (outdated)

Path aliases have been moved to a regular module, so this is no longer relevant.