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

setup.sh - Fixes for running in basic composer file-structure #16408

Merged
merged 3 commits into from
Jan 29, 2020

Conversation

totten
Copy link
Member

@totten totten commented Jan 28, 2020

Overview

Suppose you make a build of the form:

composer init
composer require civicrm/civicrm-core civicrm/civicrm-packages --prefer-source

which gives you folders

./vendor/civicrm/civicrm-core
./vendor/civicrm/civicrm-packages

If you do development on this build, you may need to run bin/setup.sh (aka xml/GenCode.php) periodically to update the DAO files.

Before

bin/setup.sh raises multiple errors.

After

bin/setup.sh is able to run (with caveats).

Comments

The example steps in the "Overview" are just a communication aide to describe the file-structure/context - they're not actual instructions.

This PR is an extracted subset of #16328, which was an exploratory branch aimed at supporting deployment of Civi git repos in D8 via composer. The changes are extracted to make the size of the review more manageable. It's probably best to use this smaller PR to consider topics like regression-risk and general code convention rather than trying to assess the fuller aims.

@civibot
Copy link

civibot bot commented Jan 28, 2020

(Standard links)

@civibot civibot bot added the master label Jan 28, 2020
@@ -4,7 +4,8 @@
die("GenCode can only be run from command line.");
}

ini_set('include_path', '.' . PATH_SEPARATOR . '..' . DIRECTORY_SEPARATOR . 'packages' . PATH_SEPARATOR . '..');
$includes = ['.', '../packages', '../../civicrm-packages', '..'];
ini_set('include_path', implode(PATH_SEPARATOR, $includes));
Copy link
Member Author

@totten totten Jan 28, 2020

Choose a reason for hiding this comment

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

The original mix of PATH_SEPARATOR and DIRECTORY_SEPARATOR and dots might make your eyes glaze over. (At least, it made my eyes glaze over...) Trying to summarize this bit, it:

  • Switches to array notation to make it is easier to read
  • Retains all the existing items in the search-list (., ../packages, ..)
  • Adds a new item to the search-list (../../civicrm-packages)

@seamuslee001
Copy link
Contributor

Test fail unerlated and this looks fine to me merging

@seamuslee001 seamuslee001 merged commit fa607ba into civicrm:master Jan 29, 2020
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.

2 participants