Skip to content
This repository has been archived by the owner on Jan 15, 2021. It is now read-only.

removed dependency on ext-curl #48

Merged
merged 1 commit into from Aug 3, 2015

Conversation

fabpot
Copy link
Member

@fabpot fabpot commented Aug 3, 2015

Having a hard dependency on cURL is problematic. For instance, this library is a dep of SensioDistributionBundle, which is also a dep of Symfony Standard Edition. So, it means that we are forcing everyone using Symfony SE to have the PHP cURL extension installed, which is not what we want.

So, this PR falls back to using file_get_contents when cURL is not installed.

@fabpot fabpot force-pushed the curl-requirement-removal branch 5 times, most recently from 189d17e to 6292137 Compare August 3, 2015 08:03
@fabpot
Copy link
Member Author

fabpot commented Aug 3, 2015

Another option is to drop cURL support and only keep file_get_contents support.

ping @javiereguiluz @stof

@javiereguiluz
Copy link
Contributor

👍 to remove the cURL requirement. In Symfony Installer we recently switched to use file_get_contents() for Windows users. This is the installation method which seems to work for most users:

c:\> php -r "file_put_contents('symfony', file_get_contents('http://symfony.com/installer'));"

In any case, I'd maintain cURL support because the support of file_get_contents() for URLs can be disabled via the allow_url_fopen setting in php.ini file.

$this->crawler->setEndPoint($endPoint);
}

public function doCheck($lock, $certFile)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should not be public

@fabpot
Copy link
Member Author

fabpot commented Aug 3, 2015

Comments addressed

throw new RuntimeException(sprintf('The web service failed for an unknown reason (HTTP %s).', $statusCode));
}

curl_close($curl);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could call curl_close($ch) just after getting the status code as none of the code paths after that are using $ch anymore. It would avoid having 3 different places getting it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right

@stof
Copy link
Contributor

stof commented Aug 3, 2015

👍

@fabpot fabpot merged commit d7e788a into sensiolabs:master Aug 3, 2015
fabpot added a commit that referenced this pull request Aug 3, 2015
This PR was merged into the 2.0-dev branch.

Discussion
----------

removed dependency on ext-curl

Having a hard dependency on cURL is problematic. For instance, this library is a dep of SensioDistributionBundle, which is also a dep of Symfony Standard Edition. So, it means that we are forcing everyone using Symfony SE to have the PHP cURL extension installed, which is not what we want.

So, this PR falls back to using file_get_contents when cURL is not installed.

Commits
-------

d7e788a removed dependency on ext-curl
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants