WordPress.org

Make WordPress Core

Opened 2 months ago

Last modified 8 weeks ago

#48086 new enhancement

Add required extensions and libraries to composer.json

Reported by: dingo_d Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 5.2.3
Component: Build/Test Tools Keywords: has-patch 2nd-opinion
Focuses: docs, coding-standards Cc:
PR Number:

Description

Composer offers a way to require or suggest certain platform packages: https://getcomposer.org/doc/01-basic-usage.md#platform-packages
Ticket 48081 addresses the PHP version, and this one covers the required libraries and extensions.

I've used the WP-CLI ext package (https://github.com/johnbillion/ext) to generate all the required and suggested extensions and libraries.

Attachments (2)

48086.diff (4.0 KB) - added by dingo_d 2 months ago.
48086.1.diff (1.2 KB) - added by dingo_d 2 months ago.
Updated the patch to only include the composer.json

Download all attachments as: .zip

Change History (13)

@dingo_d
2 months ago

#1 @jrf
2 months ago

Hi @dingo_d Yet another great step towards the future!

I've reviewed your patch and have two remarks:

  1. Please undo the update to the PHP_CodeSniffer package in the composer.lock file. There is an issue with a particular sniff which is why this update hadn't been done yet - see #47632 for more details.
  2. IIRC adding the lib-... requirements is only useful when you actually add versions to them. Otherwise, set the ext-... with * instead (for those extensions which are not part of the PHP core).

#2 @dingo_d
2 months ago

IIRC adding the lib-... requirements is only useful when you actually add versions to them. Otherwise, set the ext-... with * instead (for those extensions which are not part of the PHP core).

So I should just put those libraries to ext- and use wildcard, or remove them entirely (they seem to be required)? Not sure if there will be any negative impact for that.

Also, I'll exclude the lock update, but I'm unsure if this will cause any failures because composer validate complains about them not being up to date with the changes.

@dingo_d
2 months ago

Updated the patch to only include the composer.json

#3 @johnbillion
2 months ago

Related discussion on roots/wordpress: https://github.com/roots/wordpress/issues/12

#4 @jrf
2 months ago

Also, I'll exclude the lock update, but I'm unsure if this will cause any failures because composer validate complains about them not being up to date with the changes.

I think you do need to include an updated composer.lock with this change. Just run composer update --lock to only update the hash and not update dependencies.

So I should just put those libraries to ext- and use wildcard, or remove them entirely (they seem to be required)? Not sure if there will be any negative impact for that.

Yes. If they are extensions which are part of the PHP core which cannot be disabled and PHP cannot be explicitly compiled without them, leaving them out is fine.

Note: I have not checked the extensions listed. It looks very long to me - definitely compared to what's in Site Health.

#6 @jrf
2 months ago

Related ticket to update the list in Site Health: #47454

#7 @dingo_d
2 months ago

Ok, so for now, the best course of action is to get a confirmed list of required and recommended list of extensions needed. I also like the descriptions in this patch: https://core.trac.wordpress.org/attachment/ticket/23912/23912.6.patch, this could be used (with appropriate props ofc) for the suggested extensions.

And the handbook (https://make.wordpress.org/hosting/handbook/handbook/server-environment/#php-extensions) should be updated as well?

#8 @jrf
2 months ago

@dingo_d IMO, yes to both and the updated list of required and recommended extensions should also be used in #47454.

Maybe @rarst would also be so kind as to run his expert eye over this patch ?

#9 @Rarst
2 months ago

As of current patch, the list is way too wide. Many of PHP extensions used by WP core are optional, you can't rely on automated search to enumerate which of them are actually required. I did a manual pass years ago https://wordpress.stackexchange.com/a/42212/847

Also while technically being "extensions" some of these are de facto part of PHP core (e.g. date) and are redundant to require explicitly.

Last edited 2 months ago by Rarst (previous) (diff)

#10 @dingo_d
8 weeks ago

In your opinion @Rarst what would be the best way to handle the list? Remove the ones that come bundled with PHP, and the ones that are not required just push to suggested part of the composer.json?

#11 @Rarst
8 weeks ago

Since the scope of this composer.json is test environment really I would:

  1. omit anything that is part of PHP core, that's just not necessary
  2. limit require to the extensions that are hard requirement for tests to run/complete
  3. limit suggest to the extensions that are optionally covered by tests (if any)
Note: See TracTickets for help on using tickets.