Here's an example: http://drupalcode.org/project/geshifilter.git/blob/refs/heads/7.x-1.x:/g...

This would be beneficial because:

  • We could test navbar on simplytest.me very quickly on its own without using a platform build.
  • Heres the link: http://simplytest.me/project/navbar/7.x-1.x
  • If the required versions of backbone and underscore change between navbar module revisions, you can specify the right one.
  • Saves time for those including navbar in their own profiles.
  • We can skip a step when installing.
  • Less human error will happen involving libraries folders.
  • Distribution and Profile maintainers will have two less libraries to worry about.

navbar.make

api = 2
core = 7.x

; Dependencies

libraries[backbone][directory_name] = backbone
libraries[backbone][download][type] = get
libraries[backbone][download][url] = https://github.com/documentcloud/backbone/archive/master.zip

libraries[underscore][directory_name] = underscore
libraries[underscore][download][type] = get
libraries[underscore][download][url] = https://github.com/documentcloud/underscore/archive/master.zip
CommentFileSizeAuthor
#9 0002-Issue-1968252-by-theohawse-Added-Include-a-makefile-.patch607 bytespol
#6 0001-Issue-1968252-by-theohawse-Added-Include-a-makefile-.patch1 KBpol

Comments

theohawse’s picture

theohawse commented

Looks like I missed these:
http://drupal.org/project/backbone
http://drupal.org/project/underscore

... Perhaps this issue should be with those modules separately?

-edit- turns out the modules dont get used in navbar, just the 2 js files.

saltednut’s picture

saltednut
she/her
Primary language English
Location Chicago
commented

As this is part of the spark project, the necessary libraries are downloaded by the drupal-org.make that comes with that distribution.

If the module also includes its own makefile, then the libraries will unnecessarily be downloaded twice.

So if this is added, the Spark team should remove these libraries from the Spark drupal-org.make and allow Navbar to maintain its own dependencies.

theohawse’s picture

theohawse commented

Yes, I include my own makefile as well in a custom distro, but it would be easier if modules maintained the library versions rather than distributions. Geshi Filter and Chosen already use this method.

saltednut’s picture

saltednut
she/her
Primary language English
Location Chicago
commented

@theohawse I disagree - it seems cumbersome dealing with these makefiles in multiple places and its terrible not having control over whats being downloaded. Panopoly does this too and its maddening that I have to patch the makefile inside panopoly_widgets just to use the latest Media module. That is not easier.

The most widely used distributions (Commerce Kickstart, Drupal Commons) manage all module dependencies in the drupal-org.make.

theohawse’s picture

theohawse commented

I know what you mean by having to patch panopoly... here's an issue about the navbar in panopoly_admin: http://drupal.org/node/1971292

Panopoly should really move its makefiles back into drupal-org.make or stub makefiles.

Perhaps then only modules which make exclusive use of a library (superfish, geshi filter) should contain make files for their respective libraries. This I guess is not the case with navbar.

pol’s picture

pol
Location Brussels
commented
StatusFileSize
new 0001-Issue-1968252-by-theohawse-Added-Include-a-makefile-.patch1 KB

Here's the patch

pol’s picture

pol
Location Brussels
commented
Status: Active » Needs review
pol’s picture

pol
Location Brussels
commented
Status: Needs review » Needs work

I will redo a patch using git repositories instead of a single file. So the navbar_requirements() hook will be ok.

pol’s picture

pol
Location Brussels
commented
Status: Needs work » Needs review
StatusFileSize
new 0002-Issue-1968252-by-theohawse-Added-Include-a-makefile-.patch607 bytes

Here it is !

saltednut’s picture

saltednut
she/her
Primary language English
Location Chicago
commented

Really hope this doesn't go in. Seems like an anti-pattern. See #4

jessebeach’s picture

jessebeach commented

I have to agree with brantwynn on the point that when including this module in a larger build, its included libraries might be in conflict with those of the larger project.

Pol, is there any way to check if the dependencies are met and load them only if they're not? Maybe on hook_enable? Or in hook_requirements? Maybe something in the make flow can handle this? I undertand the frustration of needing to get these libraries into your build if you're just using the module and not a distro. On the other hand, Backbone and Underscore can be very common and might be present already. We wen through pains to make sure we're using the Libraries architecture so that we play nice in a larger setup.

pol’s picture

pol
Location Brussels
commented
Status: Needs review » Closed (won't fix)

As you wish then ! Thanks for your point of view!

pol’s picture

pol
Location Brussels
commented

*double clicked, sorry*

saltednut’s picture

saltednut
she/her
Primary language English
Location Chicago
commented

Maybe something in the make flow can handle this? I undertand the frustration of needing to get these libraries into your build if you're just using the module and not a distro.

The only workaround I know is... say you wanted to use a different version of the libraries:

Since make files build from the bottom up, if you include this module at the bottom of your makefile and then reference the libraries earlier on, it will get you what you need. But at the cost of some sanity.

Basically, it would download the libs first using the make provided by the module (in this case a navbar.make). Then it would recusively override and download them again (or alternate versions) from your drupal-org.make provided by the profile.

Not ideal and slows down your build process on wasted cycles. Therefore, yet another reason why I think its more sane to just include the libs yourself.

adamdicarlo’s picture

adamdicarlo commented

A good compromise is to name the file MODULE_NAME.make.example.

That way drush make won't recurse into an abyss, and people know where to quickly copy-paste from.

Chris Charlton’s picture

Chris Charlton
Primary language English
Location Los Angeles
commented
Issue summary: View changes

There's a Drush Make flag to help with this: https://github.com/drush-ops/drush/issues/15
With that we should really consider adding a make file for the many devs that rely on Drush Make to do the repetitive grunt work.

add a command-line --no-recursion option to disable recursion across all projects

btopro’s picture

btopro commented

MODULE_NAME.make.example is pretty standard way of avoiding this obnoxious issue (auto recursion)