-
Notifications
You must be signed in to change notification settings - Fork 395
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BLT needs to support our minimum php version requirement (5.6) #2661
Comments
Just curious, what is preventing an upgrade to PHP 7?
PHP 5.6 was only supported in beta versions of BLT 9.0.x, which is why a change to requiring 7 was permitted in the 9.0.x branch.
This change was discussed internally, though not with all product stakeholders. In any case, it seems permissible to introduce a major change like this even without unilateral support for two reasons:
Depending on customer demand or requirements, I'd be happy to extend the LTS coverage of 8.9.x to a later date and ensure that the BLT 8.9.x branch is compatible with Drupal 8.6 when it is released. |
We're prevented from upgrading to PHP7 because the minimum requirements that we've (Acquia Products, Drupal Core) agreed to is PHP 5.6. While Acquia hasn't announced an EOL for 5.6, I'm guessing it'll be around PHP's EOL, which is the end of 2018. Drupal Core will EOL 5.6 with Drupal 8.7 Unfortunately, until that time, we have customers that are on 5.6 because of platform (especially ACSF) or other requirements, and because we officially support 5.6 our products, including BLT, should support 5.6 as well. Regarding 8.9.x, there is little reason why we should be supporting 8.9.x for another year just so we can get away with using some php7 constructs early. The issues raised with needing 5.6 has already been addressed in the PR. It'd be nice if customers who need to use 5.6 can still get the other goodies from 9.x. On a more practical level, the contenthub team spent a sprint upgrading to 9.x BLT, which we use as our testing platform, and to undo this work would be time consuming and unnecessary. |
So to pile on here... The justification for moving to 7 internally to BLT (when I looked at the PR/commit log) was that w/o doing so, it was possible to introduce dependency calculation issues within your code base. Respectfully, that's not BLT's problem. Even if moving to PHP 7 makes that less likely, enforcing that requirement seems non-essential to the project and to teams/products using it. Furthermore, while I'm not certain what Acquia's minimum PHP version is (I'm told it's 5.6) but I can guarantee that core's is still 5.5, and will continue to be for 1 more year. I like working in PHP 7+ too... I'm just not sure that moving to PHP 7 is something BLT should be doing just yet. Even with Drupal core, we're giving people a LOT of notice of this change, and I think BLT should emulate core in this regard. If you want to make this sort of change, that's fine, but a lot of us are depending on it at this point, and while we can be cut out of the decision making process, you should message it more clearly, and give an appropriate amount of time for us to respond. Changing mid-tag release seems completely out of the blue for people building testing infrastructure on the platform, and since products are using BLT to have a consistent testing environment, that means when a team like ContentHub converts to the newest new BLT w/ no visibility around this, we're going to have issues. Our products still support customers on Acquia's minim PHP version. I feel really strongly the platform we're all using together needs to support those products. |
In counterpoint, the move from BLT 8.x to 9.x is an opportunity to focus on removing cruft (old version of PHP) that has a known end of life stated for the end of this year. Upgrading to BLT 9.x is not mandatory for significantly older software and keeping 8.x around to maintain that compatibility improves the supportability and agility of the 9.x code base. With so many moving parts involved in D8 building sites on a more maintainable automation layer as provided by BLT in this instance separates the concerns of backwards compatibility. If 8.x were to be EOL'ed prior to PHP 5.6 being EOL'ed that would be more problematic but in this instance that also is not the case. As it stands there is sufficient difficulty in navigating newer versions of PHP that forcing an as of yet unproven need for backwards compatibility with PHP 5.6. I would suggest that instead the EOL for BLT 8.x be extended to correspond to the EOL for Drupal support of PHP 5.6. I'd also note that while Drupal supports but does not recommend PHP 5.5 BLT does not support PHP 5.5. |
PHP 7's EOL is before PHP 5.6's, and while I recognize that BLT is actually dependent on 7.1 at this point, keep in mind that the actual changes in the patch referenced by this issue have absolutely nothing to do with "cruft" and are in fact just json/yml level enforcement of a policy. I'm not certain how the trait usage in the patch applies in the grander scheme, but there's only one php file touched here. Drupal 8's life extends past PHP 5.5. (i.e. mid 2016 vs now) http://php.net/supported-versions.php yet we still officially support it. BLT choosing NOT to support 5.5 because Acquia doesn't support it seems perfectly kosher, but Acquia DOES continue to support 5.6, and the EOL on our arch for that is not imminent. My main point in all of this is that this sort of change requires public messaging and a time frame associated with that which is reasonable to allow those who depend on the tool to get their ducks in order. Neither of those things happened in this case and that is problematic at the bare-minimum for the ContentHub team. TBD whether it affects anyone else, but it certainly could have which is the risk associated with these sorts of changes. I'm not trying to antagonize here, but I think the simple fact is that maintaining BLT in this way going forward will definitely bite other projects, and as more projects depend on it, that's going to cause some serious friction. Better to adopt a policy around this sooner rather than later. Preferably one that gives teams dependent on it notice of upcoming changes that affect their entire build environment and what customers they can serve. |
I'm willing to introduce 5.6 support into BLT 9.x if it's necessary. But I feel like we're having separate conversations in this thread and I'd like to understand the need on a more concrete level before making the change. I understand the argument that BLT should support the same versions of PHP that Acquia supports. That makes sense to me. At the moment, BLT does support all the versions of PHP supported by Acquia. BLT 8.9.x supports PHP 5.6, BLT 9.x requires 7.1.x. The update to BLT 9.x is not required or urgent. Both BLT 8.9.x and BLT 9.x are compatible with Drupal 8.5.x. We can extend the LTS coverage for BLT 8.9.x it until Acquia drops PHP 5.6 support. I'm very willing to do that, it shouldn't be difficult.
People can take all the time they'd like to get their ducks in order. BLT released a new major version that is optional. Is there some pressure to update that is putting people in a bad position? If so, maybe we can address that? Why is it insufficient for BLT 8.9.x to support PHP 5.6? |
There are three major reasons:
|
Yeah so, as I said earlier, the change to PHP 7.1 was not messaged for BLT 9.x at all. The CH team pushed to get CH working with BLT 9.x, and then in a minor pre-release of BLT, PHP 5.6 support was dropped for non-technical reasons. Had the intentions been clearly messaged, I'm sure the CH team would have pushed to make sure everything worked on BLT 8.9 with D8.5 support instead. My point about "duck ordering" is 100% related to this because we can't very well determine what direction we're going to go if there's not a very clear and obvious roadmap for support of things like PHP version with an associated time table. If that were the case, we'd be debating the merits of dropping 5.6 support in BLT 9 instead of saying "Whoa whoa, you didn't tell us this was going to happen." which is what we're essentially saying here. |
The PR was merged and will be released in 9.1.0. It is currently available in the 9.x branch. |
My system information:
When I run this command:
I get the following output:
And I expected this to happen:
Drupal 8.5 and Acquia hosting still supports php5.6 (and not all environments are upgraded to php 7 yet), which means that I'm stuck on 9.0.0-beta3 until Drupal 8.7, see this comment for official 5.5/5.6 support:
https://www.drupal.org/project/drupal/issues/2842431#comment-12379484
Since the 9.0.x branch did support php 5.6, it probably shouldn't be removed until 9.1. Otherwise, Acquia's products which use BLT to test their modules won't be able to update BLT until Drupal 8.7. Acquia's customers who use 5.6 would then run into this issue in a few months when 8.9 is EOL.
Lastly, the commit didn't appear to have any comments around it: #2601
Changing the underlying php version minimum requirement is not a small task. Before making these changes in the future, it should be vetted with product stakeholders to validate it follows larger product roadmap version support plans.
The text was updated successfully, but these errors were encountered: