Problem
- It is not clear how we're going to maintain and handle Symfony code (at the core of Drupal) in case of API-incompatible fixes/changes (e.g., caused by security issues).
Goal
- Clarify the problem space of hard dependencies between independent software projects.
- Check Symfony's process, paradigm, and handling of security issues.
- Decide on a policy for how we're going to maintain Symfony code.
Details
Over in #1424924: Use Composer for updating Symfony components (without removing Symfony code from repo), the following was mentioned:
no one should be editing code in there [Symfony components in Drupal core] anyway.
That absolute statement is problematic and has a very good chance to not hold water. Here's why:
- We're pulling in code from an external software project. Both that project as well as Drupal claim to retain API compatibility in stable branches.
- We're going to update the external software to its latest minor/point release when we're about to create a new minor/point release ourselves.
- As we've learned at the WSCCI sprint, we should be safe to use any component that is tagged with @api (or similar).
- If we find any issues, we'll surely file a patch upstream.
So far, so good. But this train of thought has to be continued:
- Symfony is not a dumb frontend library like jQuery. We're talking about bare essential code that is going to drive Drupal at its core.
- If we find a security issue, then we'll attempt to file a patch upstream.
- As you know, security issues require ugly hacks or even API changes at times, so there's a pretty good chance that the fix we're filing upstream won't be accepted for various reasons, or will only land for the next major version.
- At the same time, we likely still want to update to newer minor/point releases of the external software, because they are likely to fix minor bugs down the line (which equally may affect our core code or contrib code).
- This inherently means that we might have to touch our copies, and actually have to merge in upstream changes instead.
- In the end, there's absolutely no guarantee that we will be able to always use those components as-is, for the entirety of the next ~5+ years.
This is a fact. Even more so, since we never did something like this before. Whoever argues against that argues on the base on hypothetical assumptions, which may or may not be true. You can happily point me to this issue in 5 years and tell me that I was wrong. But until that future point in time has elapsed, this boils down to pure risk management. And there is a 99% risk in my book.
Again, this is not about jQuery or jQuery UI libraries. They're not comparable, at all.
Apparently, a Symfony developer clarified that there have been backwards-incompatible changes to components flagged as @api in some minor/point releases. That's a topic which comes actually on top of what I outlined above, because the API change is reversed; it originates from upstream instead of downstream.
Comments
Comment #1
cosmicdreams CreditAttribution: cosmicdreams commentedPerhaps we should maintain our own branch for Symfony 2 and merge in changes that are proved to not break core. This would cause the diversion that sun's talking about but I think stability of Drupal trumps progress of Symfony. If an API breaking change must occur then that should be a new branch. And if during the lifetime of D8 Symfony moves from 2.1 to 3 then we'll make an attempt to rework our implementation of Symfony in Drupal Next
Comment #2
lsmith77 CreditAttribution: lsmith77 commentedFirst up, we do everything possible to not break "@api" labeled code, however indeed in some rare cases this will not be possible (see for example the refactoring of the session system for 2.1). so the issue is indeed real. that being said, 2.0 components will remain unaffected of these changes for 2.1. furthermore there will be an LTS (long term support) release of Symfony2 likely with 2.2 tentatively scheduled for late summer or early fall this year which will have support for 5 years at least.
Now of course as pointed out in the ticket, even in this LTS release we might find some critical security bug that requires us to break BC. In this case we will of course communicate with all our major users to find the best solution.
Do note however that thanks to OO principles (and dependency injection) it is much easier for projects using these components to make their own choices. In most cases it would be possible to simply extend the given class with your own solution and if you are using dependency injection it would be trivial to then start using your custom class.
Comment #3
RobLoachQuick note.... I've put together a Symfony Update module which, in hook_init(), adds an updated version of Symfony to the Symfony module. The key is registering the namespaces with $prepend = TRUE... Then, when finding the classes, it checks the Symfony Update paths before moving up to the Symfony module's paths.
#1424924: Use Composer for updating Symfony components (without removing Symfony code from repo)
Comment #4
deviantintegral CreditAttribution: deviantintegral commentedHere's the documentation for how Symfony handles security reporting; in essence, they follow the same process we do. What I don't see is a definition for how they classify vulnerabilities similar to https://drupal.org/security-team/risk-levels.
We should work with the Symfony team to have some sort of communication so that we can try to release security updates in parallel with Symfony itself.
Comment #5
cfennell CreditAttribution: cfennell commentedFWIW, the approach suggested by the Composer lead developer, @Seldaek, on the issue @Rob Loach mentions in #3:
http://drupal.org/node/1424924#comment-5615070
Comment #6
marfillaster CreditAttribution: marfillaster commentedThis is indeed the way to go. Drupal will maintain its own composer.json and you can easily provide temporary custom repo. Or in the event of full blown diversion, a fork.
Comment #7
cfennell CreditAttribution: cfennell commentedRails also handles this problem in a similar fashion with Bundler, the inspiration for Composer.
I tested the process of forking and posting an alternate Composer package just to see what it was like. I was surprised at how easy it actually was; it took me about two minutes to publish an alternate HttpKernal package: http://packagist.org/packages/symfony/http-kernel-test123
The process took three steps: I (1) forked the original on github, (2) changed the HttpKernal composer.json file to rename the package (packages must be uniquely named), and (3) submitted the git uri to http://packagist.org for publication.
Not that there won't be other considerations to work through, but does this workflow not address the primary concern raised by this issue?
Comment #8
BerdirI am unsure as to what needs to be done to be able to close this issue, same as cfennell in #7.
- As mentioned by lsmith77 in #2, there are plans for an LTS release of symfony that should be released in time for Drupal 8, being supported/maintained for at least 5 years.
- cfennell in #7 shows that it's trivial to maintain our own backwards compatible version if really necessary.
- I'm sure that any BC-breaking change in a minor update will receive a big bold warning in the changelog (e.g. https://github.com/symfony/symfony/blob/2.0/CHANGELOG-2.0.md).
- The changes in a major 2.x update seem to be documented like this: https://github.com/symfony/symfony/blob/master/UPGRADE-2.1.md.
- As deviantintegral mentioned in #4, they do have a similar process for security releases.
So, here is an attempt at defining a policy:
We are going to switch to the LTS version (whether that will be 2.1 or 2.2) once available, regularly update to the newest minor version to include bugfixes but otherwise stay with that version. Our security team will stay in close contact with theirs in case there is a need for a coordinated security release. *If* they introduce a BC break *and* we know a way to fix whatever the issue is without doing that, it is a matter of minutes to set up a fork on github, create our own composer package and continue with that.
What else to we need to define/know? Setting to needs review.
Comment #9
cfennell CreditAttribution: cfennell commentedAnother, perhaps better, approach for us would be to create a http://packages.drupal.org "micro-packagist" site and deliver core external dependencies from our own servers. This has a few noteworthy advantages over deploying from packagist.org:
I read through the Composer documentation and implemented an example package site (http://packages.libsystems.org/). This process was almost as easy as publishing to packagist.org. Satis, the software that powers packagist.org sits on your machine and produces a package index of flat HTML files based on a composer.json file where you list all of your available packages.
Since we are currently talking only about forked versions/downstream patches, I think a micro-packagist site makes some sense. For things that we'd want distribute back to the wider PHP community down the road, packagist.org might be a good option, especially with a little more experience under its belt.
Anyway, just thought I'd mention the localy installed package site option since it hasn't come-up just yet.
Comment #10
cfennell CreditAttribution: cfennell commentedInadvertent status change while @Berdir was posting, re-assigning to "needs review"
Comment #11
RobLoachThis is an absolute non-issue. If we need to run our own Symfony fork, we could host our own with Satis, as outlined in #9. We could also have contrib use the ClassLoader to switch which Symfony classes are loaded, as outlined in #3. So far though, we haven't had any requirement to do this.
We run the master branch of Symfony currently, which will soon become Symfony 2.1 beta. When 2.2 comes out, we'll be able to either upgrade within Core, or pass that off to the Symfony module to handle via the ClassLoader. Both solutions are viable options, and both solutions will most likely still work in tandem as well.
Taking that into consideration, the concerns brought up in the issue summary are mute. Being part of the Symfony community means we're part of the Symfony community. If we run into issues, we'll easily be able to work with them to sort the issues out. We should probably close this issue as it just spreads fear, uncertainty and doubt about adopting proven tools.
Comment #12
Crell CreditAttribution: Crell commentedGiven Symfony's release schedule, I suspect when D8 actually ships we'll be shipping Symfony 2.3 or 2.4. :-) We want to remain able to continue contributing to Symfony for our benefit for as long as possible rather than fixing ourselves to 2.2 LTS, which should be out close to a year before Drupal 8.
I otherwise agree with Rob in #11. Based on the speed with which pull requests we've filed have gotten merged, I'd much rather see our energy put into #1424924: Use Composer for updating Symfony components (without removing Symfony code from repo) and then moving to Composer with a .lock file entirely.
Comment #13
catchLet's do this. I don't see anything problematic at the moment that needs sorting out right now.
Comment #14
cfennell CreditAttribution: cfennell commentedW00t! Dependency management is yet another area where PHP applications have been unusually siloed (if they address it formally at all) as compared with other projects. +1 on collaborating with the Symphony folks to change that pattern.
Comment #15
scor CreditAttribution: scor commentedThe Drupal Security team is in contact with Fabien Potencier in order to formalize how we deal with security issues in Symfony (upstream and downstream). I'll post back here once it's been formalized. Feel free to chime in the more general discussion on gdo about Planning for Security implications of using external libraries.
Comment #16
mallezieDon't know if it's relevant, but i'll mention it here.
Symfony is planning to change release cycle policy. https://groups.google.com/forum/?hl=en_US&fromgroups=#!topic/symfony-dev...
Comment #17
catchYes I think that is relevant!
The really important bits from that post:
- Symfony 2.3 comes out around May next year.
- it will remove bc layers for Symfony 2.0
- it will be an LTS release.
- all future releases after 2.3 will keep bc (with some caveats).
Also directly quoting this section:
,
So for Drupal 8 I think this means:
- Pretty much everything we relay on is in the first group, a couple in the second.
- we need to be careful before adopting components in the last group (I'd be tempted to bump any such issue to Drupal 9).
- switch to 2.3 (or ideally 2.2-dev) asap once it's available so we can ensure we're not relying on any bc layers before they're ripped out (in time for code freeze)
- we should be able to keep up with major 2.x releases from Symfony since they'll only add new features and not break bc, after Drupal 8 is released.
This sounds really, really good to me and I think it removes a tonne of the uncertainty which prompted this ticket being opened in the first place.
Comment #18
effulgentsia CreditAttribution: effulgentsia commentedThe only thing currently in HEAD from the 2nd group is Yaml.
For Symfony, I agree. What about other libraries? See #1787222: [meta] Strategy for updating vendor JS libraries within a major stable version
Comment #19
webchickMarked #1447712: Evaluate Symfony form component as a replacement for Drupal FAPI postponed, gave it a "Drupal 9" tag, in absence of another option.
Comment #20
Crell CreditAttribution: Crell commentedAs I just posted to the Symfony-Dev list, that puts the 2.3 LTS release date slightly after our planned code freeze. My recommendation is to continue to track Symfony master until then, and grant an exception to Symfony itself on code freeze until 2.3. When 2.3.0 ships, we change our dependency to that and lock in on 2.3 for the Drupal 8 cycle.
Even with improved BC commitments for 2.4+, I'd want to be cautious about upgrading core itself to those. Perhaps that's better left to a jquery_update-style module, or just telling people how to tweak their composer file if they want to do that on their site for some reason.
That also means we have until 2.3 to get in any Drupal-improving additions to Symfony we want for Drupal 8. :-)
Comment #21
catchI don't think we can stay on 2.3 for the entire 8.x release cycle, because it simply won't be supported that long:
Unless Drupal 9 and 10 each have strict, 18 month release cycles, we'll go past that.
To me that leaves two options, apart from supporting Symfony 2.3 for security releases for 1-3 years after they drop support for it:
- update every 6 months (albeit cautiously) - we can commit to dev and allow 2 months before cutting the next 8.x tag for example.
- update each LTS release. That means one or at the outside two updates during the entire 8.x lifecycle and we'll have a full year to evaluate the next one before support for the last one is dropped.
Comment #22
Crell CreditAttribution: Crell commentedI am not against tracking Symfony stables post-2.3 if the D8 maintainer is OK with it. :-) Mostly I just wanted to make sure that the 2.3 final-release being scheduled for after our code freeze won't be an issue on our end. If it is, we can work with Symfony to try and mitigate. If we can work with it, then we should give Fabien a thumbs-up (or whatever other feedback would be appropriate as a major client of Symfony).
Comment #23
catchI don't see that being a problem at all, we should try to run dev for the final month of code freeze to iron out anything that'll break due to the bc removal, apart from that it'd be complete madness to release running 2.2 IMO.
Comment #24
webchickYeah, agreed. There are code freezes and then there is common sense.
Comment #25
chx CreditAttribution: chx commentedApril 1 is API freeze, isn't it? That doesn't mean we can't commit something that doesn't break APIs. So yeah, I like this plan.
Comment #26
scor CreditAttribution: scor commentedFabien Potencier posted a proposal for dealing with downstream projects (such as Drupal) at
https://github.com/symfony/symfony-docs/pull/2639/files
Comment #27
David_Rothstein CreditAttribution: David_Rothstein commentedAgree with this question. Symfony is obviously the most important, but there are lots of others. There's a good list of criteria at http://groups.drupal.org/node/250578 but how many of the external libraries that have already been added to Drupal meet those criteria?
Also, for what it's worth I've think we've seen this issue already with jQuery (see http://drupal.org/SA-CORE-2013-001); if we had a formal procedure in place maybe that issue would have been discovered sooner and would have been able to be handled better in conjunction with the corresponding jQuery release (though from the point of view of jQuery that particular one was more of a security hardening fix than an actual security issue). In any case, jQuery is a much much bigger and more mature software project than any of the other libraries that have been added to Drupal 8, so if it can happen there it is quite likely to happen for the others as time goes on, and Drupal could really use a clear way to deal with all of them.
Comment #28
fgmJust to complete the scene, we are also using the Symfony CMF routing component, which is not included in the list above at #17.
Comment #29
chx CreditAttribution: chx commentedWe have been careful and only added the last three out of the six.
Comment #30
Gábor Hojtsy@chx: I don't believe we added the Translation component, here is a quick excerpt from core/.gitignore:
We definitely experimented with using that component instead of coding our own translation handling, but there were too many incompatibilities so we needed the one-off Drupal implementation for Drupal 8. See #1570346: Let Symfony Translation component handle language messages.
Comment #31
catch#2157189: [Policy, no patch] How to handle upstream fixes that don't get committed fast enough is a lot more up to date, marking this as duplicate.
Comment #32
catchAlso untagging.