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

CLI-1398: ACLI does not copy the paths defined in installer-paths when {$name} is used #1797

Open
camoa opened this issue Sep 12, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@camoa
Copy link

camoa commented Sep 12, 2024

Describe the bug
When using acli push:artifact to deploy code ACLI by default does not copy the compiled assets of the custom themes.

Acquia provided a solution per this doc: https://docs.acquia.com/acquia-cloud-platform/add-ons/acquia-cli/known-issues#section-artifact-does-not-include-front-end-build-assets

This does not work when the installer path is created like this in composer.json:

      "docroot/themes/custom/{$name}/css": [
        "type:npm-asset"
      ],

The files are not copied.

This type of installer path is important when using several tcustom themes like multisites and ACSF.

To Reproduce
Steps to reproduce the behavior:

  1. Add an installer-path (per the docs) to composer.json as shown in the description.
  2. Add the necessary step (composer script called pre-acli-push-artifact) that compiles the them assets.
  3. commit your changes
  4. test acli push:artifact
  5. Check the messages you will see a message like: "Forcibly coping /themes/custom/css"

Expected behavior
ACLI should follow the standard use of installer paths and when finding {$name} in a path definition, it should create 1 entry per each subfolder or use some sort of * pattern to copy all possible ocurrences of the folders.

Screenshots

> acli push:artifact eecospina.dev --no-push --no-sanitize -vvvv |grep -i forcibly
[debug] Forcibly adding vendor
[debug] Forcibly adding docroot/core
[debug] Forcibly adding docroot/modules/contrib
[debug] Forcibly adding docroot/modules/custom
[debug] Forcibly adding docroot/profiles/contrib
[debug] Forcibly adding docroot/profiles/custom
[debug] Forcibly adding docroot/themes/contrib
[debug] Forcibly adding docroot/themes/custom
[debug] Forcibly adding docroot/themes/custom/dist
[debug] Forcibly adding docroot/themes/custom/css
[debug] Forcibly adding docroot/themes/custom/js
[debug] Forcibly adding docroot/libraries/mmenu
[debug] Forcibly adding docroot/libraries
[debug] Forcibly adding drush/Commands
[debug] Forcibly adding docroot/.csslintrc
[debug] Forcibly adding docroot/.eslintignore
[debug] Forcibly adding docroot/.eslintrc.json
[debug] Forcibly adding docroot/.ht.router.php
[debug] Forcibly adding docroot/.htaccess
[debug] Forcibly adding docroot/example.gitignore
[debug] Forcibly adding docroot/index.php
[debug] Forcibly adding docroot/INSTALL.txt
[debug] Forcibly adding docroot/README.md
[debug] Forcibly adding docroot/robots.txt
[debug] Forcibly adding docroot/update.php
[debug] Forcibly adding docroot/web.config
[debug] Forcibly adding docroot/sites/README.txt
[debug] Forcibly adding docroot/sites/development.services.yml
[debug] Forcibly adding docroot/sites/example.settings.local.php
[debug] Forcibly adding docroot/sites/example.sites.php
[debug] Forcibly adding docroot/sites/default/default.services.yml
[debug] Forcibly adding docroot/sites/default/default.settings.php
[debug] Forcibly adding docroot/modules/README.txt
[debug] Forcibly adding docroot/profiles/README.txt
[debug] Forcibly adding docroot/themes/README.txt
[debug] Forcibly adding docroot/autoload.php

Additional context
In

foreach ($composerJson['extra']['installer-paths'] as $path => $type) {
the {$name is replaced with and empty string.

@camoa camoa added the bug Something isn't working label Sep 12, 2024
@github-actions github-actions bot changed the title ACLI does not copy the paths defined in installer-paths when {$name} is used CLI-1398: ACLI does not copy the paths defined in installer-paths when {$name} is used Sep 12, 2024
@joshirohit100
Copy link
Contributor

joshirohit100 commented Sep 12, 2024

Not sure, If I am missing anything, but I tried on local and couldn;t reproduce.

Steps I follower -

  • Get code from https://github.com/acquia/drupal-recommended-project
  • Added two custom themes - theme_a and theme_b in docroot/themes/custom/
  • For each theme added - *.info.yml and css/theme_a.css and css/theme_b.css
  • Ran the acli push artifact command without push and noticed the artifact generated and can see that has those css directory and files

sorry, pls ignore my ^^ comment. I am able to reproduce the same

@camoa
Copy link
Author

camoa commented Sep 12, 2024

There is more to it.

I was testing and I believe the "Forcibly copy" part is also wrong, or the timing of the pre acli push artifact hook.

  • Added the entries for the theme dist files in composer.json, not using ${name} but the name of the theme.
  • Tested several times: acli push:artifact site.dev --no-push --no-sanitize -vvvv |grep -i forcibly
  • Monitored the artifact folder and noticed a couple things:
  1. The pre-acli-push-artifact runs over the development code not the Acquia repo, this is done before we clone Acquia into the artifact.
  2. The system does some copy, starting with the .gitignore and then commits changes
  3. The command then "Forcibly" adds the files which is not a Copy from the source, it is a git add -f, at this point there is no CSS nor JS, or dist folder in the artifact.

Weirdly I am getting a css folder which contains something unrelated to the theme, no idea why. This was because I used npm-asset as the type for the installer path composer got confused for some reason, use drupal-custom-theme.

@joshirohit100 proposed using a post-install-cmd script in composer, which could potentially work in unison with the installer path, but we come back to the multisite / ACSF issue of having multiple files and also, this forces devs to go through the theme compilation every time composer install runs.

That is not ideal.

@joshirohit100
Copy link
Contributor

Adding this should work and not pre-acli hook

"post-install-cmd": [
            "./scripts/to/compile/buils/assets.sh"
        ]

The pre-acli hook initiates before all command in very early phase (even before execution of push:artifact command logic)

Now later in push artifact command, steps are -

  • clones the source repo
  • removes the vendor directories
  • do composer install
  • Commit and push

The step removes the vendor https://github.com/acquia/cli/blob/main/src/Command/Push/PushArtifactCommand.php#L239 -> this removes all the themes and hence FE assets also lost

When we do later in post composer step, then assets is available while acli commiting the full directory

@camoa
Copy link
Author

camoa commented Sep 12, 2024

@joshirohit100 that is a viable workaround. Still forces the theme compilation on every composer install or any task that runs composer install. If the theme compilation migh need more complex task, and add several possible themes doing this, this can add a wait of several minutes on every composer install. It is workable, but I don't believe it will be nice.

I confirm that rohit workaround works, I still believe we need a native Acquia solution to deploy code to Acquia.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants