Improve handling of ddev exec, ddev composer and exec hooks by adding --raw, fixes #2547

[rfay]

The Problem/Issue/Bug:

People have had an enormous number of problems with complex ddev exec and ddev composer commands, especially with quotes.

• 'ddev composer', ddev exec, and shell commands like ddev drush do not support quotes, breaking lots of things #2547

There are lots of examples in that issue, and loads of them will need to be tested.

How this PR Solves The Problem:

• Add ddev exec --raw, which will make the exec not use bash to interpret the command. See ddev exec -h. `ddev exec --raw -- ls -lR
• Always use raw execution for ddev composer or ddev composer create.
• Use raw for shell commands like ddev drush
• Add exec_raw to exec hooks:

  - exec: 
    exec_raw: [ls, -lR, /var/www/html]

Manual Testing Instructions:

You can use a number of ways to test this PR. Current build is at https://github.com/drud/ddev/actions/runs/1844768190 Probably easiest to use gitpod

We'll need to add lots of examples to this from #2547

• Traditional things that worked with ddev exec, including bash execution.
  • ddev exec 'set | grep DDEV'
• Exec hooks with and without exec_raw
• Test each provided shell command
  • ddev mysql -e "SHOW TABLES;"
  • ddev psql -c '\l' and ddev psql -c 'SELECT * FROM users;'
  • ddev php -r 'echo "hi";'
  • ddev magento
  • ddev typo3
  • ddev typo3cms
• Custom command examples
  • @rpkoller's example in 'ddev composer', ddev exec, and shell commands like ddev drush do not support quotes, breaking lots of things #2547 (comment) seems to work now, ddev drush role-add-perm authenticated 'administer blocks', for example.
  • @iamsophiesk's example from 'ddev composer', ddev exec, and shell commands like ddev drush do not support quotes, breaking lots of things #2547 (comment) ddev drush sqlq "DELETE FROM key_value WHERE collection = 'update_fetch_task';" seems to work fine.
• Lots of ddev composer examples.
  • The OP from 'ddev composer', ddev exec, and shell commands like ddev drush do not support quotes, breaking lots of things #2547, ddev composer require "drupal/core:9.0.0 as 8.9.0" --no-update
  • @shaal 's example from 'ddev composer', ddev exec, and shell commands like ddev drush do not support quotes, breaking lots of things #2547 (comment) ddev composer config repositories.feeds '{"type": "path", "url": "repos/feeds", "options": {"symlink": true}}' now works as is, out of the box, no trouble.

Automated Testing Overview:

• Still needs tests for several cases of raw.
• Add tests for ddev drush and ddev mysql with complex uses

Related Issue Link(s):

• 'ddev composer', ddev exec, and shell commands like ddev drush do not support quotes, breaking lots of things #2547

Release/Deployment notes:

• Users with exotic workarounds to the previous bash-interpreted strings for composer and exec commands will have to simplify them.
• Alpha testers will have a bad ~/.ddev/commands/web/php that should be removed (and will automatically be replaced)
• Remind people that customized script commands like drush or php won't be replaced by ddev, so you may want to rm them
• Remind people that ddev no longer pesters about overridden commands (when project has a drush command for example)

[shaal]

I tested it with a composer config I use in DrupalPod
Testing this PR, when I use the previous hack -
ddev composer config repositories.drupal-core1 ' '"'"' {"type": "path", "url": "'"repos/$DP_PROJECT_NAME"'", "options": {"symlink": true}} '"'"' '
I am getting this error (which is expected)

  [Seld\JsonLint\ParsingException]
  "" does not contain valid JSON
  Parse error on line 1:
  ' {"type": "path", "
  ^
  Invalid string, it appears you used single quotes instead of double quotes

But when I run the same line without the quotes hack, it works as it should
ddev composer config repositories.drupal-core1 '{"type": "path", "url": "repos/$DP_PROJECT_NAME", "options": {"symlink": true}}'

[rfay]

Thanks for that test. I think it would be worth it to people to be able to use ddev composer config repositories.drupal-core1 '{"type": "path", "url": "repos/$DP_PROJECT_NAME", "options": {"symlink": true}}' instead of the other thing.

However, note that the resolution of $DP_PROJECT_NAME has to happen on the host side, and I don't know how that can happen in your current situation where the full '{"type": "path", "url": "repos/$DP_PROJECT_NAME", "options": {"symlink": true}}'` is enclosed in single quotes.

[rfay]

Assuming that DP_PROJECT_NAME is defined on the host (what's a decent default value?) then this may work:

ddev composer config repositories.drupal-core1 "{'type': 'path', 'url': 'repos/$DP_PROJECT_NAME', 'options': {'symlink': true}}"

That lets bash interpolate the $DP_PROJECT_NAME.

[shaal]

@rfay when I run this command
ddev composer config repositories.drupal-core1 "{'type': 'path', 'url': 'repos/$DP_PROJECT_NAME', 'options': {'symlink': true}}"

I get this error

[Seld\JsonLint\ParsingException]                                            
  "" does not contain valid JSON                                              
  Parse error on line 1:                                                      
  {'type': 'path', 'url                                                       
  ^                                                                           
  Invalid string, it appears you used single quotes instead of double quotes

I tested it on Gitpod using this PR.
I ran first export DP_PROJECT_NAME=feeds
and then the ddev composer config command, from the /workspace/d9simple directory.

[rfay]

Please start with ddev composer config repositories.feeds '{"type": "path", "url": "repos/feeds", "options": {"symlink": true}}' and work back from there. You'll need to inject your repo name somehow, but that's a problem on the host side.

I also get your results also with ddev composer config repositories.feeds "{'type': 'path', 'url': 'repos/feeds', 'options': {'symlink': true}}", but your problem is just "how to I inject the project name". There are many ways to do it. You don't have to use an environment variable directly. You can construct the command in stages, adding what you want, then execute it.

The problem you're up against is just how to generate a valid compose string with json in it, and it's something you do on the host with the tools you have available, including concatenation, etc.

[iamsophiesk]

Can confirm that this has resolved the issue I was having, and it seems to fix the problem for other examples too. Thanks for jumping on this so quickly. 👍 seems like a neat solution.

[rfay]

Thanks for testing @iamsophiesk !

[jonaseberle]

Sorry I haven't followed the suggested test regime but I found this to be very resilient. Great!
Stuff that was properly escaped before (ddev exec 'echo \"a\"') will keep working.
This ddev mysql -e 'SELECT "a"' works now properly. Same for ddev composer "..".

Although I haven't tested hooks I am confident to give a +1.

[rfay]

Thanks to all of you for testing. I did discover a bug from a previous PR, so please rm ~/.ddev/commands/web/php and it will automatically be replaced with the correct version if you get latest here or when the next alpha comes out.

[rpkoller]

apologies am a little bit late to the game; haven't noticed the pull request and call for testing. went through a few of the suggested manual tests from the initial post with alpha5. ran into two problems

ddev php -r 'echo "hi";' leads to:

PHP Parse error:  syntax error, unexpected end of file in Command line code on line 1

Parse error: syntax error, unexpected end of file in Command line code on line 1

and ddev drush role-add-perm authenticated 'administer comment types' leads to

 Too many arguments, expected arguments "command" "machine_name" "permissions".

Failed to run drush role-add-perm authenticated administer comment types: exit status 1

the other two statements i'Ve tried worked:
ddev exec 'set | grep DDEV'
ddev mysql -e "SHOW TABLES;"

[rfay]

@rpkoller I think you got the old php that didn't get updated. Please rm ~/.ddev/commands/web/php and ddev start and try again. (the php script shipped previously didn't have #ddev-generated so didn't automatically get updated to the current working one.

I had not trouble with your example ddev drush role-add-perm authenticated 'administer comment types' except where the comment-types permission didn't exist (so I used 'administer content' I think). Please update again and ddev start and see how you do.

[rpkoller]

@rfay you were spot on with your assumption. removing php from .ddev/commands/web solved the error. after a restart the php echo statement worked!
but about the drush command that still keeps on failing. Either with administer comment types or administer content like you suggested and i've also tried with administer blocks (checked the permissions yaml of the core blocks module beforehand). but all lead to the same too many arguments error i've pasted in my previous comment. if i ssh into the container and perform the commands there each of the three works flawlessly.

*tested on drupal 9.4.x-dev and drush 11.0.5

[rfay]

Is it possible you have a customized drush? Please remove any drush from ~/.ddev/commands/web/drush and <project>/.ddev/commands/web/drush

[rpkoller]

oh! i was under the impression to be able to use drush without the need of exec or . you had to add a global drush command using /var/www/html/vendor/bin/drush $@ inside. that is what i've used and was still there in ~/.ddev/commands/web/drush . Deleted the file and now the drush commands work as expected!

[rfay]

The global drush command has been built into ddev for some time now, but you had created your own a long time ago and ddev respects what people modify if they don't have #ddev-generated in there.

[rpkoller]

ahhh just totally missed that you've added a global drush command at some point. that explains things :) and double thumbs up for this pull request. works as expected! thank you for the pull request as well as the explanation.

p.s. maybe a reasonable step for the release notes of 1.19.0 might be to remind people like me who have their own drush command or php to delete those in case nothing custom or special is in there and let ddev replace those with the recent ones. might prevent a few support requests in the context of the issue this pull request fixes?

[rfay]

Added to caveats in release notes, https://github.com/drud/ddev/releases/tag/v1.19.0-alpha5

[shaal]

Using set -x in a bash script, I was able to find the solution that works, that use env var as well -

ddev composer config repositories.core '{"type": "path", "url": "repos/'"$DP_PROJECT_NAME"'", "options": {"symlink": true}}'

[rfay]

Oh, I see - you're putting the three parts together:
ddev composer config repositories.core '{"type": "path", "url": "repos/'"
and "$DP_PROJECT_NAME"
and '", "options": {"symlink": true}}'

That's a good strategy. Awkward but suitable :)

[rpkoller]

@rfay fyi. one observation about a phenomenon i've reported where certain drush commands were hanging indefinitely after being ran successfully. guess i only reported it via dischord and we had a thread there (cant find it right now). but after removing the custom global drush command recommended in relation to this issue here somehow the indefinite hang issue is also gone for a while now! just realised that this morning.

[rfay]

Oh, so the problem you had might have been because you had a one-off custom drush command. That's super interesting :)

[rpkoller]

buuuut the issue happens again and the commands hang again like before. seems like it was just a temporary improvement and things keep on happening on my setup with the standard global drush command again. just wanted to give you the follow up.

[rfay]

I hope you'll be able to create a simple repro case and share it. If you just create a trivial Drupal project with drush and can make this happen it will be solvable. If possible, also try to recreate it on a current-generation computer. Please open an issue when you can recreate it with a demonstration repo+db. You can just push the repro repo up to github and then we'll have something. Thanks!