Update dependencies (Symfony and Twig) follow up fixes for Composer

That's the wrong Symfony version. It's still tracking stables. We want to be tracking master/dev, as there's patches only in master that we added that we need.

#2: 1834594.patch queued for re-testing.

The classloader change should not be contained in this monster.

Postponing on #1834594: Update dependencies (Symfony and Twig) follow up fixes for Composer

sun: Knock it off. You just postponed an issue on itself.

We really, *really* should not be sweating about the class loader we're using right now. That's a premature micro-optimization. We can futz with the class loader we're using until June if we want to, since it's a performance question. For now, whatever works and works easiest works for me.

Sorry, that was meant to be #1658720: Use ClassLoader instead of UniversalClassLoader

There's a lot of discussion over there, and it doesn't make sense 1) to repeat that here, nor 2) to review the changes within a 800 KB patch.

I'm not switching classloader unless it's in a dedicated issue. Issues that update external libraries should do one thing, and one thing only.

There's no discussion here about what the problem with the classloader even is.

Also please define both 'premature' and 'micro-optimization' before bandying the term around in relation to the autoloader. How small a performance change is 'micro'? Exactly how late do we start dealing with the myriad performance regressions introduced during Drupal 8? Can optimization ever be premature in a 12-year-old legacy system where many of the performance bottlenecks are already well documented?

In this case, I define it as "changing the autoloader is by design independent of the things being autoloated once you're using PSR-0, so we should be focusing on an autoloader that's easy for us to work with right now, not the one that's fastest". At least until we are done adding libraries, so we should focus on "it works" for now, unless someone can demonstrate that we could not change our autoloader in May without breaking all the things. (The whole point of PSR-0 is to allow the code and the autoloader to be independent of each other. It wasn't designed for performance, but for DX.)

I am not saying performance isn't important. I'm saying our focus right now should be functionality and architectural-level performance issues, not API-irrelevant optimizations.

In this case, I define it as "changing the autoloader is by design independent of the things being autoloated once you're using PSR-0, so we should be focusing on an autoloader that's easy for us to work with right now, not the one that's fastest".

That's not true either:

catch@catch-thinkpad:~/www/8$ grep -r "getNamespaces" *
core/lib/Drupal/Core/Plugin/Discovery/AnnotatedClassDiscovery.php:    $namespaces = drupal_classloader()->getNamespaces();
Binary file core/lib/Drupal/Core/Plugin/Discovery/.AnnotatedClassDiscovery.php.swp matches
core/vendor/symfony/class-loader/Symfony/Component/ClassLoader/Tests/UniversalClassLoaderTest.php:        $namespaces = $loader->getNamespaces();
core/vendor/symfony/class-loader/Symfony/Component/ClassLoader/UniversalClassLoader.php:    public function getNamespaces()
core/vendor/symfony/class-loader/Symfony/Component/ClassLoader/DebugUniversalClassLoader.php:                $loader->registerNamespaces($function[0]->getNamespaces());

Which is why this discussion should happen in its own issue. Plugin system needs to be fixed to not rely on UniversalClassLoader before anything can be changed.

All that was needed to get past the autoloader problem was to revert core/vendor/composer/autoload_namespaces.php. This does that, but also adds some other Drupal and Symfony changes that were needed to get Drupal to even install locally for me. I'll post that diff separately, but first, curious what other problems bot finds.

Here's #16 decomposed:

- 'base-min' is just the composer.json change
- 'base-full' is that plus running composer update
- 'hacks' is the stuff I did on top of that to get #16. Obviously not commit worthy, just a work in progress. Next step is fix the test failures and maybe find proper ways to clean up the hacks.

This interdiff is all of the changes relative to #18's "base-full".

This is blocking #1854902: Document possible CSRF vulnerability in REST module.

checked #21 but it's missing some changes necessary for #1831074: Send 415 Unsupported Media Type http header when serializer encounters an unsupported format (which will be used in 1 and 2). Are we tracking the right Symfony branch? The commit is in master (committed a few weeks ago).

Add list of dependent issues.

Add another blocked issue.

Are we tracking the right Symfony branch?

Hah! No, the http-kernel component was downgraded from 2.1 to 2.0 instead of upgraded to 2.2. That's why all those hacks in #21 were needed. The reason Composer chose to downgrade that component, along with event-dispatcher, is because Guzzle requires event-dispatcher 2.1.* (with no ">" sign). I'm not quite sure why that mean http-kernel needed to be downgraded to 2.0 instead of being left at 2.1, but in any case, doesn't matter, since what we need is to go to 2.2.

So, I:
- took the same composer.json as what's in #18's "base-min",
- temporarily removed guzzle/http from it,
- ran composer update, which removed core/vendor/guzzle,
- told git to discard the removal of core/vendor/guzzle
- restored guzzle/http to composer.json
- discarded changes to core/vendor/composer/autoload_namespaces.php, per #16.

And that's what's in this patch. Hopefully, good enough to commit and unblock the issues depending on this. But future composer update runs will break until Guzzle updates their composer.json to allow event-dispatcher 2.2.

Quickfix filed upstream for Guzzle: https://github.com/guzzle/guzzle/pull/187

Given that this is blocking several issues, I'm gonna be bold and RTBC it. catch or someone else could send it back if the composer process hacks in #24 are unacceptable.

Looks like that change was incorporated quickly, so the concerns in #24 about breaking composer should be addressed.

Committed and pushed to 8.x. Thanks!

I tried a composer update, but it didn't quite work.

I had a follow up conversation with the Guzzle maintainer; he needed to make one additional commit, and I had to rework composer.json a bit to be compatible (frankly, it's black magic to me why this works and the original doesn't).

Also, to not require remembering to discard autoload_namespace.php changes every composer update, I fixed bootstrap.inc (and in the process, removed an incorrect comment about PSR-0).

This also includes a couple minor Symfony updates, since we're now tracking their master branch, so it automatically came in when I ran composer update.

Finally, there's still an annoyance that before running composer update, you need to rm -rf core/vendor, because we're now tracking git branches, but not committing the corresponding .git directories (thanks to our .gitignore) in order to avoid git submodule hell. But it trips up Composer updates to have git clones without the .git info; cleaning out the vendor directory first makes Composer happier. I have no idea where to document this so that the next person running a composer update knows this.

I assume due to the rm business, this patch removes the .gitignore and README.txt files from core/vendor, which I think is unintended.

Thanks. I ran #28's src again with:

cd core
rm -r vendor/*/
curl -s https://getcomposer.org/installer | php
php composer.phar update
rm composer.phar
cd ..
git add core
git status       # to check if anything needs removal with git rm
git diff --cached > /PATH/TO/PATCHFILE.patch

File's larger, because it pulled in another day of upstream Symfony changes.

As a side note, given that #28 passed bot, we may now want to remove that core/vendor/.gitignore file, and since we're using Composer to manage the vendor directory, I don't know if core/vendor/README.txt is useful either, but that's a topic for a separate issue. This patch is just about getting composer update to be as smooth as possible.

#31: update-1834594-followup-31.patch queued for re-testing.

Would be nice for Composer to not be in a broken state in D8 HEAD, so RTBC'ing, under the assumption that the "src" file in #28 and commands in the comment text of #31 are straightforward enough for a core committer review to be sufficient.

Committed and pushed to 8.x. Thanks!