Problem/Motivation

drupal_check_profile() and drupal_verify_profile() are responsible for ensuring the presence of all the modules specified by the installation profile, and verifying their requirements. However, during a config-based install, the profile isn't as relevant; it's more important to ensure that the extensions listed in the core.extension config object are all present. But neither function currently does that. As long as the profile's modules are all present and their requirements check out, installation proceeds.

Proposed resolution

drupal_check_profile() and drupal_verify_profile() need to detect if a config-based install is in progress, and if so, they should be using core.extension to get the list of modules to locate and verify.

Remaining tasks

Write a fail patch, then a fix patch, review it, and commit it.

User interface changes

None.

API changes

TBD, but likely none.

Data model changes

None.

CommentFileSizeAuthor
#21 3005959-21.patch8.22 KBalexpott
#21 19-21-interdiff.txt743 bytesalexpott
#19 3005959-19.patch8.16 KBalexpott
#19 15-19-interdiff.txt1.36 KBalexpott
#15 3005959-15.patch9.71 KBalexpott
#15 12-15-interdiff.txt1.59 KBalexpott
#12 3005959-12.patch8.18 KBalexpott
#12 3005959-12.test-only.patch2.08 KBalexpott
#12 10-12-interdiff.txt4.19 KBalexpott
#10 3005959-10.patch3.74 KBalexpott
#10 3005959-10.test-only.patch2.05 KBalexpott
#10 5-10-interdiff.txt1.8 KBalexpott
#5 3005959-5.patch3.28 KBalexpott
#5 3005959-5.test-only.patch2.05 KBalexpott
#4 3005959-4.patch1.22 KBalexpott

Issue fork drupal-3005959

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

phenaproxima created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Issue tags: +Configuration system

Thanks for opening this one.

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

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review
StatusFileSize
new 3005959-4.patch1.22 KB

Here's a fix. Need to work out how to test this. Feels a bit tricky because it only happens when a module that is in the install list for a profile is not around when installing that profile from configuration.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 3005959-5.test-only.patch2.05 KB
new 3005959-5.patch3.28 KB

Here's a test building on the testing of drupal_verify_profile in HEAD.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Still need to handle - drupal_check_profile()

The last submitted patch, 4: 3005959-4.patch, failed testing. View results

The last submitted patch, 5: 3005959-5.test-only.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 5: 3005959-5.patch, failed testing. View results

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs work » Needs review
StatusFileSize
new 5-10-interdiff.txt1.8 KB
new 3005959-10.test-only.patch2.05 KB
new 3005959-10.patch3.74 KB

Still need to handle drupal_check_profile() but here's a fixed patch and a test-only patch to prove we've fixed the bug when calling drupal_verify_profile()

The last submitted patch, 10: 3005959-10.test-only.patch, failed testing. View results

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 10-12-interdiff.txt4.19 KB
new 3005959-12.test-only.patch2.08 KB
new 3005959-12.patch8.18 KB

Here's an update to drupal_check_profile() plus test coverage for the changes.

The last submitted patch, 12: 3005959-12.test-only.patch, failed testing. View results

daniel.bosen’s picture

daniel.bosen
he/him
commented

I successfully tested the following situations, that we had problems with, when installing from config:

a) Removing a manually disabled module, that was required by the profile.
=> Importing this configuration works now without problems

b) Removing a manually required and enabled module, that the profile did not know about.
=> This correctly fails now.

So, functionally this works perfectly!

Just a few nits I found in the patch:

  1. +++ b/core/includes/install.inc
@@ -584,10 +585,23 @@ function drupal_verify_profile($install_state) {
+    // it exists otherwise

    Missing full-stop 

  2. +++ b/core/includes/install.inc
@@ -968,11 +982,16 @@ function drupal_requirements_url($severity) {
+ *   (optional) The path to the configuration directory ff the site is being

    I guess the "ff" sould be an "if"? 

  3. +++ b/core/includes/install.inc
@@ -982,7 +1001,21 @@ function drupal_check_profile($profile) {
+    // it exists otherwise

    Another missing full-stop

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 12-15-interdiff.txt1.59 KB
new 3005959-15.patch9.71 KB

@daniel.bosen thanks for the review.

Fixed everything in the attached patch.

daniel.bosen’s picture

daniel.bosen
he/him
commented
Status: Needs review » Reviewed & tested by the community

Looks good to me!

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Reviewed & tested by the community » Needs review 
+++ b/core/modules/comment/src/Plugin/migrate/source/d6/Comment.php
@@ -34,31 +34,18 @@ public function query() {
   public function prepareRow(Row $row) {
-    // @todo Remove the call to ->prepareComment() in
-    // https://www.drupal.org/project/drupal/issues/3069260 when the Drupal 9
-    // branch opens.
     return parent::prepareRow($this->prepareComment($row));
   }
 
   /**
-   * Provides a BC layer for deprecated sources.
-   *
    * This is a backward compatibility layer for the deprecated migrate source
    * plugins d6_comment_variable and d6_comment_variable_per_comment_type.
    *
    * @param \Drupal\migrate\Row $row
    *   The row from the source to process.
-   *
    * @return \Drupal\migrate\Row
    *   The row object.
-   *
-   * @throws \Exception
-   *   Passing a Row with a frozen source to this method will trigger an
-   *   \Exception when attempting to set the source properties.
-   *
-   * @todo Remove usages of this method and deprecate for removal in
-   *   https://www.drupal.org/project/drupal/issues/3069260 when the Drupal 9
-   *   branch opens.
+   * @deprecated in Drupal 8.4.x, to be removed before Drupal 9.0.x.
    */
   protected function prepareComment(Row $row) {

This looks like it might be noise from a different issue...??

daniel.bosen’s picture

daniel.bosen
he/him
commented

True story, just took a look at the interdiff...

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 15-19-interdiff.txt1.36 KB
new 3005959-19.patch8.16 KB

Thanks @phenaproxima.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented 
+++ b/core/tests/Drupal/FunctionalTests/Installer/InstallerExistingConfigHookRequirements.php
@@ -0,0 +1,60 @@
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUpSettings() {
+    // There are errors.
+    return;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function setUpSite() {
+    // There are errors.
+    return;
+  }

These comments are not particularly clear. What does "there are errors" mean? Why are there errors? Maybe something like "prepareEnvironment() sets up an erroneous configuration on purpose, so this method must be bypassed" would make more sense.

But, that said, this ain't my issue, so I'm bowing out now. :)

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 19-21-interdiff.txt743 bytes
new 3005959-21.patch8.22 KB

Sure n.p.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs review » Reviewed & tested by the community

Restoring RTBC! I haven't deeply reviewed this patch, so I trust @daniel.bosen's judgment :)

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

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 21: 3005959-21.patch, failed testing. View results

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

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now 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.

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.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.