For me autoloading (in my case PSR-0) fails when classes are used in hook_install().

Steps to reproduce this

I use a clean minimal installation and activate the example.module via drush en -y example. It pulls in xautoload as dependency.

example.install

function example_install() {
  $x = new \Drupal\example\A();
}

example.info

name = Example
core = 7.x
dependencies[] = xautoload

lib/Drupal/example/A.php

class A {}

Expected results

Autoloading when example_install() is run regardless of whether xautoload was previously enabled or not.

Actual result

If xautoload was already enabled everything works fine. If it is pulled in as dependency it doesn't work.

  • drush en -y xautoload; drush en -y example; … works fine
  • drush en -y example; … crashes because it can't find class Drupal/example/A

Comments

donquixote’s picture

Hi!
Yes, having classes available already during modulename_install is a challenge.

Have a look at this, "Module install time".
https://drupal.org/node/1976210

It would be great if we can do better than this. Investigation is welcome!

torotil’s picture

It seems that the module-registry in xautoload is the culprit in this case. This registry seems to be the core of most features of xautoload but makes additional configuration necessary if you want to use autoloading in hook_install().

donquixote’s picture

What do you mean by module registry?

torotil’s picture

I mean the data structure that is changed when xautoload()->registerModule(__FILE__); is executed. ;)

Sorry for creating my own term.

An autoloader that is stateless (except for caching) wouldn't have this issue.

donquixote’s picture

I mean the data structure that is changed when xautoload()->registerModule(__FILE__); is executed. ;)

It is not so much this data structure that is a problem, but knowing when the registerModule() is triggered and when it is not.

An autoloader that is stateless (except for caching) wouldn't have this issue.

At some point you need to tell the class loader about the module namespaces and the associated directories.
You can only do this when you actually know which modules are enabled and which are not.
This is particularly difficult on install time.
How would you do it?

torotil’s picture

How would you do it?

With the feature set of xautoload I honestly don't know. After I've posted this issue last night I've played around a bit though and I came up with a more minimalistic approach.

For PSR-0 or PSR-4 autoloading as it is in Drupal 8 (as far as I understand it) you only need to look at the namespace of the class:

  • The first part needs to be "Drupal"
  • the second is the module or profile name -> use drupal_get_path() to get the path to the module.
  • The rest of the fully qualified class name becomes the path below lib/ or lib/drupal/$module

I've implemented this in http://drupal.org/project/psr0 and it seems to work fine even in module.install.

The new module covers only a tiny tiny subset of the features of xautoload, so I think it's fine to have an extra module for the while. Currently for my modules this is all the autoloading that I need.

donquixote’s picture

For PSR-0 or PSR-4 autoloading as it is in Drupal 8 (as far as I understand it) you only need to look at the namespace of the class:

  • The first part needs to be "Drupal"
  • the second is the module or profile name -> use drupal_get_path() to get the path to the module.
  • The rest of the fully qualified class name becomes the path below lib/ or lib/drupal/$module

I think I played around with the same idea once in the past :)
One issue could be performance (having to call drupal_get_path()). But even that could be mitigated with a cache..

About having a separate "psr0" module: I'm totally ok with that for your own purposes.
The scenario where this becomes less desirable is if you want to install two modules A and B, where A depends on xautoload and B depends on psr0.
(I am not an authority in this matter, this is just my personal opinion)

I am going to think about whether xautoload should be simplified.

donquixote’s picture

Nice and small module, btw!

torotil’s picture

The scenario where this becomes less desirable is if you want to install two modules A and B, where A depends on xautoload and B depends on psr0.

If I understood correctly, the worst case is that both modules do their work and swallow up a bit more resources than necessary.

One issue could be performance (having to call drupal_get_path()).

I'd really like to see benchmarks for that especially with the usual opcode-cache in place. Performance-wise the calls to file_exists() could be even worse. Also querying the system table directly might be faster. As long as there is no measurable benefit I'd rather stick to the simple solution though.

Thanks a lot for your feedback!

donquixote’s picture

As long as there is no measurable benefit I'd rather stick to the simple solution though.

You could do some tweaks to your module, but in general I think it is pretty ok. And you already have a static cache array to avoid having to hit the database for every class lookup.
- strpos() + substr() + str_replace() or strtr() sometimes work faster than explode() + implode().
- indirection has a cost in PHP. You could put the static cache array into the autoload callback itself, so that on average calls you have zero indirection.

If I understood correctly, the worst case is that both modules do their work and swallow up a bit more resources than necessary.

Yup. And people have two extra modules to install instead of one. And read two module pages.
All survivable.

donquixote’s picture

Version: 7.x-2.7 » 7.x-3.x-dev
Status: Active » Needs review

This should be fixed now in 7.x-3.x with this commit:
http://drupalcode.org/project/xautoload.git/commitdiff/b556bd73dd9444149...

@torotil could you try?

donquixote’s picture

Issue summary: View changes
Status: Needs review » Fixed

I'd say we consider this fixed in 7.x-3.3.
(and of course in the upcoming 7.x-4.x branch)

Status: Fixed » Closed (fixed)

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