ClassicPress / ClassicPress-v1

A copy of ClassicPress v1.x.
1 stars 1 forks source link

Support installing ClassicPress via composer #239

Closed pierre-dargham closed 5 years ago

pierre-dargham commented 6 years ago

Hello there !

Many of us are handling WordPress core as a composer dependency, using johnpbloch/wordpress . It's also the standard method to install and manage the core in a lot of WordPress boilerplates or stack like roots/bedrock or globalis/wp-cubi

I think it's necessary to make ClassicPress 1.0.0 available as a composer package, allowing a composer-friendly / developer-friendly way of doing the migration (in addition to the migration plugin way, more user-friendly), in a single commands like composer remove johnpbloch/wordpress && composer require classicpress/classicpress.

Just to make it clear : I'm not talking about adding composer / PSR-4 autoloading in ClassicPress bootstrap for now (I think it should be a 2.x.x feature, but we'll have plenty of time to discuss it in the future). We just need ClassicPress itself to be available as a composer package.

It would require a few actions (nothing too difficult, and not a single change in PHP files of ClassicPress) :

What do you all think about this ?

If everybody agrees on that, I'm willing to do the pull request for adding the composer.json and implement the classicpress-core-installer package.

Bests regards, Pierre

scottybo commented 6 years ago

+1 from me - let's let this discussion take shape and then we'll open it up to the committee

Mte90 commented 6 years ago

It is on our plan to remove the compatibility to php 5.2, in that way we can add composer.

pierre-dargham commented 6 years ago

@Mte90 I think dropping PHP 5.2 compatibility will be necessary when (if) we want to add the composer audoloader as a part of the core bootstrap, but it's not the issue here : if we add the composer.json file, it will just mean that the composer way of installing ClassicPress will be available for PHP 5.3.2+. The core itself if not modified and will remain PHP 5.2 compatible for now.

Mte90 commented 6 years ago

Yeah I know but when we will discuss that point we will see also that. We received a lot of request for composer between that point and also the minimum support bump, so to avoid confusion is better to do all together.

nylen commented 6 years ago

Hi @pierre-dargham, please submit this PR. Thanks for your interest!

nylen commented 6 years ago

@pierre-dargham any interest in continuing this? I think it is of very clear benefit, and we could already use it in our infrastructure.

pierre-dargham commented 6 years ago

Hi @nylen, yes I'm willing to do it, I'm just a little overwhelmed with production / client projects right now. I'll try to work on it and submit the PR(s) during this week, or the next

pierre-dargham commented 6 years ago

Update: work in progress cc @nylen

classicpress-composer-plugin

https://github.com/pierre-dargham/ClassicPress-Composer-Plugin https://github.com/pierre-dargham/ClassicPress

I'll submit the PR next week

Note: on the screenshot, it's pierre-dargham/classicpress, but obviously, it will be classicpress/classicpress when we're done

Bests regards, Pierre

nylen commented 6 years ago

Great, thanks!

A couple of thoughts:

nylen commented 6 years ago

Not assigning a milestone to this yet, it's something I'd really like to have for v1 but it's probably going to depend on an outside contributor to get it done.

johnpbloch commented 6 years ago

I'm very excited about this addition! @pierre-dargham thanks for getting out there and doing the leg-work on this!

I wanted to touch on the 3 steps you outlined above:

  • Add a composer.json file to the core repository (something like johnpbloch/wordpress-core/composer.json)

I didn't see anybody here specifically mention this yet, so I wanted to make sure this gets said: the composer.json file needs to go into the built version, not the dev version. Otherwise composer users will be getting unbuilt source files and things won't work as expected. So the composer manifest needs to be built by grunt and placed in the same build directory as a sibling to wp-content, wp-admin, and wp-includes. I'd even go so far as to say the composer manifest should not be committed in any branch that isn't a build branch.

  • Create a composer plugin classicpress-core-installer (we could just fork johnpbloch/wordpress-core-installer), allowing composer to install the core in the right directory, instead of the vendor directory. This package would be a dependency of the core main package.

For now, this isn't strictly necessary. As long as the composer manifest uses a type of wordpress-core my installer will work as expected. As long as both ClassicPress and WordPress share the same install routine, I don't see any urgency in duplicating the package. That said, it makes no difference to me whether CP has its own installer or not. I just wanted to mention that this step isn't strictly necessary for getting composer support added.

  • Create a classicpress account on https://packagist.org/ and add the two packages (and enable github integration so the packages are automatically synced with github.com releases)

The account can (and probably should) be created immediately, even before there are any packages to add. One of the administrators here should probably do so. Other contributors can be added to the vendor namespace later.

johnpbloch commented 6 years ago
  • Can you explain what the ClassicPress-Composer-Plugin piece does, whether here or on the PR? I think this is something we'll want to document (along with how installing ClassicPress via Composer works overall).

