Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

composer vs. git submodules issue #65

Closed
SchumacherFM opened this issue Feb 25, 2014 · 14 comments
Closed

composer vs. git submodules issue #65

SchumacherFM opened this issue Feb 25, 2014 · 14 comments

Comments

@SchumacherFM
Copy link

hey,

composer does not support submodules as your modman app therefore the Credis lib folder will stay empty.

Any other solution except forking and copying the Credis class into the lib folder?
Also adding the Credis module to a projects root composer.json is not an option...

Thanks!

@colinmollenhour
Copy link
Owner

I'm not a composer expert but credis itself is a composer module so perhaps it can be added as a dependency in the composer file? Feel free to open a PR. :)

On February 24, 2014 8:04:36 PM EST, Cyrill Schumacher [email protected] wrote:

hey,

composer does not support submodules as your modman app therefore the
Credis lib folder will stay empty.

Any other solution except forking and copying the Credis class into the
lib folder?
Also adding the Credis module to a projects root composer.json is not
an option...

Thanks!


Reply to this email directly or view it on GitHub:
#65

Sent from my Android phone with K-9 Mail. Please excuse my brevity.

@SchumacherFM
Copy link
Author

Yes you can add Credis as a composer required package ... but then you need to hack the Magento AutoLoader to load form the vendor folder.
Which leads me to: We do not have a vendor folder neither in staging nor in production so the hack gets more weird ;-)

So my only solution which comes into my mind is remove the .gitmodules file and add Credis files directly. (which I have already done in my fork :-) ). I have no idea if this is in your philosophy :-)

@colinmollenhour
Copy link
Owner

Copying files into multiple repos is definitely not my philosophy.. It seems lack of support for git submodules is a glaring omission that shouldn't be too hard to fix in composer. Save for that I would probably favor using subtree merge. I'm not entirely opposed to using subtree merge in this repo and in Cm_RedisSession, but I think that is an ugly hack for a shortcoming of composer.

@SchumacherFM
Copy link
Author

A composer dev states: "the lack of submodule support is by design."
You are totally right that it shouldn't be hard to fix in composer:
composer/composer#1600
I've just tried it now and it works. The git submodule commands runs after the magento-installer-command but due to your modman file and the linking of lib/Credis to the appropriate magento folder the Credis classes will be added later safely :-)

But it is still a hack and not everybody is aware of this work around.

@colinmollenhour
Copy link
Owner

I saw that after my comment. I disagree that they should not be supported, especially when there is a workaround that can be injected into the composer file so easily, so which is messier? lol

Would you mind trying the "scripts" method and submitting a PR if it works well?

{
     "scripts": {
         "post-install-cmd": ["cd vendor/foo/bar && git submodule init && git submodule update"],
         "post-update-cmd": ["cd vendor/foo/bar && git submodule init && git submodule update"]
     }
}

@SchumacherFM
Copy link
Author

No. Sorry I do not send a PR. But I have tried it in my root composer.json file and it works.

screen shot 2014-02-26 at 11 03 00 am

Because 1. a hardcoded vendor folder name is also messy ;-) 2. scripts will only be executed in the root composer.json file.

@colinmollenhour
Copy link
Owner

Thanks for the explanation. Seems like there is no great solution since composer guys won't support submodules, so I'll just happily continue using modman. :)

@SchumacherFM
Copy link
Author

Modman is a great tool. Kudos! 👍
But for setting up large projects in different environments it cannot be used.

@SchumacherFM
Copy link
Author

I've just run our build process and the submodules confuses and broke the process ... so I removed everything and created my own fork of Cm_Cache_Backend_Redis and replaced the submodule.

@colinmollenhour
Copy link
Owner

I've setup large projects (up to 23 repositories) with it in different environments (multi-node clusters, development, staging, etc).. I'd be curious to know specifically what shortcoming it has to make you say otherwise?

Here is a good method that lets you pull in the files into your repo while still being able to easily track upstream changes: http://stackoverflow.com/a/8396318/187780

@SchumacherFM
Copy link
Author

Wow Thanks for that! I will have a look and play around with it :-)

@yssource
Copy link

"post-update-cmd" do not git pull the lib/Credis if using the installer "Cotya/magento-composer-installer
".
it would be better to add the dependency as a composer dependency to their package. #Cotya/magento-composer-installer#104

@colinmollenhour
Copy link
Owner

I am not familiar with composer enough to determine what is the best way for this issue to be resolved and don't have time to setup test environments and play around with it. It seems it would make sense for Cm_Cache_Backend_Redis to have a composer dependency on Credis, but I have yet to see a PR that solves the issue without breaking the original install method of using modman. I would love to merge a PR that does this though.

@colinmollenhour
Copy link
Owner

There was some good discussion over on #70 but it never was resolved, in case you hadn't seen that PR already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants