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

Allow external libraries to inject non-installed Drupal commands #3175

Open
nvaken opened this issue Feb 10, 2017 · 17 comments
Open

Allow external libraries to inject non-installed Drupal commands #3175

nvaken opened this issue Feb 10, 2017 · 17 comments

Comments

@nvaken
Copy link
Contributor

nvaken commented Feb 10, 2017

Currently packages with commands that will be installed in the vendor folder will not be able to inject commands for non-installed Drupal setups. The commands are only injected when Drupal installed.

Reproduction

  1. Install a Drupal project with https://github.com/drupal-composer/drupal-project
  2. Make sure you install a Drupal console command library (eg. https://github.com/hechoendrupal/drupal-console-extend-example)
  3. Note that the commands are not available directly through drupal list
  4. Install Drupal
  5. Run drupal list again and note that now the commands are available.

Also note, that there is no solution for adding commands to non-installed Drupal setups.

Possible solution

As discussed with @jmolivas:

I can think in two options
define services on two diff files
or user tagged services

@jmolivas
Copy link
Member

@nvaken We all know naming things is hard. For this task we need to break one file into two files.

Actually DrupalConsole discover packages and reads a file named console.services.yml we need to read two files now what would be good name for the files?

@jmolivas
Copy link
Member

jmolivas commented Feb 14, 2017

After thinking this a little using tagged services could be good alternative as well.

a) Adding a new tag:

    tags:
      - { name: drupal.command }
      - { name: drupal.bootstrap.uninstall }

The tag name suggested is drupal.bootstrap.uninstall just in case we need to add more bootstrap level validation in the future.

b) Add an attribute on the tag like:

    tags:
      - { name: drupal.command, bootstrap: uninstall }

This means we keep using only one file console.services.yml for registering services and the Composer Plugin => https://github.com/hechoendrupal/drupal-console-extend-plugin is the one that will read and process those tagged services and create two separated cache files.

@jmolivas
Copy link
Member

jmolivas commented Feb 14, 2017

I tried the option b - Add an attribute on the tag and after making some changes on the composer plugin I have this working like this.

drupal-composer-plugin

The only missing piece is to update the DC compiler pass to read each file depending of the bootstrap executed process.

@nvaken
Copy link
Contributor Author

nvaken commented Feb 15, 2017

Sounds a lot better then two files, thumbs up!

@nvaken
Copy link
Contributor Author

nvaken commented Feb 21, 2017

@jmolivas I'm trying to clean my issues up a bit, or at least structure them a bit. Does your above prototype also apply to drupal console native commands? If so, this is related to #3187.

@nvaken
Copy link
Contributor Author

nvaken commented Apr 7, 2017

I've tried this a little while ago, though this did not seem to work. The following is a message I wrote a while back on Gitter, still applies for this issue:

Just changed my command tags in both drupal/console/extend.console.services.yml and my/package/console.services.yml from

      - { name: drupal.command }

to

      - { name: drupal.command, bootstrap: uninstall }

That should suffice, right? Still, when renaming settings.php to make sure DC sees my install as uninstalled, my command does not show up?

@jmolivas
Copy link
Member

jmolivas commented Apr 7, 2017

@nvaken can you remind me why adding commands on external libraries did not work for you?

@nvaken
Copy link
Contributor Author

nvaken commented Apr 14, 2017

Hi @jmolivas! I've got a custom written library, containing three different commands. I would like to make these available when Drupal is not installed. So, I undertake the following steps to try out above:

  1. I adjust my console.services.yml file and rename the tags attribute from:
      - { name: drupal.command }

To:

      - { name: drupal.command, bootstrap: uninstall }
  1. To make sure Drupal Console caches these new tags in it's own vendor/drupal/console/extend.console.services.yml I run:
$ drupal cr all

After this, I check the before mentioned file, but this still seems to contain my old tags (also see issue: #3234):

      - { name: drupal.command }
  1. To workaround this bug, I simply add the tags myself.
  2. To test if my commands now work without installing Drupal, I rename my web/sites/default/settings.php to web/sites/default/_settings.php to make sure my install is "uninstalled".
  3. I run $ drupal list to check if my commands are now available. Which does not seem to be the case.
  4. I even try running them, in case drupal list simply does not list them but I get:
[ERROR] Command "my_command", is not a valid command name.

I'm using DC 1.0.0-RC16

@jmolivas
Copy link
Member

jmolivas commented Apr 14, 2017

@nvaken The rebuild and registration code changes are on master and will be on next release.

@nvaken
Copy link
Contributor Author

nvaken commented Apr 14, 2017

Ah, you mentioned in Gitter that it was already released in RC16, hence the confusion. I'll check it out in next release.

@nvaken
Copy link
Contributor Author

nvaken commented Apr 20, 2017

Okay, I just installed the latest HEAD from master. I now notice that my commands show up in extend.console.uninstall.services.yml, but still, these commands do not show up in drupal list?

Edit: FWIW, the commands are not available in uninstalled state AND not available in installed state. Renaming the extend.console.uninstall.services.yml to extend.console.services.yml makes them available again in installed state.

@jmolivas
Copy link
Member

@nvaken I will be testing this with latest dev-master of drupal/console and drupal/console-core and ping you back.

@nvaken
Copy link
Contributor Author

nvaken commented Apr 21, 2017

@jmolivas I just found out I should also update drupal/console-core. Which I should've realised in the first place. So, I managed to install the core to the latest version by adding the following to my composer.json:

        "drupal/console-core": "dev-master as 1.0.0-rc16",

The bootstrap: uninstall now seems to work but only if I run it directly from vendor/bin/drupal, using the launcher it won't work. Since everyone here is now using the launcher, this would be unusable.

Also, I thought the command would stay available for both installed and installed state. But it seems that they are only available in uninstalled state. Would it be possible to adjust the tags so we can define:

  • Installed state
  • Uninstalled state
  • Installed & uninstalled state

We have a command which "rebuilds" the entire project, which should be available in uninstalled state as well as installed. (since it does not matter to us if the project is installed).

@jmolivas
Copy link
Member

Lets make uninstalled work on both and by default only when installed.

@nvaken
Copy link
Contributor Author

nvaken commented Apr 21, 2017

Sounds like a plan! And any idea why the launcher does not pick up the commands?

@nvaken
Copy link
Contributor Author

nvaken commented Apr 25, 2017

Since this seems to have been released (partly?) can you let me know if with rc17 they work in both states now?

@nvaken
Copy link
Contributor Author

nvaken commented May 4, 2017

Okay, I think I've been able to answer that last question, using the latest RC18 releases, I see only a extend.console.uninstall.services.yml file being created. Also, after running drupal list I only see the command in uninstalled state.

@jmolivas I'd be interested to try and fix this. Do you have any suggestions about how this should work? Do the services have to be listed in both extend.console.services.yml & extend.console.uninstall.services.yml? Or should bootstrap: uninstall services be read from only extend.console.uninstall.services.yml?

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

No branches or pull requests

2 participants