Refactor Bootstrap Process #3872

schlessera · 2017-03-02T16:09:59Z

Added scenarios: - `Install a package that overrides a command bundled in source` - `Install a package that overrides a bundled PHAR command` Both scenarios are tagged with `@bootstrap`. See #3850

Added scenarios: - `Override command bundled with current source` - `Override command bundled with PHAR` Both scenarios are tagged with `@bootstrap`. See #3850

Added scenarios: - `Override command bundled with downloaded PHAR` - `Install a package that overrides a command from a downloaded PHAR` Both scenarios are tagged with `@bootstrap`. See #3850

…endencies. Added scenarios: - `Basic Composer stack` - `Composer stack with override requirement after WP-CLI` - `Composer stack with override requirement before WP-CLI` (fails with current commit, as this is the issue to solve) All three scenarios are tagged with `@bootstrap`. See #3850

Added scenarios: - `Override command bundled with Composer stack` - `Install a package that overrides a command from a Composer stack` Both scenarios are tagged with `@bootstrap`. See #3850

The order of execution of the bootstrapping process has been heavily refactored to contain everything in separate, isolated steps as far as currently possible. There's a major issue remaining, that will need further discussion: the autoloader for the framework itself (needed for any commands to register without errors) and the autoloader for the bundled commands (that are required through Composer) would need to be split and moved independently, but reside within the same Composer autoloader setup. See #3850

…k" autoloader and a "commands" autoloader. Requires the package `wp-cli/autoload-splitter` that is yet to be created as a repository. There's still an issue with the YAML loading, because it is not class-based. See #3850

danielbachhuber · 2017-03-07T19:26:43Z

php/commands/cli.php

-
-}
-
 WP_CLI::add_command( 'cli', 'CLI_Command' );


Is it necessary to move this code? If not, can you revert the changeset so that it's possible to easily follow the file history?

I was looking for a command that would not be split out, to use for the tests. So, the tests do test all of the different ways to override this command.

For this to work, however, the command itself needs to be autoloaded on-demand, not immediately loaded upon bootstrap.

This change allows the `Spyc` through an autoloader, as soon as the related pull request to the `mustangostang/spyc` package has been merged: mustangostang/spyc#66

These tests cannot work as long as there is no released PHAR version to download that includes the bootstrap changes from this branch.

danielbachhuber · 2017-03-08T12:46:59Z

@schlessera Can you merge master?

- `"wp-cli/autoload-splitter": "dev-master"` - `"mustangostang/spyc": "dev-master#1cea7c15324025cd7b62c8d47c10e1e94f634c1e"`

…tly from GitHub.

danielbachhuber · 2017-03-17T12:36:56Z

composer.json

-		"psr-0": { "WP_CLI": "php" }
+		"psr-0": { "WP_CLI": "php" },
+		"psr-4": { "": "php/commands/src" },
+		"classmap": [ "php/export" ]


This classmap shouldn't be necessary anymore...

I'm currently trying to figure out the autoloading in general here. There's still an issue with the Spyc class, my proposed change did not really help at all.

danielbachhuber · 2017-03-23T13:29:06Z

features/command.feature

      """
    And STDERR should be empty
+
+  @bootstrap


What if we were to create a feature/bootstrap.feature file? I think breaking things apart is typically easier than using a bunch of tag annotations.

Yes, I agree that is preferable. Now that most of the package have been taken out, it can more easily be contained.

…the one that comes with Composer.

…ted with a dash.

Once this is merged to `master` branch, `dev-3850-refactor-loading-order` needs to be changed into `dev-master`.

The normal fork we were pulling in via a branch needs to be set in the `"repositories"` section and is only applied on the root package for security reasons. To avoid WP-CLI breaking for people that pull it in via Composer, we replace the fork with a properly released package under a different name. See #4017

…splitter.

With the new hooks introduced in wp-cli/wp-cli@7fcee0e , we can now only load the commands if the parent they depend on has already been added. This is needed because of the added flexibility provided by the bootstrap refactoring. See wp-cli/wp-cli#3872

See wp-cli/scaffold-package-command#104

schlessera · 2017-05-02T11:30:20Z

@danielbachhuber This should now be ready to be merged.

As we had already discussed, there's still two tests that are marked as @broken (and hence skipped on Travis):

Composer stack with override requirement after WP-CLI
The expected behavior is that the original WP-CLI command should run, not the override. The test shows that the override is run instead, though.
I'm not yet sure whether the expected behavior is even feasible, but it should not pose any problem to existing code, as such a scenario was not at all supported before.
Composer stack with override requirement before WP-CLI
This is teh same test as 1., but in the opposite order, where we would expect the override to be active instead of the original command.
The test shows that this is the case, but then the clean-up mysteriously fails and blocks Behat. I suspect this is due to the way the branch is pulled in. This will need further examination, but does not directly have anything to do with the behavior that is being tested.

masumkhan5479

ftg

With the new hooks introduced in wp-cli/wp-cli@7fcee0e , we can now only load the commands if the parent they depend on has already been added. This is needed because of the added flexibility provided by the bootstrap refactoring. See wp-cli/wp-cli#3872

schlessera added 10 commits March 1, 2017 16:41

Add scenarios for testing bootstrap loading order.

a81846e

Added scenarios: - `Install a package that overrides a command bundled in source` - `Install a package that overrides a bundled PHAR command` Both scenarios are tagged with `@bootstrap`. See #3850

Add scenarios for testing bootstrap loading order.

f2b6409

Added scenarios: - `Override command bundled with current source` - `Override command bundled with PHAR` Both scenarios are tagged with `@bootstrap`. See #3850

Make version optional when building a new PHAR.

e9a1d10

Add step to download existing PHAR release.

94a0b84

Implement step to download existing PHAR release.

40e365e

Add scenarios for testing bootstrap loading order for downloaded PHARs.

03e4210

Added scenarios: - `Override command bundled with downloaded PHAR` - `Install a package that overrides a command from a downloaded PHAR` Both scenarios are tagged with `@bootstrap`. See #3850

Fix typo and simplify composer.json file.

f57b73b

Added scenarios to test overriding commands in a Compsoer stack.

f666104

Added scenarios: - `Override command bundled with Composer stack` - `Install a package that overrides a command from a Composer stack` Both scenarios are tagged with `@bootstrap`. See #3850

schlessera added scope:bootstrap scope:framework labels Mar 2, 2017

danielbachhuber reviewed Mar 7, 2017

View reviewed changes

schlessera added 5 commits March 8, 2017 10:02

Use Spyc class instead of the procedural spyc_load_file().

63750ae

This change allows the `Spyc` through an autoloader, as soon as the related pull request to the `mustangostang/spyc` package has been merged: mustangostang/spyc#66

Modify splitter logic to include additional psr-4 namespace.

2618011

Remove bootstrap tests with downloaded PHARs.

4b87a91

These tests cannot work as long as there is no released PHAR version to download that includes the bootstrap changes from this branch.

Use workarounds to get to WIP versions of dependencies.

f204f6c

Include updated Composer lock file.

d69a32b

schlessera added 4 commits March 8, 2017 15:05

Merge branch 'master' into 3850-refactor-loading-order

42d0ad9

Update PR-specific Composer dependencies.

98d3b69

- `"wp-cli/autoload-splitter": "dev-master"` - `"mustangostang/spyc": "dev-master#1cea7c15324025cd7b62c8d47c10e1e94f634c1e"`

Use wp-cli/autoload-splitter package from packagist, instead of direc…

69477bf

…tly from GitHub.

Resolve merge conflicts.

9b80488

danielbachhuber reviewed Mar 17, 2017

View reviewed changes

Remove unneeded php/export classmap autoloader.

6a507be

danielbachhuber reviewed Mar 23, 2017

View reviewed changes

danielbachhuber mentioned this pull request Mar 23, 2017

Test suite for abstracted command package runs against wrong code #3850

Closed

schlessera added 2 commits March 23, 2017 16:14

Use wp-cli fork of mustangostang/spyc.

01bb810

Merge master back into branch.

6b3789f

schlessera added 9 commits April 17, 2017 15:41

Don't use short array syntax, we're targeting PHP 5.3+.

4093970

Alias mustangostang\spyc as 0.6.x-dev, not 6.0.x-dev.

e7800b9

Add documentation about why we need a separate autoloader apart from …

5489d2f

…the one that comes with Composer.

Explicitly declare the intent to use the same version for the Phar.

6aa3bc2

Improve regex to detect commands by allowing multiple sections separa…

73c771b

…ted with a dash.

Merge branch 'master' into 3850-refactor-loading-order

ecf9a1b

Add updated Composer lock file.

259f33f

Remove @broken tag from working cli tests.

7df2c0b

Pull in the correct version of WP-CLI for Composer stack tests.

4349cf9

Once this is merged to `master` branch, `dev-3850-refactor-loading-order` needs to be changed into `dev-master`.

schlessera mentioned this pull request Apr 25, 2017

Replace mustangostang/spyc fork with distinct package #4017

Closed

schlessera added 5 commits April 26, 2017 08:44

Rename custom command so that it is properly matched by the autoload …

39e7a61

…splitter.

Remove mustangostang folder from Phar building.

e02e069

Merge branch 'master' into 3850-refactor-loading-order

619b5e2

Fix "remove_command()" test.

f47fe72

schlessera mentioned this pull request May 2, 2017

Only add commands if scaffold is already loaded. wp-cli/scaffold-package-command#104

Merged

schlessera added 2 commits May 2, 2017 12:34

Fix loading order for scaffold package command.

92953a3

See wp-cli/scaffold-package-command#104

Fix version string when building Phars in Behat.

888ef1a

danielbachhuber changed the title ~~Refactor Bootstrap Process (WIP)~~ Refactor Bootstrap Process May 2, 2017

danielbachhuber added this to the 1.2.0 milestone May 2, 2017

schlessera mentioned this pull request May 2, 2017

Composer command override order fails #4037

Closed

danielbachhuber approved these changes May 2, 2017

View reviewed changes

danielbachhuber merged commit d9d405a into master May 2, 2017

danielbachhuber deleted the 3850-refactor-loading-order branch May 2, 2017 16:05

schlessera mentioned this pull request May 2, 2017

Behat hangs on clean-up #4038

Closed

danielbachhuber mentioned this pull request May 2, 2017

Make Spyc class in mustangostang\spyc package autoloadable. #3882

Closed

masumkhan5479 reviewed May 19, 2017

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Refactor Bootstrap Process #3872

Refactor Bootstrap Process #3872

Uh oh!

schlessera commented Mar 2, 2017 •

edited by danielbachhuber

Loading

Uh oh!

danielbachhuber Mar 7, 2017

Uh oh!

schlessera Mar 8, 2017

Uh oh!

danielbachhuber commented Mar 8, 2017

Uh oh!

danielbachhuber Mar 17, 2017

Uh oh!

schlessera Mar 17, 2017

Uh oh!

danielbachhuber Mar 23, 2017

Uh oh!

schlessera Mar 23, 2017

Uh oh!

schlessera commented May 2, 2017 •

edited

Loading

Uh oh!

masumkhan5479 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Refactor Bootstrap Process #3872

Refactor Bootstrap Process #3872

Uh oh!

Conversation

schlessera commented Mar 2, 2017 • edited by danielbachhuber Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danielbachhuber Mar 7, 2017

Choose a reason for hiding this comment

Uh oh!

schlessera Mar 8, 2017

Choose a reason for hiding this comment

Uh oh!

danielbachhuber commented Mar 8, 2017

Uh oh!

danielbachhuber Mar 17, 2017

Choose a reason for hiding this comment

Uh oh!

schlessera Mar 17, 2017

Choose a reason for hiding this comment

Uh oh!

danielbachhuber Mar 23, 2017

Choose a reason for hiding this comment

Uh oh!

schlessera Mar 23, 2017

Choose a reason for hiding this comment

Uh oh!

schlessera commented May 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

masumkhan5479 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

schlessera commented Mar 2, 2017 •

edited by danielbachhuber

Loading

schlessera commented May 2, 2017 •

edited

Loading