It installs CP in a subdirectory of the webroot instead of inside vendor/classicpress/classicpress.

pierre-dargham commented 6 years ago

Hello @nylen : Sorry for the late answer, I couldn't find the time to complete the work until now, but it's still at the top of my todo list. I do agree with you that it's better not to assign it to a milestone for now, nevertheless I will try to push the PR asap anyway, and I hope that we'll have it soon enough to merge it before v1.

Hello @johnpbloch : I'm very glad you came here and support the proposal ! Regarding what you said :

I should be able to come back at the PR tomorrow. I'll let you know, Pierre

nylen commented 6 years ago

A wild @johnpbloch appears!

I didn't see anybody here specifically mention this yet, so I wanted to make sure this gets said: the composer.json file needs to go into the built version, not the dev version. Otherwise composer users will be getting unbuilt source files and things won't work as expected. So the composer manifest needs to be built by grunt and placed in the same build directory as a sibling to wp-content, wp-admin, and wp-includes. I'd even go so far as to say the composer manifest should not be committed in any branch that isn't a build branch.

It's super effective!

(Thanks for the advice, that was a key piece that was missing in my understanding of how this needs to work.)

I've got https://packagist.org/users/ClassicPress now.

A follow-up question: which packages do we publish to packagist? Can we use our release builds as the package itself? If so, that makes things a bit easier. Anyway let me know and I'll get the necessary repositor(ies) set up - I'm already working on setting up automated builds.

To make changes to the build, we get to dive into the Gruntfile.js. Copying the manifest file to a build is pretty easy, but I do think it needs to be committed somewhere in the source tree, or at least a script that generates it in the build.

it may be clever to stick to the strictly necessary steps to provide the composer migration way asap.

Agreed, at least as a starting point. Let's see what else we need once we have this working.

pierre-dargham commented 6 years ago

As I understand it now, our issue here is that :

So I think we have 2 different ways of handling it :

1. With an unique repository :

master / source branch :

./bin/
./composer.json // !!! HERE IT IS !!!
./Gruntfile.js
./jsdoc.conf.json
./package-lock.json
./package.json
./phpunit.xml.dist
./README.md
./renovate.json
./src/
./tests/
./tools/
./webpack.config.dev.js
./webpack.config.prod.js
./wp-cli.yml
./wp-config-sample.php
./wp-tests-config-sample.php

release / build tag :

./composer.json // !!! HERE IT IS !!!
./index.php
./license.txt
./readme.html
./wp-activate.php
./wp-admin/
./wp-blog-header.php
./wp-comments-post.php
./wp-config-sample.php
./wp-content/
./wp-cron.php
./wp-includes/
./wp-links-opml.php
./wp-load.php
./wp-login.php
./wp-mail.php
./wp-settings.php
./wp-signup.php
./wp-trackback.php
./xmlrpc.php

2. With two repositories :

None of these solutions is perfect, but I think that we're stuck with historical misconceptions in WordPress architecture / versioning and building process.

If if we go with the first approach though, (unique repository) and if we use johnpbloch/wordpress-core-installer for now, the PR could be pretty simple : we add the composer.json at the root and we add a few lines in the Gruntfile.js to handle the copy during build.

Let me know your what do you think about it @nylen @johnpbloch !

pierre-dargham commented 6 years ago

If we go with solution 1, the PR would be something like this : https://github.com/ClassicPress/ClassicPress/compare/master...pierre-dargham:feature/add-composer-json-manifest

nylen commented 6 years ago

(2) We make a second github repository (something like ClassicPress-builds), that contains only the build versions of each release / tag, like they do with https://github.com/WordPress/WordPress

I'm already working on creating this repository today (we need it anyway, for our official builds), so I think this is the right solution.

If we go with the first approach though, (unique repository) and if we use johnpbloch/wordpress-core-installer for now, the PR could be pretty simple : we add the composer.json at the root and we add a few lines in the Gruntfile.js to handle the copy during build.

I think it's not quite that simple, because our build process needs to do a few other things (minify JS and CSS for example).

So where does the composer.json live that gets copied to the build? I am thinking src/composer.json (in this repository) is a decent choice.

invisnet commented 6 years ago

So where does the composer.json live that gets copied to the build? I am thinking src/composer.json (in this repository) is a decent choice.

Do we not then end up with composer.json in the webroot after install? Surely it's better to have it outside the webroot?

pierre-dargham commented 5 years ago

A little update on the situation :

As @nylen said, we will have soon a github repository for official builds, so we're finally heading to solution 2 :

Regarging @invisnet question :

I will now open the PR, that will finally just be the addition of src/composer.json. We can continue the discussion on the PR thread

nylen commented 5 years ago

I just merged this PR (#169). I'd like for us to explore options to keep composer.json out of the web root, but this will do for now.