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.

Comments

philbar’s picture

+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.

stephthegeek’s picture

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

sun.core’s picture

Version:7.x-dev» 8.x-dev
sreynen’s picture

subscribing

laura s’s picture

+1: the bee's knees.

larowlan’s picture

subscribing

Jeff Burnz’s picture

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?

sreynen’s picture

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.

rickvug’s picture

beefzilla’s picture

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

dvessel’s picture

@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?

RobLoach’s picture

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

webchick’s picture

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.

sreynen’s picture

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?

dvessel’s picture

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.

ohnobinki’s picture

+

KrisBulman’s picture

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.

Jeff Burnz’s picture

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.

sreynen’s picture

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?

Jeff Burnz’s picture

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

catch’s picture

laura s’s picture

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.

Todd Nienkerk’s picture

sun’s picture

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.

KrisBulman’s picture

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

sun’s picture

@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.

bdone’s picture

subscribing...cause it it sounds useful

Jeff Burnz’s picture

RobLoach’s picture

RobLoach’s picture

Status:Needs work» Needs review
StatusFileSize
new64.46 KB
new6.27 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch thememoduledependency.patch. Unable to apply patch. See the log in the details link for more information. View

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

Snugug’s picture

Is there any movement on this?

loominade’s picture

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

rcross’s picture

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.

rcross’s picture

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

sreynen’s picture

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().

Snugug’s picture

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.

mgifford’s picture

Status:Needs work» Needs review

go bot.

Status:Needs review» Needs work

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

yannickoo’s picture

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

Snugug’s picture

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.

thedavidmeister’s picture

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
anandps’s picture

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)

thedavidmeister’s picture

@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.

davidneedham’s picture

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. :-)

thedavidmeister’s picture

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.

sun’s picture

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

thedavidmeister’s picture

StatusFileSize
new21.92 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch theme_dependencies-474684-46.patch. Unable to apply patch. See the log in the details link for more information. View

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.

thedavidmeister’s picture

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 :)

thedavidmeister’s picture

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 :/

KrisBulman’s picture

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: [meta] 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!

thedavidmeister’s picture

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.

thedavidmeister’s picture

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 :/

sun’s picture

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().

thedavidmeister’s picture

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.

thedavidmeister’s picture

jhedstrom’s picture

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

Bumping to 8.1.x.

davidwbarratt’s picture

Version:8.1.x-dev» 8.0.x-dev
Status:Needs work» Needs review

Moving to Needs Review and 8.0.x to kick off the test bot of the patch submitted in #47.

Status:Needs review» Needs work

The last submitted patch, 47: theme_dependencies-474684-46.patch, failed testing.

The last submitted patch, 30: thememoduledependency.patch, failed testing.

mgifford’s picture

@davidwbarratt do you want to re-roll the patch or should this be bounced up to 8.1 again?

Fabianx’s picture

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

As feature request this is definitely 8.1.x material.

markcarver’s picture

Issue tags:+Needs reroll

I was really giddy when I created and enabled a sub-module that was located inside the bootstrap base theme only to find out that I couldn't actually add said sub-module as a dependency. Doing so causes the theme to become non-installable due to it thinking that it's looking for a theme and not a module.

A little sad that this feature is still out in the ether. I thought it had been taken care of with all the theme services/manager refactoring that took place :-/

dawehner’s picture

StatusFileSize
new2.73 KB

Just a quick start so far, nothing elaborated like the last patch. We cannot change the API anymore so i'm not sure adding 'modules' dependencies to modules itself would make sense.
IMHO adding theme dependencies for modules should not be added due to the potential circular dependency issues involved with it.

Miguel.kode’s picture

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

This is the re-roll for this patch.

Status:Needs review» Needs work

The last submitted patch, 65: 474684-65.patch, failed testing.

dawehner’s picture

Status:Needs work» Needs review
StatusFileSize
new6.68 KB
new4.96 KB

There we go.

This uses the implicit assumption that you cannot have themes with the same name as modules, which IMHO is a valid restriction.

Status:Needs review» Needs work

The last submitted patch, 67: 474684-67.patch, failed testing.

Cottser’s picture

Issue tags:-Needs reroll

@dawehner thanks, can you please elaborate on the potential for circular dependencies, maybe give an example?

catch’s picture

