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

GenCode, Cache::cleanKey() - Fix deploop during clean initialization #14777

Merged
merged 1 commit into from
Jul 10, 2019

Conversation

totten
Copy link
Member

@totten totten commented Jul 10, 2019

Overview

Suppose you have a clean codebase with no DAO files, and you try to run xml/GenCode.php.
This currently crashes.

The problem is not symptomatic day-to-day because we commit DAOs, so it's
very rare to run a full/clean initialization. However, it does get in the
way of stress-testing the correctness of GenCode.

These are the steps I've used to try GenCode with a (relatively) clean slate:

rm CRM/*/DAO/*.php -f
git checkout -- CRM/Contact/DAO/Factory.php CRM/Core/DAO/AllCoreTables.php CRM/Core/DAO/Factory.php CRM/Core/DAO/permissions.php
./bin/setup.sh -g

Before

Running setup.sh above crashes:

[bknix-min:~/bknix/build/dmaster/sites/all/modules/civicrm] ./bin/setup.sh -g
+ '[' -n '' ']'
+ '[' -n 1 -a -d /Users/totten/bknix/build/dmaster/sites/all/modules/civicrm/./bin/../xml ']'
+ pushd /Users/totten/bknix/build/dmaster/sites/all/modules/civicrm/./bin/../xml
~/bknix/build/dmaster/sites/all/modules/civicrm/xml ~/bknix/build/dmaster/sites/all/modules/civicrm
+ '[' -z 3307 ']'
+ PHP_MYSQL_HOSTPORT=127.0.0.1:3307
+ php -d mysql.default_host=127.0.0.1:3307 -d mysql.default_user=dmasterciv_mqc44 -d mysql.default_password=NOTREALLYTHIS GenCode.php schema/Schema.xml '' Drupal

civicrm_domain.version := 5.17.alpha1

Parsing schema description schema/Schema.xml
Extracting database information
Extracting table information
PHP Fatal error:  Class 'CRM_Core_DAO_Cache' not found in /Users/totten/bknix/build/dmaster/sites/all/modules/civicrm/CRM/Core/BAO/Cache.php on line 41
PHP Stack trace:
PHP   1. {main}() /Users/totten/bknix/build/dmaster/sites/all/modules/civicrm/xml/GenCode.php:0
PHP   2. CRM_Core_CodeGen_Main->main() /Users/totten/bknix/build/dmaster/sites/all/modules/civicrm/xml/GenCode.php:55
PHP   3. CRM_Core_CodeGen_Main->getTasks() /Users/totten/bknix/build/dmaster/sites/all/modules/civicrm/CRM/Core/CodeGen/Main.php:109
PHP   4. CRM_Core_CodeGen_Schema->__construct() /Users/totten/bknix/build/dmaster/sites/all/modules/civicrm/CRM/Core/CodeGen/Main.php:127
PHP   5. CRM_Core_CodeGen_Schema->findLocales() /Users/totten/bknix/build/dmaster/sites/all/modules/civicrm/CRM/Core/CodeGen/Schema.php:15
PHP   6. CRM_Core_Config::singleton() /Users/totten/bknix/build/dmaster/sites/all/modules/civicrm/CRM/Core/CodeGen/Schema.php:113
PHP   7. Civi\Core\Container::boot() /Users/totten/bknix/build/dmaster/sites/all/modules/civicrm/CRM/Core/Config.php:113
PHP   8. CRM_Utils_Cache::create() /Users/totten/bknix/build/dmaster/sites/all/modules/civicrm/Civi/Core/Container.php:503
PHP   9. spl_autoload_call() /Users/totten/bknix/build/dmaster/sites/all/modules/civicrm/CRM/Utils/Cache.php:186
PHP  10. CRM_Core_ClassLoader->loadClass() /Users/totten/bknix/build/dmaster/sites/all/modules/civicrm/CRM/Utils/Cache.php:186
PHP  11. require_once() /Users/totten/bknix/build/dmaster/sites/all/modules/civicrm/CRM/Core/ClassLoader.php:221

Since 76e697a, CRM_Utils_Cache::create() calls CRM_Core_BAO_Cache::cleanKey(). However, the class CRM_Core_BAO_Cache cannot be loaded because its parent (CRM_Core_DAO_Cache) does not yet exist.

After

  • The utility function CRM_Core_BAO_Cache::cleanKey() is migrated to CRM_Utils_Cache::cleanKey().
  • Thus, CRM_Utils_Cache::create() no longer loads CRM_Core_BAO_Cache or CRM_Core_DAO_Cache. Thus, it works.

Overview
--------

Suppose you have a clean codebase with no DAO files, and you try to run `xml/GenCode.php`.
This currently crashes.

The problem is not symptomatic day-to-day because we commit DAOs, so it's
very rare to run a full/clean initialization.  However, it does get in the
way of stress-testing the correctness of GenCode.

Before
------

Since 76e697a, I believe we've had a dependency-loop
in the clean-gencode scenario, which works as follows:

* We don't have any DAOs, so we run `GenCode`.
* Running `GenCode` does a partial bootstrap (i.e. `CRM_Core_Config::singleton($loadFromDB===FALSE)`)
* The partial bootstrap creates some thread-local caches (`Arraycache`)
* Before creating the cache, it normalizes the name by calling `CRM_Core_BAO_Cache::cleanKey()`
* `CRM_Core_BAO_Cache` extends `CRM_Core_DAO_Cache`
* `CRM_Core_BAO_Cache` doesn't exist - that's why we called `GenCode` at the beginning.

After
-----

This migrates the utility function `cleanKey()` to another class which is always available and
does not rely on DAOs.
@civibot
Copy link

civibot bot commented Jul 10, 2019

(Standard links)

@eileenmcnaughton
Copy link
Contributor

I confirmed this really is just moving a function (by doing it myself). I prefer that we deprecate functions more noisily so in 6-12 months we can remove them but that is non-blocking

Rationale makes sense & there is lots of cover on this

@eileenmcnaughton eileenmcnaughton merged commit 39b26f3 into civicrm:master Jul 10, 2019
@totten totten deleted the master-clean-key branch July 10, 2019 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants