🐛 N°8278 - Avoid autoloaders class name conflict by ensuring all existing have a unique name

[Molkobain]

Base information

Question	Answer
Related to a SourceForge thead / Another PR / Combodo ticket?	No
Type of change?	Bug fix

Symptom (bug)

When running a setup, it crashes during compilation saying that an autloader class with the same name is already loaded:

( ! ) Fatal error: Cannot declare class ComposerAutoloaderInitd751713988987e9331980363e24189ce, because the name is already in use in C:\wamp64\apps\itop\3.2\extensions\sample-module\vendor\composer\autoload_real.php on line 5
Call Stack
#TimeMemoryFunctionLocation
[...]
110.334411825280require_once( 'C:\wamp64\apps\itop\3.2\extensions\sample-module\vendor\composer\autoload_real.php )...\autoload.php:23

PHP error occured : msg=Cannot declare class ComposerAutoloaderInitd751713988987e9331980363e24189ce, because the name is already in use, no=64, file=C:\wamp64\apps\itop\3.2\extensions\sample-module\vendor\composer\autoload_real.php, line=5

This is a very similar symptom than what we had several times (without understanding it at the time) during the development of new portal modules compatible with both iTop 2.6 & 2.7. Which was actually when we introduced autoloaders and composer in portal modules.

Reproduction procedure (bug)

1. On iTop 3.2.1+
2. With PHP 8.1.31
3. Create a module with a composer.json file that contains nothing but the license and the autoload configuration
4. Run composer install on the module's composer.json file
5. Run the setup and make sure to select the user portal in the options
6. See the error message during compilation

Note: You'll find attach to this PR a minimal module with the corresponding composer.json file to ease reproduction.

Cause (bug)

Both the itop-portal-base/portal and my module have the same PHP class name for the autoloader: d751713988987e9331980363e24189ce

The regression cause is that in 3.2.1, the itop-portal-base module's autoloaders were udpated in 9fadbb5e. Changing the autoloader class name to ComposerAutoloaderInitd751713988987e9331980363e24189ce which is kind of the default one (see root cause section).

Root cause

In the latest versions of Composer, when there is a composer.lock, the suffix of the autoloader class (e.g. d751713988987e9331980363e24189ce) is a hash (md5) based on some of the information in the composer.json file.

$relevantKeys = [
    'name',
    'version',
    'require',
    'require-dev',
    'conflict',
    'replace',
    'provide',
    'minimum-stability',
    'prefer-stable',
    'repositories',
    'extra',
];

If none of the properties are defined in the composer.json, then the hash will always be the same: d751713988987e9331980363e24189ce

Composer source references:

• AutoloadGenerator calling the Locker::getContentHash()
• Locker::getContentHash()

Note: The composer.lock file is not generated if you only dump the autoloaders. You have to install/update for it to be generated. Which is not done on all modules.

Proposed solution (bug and enhancement)

Add the missing "name" information in modules' composer.json files and re-dump the autoloaders. It makes sense as module are supposed to be individual packages. Also this is something that is done in many existing Combodo's modules.

Exception for itop-portal-base/portal/composer.json file, as it is not at the root of the module, naming it combodo/itop-portal-base would be misleading. Naming it something like combodo/itop-portal-base_portal would be weird as well.
So I went with using the autoloader-suffix conf. property (documentation) that forces the suffix of the class name.

Mind that we could consider forcing the suffix of autoloaders in all modules, that way we would ensure uniqueness without relying on Composer's black box algorythm. But as it's not the current policy in existing modules, I didn't. Feel free to tell me if you want to adjust the PR accordingly.

Workaround

For people having the issue with their module, you can juste set a "name" property in your composer.json:

{
  "name": "molkobain/some-module",
  "autoload": {
    "psr-4": {
      "Molkobain\\iTop\\Extension\\SomeModule\\": "src/"
    }
  }
}

Checklist before requesting a review

• I have performed a self-review of my code
• I have tested all changes I made on an iTop instance
• I have added a unit test, otherwise I have explained why I couldn't
• Is the PR clear and detailed enough so anyone can understand digging in the code?

Checklist of things to do before PR is ready to merge

• Discuss with Combodo how they would like the unit test to be performed, once we agreed on the solution

[Molkobain]

The sample module sample-module.zip

[Molkobain]

Proposal of unit tests:

• Check every composer.json files not in /lib nor in /**/vendor
  • Assert that its autoloader_real.php has not the "default" suffix (will be run by CI for all existing modules included in packages, as well as those with new commits)
  • If on root level of project or module, assert that it has a "name" property
  • If not on root level, asset that it has either an "autoloader-suffix" or a "name" property (for the later it could be a included lib)

[Molkobain]

@jf-cbd careful when you change the target branch, sometimes you need to "rebase onto" the original branch as well. Otherwise there is a whole lot of extra commits in the PR like now. You should either do it or poke the author so he does it.

I'll do it :)

[Molkobain]

@jf-cbd Done!

[Molkobain]

Btw, you changed the target branch to support/3.2.1 which means that it will be in a micro version (e.g. 3.2.1-1), is it on purpose?

[jf-cbd]

Alright, thanks for doing it @Molkobain !

Concerning the problem of autoloaders class name conflicts, we decided to split the problem in two part :
The ticket 8278, that fix it in iTop sources, but won't prevent 2 extensions to have conflicts. The fix (yours) will be integrated in 3.2.1-1, so your PR should be merged soon.
The ticket N°8280 (Risk of incompatibility between extensions) is about how to prevent future risks : documentation, tests (we added your proposal in the ticket) and potential other solutions, like warning in the setup.
Its fix will probably arrive in 3.2.2.

[Molkobain]

Alright, sounds great!

[steffunky]

I met the same issue while working on a client extension with a minimal composer.

Adding a autoloader-suffix did the trick

[jf-cbd]

Perfect, let's merge :)

[jf-cbd]

Thank you @Molkobain 😉