If system required modules missed and your list looks like this
function default_profile_modules() {
return array('color', 'comment', 'taxonomy');
}
then you get a fatal error: Call to undefined function _node_type_set_defaults() in profiles/default/default.profile on line 58

array('color', 'comment', 'node', 'taxonomy') -->
Fatal error: Call to undefined function user_access() in modules/comment/comment.module on line 150

array('color', 'comment', 'node', 'taxonomy', 'user') -->
Fatal error: Call to undefined function filter_xss_admin() in themes/engines/phptemplate/phptemplate.engine on line 154

and so on.

At the moment only module system is merged to the list.

The patch solves this and adds all required modules if missed.

CommentFileSizeAuthor
#11 install-require_0.patch3.07 KBwebchick
#10 install-require.patch2.05 KBwebchick
#4 required-modules-in-installation-profiles_2.patch1.67 KBrstamm
required-modules-in-installation-profiles.patch1.17 KBrstamm
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
CreditAttribution: greggles commented

How can we repeat this bug? Do we need to have a module (module2) that requires a module (module1) and delete module1 from the system?

If not, please present the steps.

rstamm’s picture

rstamm CreditAttribution: rstamm commented

Replace in file profiles/default/default.profile

function default_profile_modules() {
  return array('block', 'color', 'comment', 'filter', 'help', 'menu', 'node', 'system', 'taxonomy', 'user', 'watchdog');
}

with 

function default_profile_modules() {
return array('color', 'comment', 'taxonomy');
}

and run install.php?profile=default
then you get the php error: Call to undefined function _node_type_set_defaults() in profiles/default/default.profile on line 58

eaton’s picture

eaton CreditAttribution: eaton commented
Priority: Critical » Normal
Status: Needs review » Needs work

Replace in file profiles/default/default.profile...

Right there the red flag goes off. This issue is not critical if it requires the modification of the core install profile. the choice not to enforce absolute module requirements beyond 'system' was an explicit one. Most of the modules that this patch adds as 'required' are really only required by other commonly-enabled dependencies. During testing, valid cases were proposed where a profile developer might need to replace a particular core module with one that implements several of its functions for compatability; if we require that all profiles install the core module, that dev is out of luck. If we only require 'system', the dev can put their replacement module (say, BetterWatchDog.module) in the profiles/foo/modules directory and simply keep 'watchdog' out of the profile's install list.

This IS a documentation issue for profile developers, and it may be useful to provide a 'normal' list of baseline modules that profiles can override, not unlike theme regions in PHPTemplate. But it's definitely not a critical, given the fact that you have to hack core or make a custom install profile to trigger the error.

rstamm’s picture

rstamm CreditAttribution: rstamm commented
Status: Needs work » Needs review
FileSize
required-modules-in-installation-profiles_2.patch1.67 KB

It is not only a documentation issue.

The handling of default_profile_modules() is not really user friendly. If you forget a required module the installation runs into an error.

Drupal should prevents from forgetting it. But you are right. To be to rigid is not good too.

So the new patch supports both sides and allows to write

function default_profile_modules() {
  return array('color', 'comment', 'help', 'taxonomy');
}

and if you want to replace a required module

function default_profile_modules() {
  return array(
    'default' => FALSE,
    'modules' => array('block', 'color', 'comment', 'filter', 'help', 'menu', 'node', 'system', 'taxonomy', 'user', 'BetterWatchDog')
  );
}

In the first case the patch merges required modules and in the second not.

RobRoy’s picture

RobRoy CreditAttribution: RobRoy commented

Old issue at http://drupal.org/node/104764 was marked dupe of this.

I'm making the docs at http://drupal.org/node/67921 a little more explicit re core required/optional.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Version: 5.x-dev » 6.x-dev
Status: Needs review » Postponed

If this behavour truly is "by design," then I support something like Ralf's patch in #4. It's silly that essentially every installation profile under the sun is going to have to re-input those values; error-prone and confusing.

However, it'll be an API change, so marking for 6.x. menu module is no longer required, but this may change as the menu system gets re-done. Marking postponed.

However, all of these modules are in fact required, at least the way that system.install is setup. You get fatal errors if you don't remember each of them. I'll file a separate task for that, to move the creation of the user login block and such to default_profile_final rather than system.install.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Postponed » Needs work

Well, no. That doesn't make sense either. :P Because then every install profile is going to have to copy/paste this chunk. I guess throwing SQL in documentation to delete the user login block for people who want to make a "barebones" profile could work...

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Needs work » Postponed

Oops. didn't mean to change status.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Postponed » Needs work

Actually, I've changed my mind. ;)

So the logical way to separate stuff from system.module is to put all the SQL for blocks and whatnot in their own module.install files. However, when you do that you now run into issues on where to put the updates, you create another 5-6 elements on the system upgrade page to be checked, etc. In short, this makes it a huge pain in the rear. And all of this is just so that the 1% case will work (probably more like 0.000001% case). Meanwhile, the 99% case have to copy and paste this code every single time. Doesn't make sense to me.

Furthermore, if someone really wants a BetterWatchdog module, they can easily do that by overriding watchdog module's menu paths in the module, and and just increasing its weight in the system table as part of its install. Again, no reason for 99% of profile builders to have to suffer. Required modules are required modules; if you want to do something fancy to get around that, then you should be the one who has to do extra work, not everyone else.

So now I'm back in favour of the first patch, although I'm marking code needs work because we should look at refactoring that somehow so that the list is only in one place and doesn't need to be updated twice every time we change this list. Maybe a _drupal_default_required_modules() function or something?

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Needs work » Needs review
FileSize
install-require.patch2.05 KB

Let's try this.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
FileSize
install-require_0.patch3.07 KB

Oops. There's another instance of that array over in system.module. Because install.inc is out of scope here, moved the drupal_required_modules function to module.inc.

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Needs review » Fixed

I like webchick's patch. Committed. Thanks.

Anonymous’s picture

(not verified) CreditAttribution: commented
Status: Fixed » Closed (fixed)