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

Installation - If available, use the new installer package #121

Merged
merged 2 commits into from
Jan 19, 2018

Conversation

totten
Copy link
Member

@totten totten commented Jan 18, 2018

civicrm-core includes an installer (civicrm/install/index.php), but this
code is difficult to update/maintain. The civicrm-setup
(http://github.com/civicrm/civicrm-setup/) package is a "leap" to replace it
with a more maintainable library.

This patch uses whichever is available.

Before

  • The page civicrm-install always uses civicrm/install/index.php.

After

  • The page civicrm-install picks the first available installer UI, either:
    • civicrm/vendor/civicrm/civicrm-setup/* (Civi\Setup::createController())
    • civicrm/packages/civicrm-setup/* (Civi\Setup::createController())
    • civicrm/setup/* (Civi\Setup::createController())
    • civicrm/install/index.php

Comments

`civicrm-core` includes an installer (`civicrm/install/index.php`), but this
code is difficult to update/maintain.  The `civicrm-setup`
(http://github.com/civicrm/civicrm-setup/) package is a "leap" to replace it
with a more maintable library.

This patch uses whichever is available.

Before
------
 * The page `civicrm-install` always uses `civicrm/install/index.php`.

After
-----
 * The page `civicrm-install` picks the first available installer UI, either:
   * `civicrm/packages/civicrm-setup/*` (`Civi\Setup::createController()`)
   * `civicrm/install/index.php`
@kcristiano
Copy link
Member

@totten This looks great. Just tested and it works as described.

WP 4.9.2 CiviCRM master. Added patch and civicrm-setup to civicrm/packages

new installer is run, able to edit the DB credentials. Upon entering incorrect credentials I get a failure notice. Once corrected the installer is ready and installation proceeds and succeeds.

Is the plan to include civicrm-setup when we package the tarballs?

@totten
Copy link
Member Author

totten commented Jan 19, 2018

@kcristiano Exactly. We'll probably make two builds for WP -- the current one, and a new one with the updated installer.

@kcristiano
Copy link
Member

@totten I would love to see this in early in the RC cycle so we can get be testing as much as possible

At the moment, we don't bundle `civicrm-setup` directly with all tarballs, and
it would be easy for an admin to mistakenly use an incompatible build. This
generates an error if the library is mismatched.
@totten
Copy link
Member Author

totten commented Jan 19, 2018

Thanks @kcristiano. Agreed, I'd like to get this in soon, and I don't think there's much downside since (by default) it'll fallback to existing behavior.

@totten totten merged commit 4fd42de into civicrm:master Jan 19, 2018
@totten totten deleted the master-setup branch January 19, 2018 23:37
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.

3 participants