October: October modules overwritten via composer install

[petehalverson]

This seems to have been an oversight. The issue goes wholly unnoticed when the composer/packagist sources are in sync with this repository's /modules folders.

Loading a copy of the modules/ folder via composer seems unnecessary and is prone to cause issues.

Expected behavior

$ composer install adds dependencies to the /vendor folder.

Actual behavior

$ composer install unexpectedly replaces the /modules/system, /modules/backend and /modules/cms from other sources.

Reproduce steps

$ git clone https://github.com/octobercms/october.git  .
$ composer install

Compare timestamps of the files/folders initially pulled from the repo with the vendor folders and the modules folders that have been overwritten by composer.

$ ls -l . modules/

The issue is more obvious when checking out the l55upgrade branch:

$ git clone https://github.com/octobercms/october.git -b l55upgrade .
$ composer install
$ git status

Suggested Action

I'd suggest october/system, october/backend and october/cms be removed from the composer.json

Solution test

$ git clone https://github.com/octobercms/october.git -b l55upgrade .
$ sed -i '/october\/[system|backend|cms]/,+0 d' composer.json
$ composer install
$ git status

This took me a while to wrap my head around, so please follow up if you want me to provide some more detail!

Thanks!

[daftspunk]

This doesn't happen when your composer file looks like this

        "october/rain": "dev-l55upgrade",
        "october/system": "dev-l55upgrade",
        "october/backend": "dev-l55upgrade",
        "october/cms": "dev-l55upgrade",
        "laravel/framework": "5.5.*@dev"

However, I do understand the concern. It means you need a clean workspace before running composer update.

# Expect a clean workspace
git status 

# This may spam modules/ with old changes
composer update

# Revert stale changes
git checkout modules/*

It's not great, but it isn't an issue generally faced by consumers. The same consumers who want us to use composer.

Hope this helps! =)

[daftspunk]

Removing these packages entirely is another valid solution? However it means you must maintain these modules yourself.

[petehalverson]

@daftspunk, it does happen when the composer.json references 'dev-l55upgrade'. That branch includes that change: https://github.com/octobercms/october/blob/l55upgrade/composer.json#L30-L33

The modules are in the repo already, it's not clear why they're being pulled in again via composer.

[daftspunk]

It is due to the architecture of the October project. Without going in to too much detail, the vendor folder is too scary a place for modules, which act as foundations to the platform, in both code and comprehension.

This segment of the composer.json file found in modules instructs composer to install it in the modules/ directory. Plugins and themes have a similar type definition.

"type": "october-module",

What you are experiencing is most likely due to the fact that l55upgrade is a development branch and does not get updated daily like the others. As you may notice, the modules that exist in the repo are used as the source for the composer packages themselves. So theoretically they are one and the same, however, it is prone to the aforementioned stale changes.

[petehalverson]

Agreed, the issue is noticable on the l55upgrade branch, but we've run into issues (albeit infrequently) when the packagist repos were stale.

Since the current modules are always included with the repository, why are they required in composer.json?

From my tests, removing them from the composer.json has no effect. Am I missing something?

[daftspunk]

They are included in composer.json so running composer update will deploy the latest code.

1. create-project october/october pulls down the app files
2. composer update pulls down the latest module files

The october/october files are not included as part of this update process, things like boostrap, config, etc. Only the modules are included in the composer update process.

When you remove from composer.json, delete composer.lock and use composer install.

Perhaps consider the inverse solution, creating your own application repo and add modules/* to .gitignore? This should clear up some confusion.

Keep in mind by cloning the application repo directly, you are in the developer workflow right now, a channel for developers who want to contribute code back to the project. Modules are part of the application layer, so it exists in the application repo. There are some gotchas involved here, discussed already.

[daftspunk]

I think we may be entering X Y territory here.

[daftspunk]

Sorry, that's not to say you can't still respond to my previous comment @petehalverson. It just means no action will likely come of this.

Also I'm not entirely sure what the problem is you are trying to solve. Hence the XY problem (http://xyproblem.info/), otherwise we could talk about this for hours.

[anatoliyarkhipov]

Hi guys! Do I correctly understand, that modules, plugins and themes are handled by Composer and it's not a good idea to place them under a version control?

[LukeTowers]

Modules is controlled by composer, but plugins and themes are not.

[anatoliyarkhipov]

Thanks for the clarification @LukeTowers! Can you also tell something about best practicies of using October with a version control? Or may be you can point me to a document about it? The main concern for me is what should be controlled and what shouldn't.

[LukeTowers]

I'll try to remember to post a link to my article on my development processes when working with October here whenever I get around to finding the time to finish it.

[petehalverson]

@daftspunk, forgive me for resurrecting the discussion.

While troubleshooting unit test result discrepancies, I stumbled upon your commit that handles the issue I was referring to earlier in this conversation.

In the Travis CI build process, the module folders are pulled in via composer, then the repo is reset to make sure the modules contained match those in the repo.

With that context, I realize you probably understood the issue I was complaining about, but just believe it's worth forcing contributing developers to work around to accommodate the 'composer only' workflow. Is that correct?

I have since discovered the installer method curl -s https://octobercms.com/api/installer | php. It is really well done and quite slick. I would have assumed that install process would be more popular than create-project october/october, no?