Rename app/code to app/modules

[colinmollenhour]

It seems that with all of the module components being moved into the app/code directories that the name "code" for this directory is far too specific and "modules" would be more appropriate.

[ArnaudLigny]

👍

[sshymko]

Internally it's already named Mage_Core_Model_Dir::MODULES.

[amenk]

👍

[andreasemer]

👍 :-)

[waynetheisinger]

👍

[sankalpshekhar]

👍

[ontic]

+1

[robouk]

👍

[RahulKachhadia]

👍

[kandy]

It maybe better to rename the "app/modules" in the "vendor"?

[tzyganu]

http://martinfowler.com/bliki/TwoHardThings.html

[fooman]

I support @kandy's suggestion - currently I do not see a reason to use the app/code/ location (except maybe historical reasons). If we could move all modules into the composer vendor directory (vendor/) this would be a consistent and permanent location for all modules without having to copy/symlink non-core extensions into the app/code directory when installing additional extensions.

[alankent]

There was a big proposed directory structure change that did not make it before dev beta - too big/risky. One of the challenges at present is all modules are located by looking for app/code///etc/module.xml (I think). So all modules need to be under the same directory. With the new config.php approach (the list of modules is created by setup script, not at runtime) one idea being played with was to include the path name of each module root directory in config.php as well. Then modules could be located in different places more easily without extra overhead per http request. (There would be more overhead when setup is run, but that does not matter so much.) That would make it easier to put modules under app/code (or better app/modules) as well as vendor/.

Oh, themes into a 'themes' directory instead of 'design' was another possible change.

[TexanHogman]

As @alankent mentioned we wanted to make this change (to /vendor) as part of dev beta and again we've looked at it for dev rc. Our struggle was to complete before the release date so the work was suspended. The complexities highlighted were a result of a single base path variable used throughout the code. When the modules were moved this required a new separate variable and we need to address each usage of base path to map to new structure. Not a difficult task but just proved very time consuming and the decision was made to not hold release for this change. I believe it was the primarily the tests that were highly impacted.

[mmenozzi]

+1 for modules under vendor.
This will maybe remove the needs of magento-composer-installer (https://github.com/magento/magento-composer-installer) for installing third party modules. No more copies or symlinks to exclude from version control.

[colinmollenhour]

A concern that I heard voiced by several at the beta forum this week was that it is messy to package an extension into a repo in such a way that is compatible with composer. An example repo would ideally look something like this (subjective of course):

• docs/
• examples/
• lib/
• scripts/
• src/ (this is the only part that belongs in app/code/)
• tests/
• README.md
• LICENSE.txt
• composer.json

Currently I do not know of a way to map the contents of src/ into app/code/ using composer because composer wants to instead place the src/ directory itself into app/code/ like app/code/Vendor/Module/src/.

Of course this can be resolved by using modman but TBH I'd like to see a solution that works without requiring modman since so much progress has been made to reduce the need for symlinks already. Of course I could just be missing some feature of composer that allows one to do this currently (aside from magento-composer-installer which would also be nice to not have to depend on).

[mmenozzi]

Hi @colinmollenhour,
yes you can map src/ in app/code/Vendor/Module/ with magento/magento-composer-install. To do so, if you have a repo like that, you can put the following mapping in the extra key of the composer.json:

"map": [
    [
        "src",
        "app/code"
    ]
]

But this is not the point. IMHO the real problem is that Magento requires to have modules in app/code. This requirement is a problem because, if you want to manage external modules with composer (and every PHP project should manage external dependency with composer), you need something that copy or symlink files in that location. Actually this something is the magento/magento-composer-installer and the usage of this tool introduces other problems. For example:

• You have to add to VCS ignore every folder/file copied by magento/magento-composer-installer every time you add a module to your composer.json.
• It's hard to modify and push to related upstream a module that you maintain installed with composer in your project. This because module's files are copied to app/code and you have to repeat the changes even in module directory under vendor. It would be much more better to have the module only under vendor so when the module is changed you can commit and push it to the related upstream.
• In the project there are two files defining the same class and using go to method IDE functionality it's messy because it asks to the user which file to open.

I think that a better approach for loading modules is like it's done by Symfony.
In Symfony to add a new bundle (or module) you have to add an instance of a dedicated class to the AppKernel (https://github.com/symfony/symfony-standard/blob/2.7/app/AppKernel.php#L8). Then is the bundle instance that knows how it's configuration files are located. This allows to have bundles under vendor because is the autoloader that knows where to locate class files.

[orlangur]

@mmenozzi, is there some crucial difference between Symfony approach and module bitmap in app/etc/config.php?

Thus currently you may change composer.json in order to specify modules you need and after that list of active modules will be generated during installation automatically.

[colinmollenhour]

@mmenozzi Yes, this is why I stated at the end:

(aside from magento-composer-installer which would also be nice to not have to depend on)

And thank you for mentioning the headache that comes with copying files.. IMO a good solution definitely does not involve duplicating files.

I don't have a solution, but one idea just to get the point across is something like modifying the include path in the autoloader to expect an intermediate directory such that instead of loading My/Module/Model/Foo.php it becomes My/Module/src/Model/Foo.php. However, this breaks PSR convention and many will probably hate this idea because it is ugly..

I personally don't think modman is a bad solution, it just seems like this is a good opportunity to make it unnecessary.

[mmenozzi]

Hi @orlangur,

@mmenozzi, is there some crucial difference between Symfony approach and module bitmap in app/etc/config.php?
Thus currently you may change composer.json in order to specify modules you need and after that list of active modules will be generated during installation automatically.

maybe I'm missing something but from what I can see the app/etc/config.php it's used only for enable/disable modules but these modules must be already loaded by Magento\Framework\Module\ModuleList\Loader::load(). The difference is that with Symfony a minimum bundle is simply a class implementing Symfony\Component\HttpKernel\Bundle\BundleInterface so Symfony can demand to the Composer autoloader the responsibility to locate a bundle on the filesystem. With Magento2 a minimum module is the etc/module.xml file and is the Magento\Framework\Module\ModuleList\Loader that decide that these module.xml files must be under DirectoryList::MODULES/*/*/etc/module.xml and from here there is the constraint to have modules under DirectoryList::MODULES directory (app/code). This constraint introduces the needing of magento/magento-composer-installer.

It's good to have modules automatically in app/etc/config.php but I'm not worried about manual work that have to be done to enable a module (even in Symfony you have to add a bundle in the AppKernel::registerBundle()); I'm worried about duplicated class files as stated above.

You asked me if there is any crucial difference with Symfony... With Magento we have duplicated class files around the project (so the problems stated above) this doesn't happen with Symfony. So from a pragmatic point of view I answer: yes I think that there is at least one crucial difference.

[orlangur]

Symfony can demand to the Composer autoloader the responsibility to locate a bundle on the filesystem

I would like to avoid such flexibility by convention over configuration.

So, if there is no app/code directory and module list loader searches for module.xml in vendor/*/*/etc/module.xml - is there still some crucial difference between app/etc/config.php and Symfony approach?

[mmenozzi]

I would like to avoid such flexibility by convention over configuration.

Why?

So, if there is no app/code directory and module list loader searches for module.xml in vendor///etc/module.xml - is there still some crucial difference between app/etc/config.php and Symfony approach?

I already explained what are the differences between the two approaches. I don't know if these differences are crucial for you.

Anyway I agree with your proposal of searching modules in vendor/*/*/etc/module.xml so at least we can remove the dependance on magento/magento-composer-installer and not have duplicated files anymore.

Would you like that I work on PR for this?

[alankent]

I would like to see modules, themes, lang packs under app or vendor. So can have local modules for your project without uploading to a repo. Problem was base directory (look at prev comments above). Please let us know where this effort occurs so can keep in sync with internal work

[orlangur]

Why?

To make the things simpler. If we have vendor folder, no need to invent some additional places for modules. Let's just make all of them follow the same convention.

I would like to see modules, themes, lang packs under app or vendor

Totally second this, also, would like to see magento/framework split into components. With such approach no custom composer package types will be needed.

[alankent]

We tried to allow downloaded modules to stay under vendor, but hit a problem with the code not being flexible enough. Architecturally, I personally like the main site getting the list of module directories from a configuration file created by the setup process. That is, let them be anywhere! Setup can have conventions like only looking in vendor and app/code (or app/modules) to create this config file. Scan back up the comment history for "The complexities highlighted were a result of a single base path variable used throughout the code" for more details. We want to do it, there is a plan, it just proved harder to do than expected. We trade effort off against business value when defining our internal backlog.

[mmenozzi]

Hi @alankent,

We tried to allow downloaded modules to stay under vendor, but hit a problem with the code not being flexible enough.

Yes it's true. Unfortunately, there are a lot of components in Magento Framework that assume that there is only one modules base directory. It's sufficient to search the usages of the \Magento\Framework\App\Filesystem\DirectoryList::MODULES constant in the Magento Framework to find a list of components based on this assumption.

We want to do it, there is a plan, it just proved harder to do than expected. We trade effort off against business value when defining our internal backlog.

Ok I understand that, it's fine. Can you tell us when approximately this could be fixed?

[orlangur]

@davidalger, @mmenozzi thanks for the use case I was not aware of. Do not insist on "2." but just want to understand whether vendor-only approach is suitable at all.

Didn't try to use vendor folder for development purposes yet but looking at http://stackoverflow.com/questions/12624653/composer-develop-directly-in-vendor-packages and composer/composer#1188 it seems to be possible for sure. Are there any downsides in such way?

So, site-specific code could be placed in separate repository and added via Composer (no need to register package - it is possible to specify repository URL) or placed in the same repository by adding exception to .gitignore like

/vendor
!/vendor/specific-site-name

It would be something similar to app/code/local code pool in Magento 1.

[davidalger]

Not sure if any of you have seen it or not, but looks like @fooman is set on finding out how difficult this would be to work out over in #897

@orlangur hate to point out the semantics of it, but the term "vendor" itself implies that what's inside is provided by somebody, not built by self. Even if it's possible, why would we want to put custom code in vendor outside of avoiding the work of having the system support multiple locations for loading modules?

[alankent]

Regarding the 'when' question, this one we want to do, but are not committing to do by GA. We try to slot in as much as we can, but the ROI on this one may mean it won't be done by GA.

[fooman]

@davidalger I have started with some trials to see what would be involved. But did stop on this when I heard that some internal thinking was going on around this as well. The main problem at the moment seem to be the hardcoded tests.

@alankent one thing which I hope you can commit to is to have the individual repositories of the Magento modules available online. My guess is that these already exist internally and already come with a composer.json file but I might be wrong. This would allow for further experimentation without having to actually check in the moved code under the vendor folder https://github.com/fooman/magento2/blob/modules-under-vendor/.gitignore#L48

[alankent]

Hi @fooman. The internal repository is the same structure as the external one. That is why its easier to process pull requests etc. That is, one big repo that we generate multiple packages from.

While we are doing global edits and platform changes, this makes life easier. Commits are atomic across the platform. We don't have it with half the modules updated. Once the framework level is more stable (fewer global edits) then we will review the benefit of splitting per module. It makes complete sense, but it requires our internal test automation framework and practices to be adjusted. Its doable, but needs thought and planning. The question is how important before 2.0 ships (relative to other work). The main value appears to be post 2.0 - do you agree?

Oh, and re-reading your request, I wanted to expand upon one point. There is the GIT repository and the Composer repository of released packages. We don't rely on GitHub to check out the modules - we use a snapshotted ZIP on the Magento master Composer repository. This is not definite, but it means we can package CE and EE modules in the one place, and paid extensions could go there as well. (We password protect the download area of course.) The EE git repo will not be public (!), so we must support ZIPs or similar for Composer downloads. I would prefer to do all Magento downloads the same way (for consistency), so the need for separate Git repos per modules may be lower as a result.

[fooman]

Thanks for the insights - makes sense.

The main value appears to be post 2.0 - do you agree?

The pessimist in me says that if this is not done soon it would only happen for 3.0 - as deemed too risky / too big a change.

Another reason to having the file structure finalised and no further planned changes sooner rather than later is that it would also help with documentation and generally developers only having to learn one new way of doing things. I am just picturing all those blog posts, stack exchange answers et all which become outdated and creating unnecessary confusion.

[fooman]

One further thought - as the file structure will likely impact on the New Connect as well I think having this stabilised early would benefit that effort as well.

[alankent]

I don't think Connect will be impacted because I believe Connect will host ZIPs of modules etc. We are not talking about changing the structure inside a module here, but rather the directory structure above modules (e.g. allow them in 'vendor' or 'app/modules'). The Composer packages themselves should not care which directory they are installed within.

Would love to do sooner, but not sure internal timelines and priorities will allow this. The problem we hit when we tried was mainly in the test infrastructure being too dependent on the overall directory structure. We need to get that cleaned up first. A part of the issue is I believe the developer experience will be better with downloaded modules under 'vendor', but there are workarounds. That pushes it down the priority list. (Just being open here.)

[alankent]

This issue was originally to rename app/code to app/modules, which is a good request in its own right. I have spun out the 'vendor' question into #1013.

[davidalger]

@alankent Thanks for keeping up with the thread!

Going back to the original question though, I would like to "upvote" the recommendation to rename app/code to app/modules I'm curious to know what the likely hood of this happening is.

[alankent]

There seems to be two ways forward. Rename it (which requires all internal teams to commit all their work at the same time - causes lots of conflicts for work in progress otherwise), or support multiple directories for modules (which would allow app/code, app/modules, and vendor to all work at same time - making incremental migration easier). The former is up to @maksek as the main effort is scheduling (with some fixing of tests etc to adjust paths). The latter would depend on someone taking on #1013, which I am not sure the community is up to.

[colinmollenhour]

git handles file relocations pretty well in my experience. This would definitely be a massive update, but I would be surprised if the upstream did not merge cleanly into the various branches in progress.. Would be worth testing I think.

[alankent]

"Would be worth testing I think..." - now that is a candidate for understatement of the year! ;-) ;-) ;-)

[alankent]

Sorry, in hindsight you were talking about testing how Git works, not testing the change itself. Sorry if my warped humor got the better of me. We have done a number of global changes in the past so have good experience with this. It was much easier to coordinate across teams and minimize checked out code - less time wasted all around.

[colinmollenhour]

Hah! I meant would be worth testing to see if it is easy rather than assuming it will create lots of difficult conflicts and not trying.. But yeah.. testing is good. :)

[davidalger]

Lol. @alankent Your "warped humor" gave me a good laugh anyways! :)

FWIW, as I understand from looking at the branch @fooman was experimenting on, the automated tests should work if the location is changed to app/modules and the correct constants updated. I'd PR it it to verify this, but figured if this is done it'd be better handled by the core team for stated reasons possibly necessitating internal cooperation between teams. Git is pretty good about renames… if you guys do test how easy it is, make sure you have the rename detection variable set high enough to allow for it to do rename checks on as many files as exist in the repo and then some.

[colinmollenhour]

Ahh, thanks for chiming in @davidalger, I was not aware that there was git config that could affect the rename detection. Might be useful for everyone to observe before doing merges.
http://stackoverflow.com/questions/13805750/git-fails-to-detect-renaming

[EliasZ]

Now that the developer RC is out and this has not been implemented, what are the future plans?

[alankent]

Developer RC slows down making changes like this, but we are reviewing an internal idea around allowing multiple directories. If we can support multiple, then migration would be much easier (we could support the old and new directory trivially). So its not completely dead.
I did create #1013 (up for grabs) but no-one in community put their hands up for this one either.

[joshdifabio]

Given that so many people seem to care about this, I'd recommend taking a look at PR #1206 and providing some feedback.

[piotrekkaminski]

Thank you for your submission.

We recently made some changes to the way we process GitHub submissions to more quickly identify and respond to core code issues.

Feature Requests and Improvements should now be submitted to the new Magento 2 Feature Requests and Improvements forum (see details here).

We are closing this GitHub ticket and have moved your request to the new forum.