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

Update Drupal 8 demo buildkit build and create a Drupal 9 buildkit bu… #537

Merged
merged 4 commits into from
Aug 13, 2020

Conversation

seamuslee001
Copy link
Contributor

…ild as well for use as demo sites

ping @MikeyMJCO @totten @demeritcowboy

CIVI_DOMAIN_NAME="Demonstrators Anonymous"
CIVI_DOMAIN_EMAIL="\"Demonstrators Anonymous\" <info@example.org>"
CIVI_CORE="${CMS_ROOT}/vendor/civicrm/civicrm-core"
CIVI_UF="Drupal8"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6 => Drupal6
7 => Drupal
8 => Drupal8
9 => Drupal8

I get it but ... yeah, drupal.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeh d8/9 are very similar so no real reason creating a separate UF for it

@demeritcowboy
Copy link
Contributor

I'm noticing drush dl has been deprecated in D9. And I'm wondering should these be using drupal recommended project now, or at least for D9?

Also is exportui part of core now? Does it need the extension?

@homotechsual
Copy link
Contributor

homotechsual commented Aug 11, 2020

Issue applying patch:

537.patch:913: new blank line at EOF.
+
warning: 1 line adds whitespace errors.

This was user-error...

@homotechsual
Copy link
Contributor

I'm noticing drush dl has been deprecated in D9. And I'm wondering should these be using drupal recommended project now, or at least for D9?

Also is exportui part of core now? Does it need the extension?

Yeah, can we go the "proper" way of doing this please instead of trying to recreate D7 methodologies. Composer-first workflows for D8/D9.

@homotechsual
Copy link
Contributor

homotechsual commented Aug 11, 2020

Okay so D8 build works:

Drupal 8

Some feedback - be good to add back in the login block for the demos. I'd like us to switch the default theme on D9 to Olivetti - as it'll be the Default theme for D9 going forward.

Drupal 9 errors on me:

Error: Call to undefined function update_fix_compatibility() in update_main() (line 136 of                   [error]

@seamuslee001
Copy link
Contributor Author

thanks @demeritcowboy @MikeyMJCO I have updated it to be all composer and also have re-added in those blocks @MikeyMJCO can you make sure that you have the latest drush8 there i think try running composer install in your buildkit dir

@demeritcowboy
Copy link
Contributor

Thanks @seamuslee001. Is exportui needed anymore? It's been in core since 5.23.

@homotechsual
Copy link
Contributor

thanks @demeritcowboy @MikeyMJCO I have updated it to be all composer and also have re-added in those blocks @MikeyMJCO can you make sure that you have the latest drush8 there i think try running composer install in your buildkit dir

Issued a PR to update the Drush 8 version in composer.json. Will be required for D9 builds.


drush8 -y make --working-copy "$MAKEFILE" "$WEB_ROOT"
pushd "$WEB_ROOT" >> /dev/null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could use the composer install function from civibuild.lib.sh like the d8prj builds do.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for D9 :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MikeyMJCO is that not what is called in L25 there?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, ignore me :-)

@homotechsual
Copy link
Contributor

D9 builds are still causing me issues:

+++ '[' '!' -d /opt/buildkit/build/d9.demo/vendor/civicrm/civicrm-core/bin -o '!' -d /opt/buildkit/build/d9.demo/vendor/civicrm/civicrm-core/CRM ']'
+++ cvutil_fatal 'Failed to locate valid civi root: /opt/buildkit/build/d9.demo/vendor/civicrm/civicrm-core'
+++ echo 'Failed to locate valid civi root: /opt/buildkit/build/d9.demo/vendor/civicrm/civicrm-core'
Failed to locate valid civi root: /opt/buildkit/build/d9.demo/vendor/civicrm/civicrm-core
+++ exit 90

@seamuslee001
Copy link
Contributor Author

That is really strange
would have thought this would solve it https://github.com/civicrm/civicrm-buildkit/pull/537/files#diff-5250fce057ad705626e39890b8affc6aR25 that is what solves it for me but maybe something is screwy

@homotechsual
Copy link
Contributor

That is really strange
would have thought this would solve it https://github.com/civicrm/civicrm-buildkit/pull/537/files#diff-5250fce057ad705626e39890b8affc6aR25 that is what solves it for me but maybe something is screwy

Lemme grab a fresh copy of that file into my environment and try again - in case the patch went screwy

@seamuslee001
Copy link
Contributor Author

Merging as ok by Mikey

@seamuslee001 seamuslee001 merged commit c78ca22 into civicrm:master Aug 13, 2020
@seamuslee001 seamuslee001 deleted the d89_demo branch August 13, 2020 21:48
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

Successfully merging this pull request may close these issues.

3 participants