Split internal commands into distinct packages

[danielbachhuber]

WP-CLI has organically evolved into a monolithic codebase that can now be abstracted to a set of discrete packages. In order to better support a future with lots of disparate WP-CLI features, let's split internal commands into a series of distinct packages, beginning with:

• wp (cache|transient) * https://github.com/wp-cli/cache-command
• wp checksum https://github.com/wp-cli/checksum-command
• wp config * https://github.com/wp-cli/config-command
• wp core * https://github.com/wp-cli/core-command
• wp cron * https://github.com/wp-cli/cron-command
• wp db * https://github.com/wp-cli/db-command
• wp (eval|eval-file) https://github.com/wp-cli/eval-command
• wp export https://github.com/wp-cli/export-command
• wp (option|post|comment|user|term|site) * https://github.com/wp-cli/entity-command
• wp import https://github.com/wp-cli/import-command
• wp language https://github.com/wp-cli/language-command
• wp media * https://github.com/wp-cli/media-command
• wp package * https://github.com/wp-cli/package-command
• wp (plugin|theme) * https://github.com/wp-cli/extension-command
• wp rewrite https://github.com/wp-cli/rewrite-command
• wp (role|cap) * https://github.com/wp-cli/role-command
• wp scaffold * https://github.com/wp-cli/scaffold-command
• wp search-replace https://github.com/wp-cli/search-replace-command
• wp server https://github.com/wp-cli/server-command
• wp shell https://github.com/wp-cli/shell-command
• wp super-admin * https://github.com/wp-cli/super-admin-command
• wp (widget|sidebar) * https://github.com/wp-cli/widget-command

When we split the packages, we should:

• Preserve Git history
• Mirror Travis' test suite matrix in the package

Previously: #3652 (comment)

[szepeviktor]

Does it mean that we will be able to build smaller phar-s?

[danielbachhuber]

Does it mean that we will be able to build smaller phar-s?

No — the Phar will still contain these commands for the foreseeable future.

[nyordanov]

It would be nice if, when the time comes to remove those commands from the Phar release, we can make the update processes automatically install any packages necessary for preserving backwards-compatibility.

[jonathanbossenger]

Would it make sense to raise separate issues for each package to be split? Thereby allowing different developers to work on different packages.

[danielbachhuber]

It would be nice if, when the time comes to remove those commands from the Phar release, we can make the update processes automatically install any packages necessary for preserving backwards-compatibility.

They'll be included in the Phar release, actually, for the foreseeable future (treated as Composer dependencies).

Would it make sense to raise separate issues for each package to be split? Thereby allowing different developers to work on different packages.

After the packages are split out, each will have their own issue tracker.

[danielbachhuber]

Loading WordPress is the slowest part of the test process. To assess which commands to split out based on their impact on the build time:

ack "When I (run|try)" *.feature --count | ack "([a-z]+).+:([\d]+)" --output="\$1:\$2" > wp.log
awk -F ':' '{a[$1] += $2} END{for (i in a) print a[i], i}' wp.log | sort -n

Here are the results:

2 network
2 server
4 formatter
4 shell
4 sidebar
4 taxonomy
4 validation
6 eval
8 cap
10 super
12 rewrite
12 transient
16 roles
17 runcommand
18 cache
18 framework
19 flags
20 help
28 upgradables
29 aliases
31 skip
40 media
40 menu
40 option
42 cli
44 command
45 config
45 import
47 cron
47 term
50 widget
58 db
61 scaffold
65 comment
66 package
70 search
81 theme
83 post
85 export
87 site
101 plugin
103 user
180 core

Total instances: 1748

[szepeviktor]

Is ack really awk?

[szepeviktor]

Is ack really awk?

Yes it is 😄 No it isn't.
Try https://github.com/ggreer/the_silver_searcher it is a better product.

[schlessera]

No, ack is a specialized version of grep, meant to be used to search through source code.
awk is a tool to extract and reformat text.

[danielbachhuber]

I ran #3893 locally.

./ci/test.sh 2>&1 | tee -a test.log
ack '(behat features/[a-z-]+\.feature|[0-9]+m[0-9\.]+s)' -o test.log

Here are the test times for each .feature file: https://gist.github.com/danielbachhuber/2a5aafab5005458dd673d4e3a3b2fbf9

The outliers (greater than 45s) are:

behat features/cli.feature
0m51.47s
behat features/core-download.feature
1m27.182s
behat features/core-update.feature
1m11.436s
behat features/framework.feature
2m14.351s
behat features/package-install.feature
2m54.283s
behat features/plugin.feature
1m10.017s
behat features/runcommand.feature
0m47.6s
behat features/scaffold.feature
0m58.877s
behat features/search-replace.feature
0m51.375s
behat features/theme.feature
1m43.989s
behat features/upgradables.feature
0m48.318s

In addition to When I (run|try), I think Given a WP install contributes significantly.

[schlessera]

Profile of package-install.feature: https://blackfire.io/profiles/989896cf-c6d2-4a3d-b7b2-4bcbec688656/graph

[danielbachhuber]

I think continuing to break these commands apart is the most reasonable short-term task, then we can look at optimizing the performance of some features later on.

[schlessera]

Profile of framework.feature: https://blackfire.io/profiles/9b097d0b-b939-43d0-83ff-c57069d29664/graph

[schlessera]

Yes, the profiling does not tell us much we don't know yet.

[danielbachhuber]

Well, the Yaml parser package we're using is basically one class only as well.

Sure, but we're not actively maintaining the Yaml parser package. It's basically set it and forget it.

What do you think will be more complicated when splitting it up like this?

The developer experience ("Now I need to make this change here, this change there, update tests in both places", etc.). It's much easier to have everything for a specific feature in one place.

[schlessera]

The tests that cover the general "dealing with a db object" would be in that separate package. And the command packages would only include their respective, specific tests.

I'll think some more about this. I don't see an immediate issue with having small, granular packages per se. But I need to think through how the tests are being done in more detail.

In general, I always assume that you should be able to split packages at logical boundaries. If that should not be easily feasible, it might hint to a design flaw somewhere (which we might or might not want to change as a result).

[szepeviktor]

Is wp config new or it is a new name for old commands?

[danielbachhuber]

Is wp config new or it is a new name for old commands?

It will be the existing wp core config, plus new ones like wp-cli/ideas#4, wp-cli/ideas#34, and #3831

[szepeviktor]

Thanks.

[danielbachhuber]

Suggestions on how we should name the package for wp (post|comment|user|term|site), which extend WP_CLI\CommandWithDBObject?

Going with https://github.com/wp-cli/entity-command and https://github.com/wp-cli/extension-command

[danielbachhuber]

🚢 🔨