Module files can be loaded through the .info file in D7. It would be nice if we could extend this feature to library files. Fortunately, the API provides hook_registry_files_alter() that should allow us to do just that.

I have attached a patch that will allow modules to define whether or not their files should be autoloaded.

Files: 
CommentFileSizeAuthor
#18 1092270-17-libraries-autoload-alternate.patch2.74 KBtstoeckler
FAILED: [[SimpleTest]]: [MySQL] 128 pass(es), 0 fail(s), and 71 exception(s).
[ View ]
#13 1092270-13-libraries-autoload.patch919 byteststoeckler
PASSED: [[SimpleTest]]: [MySQL] 126 pass(es).
[ View ]
allow_libraries_to_autoload-D7.patch1.21 KBelliotttf
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch allow_libraries_to_autoload-D7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

Title:Allow modules to have their librarie files autoloadedAllow modules to have their library files autoloaded

Status:Needs review» Needs work

Hmmm, interesting patch.

I don't quite get what the purpose would be for modules to have their library files autoloaded. Autoloading only makes sense for PHP files, and only for those containing classes.

What I would imagine is for modules to declare that they depend on a library and we automatically load that library in libraries_init().

Or something like that. Haven't really thought the whole thing 100% through, but I'm not sure that your patch is the right way. Would love to hear you defend it, though :)

Hm, good point, I was going down the "bigger hammer" route without thinking too deeply about what I was doing.

Perhaps another option would be allowing individual files to be added to the registry, i.e.:

<?php
function hook_libraries_info() {
 
$libraries['foo'] = array(
   
'name' => 'Foo',
   
'vendor url' => 'http://foo.com',
   
'download url' => 'http://foo.com/download',
   
'files' => array(
     
'php' => array(
        array(
'file' => 'file.php', 'autoload' => TRUE),
      ),
    ),
  );
}
?>

Then in libraries_registry_files_alter() only add the files that have autoload set to true. There would obviously need to be a few other minor changes to check if the file was an array or a string (if we intended to to support both) but I think this method would be a good middle ground.

#3 sounds better, although it's probably incompatible with our file definitions.

Overall, I wouldn't mind to simply register any kind of PHP file in the class registry. Most external PHP libraries contain classes anyway these days.

The issue with that, sun, is that some libraries have their own autoload logic that they may rely on, especially if they're using the SplObjectLoader approach. There is also the potential for class name collisions with the "blind parse everything" approach. In principle I agree that would be nice, but in practice I think it will be impractical.

Thanks, @Crell! Your input is extremely valuable for these kind of issues :)

Alright, so right now, we have the following code for 'php' files:

<?php
 
// Load PHP files.
 
if (!empty($library['files']['php'])) {
    foreach (
$library['files']['php'] as $file) {
     
$file_path = DRUPAL_ROOT . '/' . $path . '/' . $file;
      if (
file_exists($file_path)) {
        require_once
$file_path;
       
$count++;
      }
    }
  }
?>

Considering that we'd be registering files containing classes to autoload via Drupal's class registry separately and those wouldn't have to be included manually, it might make sense to add a new/separate file group next to 'php' then?

Disclaimer: I don't actually know the guts of libraries API at all, so I'm speaking in general terms.

For OO PHP code (which is to be fair, the vast majority of it at this point), really all the libraries API needs to do is handle autoload registration. The actual loading of classes is something PHP itself can/should handle. The only catch there is whether we use our autoloader or one that the library provides / expects. I can't say for sure if any systems will actually break if they use our autoloader instead of their own. The most likely autoloader for them to use is the SplObjectLoader standard (aka Java-esque).

So... I suppose there's 3 possibilities:

1) Specific files should always be loaded when Drupal loads, period, because it's procedural code.

2) Specific files should be parsed by the registry, and then the autoloader takes it from there.

3) Some other autoload mechanism provided or specified by the library needs to be registered with PHP, and it takes it from there.

Make sense?

Trying to translate this into Libraries API terms:

1. We need to differentiate between procedural PHP files, which we simply include upon loading and OO PHP files.

2. For OO PHP files we need some sort of 'autoload callback' which defaults to the drupal code registry (something like the path in the op) and can be set to something that uses SplObjectLoader. Not really sure how that would work, but I think it should be doable.

3. Does it make sense to register the files for auto-loading during the 'loading' phase of the library or do we need to make a special provision for that?

The autoload needs to be registered at Drupal startup. I suppose you could do it at library init, actually, but it would be really nice if you didn't even need to do that step and Drupal/PHP "just found it".

For the callback, hm. I don't know that it makes sense to register the actual autoload function itself. Rather, you want to register "something callable", which could be a function or a method of an object as in SplObjectLoader. You also do NOT want to register more than once. So if using the core registry, you would want to at install time have those files parsed and then forget about it.

I'm actually wondering if any Spl-structured libraries would indeed be incompatible with the registry. I'm not sure. The caveat is that they tend not to have easily enumerated files like our files[] array, so finding all of the files to parse may not be trivial.

Over in #1023258: Make 'files' (and 'integration files') declarations consistent PHP files get converted from:

<?php
$library
['files']['php'] = array('foo.php', 'bar.php');
?>

