Drush site-install and sites using an installation profile cannot work with less.module as it's too strict about finding lessphp

I would like #2 or its equivalent.

For "normal" users that download the module and install it on the module list page, I want the installation to fail if the library is not found.

I also want it to fail if the library is not found in a Drush or install profile install.

I believe that its important for the module to not be enabled if it is not able to perform its function through the absence of the required lessphp library.

I don't want to give the user a sense that things are fine, when the module will fail to perform, especially if there are modules that depend on the this module to function.

I agree that this needs to be resolved, but it still needs to block installation if the library is absent.

Really? I can't think of any other modules that block installation when they can't find a third party module or API key they need to function. All the modules I can think of that rely on something external politely throw an error and then proceed to do nothing - WYSIWYG, Google Analytics, plupload..

I personally don't see the advantage for "normal" users "fail to install" has over "install but don't do anything, and throw an error on every page".

Since the lessphp library was removed from being bundled with this library (for good reason), for the sake of "normal" users this module causes any profile built by professional users that include the module to completely fail building.

Anyway, comment out line 57 of less.install, this one:

  case 'install':

And the error becomes a runtime problem rather than an install time problem. I'm going to get lunch, and then I'll send through some proper patches. At least my build isn't dead anymore >.<

Here's a patch that implements suggestion #2, it just loads the libraries' module file and then checks for existence of libraries_get_path() before calling it. Ends up being that little bit more robust when you're installing LESS with a profile instead of the "normal" way through the UI. Well.. it works on my setup at least! :P

IMO if the decision is made not to do #1 because corey doesn't like that idea, i think we probably *should* try to look harder for the library when the fate of the whole site build hangs in the balance.

not recommended, as you just keep having to hardwire lots of special cases that libraries.module should "just work out" for you

Well, that's a bit of FUD. The entire Libraries module is 102 lines long across just two functions, they're not really about catering for module developers with "special cases" there. The opposite is more true in that they're encouraging good standards by forcing developers who want to leverage the Libraries API to do things the "right" way.

Libraries currently does this in libraries_get_path (and nothing else):

• Look in "profiles/[profile-name]/libraries"
• Look in "sites/all/libraries"
• Look for a libraries folder within the folder where settings.php for this site is located
• If nothing is found, return at least "sites/all/libraries" as a default fallback

It wouldn't be very hard to check the first 3 places as a fallback if the libraries module is not installed on a site and that's inline with what is supported by D7, and was the same in d6 core anyway. You'd only have to update the list once every major release of Drupal at the most.

I reworked less_requirements() and _less_inc_path() a bit here to make something a bit better (hopefully) than the last patch I pushed.

Here's a quick summary of what I'm attempting here:

• less_requirements was re-implementing what _less_inc_path was written to achieve, so I deleted the repeated code and pulled less.module in with module_load_include() so we could re-use the main function
• _less_inc_path what I did in the last patch where it makes sure to try and include libraries explicitly before trying to use it
• added _less_inc_path_filter_path_array() which is a simple helper function that you can pass an array of paths to test and it returns the first one that exists, this should make it pretty easy to manage the list of fallback paths that we're testing while we're looking for the lessphp library. The path list/order I'm using mirrors the way core looks for modules.

I'm hoping that by putting the path search all in the one place, it becomes a bit easier to maintain (as well as fixing the issue raised here) :)

Update: I've done some testing for that previous patch by building a platform in Aegir and creating a bunch of new sites with this profile and doing a basic test with nested styles in my .less file to see that it is working ok after installation.

With Libraries, lessphp is in:
* /profiles/profilename/libraries - works
* the less module's directory - works
* sites/all - works

Without Libraries, lessphp is in:
* /profiles/profilename/libraries - works
* the less module's directory - works
* sites/all - works

Wasn't sure how to test putting the library in the site's folder at install time with a profile, so I couldn't test that it was working at install time.

Hope that helps get this committed :)

Just tested #4 on BOA. Works fine! Thanks!

Thanks for patch #4, works for me :) (lessphp in profiles/profilename/libraries)

Ok, I like most of the work in #1536582-4: Drush site-install and sites using an installation profile cannot work with less.module as it's too strict about finding lessphp but I'm wondering about the "looking harder" part.

Since the latest version of LESS now has Libraries marked as a dependency, isn't it redundant to do the same work that the Libraries module does, again?

This issue is that including this module in an install profile currently makes it impossible to install sites on that profile. No module should cause a whole profile to fail installation simply by enabling it without a stellar reason.

If you're going to cause installation of a whole site to fail when the LESS module can't find it's library, then you should be doing everything you can to ensure that if the lessphp library is discoverable you do discover it.

I didn't actually realize that Libraries was added as a dependency when I was testing my code, which is why I added that "try harder" bit. Removing that aspect of my patch is probably fine if you've also added the dependency for the Library. That said, it looks like installation of modules is subtly different during site installation to standalone post-installation so if you're going to remove the "safety" part of my patch please test the shit out of your changes with the Libraries module absent so that profiles aren't needlessly broken again.

Thanks for reviewing the patch btw.

Relying on libraries API by itself won't work for fresh installs. This is because the requirements are checked before any modules are installed (and, hence, libraries API is not available to help you with the check).

Either the requirements code needs to be better (which patch 4 provides) or this module needs to stop blocking the installation process (which sounds like a much better option to me).

If you're stuck until this patch is committed, the following in your makefile will let your site install:

projects[less][version] = 2.5
projects[less][patch][] = "http://drupal.org/files/1536582-look-harder-for-library-4.patch"

libraries[lessphp][download][type] = "file"
libraries[lessphp][download][url] = "http://github.com/leafo/lessphp/tarball/master"

@ruperj - actually, in the patch in #4 because I'm using module_load_include('module', 'libraries'); just before I call the libraries API, we can use it even in fresh installs because we're manually including the relevant files to get access to the functions we need.

#4 will leverage Libraries if you have it, and simulate it if you don't and will do both in either a fresh site install, or at a later date.

Thanks for the makefile example, I'd only suggest that you lock your lessphp download to a specific revision rather than rely on "master"; I've personally had some... interesting experiences with bug regressions and newly introduced syntax changes across different versions of lessphp.

libraries[lessphp][download][type] = "git"
libraries[lessphp][download][url] = "git://github.com/leafo/lessphp.git"
libraries[lessphp][download][revision] = "cd4af26010a7ed6faff48fcc0cfc091188422621"

Obviously replace that revision hash as required, that's just the one that is current today :)

When including a pull directly from a repository like this, I think you can specify a tag:

libraries[lessphp][download][type] = "git"
libraries[lessphp][download][url] = "git://github.com/leafo/lessphp.git"
libraries[lessphp][download][tag] = "v0.3.4-2"

This allows you to make it a little more "official" if you prefer.

lol, we're drifting off topic now, talking about best way to write a make file.. can we get anything committed to fix the original issue? the "looking harder" bit is only one array and one (small) function so surely you could drop it in as-is because we know it works and is never even fired if Libraries API is present.

We can always open a separate issue to "rely on libraries API exclusively to scan file paths" and link to this thread so we remember what needs to be tested in order to be comfortable that we are indeed always attempting to do something redundant here and it is safe to crop out that extra step.

To be honest, not sure when I'm next going to feel like doing a round of testing for this thread, and nobody else looks like they're jumping at the chance to help out so I feel kinda bad if we're holding back a working fix for a legitimate problem because everyone is waiting on me to watch spinny wheel in Aegir for an hour or so.

@thedavidmeister I had spotted you were loading the include manually, I just wrongly assumed that because the libraries module isn't enabled yet, the include wouldn't load. That's pretty cunning.

I'm in total agreement with commiting the patch in #4 and opening a new issue for libraries. Given how 3 people have said it fixes their issue, can't we call it RTBC now?

sure, why not

Couple of updates

Install doesn't block on absence of lessphp library.

If library is missing, files will be output to the page without modification, meaning with their .less extension.

This works fine in install profiles, but I don't have proof about Drush Make, tho it should work fine.

I'm also committing this, so you should be able to grab a dev of this soon.

Hey Corey, is this in dev yet? I like the work that I see in that latest patch.

Also, totally a technicality but Drush Make doesn't actually do anything with regard to installing sites, it's just a tool for downloading Drupal projects and applying patches to code so nothing we've discussed here can cause it to fail ever. The name of this issue is wrong really so I'll update it to make it more google-able. Drush site-install is the bit that actually tries to fire up a new site with a database and if that fails then developers using Aegir/Hostmaster will see things failing too.

I really think the crux of the problem was to do with install profiles so I think we're in the clear here if that is fixed. I also saw the bit in that patch about downgrading the severity of the requirements warning being thrown, which is nice.

Trying to apply these patches within a drush.make file. (Using projects[less][patch][] = URL).

- #4 works fine, installation doesn't break anymore
- #17 gives error on drush.make ( "Unable to patch less with less.patch.")

Composer support would be neat. Has no issues installing using that.

Fix for this should be in the latest dev.

Will be pushing new version soon.

The reason I think that #1536582-17: Drush site-install and sites using an installation profile cannot work with less.module as it's too strict about finding lessphp was not applying is that it was based of a version of dev, rather than release.

Also it was not intended to be used, but purely for review.

Forgot to mark as mixed.

I am tying to use lesscss module for drupal5 (installation has been sucesfull) but not working/not compiling less files for me can any one help me out.

NOTE: I have added less file as drupal_add_css('myfile.css.less'). Also I am not getting any error on page.

LESS CSS Preprocessor version: 2.6

@anilnaut111 - that question has nothing to do with this thread. Please open your own ticket, as this thread isn't even open atm..