Issue Summary
We're using non-specific version constraints for all of our dependencies. This causes unnecessary problems for very minimal gain.
With the current process, if I need to update one dependency, I edit the composer.json, run composer update, and not only does it update my dependency, but all others that have new releases that satisfy our version constraints.
Now I have to create a patch that includes any dependency that got updated, which is not ideal considering that the one issue I post to now has to be seen by anyone who is working with an updated dependency. Alternatively, I could selectively add just the package that I needed changed, but then the composer.lock will be pointing to updated versions of other packages that I didnt include. I guess I could hand edit that, but no thanks.
I propose that we should *never* use fuzzy version constraints. Specific stable/beta/alpha tags should be our first choice, and then if we need changes that haven't made it into a tag yet, we use the specific git hash for that commit.
This way, if I need to update one dependency in the composer.json, only that dependency is changed when I run composer update. This will be much easier for both contributors and reviewers.
This patch changes all the version constraints to the latest tags that fit within our previous version constraints, with the exception of easyrdf where I just specified the latest hash. This patch also brings a bunch of updates to dependencies because that's what happens when you run composer update currently. Hopefully this can be the last monster dependency updating patch of its kind.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| pin_dependencies.patch | 354.77 KB | Idle | FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed. | View details |
Comments
#2
The last submitted patch, pin_dependencies.patch, failed testing.
#1
The last submitted patch, pin_dependencies.patch, failed testing.
#3
This is a legit fail because Symfony changed RouteCompiler::compile to be static. I went ahead and changed it, which required making all the methods in our version static, just to get this to work, but Larry needs to look at this for a possibly better solution.
#4
The last submitted patch, pin_dependencies-3.patch, failed testing.
#5
#6
The last submitted patch, pin_dependencies-5.patch, failed testing.
#7
As long as we can still pin to specific commits (since right now we need some post-beta1 but pre-beta2 Symfony component updates), I'm OK with this in general for the time being.
#8
This patch has an example of pinning to a specific commit with easyrdf, totally doable.
#9
easyrdf should use the tag
0.8.0-beta.1. This patch only touches easyrdf related fixes.#10
According to Fabien, Symfony 2.2-beta2 is due out in a day or two (try saying that 5 times fast), so we should probably sit tight for a bit and then pin that.
#11
I narrowed down the test fails to this API change in symfony-cmf, which will be handled in #1894934: Update CMF Routing component.
#12
It looks like a release of easyrdf came out a couple days ago. Is there anything that's holding us back from targeting 0.8.0-beta.1?Ah, scor got it already :-) .
[EDIT] Once the updates go through, I'd like to #1805316: Move composer.json to the project root.
#13
updating patch after #1894934: Update CMF Routing component was committed. we should still wait for the next beta release of symfony before committing this.
#14
The last submitted patch, 1894002_pin_dependencies_13.patch, failed testing.
#15
http://symfony.com/blog/symfony-2-2-0-beta-2-released
beta2 is out, let's do our thing.
#16
#17
The last submitted patch, 1894002_pin_dependencies_16.patch, failed testing.
#18
symfony beta2 seems to wreck the installer (both the UI and drush cli) with the error
You have requested a synthetic service ("request"). The DIC does not know how to construct this service.trying out with beta1 to see if at least we can get our first green in this issue...
#19
The last submitted patch, 1894002_pin_dependencies_beta1_18.patch, failed testing.
#20
git apply complains about a missing full index. rerolling with --binary --full-index
#21
The last submitted patch, 1894002_pin_dependencies_beta1_20.patch, failed testing.
#22
#20: 1894002_pin_dependencies_beta1_20.patch queued for re-testing.
#23
I haven't been able to review all the changes all the way up to BETA2 yet, but I found another API change in synfony (94f6116) which required to update our codebase (included in this patch).
The installer is still broken with the error "You have requested a synthetic service ("request"). The DIC does not know how to construct this service.", but commenting out the line that generates this message in createService() allows the installer to work. I'm curious to see how many tests are passing/failing with this hack, and hopefully that will give more insight on why this error is occurring during the install process.
#24
The last submitted patch, 1894002_pin_dependencies_23.patch, failed testing.
#25
Crell recommends to upgrade one at a time. I was able to do that until I hit dependency-injection. Here is a patch with everything pinned to beta2 except dependency-injection. let's see if everything works so far.
#26
The last submitted patch, 1894002_pin_dependencies_25.patch, failed testing.
#27
Turns out the problems occurring when upgrading to beta2 come from the fact that we register the "request" service as a synthetic service, and symfony doesn't know how to handle this: You have requested a synthetic service ("request"). The DIC does not know how to construct this service.
The installer and the JSON-LD and REST tests pass if we register "request" as a normal non-synthetic service. I'm not sure what the implications are, but let's see first how many tests are failing with this change.
Edit: possible explanation for this: that there was an API change a while back but we were not affected by it, and this API was only recently enforced with 4119ca. This is the only change I could find between our old version of symfony and beta2 involving isSynthetic(). Not being an expert on symfony, I could very well have missed something.
#28
The last submitted patch, 1894002_pin_dependencies_27.patch, failed testing.
#29
#27: 1894002_pin_dependencies_27.patch queued for re-testing.
#30
The last submitted patch, 1894002_pin_dependencies_27.patch, failed testing.
#31
+++ b/core/lib/Drupal/Core/CoreBundle.php@@ -117,8 +117,7 @@ public function build(ContainerBuilder $container) {
- $container->register('request', 'Symfony\Component\HttpFoundation\Request')
- ->setSynthetic(TRUE);
+ $container->register('request', 'Symfony\Component\HttpFoundation\Request');
The 'request' service is logically a synthetic service, so this is masking a real bug: #1907750: Symfony update exposes fatal error during installation due to 'access_manager' service depending on 'request'. I don't mind us doing this as a temporary measure if the rest of this patch becomes ready for commit prior to solving that other issue, but we do need to solve that other issue.
#32
#27: 1894002_pin_dependencies_27.patch queued for re-testing.
#33
For easier review, here's all the manual changes that are in #27. The rest is fetched by Composer. This looks fine to me other than what I wrote in #31.
However, when I try running a composer update, I get this error:
[RuntimeException]
Failed to clone http://github.com/njh/easyrdf.git via git, https and http protocols, aborting.
- git://github.com/njh/easyrdf.git
fatal: No such remote 'composer'
- https://github.com/njh/easyrdf.git
fatal: No such remote 'composer'
- http://github.com/njh/easyrdf.git
fatal: No such remote 'composer'
So, I can't verify that #27 represents the expected versions of everything. Any ideas on what's wrong with easyrdf, composer, or my computer?
#34
@effulgentsia I often get this kind of error on random symfony components when running composer update, but now I always run
rm -rf core/vendorbefore running composer update to avoid this.Edit: this is the script I use:
cd corerm -rf vendor
git co vendor/.gitignore
git co vendor/README.txt
composer update
#35
Oh, right. I forgot about the need to do that. Thanks. #27 is good to go then.
#36
I've posted a fix for the other issue as well. I agree we shouldn't leave this hack in here for too long, but I'm OK with this issue going in first since it's already RTBC.
#37
Tagging, since this is a dependency for the revised WSCCI and SCOTCH sub-request stuff.
#38
Seems like an ok hack for now to get this in. #27 looks great to me.
#39
I got whitespace errors when applying the patch:
warning: squelched 3 whitespace errorswarning: 8 lines add whitespace errors.
Fine with the other issue being a follow-up, especially given it's major bug and already has patch at CNR. Committed/pushed this to 8.x.
Good that we're moving to specific versions in composer.json that makes a lot more sense.
#40
Automatically closed -- issue fixed for 2 weeks with no activity.
#41
People here may be interested in #1977570: Update third-party vendors and fix composer.json which is suggesting to remove version pinning via composer.json.