I'd want to see the use case for adding dependencies on themes to modules before adding that, seems considerably more obscure than the other way 'round, and hook_requirements() can be used for obscure cases if they exist.

dcrocks’s picture

Is an attempt to install module dependencies the right response to missing dependencies for themes or would an error message be better, giving a themer a chance to install modules thru existing interfaces? Given the potential for specification mistakes, it seems it would be easier to edit a theme info file rather than uninstalling an unwanted module. And given module dependency needs it might be confusing for a themer to get error messages from the module system when messages fro the theme system are expected.

davidwbarratt’s picture

I would assume that themes would always depend on modules, not the other way around. I can't see a scenario in which a module would need to depend on a theme. Is there anything in a theme you can't do with a module? I can't think of anything.

markcarver’s picture

Re: module's depending on themes

There is precedence when a module should depend on a theme.

Take for instance https://www.drupal.org/project/bootstrap_core which provides a Bootstrap Carousel Formatter in the sub-module bootstrap_field. The field doesn't work if the Drupal Bootstrap base theme isn't enabled (which provides the bootstrap_carousel theme hook).

Also, my goal in 8.x would be to package bootstrap_core with the base theme itself (instead of having an entirely separate project).

Re: theme's depending on modules
The Drupal Bootstrap base theme does not currently require bootstrap_core, however given that a module has more power (e.g. services) to alter things like properly replacing/extending the theme registry, it would make sense to move that stuff to bootstrap_core and have the Drupal Bootstrap base theme depend on the module as well.

---

I can see the dilemma with how the current dependencies logic makes this a problem. Currently and historically, a dependency means that it's a "hard/required" dependency and should be installed first. Thus, if both the base theme and packaged module depended on each other, this would thus create "circular dependencies".

IMO though, that's really the problem: we're assuming that there's just "one type" of dependency.

The reality is far from this case though. If anything, I would say that we should also have a way to declare "soft" or rather "peer dependencies".

Would the "circular dependencies" issue still technically exist then, sure. However, that would be "works as designed" IMO, especially if we add "peer dependencies" to account for this use case.

When a module/theme combo runs into this issue, they can use "peer dependencies" instead and core would just keep track of them in a "to be installed list" (install after, not before) and just skip when already installed depending on which is installed first.

markcarver’s picture

seems considerably more obscure than the other way 'round, and hook_requirements() can be used for obscure cases if they exist.

The problem with this is that this (currently) only applies to modules. Theme's cannot implement hook_requirements() or any install/update hooks for that matter.

Besides, this feels like a very hackish way to do something that core should take into account from the get go IMO. I think the "peer dependencies" solution is a much better approach.

markcarver’s picture

Issue tags:+needs backport to D7

Also, considering that this is purely an addition. I'm tagging in hopes to see this backported (in some fashion, I know it won't be 1:1), but hopeful. This feature can alleviate a lot of issues (that are created by people who don't read project documentation) with themes and modules (e.g. a theme requiring jquery_update).

markcarver’s picture

Title:dependencies[] for themes, so they can depend on modules» Allow themes to declare dependencies on modules
Related issues:+#2659938: Allow extensions to declare "peer dependencies"

Just created a similar ticket for the "peer dependencies" concept so it doesn't highjack this issue, which is really about theme's being able to declare dependencies (of any kind) in the first place.

dawehner’s picture

So this issue clearly is about adding module dependencies for themes, not the other way round.

markcarver’s picture

Yes, which is why created the other issue.......

I was simply explaining (for @catch above) what the "circular dependency" issue you brought up is and how to "fix it". Yes, even though this issue would technically "create the circular dependency" conundrum, that's really just a byproduct and can be avoided with common sense of how dependencies work. Also, it's not like the "circular dependency" issue wouldn't be very self-evident when a page/drush command fails due to recursive/nested calls.

This is certainly fine with me as long as we plan to address it with "peer dependencies" (or whatever)... thus making the "circular dependencies" a "known/works as designed issue".

mdrummond’s picture

One note: challenge with modules depending on themes is that a sub theme that extends a parent theme would fail that test even if generally filled the requirements the module needed. Checking the parent tree for a theme could alleviate that I suppose. That could be useful.

Having themes declare module dependencies could be very useful particularly because there are many things a theme just can't do but an associated module could provide.

catch’s picture

The problem with this is that this (currently) only applies to modules. Theme's cannot implement hook_requirements() or any install/update hooks for that matter.

Besides, this feels like a very hackish way to do something that core should take into account from the get go IMO. I think the "peer dependencies" solution is a much better approach.

Yes, I was talking about the obscure case of modules depending on themes. Since that's something that only modules would have a problem with, hook_requirements() is available in that case.

Yes, even though this issue would technically "create the circular dependency" conundrum, that's really just a byproduct and can be avoided with common sense of how dependencies work.

Not if we only add support for themes to depend on modules here.

andypost’s picture

Issue tags:+Needs issue summary update
Related issues:+#820054: Add support for recommends[] and fix install profile dependency special casing, +#328932: Modules and partial dependencies - enhances[] and enhancedby[] field in modules' .info.yml files

Summary is outdated, not descriptive and there's not enough arguments.
To bundle config and module requirements there's install profiles.

The only dependency for themes is libraries everything other is a distribution/profile.
Knowing an external library that theme require to render "cool slider" is more needed then module deps.

Also enhances[] is good key to point which modules the theme supports

dawehner’s picture

Status:Needs work» Needs review
StatusFileSize
new6.71 KB
new623 bytes

@markcarver
Oh sorry yeah, partially caused by a browser tab being open for a day or two.

/me sighs about the failure.

mdrummond’s picture

It is absolutely possible for a theme to need a dependency on a module and not just a library.

A theme geared towards panels-based sites would not make much sense if panels is not installed for example. Lots of possible reasons a theme could need a module.

hass’s picture

A good example is a theme that implements HTML5 stuff and requires https://www.drupal.org/project/elements module.

markcarver’s picture

The only dependency for themes is libraries everything other is a distribution/profile.

Assuming that a theme can supplement the need for modules with "libraries" is simply not true and horribly misleading.

As stated above, a very common use case (in 7.x anyway) would be to specify that a theme requires jquery_update (which, in this case is a module not a library). I know jquery_update is touted as "not being needed in 8.x because we promise to update it", but we really don't have any real world data for this yet. Besides, this is just one example. To assume that this is just "one example/special use case" would be rather ignorant.

Just because a theme has the ability to provide libraries (just for itself btw), does not negate the fact that there are often better and more abstract solutions that already exist in existing modules.

However the real, primary, reason a theme would require dependencies on modules is for the fact that themes are continuing to be placed on the chopping block (as evidence by your statement above) for what PHP hook's they're "allowed" to participate in.

If this "idea" continues (as I'm sure it will given that themes are essentially becoming 3rd class citizens), then there's will literally be nothing more a theme can do other than to provide templates (which is horrible mistake IMO, but what do I know).

At the very least, this issue would alleviate a lot of my concerns with this process happening. It would allow themes to be shipped with a companion module that does the "PHP side of things" (see related issue I'm attaching now).

markcarver’s picture

Also enhances[] is good key to point which modules the theme supports

The key "enhances" !== "theme supports". Naming things is hard, sure, but I see very little relation between either of those, let alone what this issue is attempting to accomplish.

This issue is about declaring a hard requirement for a module in a theme (e.g. installing it when the theme is installed or preventing the installation of a theme until the module is at least present).

markcarver’s picture

Re: @catch in #80:

Not if we only add support for themes to depend on modules here.

What I was also trying to get at is that creating special cases around extension dependencies is pointless (profiles/modules/themes, etc.). They're all extensions and how we handle dependencies should work the same for all of them.

dawehner’s picture

What I was also trying to get at is that creating special cases around extension dependencies is pointless (profiles/modules/themes, etc.). They're all extensions and how we handle dependencies should work the same for all of them.

I can see where you are coming from, but when we do that, we need a way better approach, because we really want to avoid potential circular dependencies, or at least detect them. So in order to do that we would need like a global dependency tree of all the extensions. As you could imagine, this is not just a small change like this issue and needs more throughout thoughts.

For me there are other reasons why we don't want to support a module depending on a profile. Some people consider it as a good idea, to vendor login users of their distribution to their distribution, by trying to put in dependencies from their custom modules to their install profile, without a technical reason underneath it. Being able to make that impossible from the system itself, is IMHO an excellent feature, given our promised openness.