Magento still assumes that modules reside in app/code directory

[joshdifabio]

There are a number of places in the Magento 2 code base which still assume that modules reside in the app/code directory. As a result, the module registrar which was implemented in #1206 isn't yet usable.

As people are beginning to experiment with the module registrar and are finding that it doesn't work as expected I think we should work out what needs fixing. I would like to use this ticket to maintain a list of the issues which still need to be resolved so that a number of us can tackle these issues together without duplicating effort.

• Magento\Framework\Module\Dir\Reader::getConfigurationFiles() and getComposerJsonFiles()
• Magento\Framework\Module\Dir\Reader::getActionFiles()
• Consider implications of Magento\Framework\Module\Dir\Reader::setModuleDir() and getModuleDir()
• Eliminating relative paths for XML schematic references (using URNs?) is another one. Relative paths a pain when the module moves.

This list is far from complete. I'll add more items to the list as time allows, and I invite others to contribute.

[alankent]

Eliminating relative paths for XML schematic references (using URNs?) is another one. Relative paths a pain when the module moves.

[joshdifabio]

Eliminating relative paths for XML schematic references (using URNs?) is another one. Relative paths a pain when the module moves.

Great point, thanks.

[Vinai]

@alankent Do you really think relative schema paths are such an issue?
I thought the dev would simply use the path depending on how she would want the extension to be installed.
For example:

Module residing in vendor/:

<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
        xsi:noNamespaceSchemaLocation="../../../../lib/internal/Magento/Framework/Module/etc/module.xsd">

Module residing in app/code/:

<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
        xsi:noNamespaceSchemaLocation="../../../../../lib/internal/Magento/Framework/Module/etc/module.xsd">

So modules could not be moved from one location to another without changes. But from what I understood that isn't the goal of this PR. Right?

[joshdifabio]

@Vinai I think it's a big grey. What if Magento\Framework moves to vendor? Essentially, that could never be done without a BC break as doing so would (soft) break all existing module implementations due to the broken XSD references. And without allowing Magento\Framework to reside in vendor, it can't be used as a dependency without using Magento Composer installer. What do you think?

[Vinai]

True, I dind't think of that. If the location if the XSD changes it is a (soft) BC break. I don't think I can understand all the consequences. From my (limited) perspective I would like to see Magento\Framework move to vendor/.
On another note, maybe the XSD files could be moved into the separate packages containing only the interfaces which Alan spoke about somewhere.
Besides the origin moving, I don't see an issue for third parties using relative paths to the XSD files.
But to be honest, decoupling the physical location of the XSD from the URI is probably a good idea.

[davidalger]

@joshdifabio What would really help here is to create an example module which leverages the different aspects of the architecture. Write unit tests for it, and run them with the module in both locations. They should break in one location or the other if the framework isn't using the registry properly.

Another (long-term) option would be to find the integration tests which verify the functionality of these core pieces of the framework and alter them so they apply to multiple module location scenarios.

[Vinai]

@joshdifabio I think in the "todo" list you missed the class name Reader after the namespace Magento\Framework\Module\Dir, right?

[joshdifabio]

@joshdifabio I think in the "todo" list you missed the class name Reader after the namespace Magento\Framework\Module\Dir, right?

Good spot!

[Vinai]

The issue seems to be that \Magento\Framework\Filesystem\Driver\File::getRelativePath() is not fully implemented.

    /**
     * Retrieves relative path
     *
     * @param string $basePath
     * @param string $path
     * @return string
     */
    public function getRelativePath($basePath, $path = null)
    {
        $path = $this->fixSeparator($path);
        if (strpos($path, $basePath) === 0 || $basePath == $path . '/') {
            $result = substr($path, strlen($basePath));
        } else {
            $result = $path;
        }
        return $result;
    }

If $path is not within the $basePath it simply returns the $path as it is, not the relative path.
This causes the later code to fail. From what I can see, once this is fixed all the methods you listed above should "just work". I'll have a go at it soon (but not tonight) 💤

[joshdifabio]

I'm going to take a look at Framework\App\Router\ActionList and the related functionality some time soon unless someone is already looking at it? @vrann @Vinai

At present, it is assumed that file paths relative to the module directory map directly to class names (e.g., app\code\Magento\Catalog\Controller\Category\View.php is assumed to contain Magento\Catalog\Controller\Category\View), but this will not work for modules located in vendor. If someone is already looking at this then please let me know.