Update to Guzzle 5

Wow, I have a feeling that you will be at Guzzle 17 before we can release Drupal 8 ;)

Is there a way to provide guzzle as versionable library?
Why core depends on interfaces from guzzle internals?

Is there a way to provide guzzle as versionable library?
Why core depends on interfaces from guzzle internals?

Depending on the bc breaks we might not be able to do this, but switching to task since we need to sort it out before release.

@mtdowling you can use core/scripts/run-tests.sh to run simpletest, but simplest way is to upload a patch here.

attached patch to see what breaks

note this patch is NOT FOR COMMIT, just for review.
Obviously we don't want dev-ring and minimum stability dev.

so 75 fails

@mtdowling no - HEAD is green (https://qa.drupal.org/8.0.x-status)

The fails relate to the update status module, which uses Guzzle to fetch json or xml from drupal.org and report security updates (not sure that the test actually does this, but that is what the module does).

The fails are 'cannot serialize closure'. So I'm guessing that is something to do with the Queue entries that are created to fetch each module update in the background. These are objects that are serialized into the {queue} table with the default queue backend.

+++ b/core/modules/update/src/UpdateFetcher.php
@@ -8,6 +8,7 @@
+use Drupal\Core\DependencyInjection\DependencySerializationTrait;

@@ -16,6 +17,8 @@
+  use DependencySerializationTrait;
+

So this is the only core change required to support the current state of the ring branch of Guzzle (which I assume to be Guzzle 5 once merged)

And we need to cast the getBody from a stream to a string

+++ b/core/modules/update/src/UpdateProcessor.php
@@ -161,7 +161,7 @@ public function processFetchTask($project) {
-      $data = $this->updateFetcher->fetchProjectData($project, $site_key);
+      $data = (string) $this->updateFetcher->fetchProjectData($project, $site_key);

That's strange... I think there should be a clean way (reusable) - fetchProjectData() should care about data and its type

The difference is upstream, getBody returns a guzzle stream object which can be cast to a string.
In the current version it returns differently for empty - you need to cast it to a string before checking if empty.

our head composer.json is invalid :(

composer self-update
composer validate
./composer.json is invalid, the following errors/warnings were found:
require.php : invalid version constraint (Could not parse version constraint >5.4.4-13: Invalid version string "5.4.4-13")
require.symfony/yaml : unbound version constraints (dev-master#089129be9f9828f41f0f345e09ff322ecf92cfb2) should be avoided
require.doctrine/common : unbound version constraints (dev-master#a45d110f71c323e29f41eb0696fa230e3fa1b1b5) should be avoided
require.phpunit/phpunit-mock-objects : unbound version constraints (dev-master#e60bb929c50ae4237aaf680a4f6773f4ee17f0a2) should be avoided

So assume we meant 5.4.13?

That was commited in #2267551: [meta] Deal with PHP requirement vs. Debian PHP version numbering

Missed updates to guzzlehttp/stream too
This should be pretty close

1. +++ b/composer.json
   @@ -4,7 +4,7 @@
   -    "php": ">5.4.4-13",
   +    "php": ">5.4.13",
   

   haha, nope :)

2. +++ b/composer.json
   @@ -30,6 +30,7 @@
      },
   +  "minimum-stability": "dev",
      "autoload": {
   

   Do we really need this? Won't that bite us in the future when people do other updates in other packages?

3. +++ b/composer.lock
   @@ -2498,9 +2578,7 @@
   diff --git a/core/includes/theme.inc b/core/includes/theme.inc
   
   diff --git a/core/includes/theme.inc b/core/includes/theme.inc
   index 1015f67..e8b9bde 100644
   
   index 1015f67..e8b9bde 100644
   --- a/core/includes/theme.inc
   
   --- a/core/includes/theme.inc
   +++ b/core/includes/theme.inc
   
   +++ b/core/includes/theme.inc
   +++ b/core/includes/theme.inc
   @@ -1492,6 +1492,7 @@ function template_preprocess_feed_icon(&$variables) {
   
   @@ -1492,6 +1492,7 @@ function template_preprocess_feed_icon(&$variables) {
        '#height' => 16,
        '#alt' => $text,
      );
   +  $variables['attributes']['class'] = array('feed-icon');
      // Stripping tags because that's what l() used to do.
      $variables['attributes']['title'] = strip_tags($text);
    }
   diff --git a/core/modules/update/src/UpdateFetcher.php b/core/modules/update/src/UpdateFetcher.php
   
   diff --git a/core/modules/update/src/UpdateFetcher.php b/core/modules/update/src/UpdateFetcher.php
   index 14bdb29..5f22265 100644
   
   index 14bdb29..5f22265 100644
   --- a/core/modules/update/src/UpdateFetcher.php
   
   --- a/core/modules/update/src/UpdateFetcher.php
   +++ b/core/modules/update/src/UpdateFetcher.php
   
   +++ b/core/modules/update/src/UpdateFetcher.php
   +++ b/core/modules/update/src/UpdateFetcher.php
   @@ -8,6 +8,7 @@
   
   @@ -8,6 +8,7 @@
    namespace Drupal\update;
    
    use Drupal\Core\Config\ConfigFactoryInterface;
   +use Drupal\Core\DependencyInjection\DependencySerializationTrait;
    use GuzzleHttp\ClientInterface;
    use GuzzleHttp\Exception\RequestException;
    
   @@ -16,6 +17,8 @@
   
   @@ -16,6 +17,8 @@
     */
    class UpdateFetcher implements UpdateFetcherInterface {
    
   +  use DependencySerializationTrait;
   +
      /**
       * URL to check for updates, if a given project doesn't define its own.
       */
   diff --git a/core/modules/update/src/UpdateProcessor.php b/core/modules/update/src/UpdateProcessor.php
   
   diff --git a/core/modules/update/src/UpdateProcessor.php b/core/modules/update/src/UpdateProcessor.php
   index d37f643..f129845 100644
   
   index d37f643..f129845 100644
   --- a/core/modules/update/src/UpdateProcessor.php
   
   --- a/core/modules/update/src/UpdateProcessor.php
   +++ b/core/modules/update/src/UpdateProcessor.php
   
   +++ b/core/modules/update/src/UpdateProcessor.php
   +++ b/core/modules/update/src/UpdateProcessor.php
   @@ -161,7 +161,7 @@ public function processFetchTask($project) {
   
   @@ -161,7 +161,7 @@ public function processFetchTask($project) {
        $project_name = $project['name'];
    
        if (empty($this->failed[$fetch_url_base]) || $this->failed[$fetch_url_base] < $max_fetch_attempts) {
   -      $data = $this->updateFetcher->fetchProjectData($project, $site_key);
   +      $data = (string) $this->updateFetcher->fetchProjectData($project, $site_key);
        }
        if (!empty($data)) {
          $available = $this->parseXml($data);
   

   Seems totally unrelated :)

We can do "prefer-stable": true then the "minimum-stability" shouldn't hurt.

1 and 2 will fix. 3 some are needed, some aren't

Fixes #28 for explanation of changes to core/modules/update/src/UpdateFetcher.php core/modules/update/src/UpdateProcessor.php see fails/comments above.

I think this is ready

re-roll

+++ b/core/modules/update/src/UpdateProcessor.php
@@ -161,7 +161,7 @@ public function processFetchTask($project) {
 
     if (empty($this->failed[$fetch_url_base]) || $this->failed[$fetch_url_base] < $max_fetch_attempts) {
-      $data = $this->updateFetcher->fetchProjectData($project, $site_key);
+      $data = (string) $this->updateFetcher->fetchProjectData($project, $site_key);

I think this should be casted to a string inside UpdateFetcher::fetchProjectData(). The documentation says it returns a string and the API there is clearly wrong (there is no TRUE argument anymore to getBody()).

good catch

Everything is addressed so RTBC.

Committed/pushed to 8.0.x, thanks!

Added follow up #2401159: Convert instances of Guzzle getBody(TRUE) for converting instances of getBody(TRUE)

Added follow-up #2442411: Update react/promise to 2.2.0 (related only in that the library was introduced by Guzzle 5).