ISSUE-220: do not use npx for action validator, use binaries #676
Conversation
|
I wonder if we should instead install the binary at the same time as all the other binaries we download. It feels a bit odd to be doing this dynamically in a task? |
|
Actually, it looks like we already install the binary, so we just have to call vendor/bin/action-validator instead? |
|
@deviantintegral : for the change I made, we are running the So, I think there is no action-validator binary at that moment. Am I right? |
tasks/github.yml
Outdated
| find $directories -type f \( -iname '*.yaml' -o -iname '*.yml' \) | xargs -n1 npx @action-validator/cli | ||
| yamls=$(find $directories -type f \( -iname '*.yaml' -o -iname '*.yml' \)) | ||
| action_validator_url_base="https://github.com/mpalmer/action-validator/releases/download/v0.6.0/action-validator_" | ||
| action_validator_bin_path="/usr/local/bin/action-validator" | ||
| [ "$(arch)" = "x86_64" ] && bin_name="linux_amd64" | ||
| [ "$(arch)" = "aarch64" ] && bin_name="linux_arm64" | ||
| curl -Lo $action_validator_bin_path "$action_validator_url_base$bin_name" || true | ||
| chmod +x $action_validator_bin_path | ||
| echo $yamls | xargs -n1 $action_validator_bin_path |
There was a problem hiding this comment.
What do you think about moving the binary install work to https://github.com/Lullabot/drainpipe/blob/main/drainpipe-dev/src/DevBinaryInstallerPlugin.php ? Then it would be available in vendor/bin/action-validator like other apps we download.
…px-action-validator-cli-is-really-slow-compared-to-calling-the-binary-directly--2
|
These two files are very similar.
For instance, if we update the action validator version in one place, it should be the same version in the other binary installer. |
|
@deviantintegral, this is ready for review again. Thanks! |
|
@beto-aveiga the DevBinaryInstaller only triggers if you're installing drainpipe-dev, which should be in require-dev. Those packages don't get installed when you do composer install --no-dev. So, I think we should move the actions validator binary to the DevBinaryInstaller class. |
|
@deviantintegral, we already have drainpipe/src/BinaryInstallerPlugin.php Lines 38 to 48 in 9a24bfc |
|
@deviantintegral removing action-validator from src/BinaryInstallerPlugin.php is causing an error on the first test. |
|
The failure makes sense - the test job isn't installing drainpipe-dev. Let's see if this fixes it. |
…px-action-validator-cli-is-really-slow-compared-to-calling-the-binary-directly--2
|
e20e351 is a great case study in why objects are better than arrays! The actions validator was nested in the wrong section in a way that was not obvious at all. The tugboat failure looks to be something new, because #688 was passing and then I rebuilt it and got the same failure here. perhaps it has to do with something merged recently, but I think it's more likely that there's a problem due to us doing some trickery to move the .tugboat directory around. |
justafish
deleted the
issue-220-npx-action-validator-cli-is-really-slow-compared-to-calling-the-binary-directly--2
branch
Uses binaries instead of NPX. See #220