Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add ability to invoke CLI commands on Composer scripts #1186

Merged
merged 7 commits into from Oct 22, 2012

Conversation

Projects
None yet
6 participants
Contributor

johnkary commented Oct 6, 2012

This new functionality allows command-line tasks to be executed when any Composer event is fired. It currently preserves backwards compatibility with the ability to intermix PHP callbacks and CLI commands in the same array of event script commands. It also includes some expansion of the scripts documentation and has also been updated to explain the new feature.

This ability to execute CLI commands with events exists in npm, and while this feature addition does not fully replicate it, it provides the basic ability to start doing so.

Jordi and I discussed this feature at the hack day during SymfonyLive 2012 in San Francisco and I implemented it there with some feedback from him.

Basic Usage

{
    "scripts": {
        "post-install-cmd": [
            "phpunit -c path/to/phpunit.xml"
        ]
    }
}

Use Cases

Test suite execution

My initial use case for my own needs was to run my project's functional and unit tests after pulling in new versions of my dependencies to make sure all tests still pass.

Integrate with other package managers

Some projects offload package management for Chef cookbooks (e.g. librarian-chef), while many others also require frontend web assets.

There are currently several frontend package managers vying for a top spot, but I don't think any one has taken hold as a standard. Jordi has written on this in the past and it would be great if someday we could manage our frontend assets more natively through Composer. But for now each frontend package manager operates in mostly its own ecosystem.

The functionality in this PR allows Composer to delegate to your project's chosen frontend package manager, and to pull in all of your frontend assets as part of updating your PHP dependencies. This allows our reliance on frontend package management to evolve independently from Composer.

Delegate to other command-execution tools

Many projects leverage additional command-line tasks from tools like Phing, Capifony, Rake, custom shell scripts, etc. You could go wild and integrate deployment as part of running Composer if that fits your project and team's mindset.

Lay groundwork for other command-line integrations

Feature request #1055 asks for a reliable way to execute a project's test suite. A command like composer test could executes a project's test suite. So a developer could jump into any project that uses Composer and instantly know how to run the test suite. With PHPUnit 3.7.3 now (properly) available via Composer, now seems like the right time to add composer test functionality to Composer:

 {
    // NOT IMPLEMENTED IN THIS PR, EXAMPLE ONLY
    "tests": [
        "phpunit -c path/to/phpunit.xml"
    ],
}

Further Improvements

Separate lists for PHP and CLI commands

I think the mixing of PHP callbacks and CLI commands in the same JSON array is a bit error-prone. Right now the code simply checks if the string contains :: and tries to execute it as a PHP callback, else tries to execute it as a CLI command. Theoretically a valid CLI command could contain :: somewhere in the command causing the wrong execution path.

Splitting a script's events into two members would make for not only a cleaner approach in the code (no if statement required) but also easier to predict results:

{
    "scripts": {
        "post-install-cmd": {
            "php": [
                "MyVendor\\MyClass::someMethodName"
            ],
            "commands": [
                "phpunit -c path/to/phpunit.xml"
            ]
        }
    }
}

Better support for executing CLI commands

NPM supports much more in the way of CLI command execution, some of which our users may require.

Right now this functionality passes CLI commands to the Symfony Process component. Hopefully this component provides good compatibility for CLI execution on multiple platforms (e.g. Windows). I haven't tested any of this code against a Windows platform.

Any limitations in the CLI functionality in Composer will probably be traced back to the Process component.

Tests for CLI output

This PR includes a test for the "happy path" of executing a CLI command, but I could not figure out how best to write a test to verify the console output was correctly passed to the IO stream. I have pushed my work in progress for writing this test to a branch in my fork. Your help in writing a test for this, or making changes to make it more testable, would be appreciated:

https://github.com/johnkary/composer/tree/cliEvents-testCliOutput

jmather commented Oct 6, 2012

I'm 100% for this. I wonder if @phiamo might have any suggestions for inclusion that could help ease the hoops that the bootstrap bundle jumps through.

jmather commented Oct 6, 2012

Another thought -- what if we wanted to run some php based commands, and then some shell commands, and then another php based commands?

I think we could maintain BC if we did something like:

{
    "scripts": {
        "MyVendor\\MyClass::someMethodName": "php",
        "phpunit -c path/to/phpunit.xml": "cli"
    }
}

As you could iterate over scripts, and if the key is_numeric, then clearly it's old style and php only, otherwise look at the value to determine how to run it?

Though that wouldn't allow for the same command to be repeated ... maybe nested arrays?

Contributor

johnkary commented Oct 6, 2012

@jmather The current code in this PR already allows for intermixing PHP and CLI commands in any order for a single event.

jmather commented Oct 6, 2012

@johnkary Ah! I took the "Further Improvements" for "code already completed" and was concerned the format of it may block such things.

@stof stof and 1 other commented on an outdated diff Oct 6, 2012

src/Composer/Script/EventDispatcher.php
@@ -126,4 +142,15 @@ protected function getListeners(Event $event)
return $scripts[$event->getName()];
}
+
+ /**
+ * Checks if string given references a class path and method
+ *
+ * @param string $callable
+ * @return boolean
+ */
+ protected function isPhpScript($callable)
+ {
+ return false !== strpos($callable, '::');
@stof

stof Oct 6, 2012

Contributor

you should also check if there is a space in it. This would allow detecting properly some of the CLI commands containing :: as a PHP callable cannot have a space in it.

@johnkary

johnkary Oct 6, 2012

Contributor

Good idea. Should we check for a space instead of :: or in addition to it?

@stof

stof Oct 7, 2012

Contributor

In addition to it, otherwise running simply phpunit would try to call a PHP function.

Contributor

johnkary commented Oct 7, 2012

Added check for space in PHP callable and expanded test scenarios.

Make test less brittle
Shouldn't really care about whether the IO is touched.
That's the test knowing too much about the implementation.

@Seldaek Seldaek referenced this pull request in symfony/symfony Oct 8, 2012

Closed

Ability to pass the environment to composer #5696

Contributor

johnkary commented Oct 10, 2012

I think this PR is ready to be merged. The topics I outlined in "Further Improvements" above could be addressed in future PRs if deemed worthy.

The only improvement that may be in scope for this new feature is "Separate lists for PHP and CLI commands". But it's a backwards compatibility break for those currently leveraging scripts. Anyone updating composer and expecting their scripts to run would need to update their composer.json. What do you think?

jmather commented Oct 10, 2012

I think the inter-mixed list is likely fine, as it is cleaner and doesn't introduce a bc break.

Contributor

stof commented Oct 10, 2012

@johnkary separating the lists is bad IMO as it would mean that the order is then mandatory (all PHP scripts first or all CLI ones first depending of the implementation)

Contributor

johnkary commented Oct 10, 2012

@stof Good point, I hadn't thought of that. Let's stick with the current functionality.

Owner

Seldaek commented Oct 10, 2012

@johnkary I think it's fine too, but gotta review in finer details before merging. It's fairly standalone and risk free though so I'll try to do it soonish.

@nicodmf nicodmf commented on an outdated diff Oct 21, 2012

doc/articles/scripts.md
-- **pre-install-cmd**: occurs before the install command is executed.
-- **post-install-cmd**: occurs after the install command is executed.
-- **pre-update-cmd**: occurs before the update command is executed.
-- **post-update-cmd**: occurs after the update command is executed.
+Composer fires the following named events during its execution process:
+
+- **pre-install-cmd*: occurs before the `install` command is executed.
@nicodmf

nicodmf Oct 21, 2012

Little typo here: an * to add

@Seldaek Seldaek merged commit a069f7a into composer:master Oct 22, 2012

Owner

Seldaek commented Oct 22, 2012

Merged, thanks!

it looks like this might answer a question i had for a Larvel installation on bluehost.

Question: is this implemented & if so - what would be the correct language to run artisan script commands both using php & php-cli ?

For instance - i'd like to run 'php artisan' on mac machines, and 'php-cli' from the bluehost environment.

Original composer.json includes:

"scripts": {
    "post-install-cmd": [
        "php artisan optimize"
    ],
},

Would updated version look like:

"scripts": {
    post-install-cmd": [
        "php artisan optimize"
    ] : "php"
    post-install-cmd": [
        "php-cli artisan optimize"
    ] : "cli"
},

or run the composer update --no scripts option & add a command section?

"commands": {
    post-install-cmd": [
        "php-cli artisan optimize"
    ]
},

Sorry if this is more obvious. I didn't see it in the documentation.

Owner

Seldaek commented Jan 9, 2014

This is presently not possible. bluehost should fix their shit so that php is php on the cli. Anything else is madness. That said, if you can't use another host and they can't fix it, you can maybe work around it using a PATH hack, e.g. by running this in the terminal:

mkdir bin
echo "#!/bin/sh\nphp-cli $*" > bin/php
chmod +x bin/php
export PATH="`pwd`/bin:$PATH"
hash -r

Then calling php should proxy to php-cli

Thanks @Seldaek ! I'm going to email bluehost to ask if there is an official override possible. In the meantime your response is helpful!!!

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