Deploy PEAR Log package and upgrade to latest version in the process

[seamuslee001]

Overview

This aims to upgrade our version of the PEAR Log package and deploy it via composer now

Before

Old version of Pear Log package used and deployed using civicrm-packages

After

Latest version deployed and deployed via compsoer

Technical Details

this is a diff between the two package installs https://gist.github.com/seamuslee001/4cae97df85c25c32264f4f5c53929e49

ping @totten @eileenmcnaughton @mfb

[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).

[mfb]

Seems to work fine in my quick testing (I also removed it from packages directory).

[mfb]

...tbh I don't know what PEAR Log does, but I was assuming it creates the ConfigAndLog .log files - if there's more I should test out I can.

[seamuslee001]

I'm pretty certain that is what it does, @mfb the only other thing i would consider would be to deliberately cause a DB error somehow and just check that the backtrace it creates in the logfile is still sensible

[totten]

A few things:

• Did a diff between civicrm-core/packages and some older versions of Log to see what bits were unique to us. It appears that packages is using a variant of Log-1.11.5 with PHP5/7 syntax fixes (for constructors, references, and missing statics) - but the newer upstream release appears to handle these things.
• Skimmed the changelog since Log-1.11 -- nothing stood out as problematic
• Checked out the patch locally. Removed the copy of packages/Log.
• Used cv to run a small buggy script (CRM_Core_DAO::executeQuery('SELECT * from alsdkjf');) and confirmed that the same output goes to the same log with old+new versions.

Suggested follow-ups:

• Remove the old version from civicrm-packages
• Remove the old references to require_once 'Log.php'; -- the composer variant should have a decent autoloader.

[seamuslee001]

@totten have put a commit in this now to remove the require_once statements and created civicrm/civicrm-packages#244 to remove it from the packages repo

[totten]

Cool. Did another small r-run per above, and it worked.

There's a test failure, which seems related. I think it's because the test build has the copies of Log.php -- and would clear when we merge both. But I'm play a little at reproducing...

[totten]

Haven't been able to reproduce the failure locally. Forced-push an update to try again without the last commit.

[eileenmcnaughton]

merging based on comments above

[colemanw]

@seamuslee001 I'm now getting failures when running API4 tests that PEAR_LOG_DEBUG constant is not defined in CRM_Core_Error_Log.
That's a global constant that is defined in vendor/pear/log/Log.php but I don't see how that file ever gets included. Trying to access a global constant wouldn't trigger it to autoload. I'm not sure though how the old version got included from packages either.

[MegaphoneJon]

What @colemanw is describing is happenong on master right now regardless of whether API4 is installed.

[seamuslee001]

hmm the only require log was in here https://github.com/civicrm/civicrm-core/pull/13842/files#diff-7f45d6d4f863da6e4b3d020b3c5b3927L40 that seems to make sense

[mfb]

Yes the require_once 'Log.php' is required - and should still work because PEAR composer adds include path. I found out when removing other PEAR stuff previously that the requires are there for a reason :)

[seamuslee001]

Why don't they put it into the composer autoload gahh

[mfb]

There is autoload. but the pear stuff is legacy and uses include/require for constants and such.

[mfb]

In general CiviCRM shouldn't use PEAR anymore.. because it's ancient code that doesn't use namespaces etc. but, baby steps...