bin/*, extern/* - Fix leak of "$config" in global namespace

[seamuslee001]

…th backdrop

Overview

This removes the $config variable from some pre CMS boot locations to avoid issues with backdrop comparability

Before

When using a bin/ or extern/ script, the $config variable implicitly enters the global namespace.

Rest API tests fail on current Backdrop 1.x builds because Backdrop specifies its own global $config of type array.

After

Rest API tests pass again on backdrop

Technical Details

Backdrop recently made $config a global variable and its probable that our use of $config in these files whilst not declared global maybe caused to populate when backdrop is booted

ping @totten @herbdool

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[herbdool]

What's the best way to test this? I assume it has to be merged before it can be tested with automated tests with Backdrop. Seems like it needs to be tested first locally.

[seamuslee001]

@herbdool the simplest way to test this would be to check it out locally then from within the main CiviCRM folder run `./tools/scripts/phpunit 'E2E_Extern_RestTest' and see that the test Rest examples work. The other thing you could try is just deploying it locally and creating a test mailing or something with some images and links and check the tracked opens and etc urls work correctly

[totten]

Overall 👍 . Adding some supporting detail:

r-tech: So I think the main concern (which Seamus had initially raised in chat) would be that other code uses$config. Or, phrased more formally... that some code - in core or downstream - relies on this precondition:

$GLOBALS['config '] instanceof CRM_Core_Config

While it's hard to perfectly rule out the existence of such code, I'm not too concerned about it. Why? Two reasons:

1. Because such code would already be broken in normal processing. That pre-condition is manifestly not met during normal page processing. You can see this by dropping the following line into random places:

   echo '<hr><pre>'; var_dump(['global config'=>$GLOBALS['config']]);exit();

2. Because it goes against the long-standing idiom. The idiom is for functions to access the CRM_Core_Config by copying the singleton to a local variable:

   function fooBar() {
     $config = CRM_Core_Config::singleton();
   }

I also think this patch is doing the Right Thing because it still calls CRM_Core_Config::singleton(). Regardless of whether I like or dislike the bootstrap protocol on general grounds, calling CRM_Core_Config::singleton() is an important step in the protocol used by these files, so it's good that it's still there.

[seamuslee001]

@totten @herbdool has anyone done a r-run on this? E2E tests would cover the Rest and Soap Interfaces i think

[totten]

The E2E suite provides coverage of some extern use-cases. That passed when CI ran on D7, and I did a couple local runs of E2E on wp-demo and bcmaster builds, and those also passed (and demonstrated the improvement in bcmaster).