Skip to content
This repository was archived by the owner on Mar 5, 2025. It is now read-only.

ACSF support for deploy step #18

Merged
merged 6 commits into from
May 5, 2016
Merged

ACSF support for deploy step #18

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
74b3686
ACSF support for deploy step
damontgomery May 4, 2016
ce42c17
Deploy update to include .gitignore files and to use consistent tags.
damontgomery May 4, 2016
0008284
ACSF Update for composer.
damontgomery May 4, 2016
ef14a17
Update ACSF related settings in settings.php and readme.
damontgomery May 4, 2016
14d9ab8
Update deploy:acsf:init to work without drush.
damontgomery May 4, 2016
d3747c4
ACSF readme note about factory hooks.
damontgomery May 4, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions template/build/core/phing/tasks/deploy.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@

<target name="deploy:artifact:build" description="Generates a deploy-ready build in deploy.dir."
depends="deploy:artifact:clean, deploy:artifact:copy, deploy:artifact:composer:install, deploy:artifact:profile:make, frontend:build, deploy:artifact:sanitize">
<!-- If we are using ACSF, run the ACSF Deploy command. -->
<if>
<equals arg1="${hosting}" arg2="acsf"/>
<then>
<phingcall target="deploy:acsf:init"/>
</then>
</if>
</target>

<target name="deploy:artifact:clean" description="Deletes the contents of the deploy dir.">
Expand Down Expand Up @@ -99,11 +106,13 @@

<!-- Copy required files from docroot. -->
<copy todir="${deploy.dir}/docroot" overwrite="true">
<fileset dir="${docroot}">
<fileset dir="${docroot}" defaultexcludes="false">
Copy link
Contributor

Choose a reason for hiding this comment

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

Which necessary files does setting defaultexcludes="false" cause to be included?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ACSF .gitignore, see https://www.phing.info/docs/guide/trunk/FileSet.html

Included files are

Default excludes are: *~, #*#, .#*, %*%, CVS, CVS/**, .cvsignore, SCCS, SCCS/**, vssver.scc, .svn, .svn/**, ._*, .DS_Store, .darcs, .darcs/**, .git, .git/**, .gitattributes, .gitignore, .gitmodules

The cleanup task already removes .git directories.

Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than disabling all of these default excludes, I think we should simply include .gitignore files. E.g., <include name="**/.gitignore"/>. This seems like a more precise way to accomplish the end goal.

Copy link
Contributor Author

@damontgomery damontgomery May 4, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I tried that and it didn't work for me. The other option is to check for ACSF and explicitly include it's .gitignore.

I tried a general rule and I even tried another copy command just for that folder and included the file name directly with no luck.

I didn't see any reason we should be ignoring all those files and most do not apply.

<!-- This should be similar to .gitigore. -->
<include name="**"></include>
<!-- We remove default excludes because they include .gitignore files we need, like the ACSF one. -->
<include name="**" />
<exclude name="**/local.*" />
<exclude name=".gitkeep" />
<exclude name=".DS_STORE" />
<exclude name="core/**" />
<exclude name="drush/contrib/**" />
<exclude name="example.gitignore" />
Expand All @@ -113,8 +122,8 @@
<exclude name="themes/contrib/**" />
<exclude name="profiles/contrib/**" />
<exclude name="modules/contrib/**" />
<exclude name="**/node_modules/**"></exclude>
<exclude name="**/bower_components/**"></exclude>
<exclude name="**/node_modules/**" />
<exclude name="**/bower_components/**" />
</fileset>
</copy>

Expand Down Expand Up @@ -170,4 +179,9 @@
</fileset>
</delete>
</target>

<target name="deploy:acsf:init" description="Re-initialize ACSF with the settings.php changes required for artifact.">
<chmod file="${deploy.dir}/docroot/sites/default/settings.php" mode="0755" />
<exec dir="${deploy.dir}/docroot" command="${drush.bin} --include=${deploy.dir}/docroot/modules/contrib/acsf/acsf_init acsf-init -y" logoutput="true" checkreturn="true"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Will it cause any problems to run this against both the canonical repo and the deployment artifact? Why is it necessary to do in both places?

Copy link
Contributor

@danepowell danepowell May 4, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

It's a paradigm shift from how we originally supported ACSF. Instead of Bolt supporting ACSF and ACE simultaneously out of this box, this adds a flag that you set to add ACSF compatibility "on-the-fly" to a build artifact.

If we're taking this approach, we also need to update the ACSF readme, rip out the ACSF-specific logic in the existing settings.php, and remove the ACSF patch from composer.json... I'll try to see what else would need to change...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Composer is updated with this PR. I'll look into the readme and settings.php.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the readme and the default settings.php. Thanks for mentioning those.

</target>
</project>
5 changes: 1 addition & 4 deletions template/composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
],
"require": {
"composer/installers": "^1.0.20",
"drush/drush": "dev-master",
"drupal/acquia_connector": "8.1.*",
"drupal/acsf": "~8.1",
"drupal/core": "~8",
Expand All @@ -28,6 +27,7 @@
"behat/mink-goutte-driver": "*",
"behat/mink-selenium2-driver": "*",
"behat/mink-browserkit-driver": "*",
"drush/drush": "dev-master",
Copy link
Contributor

@danepowell danepowell May 5, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I think Moshe has said repeatedly that applications should provide their own version of Drush (instead of using Cloud Drush)... it seems like we're going back on that here. If so, we should discuss whether that's still best practice.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I hadn't considered that. Will running drush @alias status use the drush bin in the remote application's vendor dir?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think until ACE is fixed with 1.91 release, we should leave this here so peoples installs for D8 do not break like they did for us.

Once its released, we can move it back.

Copy link
Contributor

Choose a reason for hiding this comment

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

#34

"drupal/drupal-extension": "~3.0",
"drupal/coder": "~8.2",
"phpunit/phpunit": "4.6.*",
Expand All @@ -54,9 +54,6 @@
"patches": {
"drupal/core": {
"Ignore front end vendor folders to improve directory search performance": "https://www.drupal.org/files/issues/ignore_front_end_vendor-2329453-116.patch"
},
"drupal/acsf": {
"Allow custom settings.php": "https://www.drupal.org/files/issues/acsf-2706365-2.patch"
}
}
},
Expand Down
43 changes: 19 additions & 24 deletions template/composer.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions template/project.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,8 @@ drush:
remote: ${acquia_subname}.test
# The local environment against which all local drush commands are run.
local: self

# Hosting environment flags.
# Examples: acsf (Acquia Cloud Site Factory), ac (Acquia Cloud)
# hosting: "acsf"
# hosting: "ac"