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 install #188

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

jonphipps
Copy link
Contributor

This provides an update relate to issue #184

I started to notice that the vendor libs were getting a little out of date and once I updated analog (for instance) through Composer I had 2 copies. Then I also needed a composer-managed autoload in addition to the standard one. So I experimented a bit and found that I could simply replace the standard autoload with the Composer autoload, with a few tweaks.

So this PR does that, adds the vendor libraries to composer.json, removes the duplicated libraries, and while I was at it I threw in a script to update the permissions and run phpunit after the install. All the PHPunit tests pass and a basic install works, including installing the Newsletter app.

This may fall under the rubric of "If it ain't broke, don't fix it", but it was a little broke.

@jbroadway
Copy link
Owner

I'm not sure replacing Elefant's autoloader completely is the right approach, because it means the class map needs to be regenerated every time you add a class to an app. Having the one fall back to the other isn't so bad though, since both autoloaders are pretty fast and lightweight.

Having said that, I would like to use Composer so we don't have to manually keep those libraries up to date ourselves, and I like the post-install permission updates too. But what's stopping me from requiring Elefant sites to use Composer is that I don't want them to have to run composer.phar install in order to get the initial files, so it can still be installed the old school FTP way for those that are stuck with that on their hosts. I'm not sure what the best practice is around having some libraries preinstalled like that though.

@jonphipps
Copy link
Contributor Author

The Symfony project distributes 3 ways

  • .zip with vendors -- ready to go, includes composer.json, composer.lock, fully populated vendors (version controlled by composer.lock)
  • .zip without vendors (tiny -- 119k) -- includes composer.json, composer.lock, must populate vendors with composer install
  • composer-based install

http://symfony.com/download

I'm gonna guess that their methodology is pretty well vetted.

How about I mess with this just a bit more and see if I can't get it closer to what you want.

@jbroadway
Copy link
Owner

Sounds like a zip with vendors plus a composer-based install would be a good solution.

I was playing with Composer a bit more and I didn't realize the upgrade process was so rudimentary (unlink then pull a new copy). If you add a new file to an app that's not part of its original source, it will be deleted when Composer updates the app. That's already not recommended for apps (which is why we have overriding configs and handlers), but that approach is too simplistic for Elefant itself, as it will almost certainly wipe someone's important files.

So I wonder, does installing via composer create-project mean running composer update will update Elefant itself, or will we be okay to install that way and do updates via ./elefant update? As far as I can tell, it seems that a project created with create-project will only update its requirements, but not the core itself. If that's correct, it should still be fine for our needs.

@jonphipps
Copy link
Contributor Author

This is just a suggestion, because I was playing with it and it works quite well and I thought you'd be interested in what I found.

To make composer work, you need to make composer.json, composer.lock, lib/vendor, lib/vendor/composer, and lib/vendor/autoload.php writable by the apache user. since this simply runs composer as Apache. On my systems this required my user and Apache to be members of the same group and the files to be group writable, 775 rather than 755.

This also requires that composer be installed in '/usr/local/bin/composer' and it would be good to ask where composer is installed as part of the initial elefant setup, or have it setup automatically by the composer install, and get that location from the config.

Needs more work, obviously. But I'm moving on to other less intrusive stuff.

@jbroadway
Copy link
Owner

This is pretty cool, although the permission requirements and regenerating class maps sound like they could add confusion for less techie users, but still a really neat solution. I'm going to keep the issue open since I don't think we should make such big changes to how things work before 2.0 is out, but that way we can explore install/update approaches post-2.0.

And sorry about the slow responses lately, should be back to normal next week once I'm not on the road any more :)

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

Successfully merging this pull request may close these issues.

2 participants