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

Recent versions of buildkit won't reinstall old sites #503

Closed
MegaphoneJon opened this issue Feb 5, 2020 · 3 comments
Closed

Recent versions of buildkit won't reinstall old sites #503

MegaphoneJon opened this issue Feb 5, 2020 · 3 comments

Comments

@MegaphoneJon
Copy link
Contributor

I just ran civibuild reinstall and it failed with missing variable: CMS_VERSION [in cvutil_inject_settings]. That seems to tied to the commits from 3 weeks ago.

I looked in <civicrm-buildkit-root>/build/dmaster.sh and saw CMS_VERSION="" which I changed to CMS_VERSION="latest" but that yielded this error:

+ php -d mysql.default_host=127.0.0.1:3306 -d mysql.default_user=dmasterciv_bkv7o -d mysql.default_password=CYlnSyGXHxLMPa4k 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/jon/local/civicrm-buildkit/build/dmaster/sites/all/modules/civicrm/Civi/Core/Paths.php:174
Stack trace:
#0 /home/jon/local/civicrm-buildkit/build/dmaster/sites/all/modules/civicrm/Civi/Core/Paths.php(151): Civi\Core\Paths->toAbsoluteUrl('/', 'cms.root')
#1 /home/jon/local/civicrm-buildkit/build/dmaster/sites/all/modules/civicrm/Civi/Core/Paths.php(176): Civi\Core\Paths->getVariable('cms.root', 'url')
#2 /home/jon/local/civicrm-buildkit/build/dmaster/sites/all/modules/civicrm/Civi/Core/Paths.php(151): Civi\Core\Paths->toAbsoluteUrl('/', 'civicrm.root')
#3 /home/jon/local/civicrm-buildkit/build/dmaster/sites/all/modules/civicrm/Civi/Core/Paths.php(224): Civi\Core\Paths->getVariable('civicrm.root', 'path')
#4 /home/jon/local/civicrm-buildkit/build/dmaster/sites/all/modules/civicrm/Civi/Core/Paths.php(84): Civi\Core\Paths->getPath('l10n')
#5 [internal function]: Civi\Core\Paths->Civi\Core\{closure}()
#6 /home/jo in /home/jon/local/civicrm-buildkit/build/dmaster/sites/all/modules/civicrm/Civi/Core/Paths.php on line 174

Running civibuild destroy and recreating the site worked.

@seamuslee001
Copy link
Contributor

@totten thoughts?

totten added a commit to totten/civicrm-buildkit that referenced this issue Feb 18, 2020
The CMS_VERSION was added to the list of required inputs so that it could be
passed through to `$civibuild['CMS_VERSION']` and then eventually to
`app/drupal.settings.d/pre.d/100-file_private_path-d8.php`.

Evidentally (from civicrm#503) some old builds did not record a `CMS_VERSION`, so
the assertion raises a fatal.  I'm not certain why - but regardless, the
situation should not be a fatal for most build types (since the value
normally isn't even used).
@totten
Copy link
Member

totten commented Feb 18, 2020

It's odd that the CMS_VERSION would be missing/blank, but I suppose the condition ordinarily ought not be fatal, so I've posted a PR to relax it (#507). FWIW, in my dmaster.sh file, the value is CMS_VERSION="7.x".

The other error... I don't think that's related to CMS_VERSION (merely obscured by it). I was able to reproduce the error -- remove l10n folder and ru ./bin/setup.sh -g. Since the current practice in most app/config/*/download.sh scripts is to download l10n, we don't see the problem in CI . But it's always been valid to run GenCode without l10n data, so let me take a gander to see if there's an obvious resolution...

totten added a commit to totten/civicrm-core that referenced this issue Feb 18, 2020
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
Copy link
Member

totten commented Feb 18, 2020

@seamuslee001 @MegaphoneJon - civicrm/civicrm-core#16583 should fix the recent regression in 5.23 when executing without an l10n folder.

@totten totten closed this as completed Feb 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants