Problem/Motivation

Parsing .info files for dependency information is already implemented on the modules administration page. Adding support for the same dependencies in theme.info files, and implementing the same behavior on the admin/build/themes page, would allow designers building heavily customized themes to make safer assumptions about their target sites.

A theme might require the existence of imagecache to auto-generate variations of a header image. This would be a nice compromise between systems like Wordpress and Joomla!, which give themes much greater control over the functionality of the site, and Drupal's module-centric approach.

It also creates following new UX requirements and non UI, API requirements

  1. Represent the list of dependent modules in themes list page. Display of missing/disabled dependent modules.
  2. Extension in Drush to download and enable modules dependent on themse.

Proposed resolution

Creation of the API, which will determine the dependencies on the theme.
There are following two patches submitted with different approaches:

  1. http://drupal.org/files/theme_dependencies-474684-46.patch
  2. http://drupal.org/files/modtheme.patch

Original report by [eaton]

Issue Summary
Parsing .info files for dependency information is already implemented on the modules administration page. Adding support for the same dependencies in theme.info files, and implementing the same behavior on the admin/build/themes page, would allow designers building heavily customized themes to make safer assumptions about their target sites.

A theme might require the existence of imagecache to auto-generate variations of a header image. This would be a nice compromise between systems like Wordpress and Joomla!, which give themes much greater control over the functionality of the site, and Drupal's module-centric approach.

Files: 
CommentFileSizeAuthor
#47 theme_dependencies-474684-46.patch21.92 KBthedavidmeister
#36 modtheme.patch10.42 KBSnugug
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#30 thememoduledependency.patch6.27 KBRobLoach
PASSED: [[SimpleTest]]: [MySQL] 33,973 pass(es).
[ View ]
#30 screenshot10.png64.46 KBRobLoach
#8 build_dependencies_for_theme-474684-8.patch4.01 KBsreynen
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch build_dependencies_for_theme-474684-8.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

Comments

+1

See: #340967: Remove Dependency on Extra Voting Forms (Test Alternative Voting Modules)

I am not keen on adding a dependency, because Drigg itself doesn't depend on it. It's "default theme" does.

I'm in the process of moving the default theme to a theme project page. It will depend on Drigg and Extra Voting Form.

+1, this would be an exciting option potentially encouraging some interesting contrib themes for various site recipes/configurations

Version:7.x-dev» 8.x-dev

subscribing

+1: the bee's knees.

subscribing

Yes, this would be fantastic - right now I can't tell you often we have to deal with support/bug issues because our base theme didn't get installed, or we have add a silly long description in the hope the user will read it AND actually click the links (that our theme needs this or that module to be feature complete).

I actually think this darn near borders on being a bug - the user can install a sub-theme, site blows up, wtf?

StatusFileSize
new4.01 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch build_dependencies_for_theme-474684-8.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

This preliminary patch simply collects the dependency data from theme .info files, without actually using it anywhere. The first change is simply renaming _module_build_dependencies() as _system_build_dependencies() and moving it to system.inc to indicate it can be used for more than just modules. After that, a second parameter is necessary because the current implementation only builds a dependency graph among the files being checked, meaning themes could only depend on themes. Finally, this adds a call to _system_build_dependencies() from system_rebuild_theme_data() to match the call in system_rebuild_module_data().

This builds themes dependencies on other themes as well as modules, though modules still can not depend on themes. If that sounds right, next step is to actually apply the dependencies for themes similar to how they're applied for modules.

Yes, this would be nice to have, as I just created a theme that depends on the conditional styles module, http://drupal.org/project/conditional_styles

I'd be surprised if drupal 7 doesn't support this already

@Jeff Burnz, that's definitely a bug. Defining a base theme should implicitly act as a dependency even if we can't explicitly define dependencies.

Setting a module as a dependency will pose some UI challenges. It's mildly annoying managing dependent and disabled checkboxes in the module config page. What would you do when themes come into the picture?

Having themes depend on certain modules would be great. xCSS or SASS are good examples where themes can benefit from required module dependencies.

Status:Active» Needs work

Marking this as having a patch. I like this approach. dvessel's point about UX challenges though is not to be ignored.

I'm interested in working on the UX challenges, but it would be nice to first get some consensus on whether or not it would also be useful to have module dependencies on themes, as that would suggest a significantly different, much more complicated approach to this. The only example I can think of where that might be useful is admin module having a dependency on Rubik. Any thoughts on whether that's a use case worth allowing?

sreynen, I think that's going too far. Modules provide general functionality that themes may require but themes tend to be very specific. It might be useful for one-off custom modules on a private site but allowing the two way dependency in other cases will be a big mess.

+

I definitely consider this a bug in d6 & d7.. subthemes should not break a site if the parent theme isn't present... there should be a warning, or the theme simply should just not show up in theme settings. It almost makes a case for storing sub themes inside their parent themes directory... at least when you are the maintainer of the base and sub theme.

Category:feature» bug

@11, 13, 14, we started working on a redesign of the themes page: #1167444: Not clear which themes are enabled and disabled, if we can agree this needs to happen we can work on some of the UI stuff over there maybe.

Seems like there is consensus that this is a bug.

Merging the two issues seems like a good idea. The UI changes here are dependent on the UI changes in #1167444: Not clear which themes are enabled and disabled, but those UI changes can't happen without the dependency checking code here. Should we just mark this as duplicate and move everything over to #1167444, expanding the scope a bit? Or would it be better to postpone this until #1167444 is fixed?

OK, I postponed #1167444: Not clear which themes are enabled and disabled, we can still work on our ideas over there (UI changes).

Re UI changes in #17, I would rather see a theme grayed out with a message why (e.g., "requires foobar theme to be installed" or "requires Skinr module to be enabled").

Re #14 (theme dependencies for modules) I can't say I've encountered that, but as Drupal gets more presentation-agnostic and the web gets more platform-agnostic, I can't say this use case would not arise. I wonder if the easy "fix" there would be simply to add the themes available somewhere in the modules management area, just as themes and modules both appear on update status.

Title:Themes should support dependencies[] dependencies[] for themes, so they can depend on modules
Category:bug» feature
Priority:Normal» Major
Issue tags:+API clean-up

Better title. Regardless of annoyance, this is a feature request.

I'd highly recommend to defer the implicit base theme as dependencies[] handling to a separate issue. The idea is sound, but it leads to having a theme name in a list of module names - turning the list into a list of extension names. All of that is sound and I really support it, but the merge of modules and themes into extensions needs to be discussed in a larger scope first, and we don't want this issue to get blocked on that.

+++ b/includes/module.inc
@@ -202,39 +202,6 @@ function system_list_reset() {
-function _module_build_dependencies($files) {
+++ b/modules/system/system.module
@@ -2415,13 +2415,51 @@ function system_rebuild_module_data() {
+function _system_build_dependencies($files, $dependency_files = array()) {
@@ -2563,6 +2601,8 @@ function system_rebuild_theme_data() {
+  $modules = system_rebuild_module_data();
+  $themes = _system_build_dependencies($themes, $modules);

Unfortunately, this change hugely contradicts a major D8 effort that attempts to move all the low-level bootstrap functionality out of System module: #679112: Time for system.module and most of includes to commit seppuku

The function needs to stay in module.inc.

That said, the renaming idea itself looks bogus to me -- you're still building and checking for module dependencies.

20 days to next Drupal core point release.

Module dependencies aside, what about sub themes having base themes as dependencies? Should a separate issue be created?

@KrisBulman: A sub-theme can only have one "parent" base theme, and that's specified via the base theme .info file property already. Also, as mentioned in #24, I still recommend to leave fiddling with base themes to a separate follow-up issue.

subscribing...cause it it sounds useful

Status:Needs work» Needs review
StatusFileSize
new64.46 KB
new6.27 KB
PASSED: [[SimpleTest]]: [MySQL] 33,973 pass(es).
[ View ]

Doesn't take sun's notes from above into considering. Just wanted to throw something up there to get things rolling again....

Is there any movement on this?

#1435406: allow themes to require modules has been marked as a duplicate

Cross posting a comment here.

While by no means a perfect solution, my suggestion here is for anyone who found this entry due to searching for 'theme module dependency' (or the like).

Drupal 8 may incorporate a fancy mechanism for this, Drupal 7 and 6 do not, but in D6 and D7 you can do this, in your template.php in your mytheme_preprocess_page() function add the following:

<?php
function mytheme_preprocess_page(&$vars)
{
  ...
  if (!
module_exists('needed_module'))
  {
   
// No needed_module means we need to add it, let everyone know.
    //
   
print "<h1>"  . t( 'mytheme requires that module needed_module is installed')
        .
"</h1>"
       
. "<h6>"  . t( 'warning generated in file: %f', array ( '%f'=>__FILE__ ))
        .
"</h6>";
  }
  ...
}
?>

Yes, this is a bit kludgy, because it just prints out the error. But if your theme might be broken anyway, best to get the message out. And, if your user installed the theme without the module, they'll know instantly what's wrong. If something breaks, like your user uninstalls the dependency, they'll know right away.

So, you've got all you need to solve it right there. You could get more elaborate, perhaps making your warning include where to download the missing module, but meh.

that aside, what else is needed to move this forward?

Status:Needs review» Needs work

Looks like the next step is to apply sun's comments in #24 to the patch in #30. Specifically, move _system_build_dependencies() back to _module_build_dependencies().

StatusFileSize
new10.42 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Did a bunch of work on this at DrupalCon Munich core sprint. Still needs work, but on its way.

Status:Needs work» Needs review

go bot.

Status:Needs review» Needs work

The last submitted patch, modtheme.patch, failed testing.

Snugug, you should remove the dsm right? And I would recommend to use dpm instead of dsm ;)

Like I said, this needs more work and is not finished. This patch was how far I got in sprints today and I put it up here so I could get it once I got back to my computer at the suggestion of John Albin. Once I've got a working computer again I will be able to finish this and get it to a point to really be considered.

Been working on this today using Snugug's work as a base. Decided I have some issues with what is there beyond the dsm() and that all of the implications of what Sun was saying is not immediately clear until you're looking directly at the code and trying to refactor it.

So here's my interpretation of what Sun was saying:

  • _module_build_dependencies is an API function that "figure out the modules that these files depend on, using the module system's definition of dependency" and it should stay that way
  • Don't put this functionality in system.module
  • When determining what a theme relies on, don't lump base themes and modules into a big list of "dependencies", keep the distinction clear all the way from the underlying logic through to the UI presented to the end-user.

Here's my review of what Snugug posted - well aware that it is "unfinished" but I don't know what the intentions of this patch were/are as there aren't any comments or clues in this queue so I can only discuss what is in front of me.

  • The function _theme_build_dependencies written by Snugug should mirror _module_build_dependencies and "figure out all the themes that these files depend on, using the theme system's definition of dependency" - opposed to "figure out all modules and themes that these theme files depend on, using the theme system's definition of module/theme dependencies"
  • The logic that prevents a theme with missing dependencies from being enabled should not start in the theming of the system form as this won't help with drush or direct calls to theme_enable().
  • Shouldn't be querying the system table directly to get a list of modules that are missing/disabled/enabled, we can use system_rebuild_module_data()

Also, kind of related post - #1067408: Themes do not have an installation status

So, I ran out of time today, I've been building on the patch for Snugug but don't have anything to post yet. I'll try to keep working on it this weekend.

Here's the approach that I'm taking:

  • Turn Snugug's _theme_build_dependencies into a way for the theme system to define theme dependencies for files, based on base themes defined in .info files
  • Change the documentation for _module_build_dependencies slightly to say that it checks what modules are dependent on the list of *files* rather than a list of *modules*
  • Write a separate function that checks both the theme and module dependencies for a given theme for use in the theme enabling process and the general UI
  • Make sure that theme_enable() can't enable theme's that have missing dependencies
  • Update the UI to represent what's going on now - Snugug did most of the work here

Just piping in my own two cents...

Wanted to mention that this would be awesome for advanced themers that use all sorts of tweaky-modules like Browser Class, CSS3Pie, Entity View Mode, Prepro w/ Sassy and tonnes of others.

I see many, many benefits to this and just wanna say, keep the awesomeness rolling! Really think this will help make packaging Drupal as a product that much more end-user-friendly.

Additional thoughts and ideas... ;o)

On the side, here's a question - if a User decides to enable a theme that has disabled dependencies, would we get that warning page that asks if we want the said dependencies to be enabled?

I can think of one other user friendly perk that could be clubbed into this patch/update. Would it be possible to offer to download and install those modules (a la Drush)? If not that, then it would be wonderful if we could at least show the user links to the missing modules if they are not found in the site install.

Once again, keep up the awesome work and thank you very, very much for your time and efforts!

PS... Search terms were "theme module dependency" and the reason I searched was cause of an admin-theme that I've customised off of Rubik where I'm now using Sassy to handle the .scss files...

Cheers :o)

@anandps - Yeah, I use prepro and related modules for a lot of my stuff, which is my main motivation for looking into this too. I definitely agree that whatever comes out of this thread needs to play nice with methods of enabling extensions outside of the UI (like drush).

As for how the UI should work... having themes give you all the same warnings, etc.. as modules across the board might need to be implemented at least partially by other modules, like Features or even drush.

It makes sense to provide a convenience screen to enable dependent modules, but we need to get the nuts-and-bolts right in the API first before we can polish up the UI.

Totally. I was giving snugug some moral support at DC Munich, so I know we were building this to match the user experience from module dependencies. This patch should already incorporate your suggestions around warning, prompts and enabling dependencies. :-)

Looking at this some more, I think the right place to call _theme_build_dependencies is in system_rebuild_theme_data() as the only place _module_build_dependencies is called is within system_rebuild_module_data(). (Both of which should be moved to theme_rebuild_theme_data() and module_rebuild_module_data() at some point, according to the post that Sun referenced).

To keep requires and required_by split by extension type, we kind of have to introduce extension-type specific keys at this point. Ie. where previously you would have called system_rebuild_module_data() or system_rebuild_theme_data to get a list of all available modules and themes then done something like $modules['modulename']->requires to get an array of required modules you'd now have to do this $modules['modulename']->requires['modules'] for modules and $modules['modulename']->requires['themes'] for themes, which is useless because modules don't depend on themes but it opens up the ability to do the same with $themes with a consistent interface.

Also, system_rebuild_theme_data() is not currently statically cached currently, whereas system_rebuild_module_data is so if we were to call system_rebuild_theme_data with module dependencies in there we rebuild all module dependencies every single time, bypassing that static cache, which is a bit nasty.

I'd like to suggest that because of this we can't really commit this feature without some static caching on system_rebuild_theme_data() either because we now have to invoke the module dependency system that function becomes much heavier.

Most of that is going to be resolved by #1067408: Themes do not have an installation status

StatusFileSize
new21.92 KB

Hmmm. I got this to the point where the dependencies stuff was working and I just had to update Snugug's UI work to use the new data structure I set up but it looks like something that was committed to system_rebuild_theme_data() in the last week or so is now causing fatal errors in my installation when I checked out the latest version from git :(

As far as I can tell it's because that function was converted to use a new config('system.module')->get('enabled') style format. I might just have to re-install D8 and give it another once-over.

Anyway, here's a patch against the lastest version of D8 so others can see what approach I've been taking.

Ah, before I do any more work here I'm going to apply that patch you referenced in the installation status issue first. There looks to be a lot of good stuff in there :)

I just wanted to mention that I haven't forgotten this thread, there's just a new patch going up on the issue Sun referenced in #46 every couple of days at the moment. I don't think that anything here will be able to be committed cleanly/stably until that thread is closed so I'm waiting for that thread to die down a bit before attempting to re-roll #47 here. Hopefully that happens in the next couple of weeks or we'll very likely miss the feature freeze deadline for this :/

The rabbit hole on this issue seems pretty deep at first glance: #1067408: Themes do not have an installation status -> #1798732: Convert install_task, install_time and install_current_batch to use the state system -> #1530756: Use a proper kernel for the installer

I'm really worried this may not make it in, however sun seems to be quite involved in all issues that this relies on so that's something!

Yeah, me too. But really, the outcome of #1067408: Themes do not have an installation status could easily mean this functionality should be implemented in a completely different way to how the current patches work, and there's no point in duplicating efforts with Sun when he's rolling patches new patches so consistently each time the test bot fails. I think we'll just have to wait and see what happens.

considering how the other threads are going it's highly likely this feature has missed the boat. I'd move it to 9.x-dev right now if the tag existed :/

At this point, I'd recommend to try to move forward independently here.

I'd rather object to the _module_build_dependencies() changes here though - themes are currently using the 'base theme' property only and implement their own dependency checking for that. Thus, the $theme->requires and $theme->required_by properties are not occupied, still available, and can be used to denote module dependencies in the same way as modules are doing.

That should happen in _system_rebuild_theme_data(). You should be able to copy/paste the necessary lines from _system_rebuild_module_data().

But it isn't $theme, it's $files in the _build_dependency functions.

Originally I was going to keep it is $theme->requires and $theme->required_by but then you run into this problem:

I'd highly recommend to defer the implicit base theme as dependencies[] handling to a separate issue. The idea is sound, but it leads to having a theme name in a list of module names - turning the list into a list of extension names.

Allowing themes to declare other themes as dependencies has been requested multiple times in this issue, even if nobody has updated the title of the thread to match. See #7, #8, #11, #17, #19, #25.

It only leads to having a theme name in a list of module names if you don't ask systems that can implement dependencies to namespace themselves somehow. Previously there was only one system that had a _build_dependencies function so it didn't matter. Now there would be a _theme_build_dependencies, which is more or less copied and pasted from _module_build_dependencies but for the theme system.

I didn't think that it was ok for _theme_build_dependencies to do anything at all related to deciding what a "module dependency" meant, only what a "theme dependency" meant. Themes could call _module_build_dependencies and have their info files parsed to discover anything that looks like a "module dependency" later/elsewhere.

I'm kind of just re-hashing what I said in #41 and #45 now.

tl;dr - based on what I found while debugging I don't see how $theme->requires and $theme->required_by would be sufficient to handle the requests coming in from everyone in this thread. Even if we take the "do that in a separate issue approach" we should still be polite enough not to screw whoever works on that thread by taking the good property names in advance. At least, we can't do that AND keep modules and theme dependencies in separate lists as per #24.

Also... considering my personal life commitments this week I don't think I'll get a chance to re-hash that patch before 1st Dec anyway, although I can still join in the discussion here. A re-roll of #47, with the required extra UI functionality and cleanup would be amazing if anyone gets the chance.