to
<?php
$library
['files']['php'] = array(
 
'foo.php' => array(),
 
'bar.php' => array(),
);
?>

So I guess something like:

<?php
$library
['files']['php'] = array(
 
'foo.php' => array('autoload' => 'SPLObject'),
 
'bar.php' => array('autoload' => 'Drupal Registry'),
);
?>

(please don't mind the actual array values)
isn't too far away. And I think having these values, whatever they turn out to be, defined on file level (not on library level) is the only thing we can reasonably get away with.

One architectural shift this would bring with it, is that we would have to detect all libraries in hook_init(), but I guess that's not necessarily a bad thing.

Can we just keep this simple and re-use the autoload mechanism we already have? Let's just add this to a module's .info file:

library files[my-library][] = src/my-file.php   # Adds sites/all/libraries/my-library/src/my-file.php to the autoload file registry.

StatusFileSize
new919 bytes
PASSED: [[SimpleTest]]: [MySQL] 126 pass(es).
[ View ]

That would be something like this.
Completely untested.
Pro:
* Easy
Cons:
* Non-pluggable autoload-mechanism
* Only works for module-provided libraries

Status:Needs work» Needs review

Looks pretty good already.

That, combined with the idea of also plugging into hook_requirements() in #1215376: Autoload and populate drupal_get_filename() with libraries.module during Drupal installation could be a huge step forward.

However, I'm not sure what happens if multiple modules declare the same library files in their .info files. We'd have to test that.

Having #1321372: Load the files before stating the Library is loaded would allow modules to define how to load the Libraries via the "post-load" callback. This means that AutoLoaders could kick in like in Symfony.

....Are we talking about class autoloaders? Or are we talking about the ability to declare Libraries in a module's .info file?

This is for class autoloading (aka core's file registry) so that after my module is installed I can just do $object = new MyLibaryClass(); without having to invoke any library.module APIs.

Status:Needs review» Needs work

Here is an alternate approach. It is a little more verbose, but it is a little more flexible in that it also works for libraries provided by info files.

With this patch you could do:

<?php
// In hook_libraries_info() or in an info file:
$libraries['foo']['files']['php'] = array(
 
'mySuperCoolClassLivesHere.php' => array('autoload' => TRUE),
);
// In your module, this should just work:
$bar = new mySuperCoolClass();
?>

I didn't actually test this, this is more a proof-of-concept, so I'm setting to 'needs work'.

StatusFileSize
new2.74 KB
FAILED: [[SimpleTest]]: [MySQL] 128 pass(es), 0 fail(s), and 71 exception(s).
[ View ]

Darn...

Oops, calling &drupal_static() correctly would be nice, but I hope you get the idea...

Very cool, I'm liking this....

+++ b/libraries.moduleundefined
@@ -9,10 +9,46 @@
function libraries_flush_caches() {
+  registry_rebuild();
   return array('cache_libraries');

Does the Registry flush the cache for us when hook_flush_caches() is called?

+++ b/libraries.moduleundefined
@@ -9,10 +9,46 @@
+    if (($library = libraries_detect($name)) && $library['installed']) {
+      foreach ($files as $filename => $info) {

Do you know if this will only report files that it finds? Or does the registry protect against that for us?

+++ b/libraries.moduleundefined
@@ -9,10 +9,46 @@
+        $module = (isset($library['module']) ? $library['module'] : 'libraries');
+        $module_weight = db_query("SELECT weight FROM {system} WHERE type = 'module' and name = :name", array(':name' => $module))->fetchField();

Could probably get the module weight from system_list(). Also probably not good to do it in the foreach(), might want to move that out.

Also, might be handy to have some tests ;-) .

This looks like what I would need for amazon_fps, so if/when I port that to D7 I can test this patch if it still needs it at that time.

Status:Needs work» Needs review

Moving to "needs review" for comments on the general approach here. I think this would be a really nice feature and the patch above (even though it's not 100% functional) shows it's not too hard. It just depends on how we want to it. So "needs review" doesn't mean nit-pick the code, in this case. Of course that also helps, though.

Status:Needs review» Needs work

The last submitted patch, 1092270-17-libraries-autoload-alternate.patch, failed testing.

Status:Needs work» Needs review

Oh yeah, didn't realize that...
Well then setting to "needs review" again...

Note that for 7.x-3.x we want to completely revamp Libraries API to use PSR-0 and Composer and everything. That will probably make this a lot easier.

This is possible with xautoload-7.x-2.4.
#1781794: Registration of additional PSR-0 folders in modules and sites/all/libraries

<?php
function hook_libraries_info() {
  return array(
   
'mymodule-test-lib' => array(
     
'name' => 'My test library',
     
'vendor url' => 'http://www.example.com',
     
'download url' => 'http://github.com/example/my-php-api',
     
'version' => '1.0',
     
'xautoload' => function($api) {
       
// Register a namespace with PSR-0 root in <library dir>/lib/
        // Note: $api already knows the library directory.
        // Note: We could omit the 'lib', as this is the default value.
       
$api->namespaceRoot('XALib\TestNamespace', 'lib');
      },
    ),
  );
}
?>