Hello,

I'm just opening an issue to explain my scenario, and discuss what would be the best way to solve it (which may or may not imply changes in Whois lookup module).

So, I'm massively using drush make, which makes me a perfect candidate for the included Make file which automatically downloads phpwhois library. But, something does not work well.

  • First of all, this does not allow the developer to choose which version of phpwhois library to include, which may be a drawback when one wants to ensure a constant version over time, and upgrade only when decided. Or when the latest version presents a bug and older version is recommended.
  • Secondly, this downloads phpwhois files to "modules/phpwhois/", which prevents many patterns to behave properly:
    1. modules may be downloaded in another destination than "modules/" (see below, extract of Drush make's README file)

      - `subdir`

      Place a project within a subdirectory of the `--contrib-destination`
      specified. In the example below, `cck` will be placed in
      `sites/all/modules/contrib` instead of the default `sites/all/modules`.

      projects[cck][subdir] = "contrib"

    2. if another module needs phpwhois library, then, either each module will use its own library, which may lead to conflict (if both modules load same library file, different location, on their own), or the second module has to rely on Whois lookup module, which may not be recommended
  • if two modules include a makefile downloading the same library, they will probably get in conflict, when calling drush make.

Consequently, my point is that Whois lookup make file shouldn't try to add phpwhois library.
Moreover, it is a good practice to ask users to copy libraries to sites/all/libraries folder (or equivalent), so that modules can share them if needed.
Whois lookup module could also get dependent of Libraries API module, but that doesn't seem mandatory.

What do you think?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

helmo’s picture

You make a valid point, the makefile definitely has drawbacks.

I could remove this line (libraries[phpwhois][destination] = "modules/whois") then it would end up in the libraries folder. But then libraries API would become a dependency. Which I was hoping to avoid.
But since then the Libraries API module has become stable, so it might be not such a bad thing anymore.

Is there a way to specify in your makefile that make should not do the recursion step?
If not then including this makefile might be doing more harm then good.

David Stosik’s picture

There is no way to specify in my makefile not to recurse, nor is there a way to override recursed makefiles...
I realize that libraries is not compulsory : many modules expect libraries to be in sites/all/libraries, but don't have library module as a dependency. In Whois lookup code, the only thing to change would be from
"if libraries module is absent, then find phpwhois in Whois lookup module folder"
to
"if libraries module is absent, then find phpwhois in libraries folder".

David

helmo’s picture

Status: Active » Needs review
FileSize
2.7 KB

This patch should probably do it.

I think I'll rename the makefile to whois.make.example

helmo’s picture

Committed.
I'll leave the makefile unchanged until someone who actually uses it comments...

helmo’s picture

Status: Needs review » Fixed

Nothing more to do here I guess.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.