Skip to content
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

DATE_RFC7231 already defined in PHP 7.0.19 and 7.1.5 #3169

Closed
klonos opened this issue Jun 15, 2018 · 5 comments
Closed

DATE_RFC7231 already defined in PHP 7.0.19 and 7.1.5 #3169

klonos opened this issue Jun 15, 2018 · 5 comments

Comments

@klonos
Copy link
Member

klonos commented Jun 15, 2018

This is the respective issue for https://www.drupal.org/project/drupal/issues/2877243. From that ticket:

The issue observed on Windows, IIS 10, and PHP 7.

The latest version of PHP, 7.0.19, fixes this bug in PHP and defines the constant DATE_RFC7231 in PHP itself.

As a result, Bootstrap now generates a notice:

Constant DATE_RFC7231 already defined in C:\vhosts\my-site\includes\bootstrap.inc on line 258

In my environment, a PHP notice at that stage of Bootstrap completely broke the site. I could revert the PHP version to the previous one, but there was no workaround in user interface.

This was at the top of the list of issues to port in #2776, but was marked as N/A for Backdrop(?). I know that we did not have any reports of this error in the queue, but the original issue description mentions Windows and IIS, so perhaps none of the Backdrop users work on Windows and that explains why.

Anyway, this seems like a small change, and over in the d.org issue they mention that there were no breakages.

Also from the d.org issue:

This seems like a straightforward enough quick fix, regardless of the priority level. Strictly speaking we could make it a const, instead of a define(), but the difference is marginal so... RTBC.

...

We cannot make it a const as that syntax (outside of class context) is only supported since PHP 5.3.0 (see http://php.net/manual/en/language.constants.syntax.php) and Drupal 7 officially still supports PHP 5.2.5+ (see https://www.drupal.org/docs/7/system-requirements/php#php_required).

Noting that in backdrop we require PHP 5.3.2+, so not sure if this should be a CONSTANT for us.


PR by @klonos: backdrop/backdrop#2225

@klonos
Copy link
Member Author

klonos commented Jun 15, 2018

@serundeputy, @quicksketch ...as I said, not sure if this indeed does not apply to us, but the code change seems "harmless" 😄 ...done my bit, so now deferring it to your expertise.

@klonos
Copy link
Member Author

klonos commented Jun 15, 2018

PS: you might wanna check my shameless plug over in the d.org issue.

I don't think D7 is still being taken care of... sadly, so, better add this patch to your makefiles ;-)

...

...better yet, lets fork D7 and let's take this off of drupal.org, make a new update module that pulls from Github instead.

...

I don't think this is a good idea... even if I would prefer to work on Github :-)

"Forking" Drupal 7 would makes things even more messier.

I just hope one day some people will just take care of D7 for the millions of sites yet running it.

...sorry, but I couldn't help it after having read the comments 😈

@quicksketch
Copy link
Member

We don't have this line in Backdrop because we removed this constant as part of the removal of aggregator.module from core. We could add this line back in for the purposes of contrib work and compatibility.

@klonos
Copy link
Member Author

klonos commented Jun 15, 2018

We could add this line back in for the purposes of contrib work and compatibility.

👍 ...otherwise, please feel free to close this ticket and the respective PR.

@quicksketch
Copy link
Member

It won't hurt I suppose, and could save someone a headache later, especially if porting a module from Drupal, so let's pull it in. Merged in backdrop/backdrop#2225.

I added a @since to the docblock for clarity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants