Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
This repository was archived by the owner on Nov 14, 2019. It is now read-only.

Add cURL extension requirement #200

Closed

Conversation

javiereguiluz
Copy link
Member

According to these comments, this change can prevent some errors: https://github.com/symfony/symfony-installer/issues/124#issuecomment-128694119

@weaverryan
Copy link
Member

This certainly makes sense. But does this also cover the use-case of me downloading the installer phar file and trying to run it without curl? Or is that already handled.

@ogizanagi
Copy link
Contributor

Is curl really needed anyway ? (I mean AFAIK, guzzle does not rely only on curl but has many different handlers available)
Maybe, instead of requiring curl, we could add a check through this function or similar in order to ensure one of the needed handler exist in order to make the request, or throw an appropriated exception in case it isn't.

@xabbuh
Copy link
Member

xabbuh commented Nov 22, 2015

Is the cURL extension available by default on Windows?

@emil-nasso
Copy link
Contributor

I tested this. cURL is not required, to my understanding.

I ran everything using docker with this dockerfile:

FROM debian:jessie
VOLUME .:/root/shared
WORKDIR /root/shared
RUN apt-get update && apt-get install -y php5-cli

ENTRYPOINT /bin/bash

i then built and ran it:

docker build -t symfony-test .
docker run -it -v `pwd`:/root/shared symfony-test /bin/bash

Out-of the box, the symfony installer worked on the environment. I double-checked to make sure that curl was not installed.

When I set allow_url_fopen = Off in php.ini i got this error instead, when i ran the installer:

https://dl.dropboxusercontent.com/s/cleweih8zetbawo/2015-12-09%20at%2013.41.png?dl=0

So (as it is implemented right now) guzzle in the symfony installer tries curl first and then fopen (as can be seen here: https://github.com/guzzle/guzzle/blob/5.3/src/Utils.php#L181 ).

So, to summarize: Curl is not required and you will get an error message if you lack curl and the allow_url_open-flag.

I wrote this (to replace this line: https://github.com/symfony/symfony-installer/blob/master/src/Symfony/Installer/DownloadCommand.php#L224 ) if we want to make the error message more "symfony" and a bit less confusing for new users:

        try {
            $handler = \GuzzleHttp\Utils::getDefaultHandler();
        } catch (\RuntimeException $e) {
            throw new \RuntimeException(
                'The symfony installer requires the php-curl extension or the '
                . 'allow_url_fopen ini setting.');
        }
        return new Client(array('defaults' => $defaults, 'handler' => $handler));

Should i create a pull-request or is the old message "good enough"?

@javiereguiluz
Copy link
Member Author

@emil-nasso thanks a lot for working on this issue and for your insightful explanation. I'm closing this issue as "won't fix". Besides, I like your proposal to display a custom error message. Please, consider submitting it as a pull request. Thanks!

javiereguiluz added a commit that referenced this pull request Dec 17, 2015
This PR was merged into the 1.0-dev branch.

Discussion
----------

Intercept and customize error from Guzzle

When guzzle is initalized it tries to find a suitable handler to use. If no handler can be found (for example curl) an exception is thrown.

With this change we try to find the handler before creating the client so that we can pass the handler to the client manually.
Doing this, we are able to customize the error message to make it (potentially) less confusing for the users who might have no idea what guzzle is.

The issue was found when looking at PR #200

Commits
-------

951732b Intercept and customize error from Guzzle When guzzle is initalized it tried to find a suitable handler to use. If no handler can be found (for example curl) an exception is thrown. With this change we try to find the handler before creating the client so we can pass the handler to the client manually. Doing this, we are able to customize the error message to make it (potentially) less confusing for the users who might have no idea what guzzle is.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.