Scaffold task and actions-validator via DDEV Dockerfile instead of Composer plugin

[davereid]

Fixes #948

• Gets rid of local-php-security-checker because it is abandoned in favor of composer audit
• Moves task to a scaffolded Dockerfile to be added to the .ddev/web-build/ directory. This means that ddev would need to be restarted after installing Drainpipe to make it available.

Tasks Remaining

• [x] Update documentation
• [x] Update tests
• [x] Check to see if running actions-validator via yarn is still slow, or if we need to install a binary (see https://github.com/Lullabot/drainpipe/issues/220).
• [x] Pin the version of task used instead of always installing latest
• [x] Test that upgrading in-place works for projects running on a local.
• [ ] Note that this will need a new major release bump.

[rabbitlair]

@davereid @mrdavidburns @deviantintegral, I applied some changes to this one, in order to move it forward. Can you please take a look and let me know if the chosen approach looks good for you? Thank you!

[rabbitlair]

Running action-validator via yarn is, indeed, still slow. Good news is, with the approach implemented in this PR (direct global install in the Dockerfile), execution is fast:

time (find . -name '*yml' | xargs -n1 npx @action-validator/cli)
real  0m58.743s
user  0m45.183s
sys 0m12.437s

time (find . -name '*yml' | xargs -n1 action-validator)
real  0m5.873s
user  0m7.072s
sys 0m2.275s

Also, I have implemented version pin for Taskfile, based on a file .taskfile containing the desired version, instead of just installing latest version.

@davereid, when you have a chance, do you mind taking a look to the code and let me know your thoughts? Thanks a lot!

[rabbitlair]

@davereid, both issues are now solved:

1. When running ddev task test:security, the plugin is installed properly and the command is not stall anymore

$ ddev task test:security
[test:security] Changed current directory to /home/rabbitlair/.composer
[test:security] ./composer.json has been updated
[test:security] Running composer update nedbase/composer-audit-common-report-formats-plugin
[test:security] Loading composer repositories with package information
[test:security] Updating dependencies
[test:security] Nothing to modify in lock file
[test:security] Writing lock file
[test:security] Installing dependencies from lock file (including require-dev)
[test:security] Package operations: 1 install, 0 updates, 0 removals
[test:security]   - Installing nedbase/composer-audit-common-report-formats-plugin (v1.0.0): Extracting archive
[test:security] Generating autoload files
[test:security] No security vulnerability advisories found.
[test:security] Using version ^1.0 for nedbase/composer-audit-common-report-formats-plugin
[test:security] No security vulnerability advisories found.

2. When installing/updating Drainpipe, the system-wide Taskfile binary from Dockerfile is linked from vendor/bin/task properly. To test from an existing site using a previous Drainpipe version, install the new Drainpipe version, then run ddev restart and run ddev composer install. You should see the following:

$ ddev composer install
// ...
Found existing Taskfile binary at: /usr/local/bin/task
Created symlink to system Taskfile in vendor/bin
// ...

Your vendir/bin/task file should be a symbolic link like this:

$ ls -lh vendor/bin/task
lrwxrwxrwx 1 rabbitlair rabbitlair   19 nov  4 20:05 task -> /usr/local/bin/task

And the expected version is:

$ ddev task --version
4.45.3

When you have a chance, please, can you confirm both issues are addressed? Thanks a lot!

[davereid]

It's when running ddev task test:security format=junit that it stalls.

[rabbitlair]

It's when running ddev task test:security format=junit that it stalls.

Yes, it should be working fine too in that case:

$ ddev task test:security format=junit
[test:security] Changed current directory to /home/rabbitlair/.composer
[test:security] Changed current directory to /home/rabbitlair/.composer
[test:security] ./composer.json has been updated
[test:security] Running composer update nedbase/composer-audit-common-report-formats-plugin
[test:security] Loading composer repositories with package information
[test:security] Updating dependencies
[test:security] Lock file operations: 1 install, 0 updates, 0 removals
[test:security]   - Locking nedbase/composer-audit-common-report-formats-plugin (v1.0.0)
[test:security] Writing lock file
[test:security] Installing dependencies from lock file (including require-dev)
[test:security] Package operations: 1 install, 0 updates, 0 removals
[test:security]   - Installing nedbase/composer-audit-common-report-formats-plugin (v1.0.0): Extracting archive
[test:security] Generating autoload files
[test:security] No security vulnerability advisories found.
[test:security] Using version ^1.0 for nedbase/composer-audit-common-report-formats-plugin
[test:security] <?xml version="1.0"?>
[test:security] <testsuites/>
[test:security] 
[test:security] Failed to find 'main' with 'loadFile'; Candidate 'main' does not look loadable from the fs or php stream wrappers
task: Failed to run task "test:security": exit status 1
Failed to run task test:security format=junit: exit status 201

[davereid]

I can confirm that both ddev task test:security format=junit works and ddev task --version is reporting the current version now.

[davereid]

Why do we need a version in vendor/bin now? For backwards compatibility? Does that mean that we don't actually need a major version bump?

[rabbitlair]

Why do we need a version in vendor/bin now? For backwards compatibility? Does that mean that we don't actually need a major version bump?

Yes, it is for backwards compatibility. Once you run ddev restart and ddev composer install, the binary in vendor/bin/task is removed and a symbolic link is created, pointing to /usr/local/bin/task. As a result, the only binary in DDEV container is the system wide one, while we preserve the vendor/bin/task path as valid.

So, I agree, I guess that means we should fine with a minor version bump - or even patch bump

[rabbitlair]

Would this really be a breaking change that would require a major version release?

I would say it is not a breaking change, because we are keeping vendor/bin/task as a symbolic link to the Taskfile binary installed by the custom Dockerfile for DDEV. How would it be the best way to test this?

[rabbitlair]

For the records, I just tested this branch against lullabot.com-d8, and everything looks good to me.

Just a heads up: we would need to remove this file (and related files) from the repository, as the functionality is implemented in this pull request, and it produces a conflict.

Also, I do not think we need to tag a major version when merging this one.

[rabbitlair]

@beto-aveiga, when you have a chance, please, I would like to know your thoughts on this one before merging it. Thank you!