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

[ticket/11150] Install extensions through composer #3909

Merged
merged 85 commits into from Apr 26, 2017

Conversation

@Nicofuma
Member

Nicofuma commented Sep 15, 2015

https://tracker.phpbb.com/browse/PHPBB3-11150

TODO:

  • A few language keys are missing
  • Add debug settings (to display more or less info in the composer output)
  • Titania (phpbb/customisation-db#149)
  • Display a message instead of the catalog if it is empty? (notice, blue)

Further PR:

  • Handles board updates (disable, update, enable exts/checks if exts are still compatible)
  • Deal with exts added manually (checks compatibility)

With titania only: https://blackfire.io/profiles/d8e6c3ff-d784-43cd-a080-809b03fa0d91/graph
With packagist and titania: https://blackfire.io/profiles/8d07d01b-9c7b-4518-9ae7-b75645d25634/graph

PHPBB3-11150

@Nicofuma Nicofuma added this to the 3.2.0-a1 milestone Sep 15, 2015

'CLI_DESCRIPTION_EXTENSION_REMOVE_OPTION_PURGE' => 'Purge the extensions before removing them',
'CLI_DESCRIPTION_EXTENSION_REMOVE_ARGUMENT' => 'Extensions to remove',
'CLI_DESCRIPTION_EXTENSION_UPDATE' => 'Updates extensions',
'CLI_DESCRIPTION_EXTENSION_UPDATE_ARGUMENT' => 'Extensions to update',

This comment has been minimized.

@VSEphpbb

VSEphpbb Sep 15, 2015

Member

I'm still not sure about how to call/describe MANAGE. But for the others:

'CLI_DESCRIPTION_EXTENSION_INSTALL'                 => 'Install the specified extension(s).',
'CLI_DESCRIPTION_EXTENSION_INSTALL_OPTION_ENABLE'   => 'Enable extension(s) after installation',
'CLI_DESCRIPTION_EXTENSION_INSTALL_ARGUMENT'        => 'Extension(s) to install, e.g.: vendor/package',
'CLI_DESCRIPTION_EXTENSION_LIST_AVAILABLE'          => 'List extensions available for installation.',
'CLI_DESCRIPTION_EXTENSION_REMOVE'                  => 'Remove the specified extension(s).',
'CLI_DESCRIPTION_EXTENSION_REMOVE_OPTION_PURGE'     => 'Purge extension(s) when removing them',
'CLI_DESCRIPTION_EXTENSION_REMOVE_ARGUMENT'         => 'Extension(s) to remove, e.g.: vendor/package',
'CLI_DESCRIPTION_EXTENSION_UPDATE'                  => 'Update the specified extension(s).',
'CLI_DESCRIPTION_EXTENSION_UPDATE_ARGUMENT'         => 'Extension(s) to update, e.g.: vendor/package',
public function update_data()
{
return array(
array('config.add', array('exts_composer_repositories', serialize([]))),

This comment has been minimized.

@Nicofuma

Nicofuma Sep 15, 2015

Member

add titania once phpbb/customisation-db#149 is merged

This comment has been minimized.

@marc1706

marc1706 Sep 16, 2015

Member

Why are you using serialize and not the json format?

{
return array(
array('config.add', array('exts_composer_repositories', serialize([]))),
array('config.add', array('exts_composer_packagist', true)),

This comment has been minimized.

@Nicofuma

Nicofuma Sep 15, 2015

Member

switch to false once phpbb/customisation-db#149 is merged

/**
* @var array Repositories to look packages from
*/
protected $repositories = [];

This comment has been minimized.

@Nicofuma

Nicofuma Sep 15, 2015

Member

add titania once phpbb/customisation-db#149 is merged?

This comment has been minimized.

@marc1706

marc1706 Dec 8, 2015

Member

probably a good idea, yeah

This comment has been minimized.

@Nicofuma

Nicofuma Dec 9, 2015

Member

done in the migration script

@@ -88,6 +88,11 @@
'ACP_EXTENSION_GROUPS' => 'Manage attachment extension groups',
'ACP_EXTENSION_MANAGEMENT' => 'Extension management',
'ACP_EXTENSIONS' => 'Manage extensions',
'ACP_EXTENSIONS_GALLERY' => 'Extensions gallery',

This comment has been minimized.

@VSEphpbb

VSEphpbb Sep 15, 2015

Member

We should find a replacement for gallery. maybe Catalogue

This comment has been minimized.

@rxu

rxu Sep 15, 2015

Contributor

Extensions market / store ;)

This comment has been minimized.

@VSEphpbb

VSEphpbb Sep 15, 2015

Member

that implies $$
My ideas:
Catalog
Depot
Database
Collection
Exhibit
Browse Extensions

public function write($messages, $newline = true)
{
$messages = (array) $messages;
$translated_messages = [];

This comment has been minimized.

@VSEphpbb

VSEphpbb Sep 15, 2015

Member

Is there a reason not to use traditional array syntax?

This comment has been minimized.

@Nicofuma

Nicofuma Sep 15, 2015

Member

short syntax is better ;)

This comment has been minimized.

@VSEphpbb

VSEphpbb Sep 15, 2015

Member

PHP 5.4.0 only. Is that officially the new min. req. yet? I still see 5.3.9 as our min. req.

This comment has been minimized.

@Nicofuma

Nicofuma Sep 15, 2015

Member

It's gonna change soon

This comment has been minimized.

@VSEphpbb

VSEphpbb Sep 15, 2015

Member

Well the linters don't like it. ;)

'COMPOSER_UNSUPPORTED_OPERATION' => 'Operation unsupported for the package type “%s”.',
'COMPOSER_UPDATING_DEPENDENCIES' => 'Updating packages',
'COMPOSER_LOADING_REPOSITORIES' => 'Loading remote packages information',

This comment has been minimized.

@VSEphpbb

VSEphpbb Sep 15, 2015

Member

Loading remote repositories with package information

'EXTENSION_ALREADY_INSTALLED' => 'The “%s” extension has already been installed.',
'EXTENSION_ALREADY_INSTALLED_MANUALLY' => 'The “%s” extension has already been installed manually.',
'EXTENSION_ALREADY_MANAGED' => 'The “%s” extension has already been installed manually.',

This comment has been minimized.

@VSEphpbb

VSEphpbb Sep 15, 2015

Member

Is this right? Seems a duplicate of above line.

'EXTENSION_ALREADY_INSTALLED' => 'The “%s” extension has already been installed.',
'EXTENSION_ALREADY_INSTALLED_MANUALLY' => 'The “%s” extension has already been installed manually.',
'EXTENSION_ALREADY_MANAGED' => 'The “%s” extension has already been installed manually.',
'EXTENSION_CANNOT_MANAGE_FILESYSTEM_ERROR' => 'The “%s” extension cannot be managed because the old files cannot be removed.',

This comment has been minimized.

@VSEphpbb

VSEphpbb Sep 15, 2015

Member

The “%s” extension cannot be managed because the existing files could not be removed from the filesystem.

'EXTENSION_CANNOT_MANAGE_INSTALL_ERROR' => 'The “%s” extension cannot be installed. The old extension have been restored.',
'EXTENSION_MANAGED_WITH_CLEAN_ERROR' => 'The “%1$s” extension has been installed but an error occurred and the old files have not been removed. You might want to delete the “%2$s” files manually.',
'EXTENSION_MANAGED_WITH_ENABLE_ERROR' => 'The “%s” extension has been installed but an error occurred when re-enabling it.',
'EXTENSION_NOT_INSTALLED' => 'The “%s” extension is not installed.',

This comment has been minimized.

@VSEphpbb

VSEphpbb Sep 15, 2015

Member
'EXTENSION_CANNOT_MANAGE_INSTALL_ERROR'     => 'The “%s” extension could not be installed. The prior installation of this extension has been restored.',
'EXTENSION_MANAGED_WITH_CLEAN_ERROR'        => 'The “%1$s” extension has been installed but an error occurred and the old files have not been removed. You might want to delete the “%2$s” files manually.',
'EXTENSION_MANAGED_WITH_ENABLE_ERROR'       => 'The “%s” extension has been installed but an error occurred when enabling it.',
'DISABLING_EXTENSIONS' => 'Disabling extensions',
'EXTENSIONS_GALLERY' => 'Extensions Gallery',
'EXTENSIONS_GALLERY_EXPLAIN' => 'The Extensions Gallery is a tool in your phpBB Board which allows you to see all the extensions that are available for your board.',

This comment has been minimized.

@VSEphpbb

VSEphpbb Sep 15, 2015

Member

Here you can browse all of the extensions available for your phpBB board. Extensions can easily be installed or removed with just a click. Adjust the Settings to allow instant enabling and purging of extensions.

@Nicofuma Nicofuma removed the WIP 🚧 label Sep 16, 2015

@Nicofuma

This comment has been minimized.

Member

Nicofuma commented Sep 16, 2015

The minimum-stability flag is now correctly handled and configurable.

'STABILITY_DEV' => 'dev',
'COMPOSER_MINIMUM_STABILITY' => 'Minimum stability',
'COMPOSER_MINIMUM_STABILITY_EXPLAIN' => 'TODO: why it is dangerous to change that',

This comment has been minimized.

@VSEphpbb

VSEphpbb Sep 16, 2015

Member
<strong class="error">Warning:</strong> Always use <samp>stable</samp> versions on a live forum.
Non-stable versions may still be in development and could cause unexpected problems with your
forum and should only be used for development purposes in local or staging environments.
@VSEphpbb

This comment has been minimized.

Member

VSEphpbb commented Sep 16, 2015

I wonder if the Repositories, Packagist and Min Stability settings should all be hidden and ignored except in the development environment, as these settings seem "dangerous" to have available for a live forum?

@VSEphpbb

This comment has been minimized.

Member

VSEphpbb commented Sep 16, 2015

If colors in the composer output can be controlled, the version #'s should be changed. Currently it's bright yellow which is unreadable against the grey bg of the ACP.

@Nicofuma

This comment has been minimized.

Member

Nicofuma commented Sep 17, 2015

@VSEphpbb I changed the color, should be better now. I have also fixed your issue with enable on install/purge on remove

@Nicofuma

This comment has been minimized.

Member

Nicofuma commented Sep 17, 2015

@naderman Mind have a look for the composer part and the settings?

'COMPOSER_MINIMUM_STABILITY' => 'Minimum stability',
'COMPOSER_MINIMUM_STABILITY_EXPLAIN' => 'Always use <samp>stable</samp> versions on a live forum.
Non-stable versions may still be in development and could cause unexpected problems with your

This comment has been minimized.

@VSEphpbb

VSEphpbb Sep 17, 2015

Member

the line breaks aren't really needed.

This comment has been minimized.

@Nicofuma

Nicofuma Sep 17, 2015

Member

It's easier to read in the language file... but yes. Fixed

@Nicofuma

This comment has been minimized.

Member

Nicofuma commented Sep 17, 2015

For the remove action, 2 options:

  • Purge by default when the extension is removed
  • Display the action only when purge on remove is enabled or the extension is already purged
<p>{{ MESSAGE_TEXT }}</p>
</div>
<fieldset>

This comment has been minimized.

@VSEphpbb

VSEphpbb Sep 17, 2015

Member

Wrap this fieldset inside of a <div style="overflow-x:scroll">

This comment has been minimized.

@Nicofuma

Nicofuma Sep 17, 2015

Member

thanks

@Nicofuma

This comment has been minimized.

Member

Nicofuma commented Sep 17, 2015

The remove action is now displayed only for disabled extensions (and in the catalog), and the extensions are purged when removed by default.

@naderman

This comment has been minimized.

Member

naderman commented Sep 17, 2015

@Nicofuma Won't get to it before the weekend, but put it on my calendar.

@VSEphpbb

This comment has been minimized.

Member

VSEphpbb commented Sep 17, 2015

In the extension catalog, enabled extensions still have the Remove link shown.

There is also still the possibility to Remove an extension here, before it has been purged. I think the solution is:

Remove only appears (on both pages) for purged extensions (and then we don't need the exts_composer_purge_on_remove setting)

This way, Remove will always only be able to occur after an extension is deliberately purged by the user, and will avoid errors in the ext manager like:

The “phpbb/pages” extension is not valid.
The requested file could not be found: ./../ext/phpbb/pages/composer.json

And will avoid possible deletions of extension data by the users

@VSEphpbb

This comment has been minimized.

Member

VSEphpbb commented Sep 17, 2015

Also, on further thought, I think having "Remove" and "Update" actions in both pages may be too much (clutter, confusion, redundancy). Consider the Extension Catalog just having "Install" action, and "Installed" status. And Leave the "Remove" and "Update" actions for the Extensions Manager (where they are more applicable).

<h1>{{lang( 'EXTENSIONS_CATALOG') }}</h1>
<p>{{lang( 'EXTENSIONS_CATALOG_EXPLAIN') }}</p>

This comment has been minimized.

@paul999

paul999 Sep 17, 2015

Member

Missing space after {{ (Or on line 11 there is a space to much)

$this->page_title = 'ACP_EXTENSIONS_CATALOG';
$this->tpl_name = 'acp_ext_catalog';
$this->template->assign_var('extensions', $extensions);

This comment has been minimized.

@paul999

paul999 Sep 17, 2015

Member

Why not use $template->assign_vars for these assignments?

Nicofuma added some commits Dec 9, 2015

[ticket/11150] Fix CS
PHPBB3-11150
[ticket/11150] CS
PHPBB3-11150
[ticket/11150] Update tests
PHPBB3-11150
[ticket/11150] CS
PHPBB3-11150
[ticket/11150] Wording
PHPBB3-11150
[ticket/11150] Fix wording
PHPBB3-11150
[ticket/11150] Fix comments
PHPBB3-11150

marc1706 added a commit to marc1706/phpbb that referenced this pull request Apr 26, 2017

Merge pull request phpbb#3909 from Nicofuma/ticket/11150
[ticket/11150] Install extensions through composer

@marc1706 marc1706 merged commit 1f9a269 into phpbb:master Apr 26, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@marc1706

This comment has been minimized.

Member

marc1706 commented Apr 26, 2017

Now remember kids. If something's broken, it's Nicofuma's fault.

@Nicofuma

This comment has been minimized.

Member

Nicofuma commented Apr 26, 2017

lol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment