Problem/Motivation

We want to deprecate drupal_static() and drupal_static_reset(): #1577902: [META] Remove all usages of drupal_static() & drupal_static_reset()

However, for backwards compatibility, we have to turn drupal_static_reset() into a special case that does not call @trigger_error().

This is also used in some cases where it can be called many times in a request, we should only trigger once.

Proposed resolution

Formally deprecate drupal_static_reset() and drupal_static().
Add a special name for skipping deprecations.
Add a drupal memory cache to call only once per request

Remaining tasks

Close and apply credit here #2260187: [PP-1] Deprecate drupal_static() and drupal_static_reset()
Review

User interface changes

N/A

API changes

Data model changes

N/A

Release notes snippet

Issue fork drupal-3037054

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Mile23 created an issue. See original summary.

andypost’s picture

andypost
he/him
Primary language Russian
commented

I think it needs better CR about how to prevent using static cos it brings state to services

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented

\Drupal\migrate\MigrateExecutable::attemptMemoryReclaim() needs this call too.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

> \Drupal\migrate\MigrateExecutable::attemptMemoryReclaim() needs this call too.

I'd argue it's fine to drop that without replacement as the chances that there's any memory worth reclaiming is lower and lower.

I'd argue we could just close this and use the approach that you proposed in the main issue for the few calls like this that must still call it.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

nicxvan’s picture

nicxvan commented

I think this is actually not as hard as it initially seems.

Once we deprecate all other instances in core we introduce a special key clear_all.

Then we do this:

if ($name === 'clear_all') {
  drupal_static(NULL, NULL, TRUE);
}
else {
  trigger_error.....
  drupal_static($name, NULL, TRUE);
}

Then the two remaining calls we call with clear_all as the argument and won't trigger the deprecation and all other calls will.

nicxvan’s picture

nicxvan commented
Title: Deprecate drupal_static_reset() without @trigger_error() » Deprecate drupal_static_reset() and drupal_static()
Issue summary: View changes

nicxvan opened merge request !17219

nicxvan’s picture

nicxvan commented
Status: Active » Needs review

Curious if we think we can get this in for 11.5

daffie’s picture

daffie commented
Status: Needs review » Needs work
daffie’s picture

daffie commented
Status: Needs work » Reviewed & tested by the community

All code changes look good to me.
For me it is RTBC.

catch’s picture

catch
he/him
Primary language English
commented
Version: main » 11.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

I think using a memory cache for the handful of stragglers is a good approach so we can start notifying contrib of drupal_static()'s impending demise earlier. Good to be able to commit this one seven years (!) after it was opened.

Committed/pushed to main, thanks!

Will need a backport MR for 11.x

nicxvan opened merge request !17224

nicxvan’s picture

nicxvan commented
Status: Patch (to be ported) » Reviewed & tested by the community

Ok this is ready now.

There were four additional uses in 11.x I had to mark as ignore.

2 in history which was removed from core.
2 in common for functions that were deprecated and removed already.

Failure seems random it's: EntityReferenceWidgetTest

  • catch committed 903f52dd on main 
    task: #3037054 Deprecate drupal_static_reset() and drupal_static()

By:...

  • catch committed 46ecc11e on 11.x 
    task: #3037054 Deprecate drupal_static_reset() and drupal_static()

By:...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Cherry-picked to 12.0.x because the branch was opened in between the main commit and the push actually succeeding, and backported to 11.x, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • catch committed 0bd134c1 on 12.0.x 
    task: #3037054 Deprecate drupal_static_reset() and drupal_static()

By:...
claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented

Wow!

feuerwagen’s picture

feuerwagen
he/him
Primary language German
Location Bonn 🇪🇺
commented

Thanks for your work especially @claudiu.cristea - I've been watching from the sidelines how you chipped away at this for (8+!) years. Love to see that it finally reached its goal.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.