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

xml/GenCode.php - Fix execution in default locale (without l10n data) #16583

Merged
merged 1 commit into from
Feb 18, 2020

Commits on Feb 18, 2020

  1. xml/GenCode.php - Fix execution in default locale (without l10n data)

    Overview
    --------
    
    This fixes a recent regression in which `xml/GenCode.php` fails to execute in certain configurations.
    
    Initial report: civicrm/civicrm-buildkit#503
    
    Before
    ------
    
    * If the folder `l10n` exists in its traditional location, and if you run `./bin/setup.sh -g`, then it
      correctly executes.
    * If the folder `l10n` does not exist in its traditional locacation, and if you run `./bin/setup.sh -g`,
      then it fails with an error like this:
      ```
      + php -d mysql.default_host=127.0.0.1:3306 -d mysql.default_user=dmasterciv_abcde -d mysql.default_password=abcd1234 GenCode.php schema/Schema.xml '' Drupal
    
      civicrm_domain.version := 5.23.alpha1
    
      Parsing schema description schema/Schema.xml
      Extracting database information
      Extracting table information
      PHP Fatal error:  Uncaught RuntimeException: Invalid configuration: the [cms.root] path must be an absolute URL in /home/me/.../Civi/Core/Paths.php:174
      Stack trace:
      #0 /home/me/.../Civi/Core/Paths.php(151): Civi\Core\Paths->toAbsoluteUrl('/', 'cms.root')
      #1 /home/me/.../Civi/Core/Paths.php(176): Civi\Core\Paths->getVariable('cms.root', 'url')
      #2 /home/me/.../Civi/Core/Paths.php(151): Civi\Core\Paths->toAbsoluteUrl('/', 'civicrm.root')
      civicrm#3 /home/me/.../Civi/Core/Paths.php(224): Civi\Core\Paths->getVariable('civicrm.root', 'path')
      civicrm#4 /home/me/.../Civi/Core/Paths.php(84): Civi\Core\Paths->getPath('l10n')
      civicrm#5 [internal function]: Civi\Core\Paths->Civi\Core\{closure}()
      civicrm#6 /home/jo in /home/me/.../Civi/Core/Paths.php on line 174
      ```
    
    After
    -----
    
    You can run `./bin/setup.sh -g` with or without the `l10n` folder.
    
    Comments
    --------
    
    There's an argument to be made that this shouldn't be necessary: when running
    `GenCode`, it should only create PHP files, and none of the output should depend
    on the CMS URL - because that's liable to change when you deploy the PHP code.
    If someone did try to generate URL at such an early stage, it's arguably good to
    generate an error.  In point of fact, `GenCode` is not actually using the CMS
    URL.
    
    The oddity stems from the contract of `CRM_Utils_System_*` (specifically,
    `getCiviSourceStorage()` and `getDefaultFileStorage()`) which do double-duty
    providing both path and URL.  To avoid duplicate work, `Civi\Core\Paths` uses
    the same grain of information - it tracks the pairs of path+URL.
    
    A more aggressive fix might be to split `getCiviSourceStorage()` and
    `getDefaultFileStorage()` so that it's possible to get the path and URL
    separately; then revise `Civi\Core\Paths` to take advantage of the finer-grained
    contract.  However, splitting those things would be a more invasive patch,
    and we're currently in RC.
    totten committed Feb 18, 2020
    Configuration menu
    Copy the full SHA
    b0c5bdb View commit details
    Browse the repository at this point in the history