phpro / grumphp-shim

This repository provides easy way to install GrumPHP without the risk of conflicting dependencies.
MIT License
24 stars 3 forks source link

git_hook_variables not used in phar #1

Closed ryantfowler closed 4 years ago

ryantfowler commented 4 years ago

First, I think it's great that there's a Composer plugin for Grumphp. Kudos. Next, I have found that the vendor/bin/grumphp file starts off with $(which php) which effectively nutralizes any benefits that may be present in using the pure Composer package phpro/grumphp which utilizes git_hook_variables, and therefore allows for configuring Grumphp for a native php stack/Vagrant/Docker.

This Composer plugin doesn't, from my experiments and what I can tell, support being ran on Vagrant/Docker.

My sample grumphp.yml file:

parameters:
    git_hook_variables:
        EXEC_GRUMPHP_COMMAND: 'docker exec php-fpm php'
    tasks: { phpunit: null }

My resulting .git/hooks/pre-commit:

#!/bin/sh

#
# Run the hook command.
# Note: this will be replaced by the real command during copy.
#

# Fetch the GIT diff and format it as command input:
DIFF=$(git -c diff.mnemonicprefix=false --no-pager diff -r -p -m -M --full-index --no-color --staged | cat)

# Grumphp env vars
export GRUMPHP_GIT_WORKING_DIR=$(git rev-parse --show-toplevel)

# Run GrumPHP
(cd "./" && printf "%s\n" "${DIFF}" | 'docker' 'exec' 'php-fpm' 'php' 'vendor/phpro/grumphp-shim/grumphp' 'git:pre-commit' '--skip-success-output')

Sample output:

me@computer$ git status
On branch feature/JIRA-123-local-dev-environment
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

    modified:   app/code/Vendor/Module/Block/Adminhtml/Import.php

no changes added to commit (use "git add" and/or "git commit -a")
me@computer$ git commit -am "test2"
$(which php) "$(dirname "$0")/grumphp.phar" $@
$(which php) "$(dirname "$0")/grumphp.phar" $@
[feature/JIRA-123-local-dev-environment bc5a88ad] test2
 3 files changed, 58 insertions(+), 2 deletions(-)

The above git commands were ran from my host machine, and didn't prevent the git operations, but also didn't work as I would expect (executing via the php executable within the specified docker container).

When I use phpro/grumphp instead of phpro/grumphp-shim, use the same grumphp.yml file, and end up with the same .git/hooks/pre-commit shown above, and commit file changes from my host machine, the following is output:

git commit -am "test"
GrumPHP detected a pre-commit command.
GrumPHP is sniffing your code!
Running task 1/1: Phpunit... ✔
GrumPHP detected a commit-msg command.
GrumPHP is sniffing your code!
             ▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄
           ▄▄▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▌           
         ▄▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▌
        ▐▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▌
        ▐▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▌        
  ▄▐▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▌
 ▐▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▌        
 ▐█▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▌        
   ▀█▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▌        
     ▀▀▓▓▓▓▓▓▓▓▓▓▓▓█▀▀▀▀▀▀▀▀▀▀▀▀▀▀████████████▄
      ▄████████▀▀▀▀▀                 ██████████   
     ███████▀                         ██████▀
      ▐████      ██▌          ██       ████▌      
        ▐█▌                            ███        
         █▌           ▄▄ ▄▄           ▐███        
        ███       ▄▄▄▄▄▄▄▄▄▄▄▄       ▐███         
         ██▄ ▐███████████████████████████
        █▀█████████▌▀▀▀▀▀▀▀▀▀██████████▌▐         
          ███████████▄▄▄▄▄▄▄███████████▌          
         ▐█████████████████████████████           
          █████████████████████████████           
           ██ █████████████████████▐██▀           
            ▀ ▐███████████████████▌ ▐▀            
                ████▀████████▀▐███                
                 ▀█▌  ▐█████  ▐█▌                 
                        ██▀   ▐▀                  
       _    _ _                         _ _
      / \  | | |   __ _  ___   ___   __| | |
     / _ \ | | |  / _` |/ _ \ / _ \ / _` | |
    / ___ \| | | | (_| | (_) | (_) | (_| |_|
   /_/   \_\_|_|  \__, |\___/ \___/ \__,_(_)
                  |___/
[feature/JIRA-123-local-dev-environment b6ca0ab4] test
 3 files changed, 240 insertions(+), 2 deletions(-)

This Composer plugin does solve dealing with "dependency hell," which is great. I just think that in order to be more available to more realistic and modern development environments, that the git_hook_variables ( at the least ) should be incorporated here.

veewee commented 4 years ago

Thanks for reporting. To be honest : I totally forgot about that whilst creating this package. I'll have to play around with the setup of this package to make it possible here as well.

ryantfowler commented 4 years ago

@veewee You're welcome :). My apologies for communicating a bug back to you :(. I really appreciate the work that you've done with this project. It has been very valuable to me in my work. I hope that this feedback is constructive. If I am able to, I'll also try to figure out a solution, and if I'm successful, I'll open a PR for your review.

Thank you

veewee commented 4 years ago

Hello @ryantfowler,

I've fixed this issue by directly initializing the phar scripts from the shell script. It seems to work from my POV. Can you verify if #2 fixes the issue for you?

(It will be shipped soon in 0.17.1)

ryantfowler commented 4 years ago

@veewee thanks for doing this. I'll take a look and get back to you. If I'm interested in this phar being ran inside a docker container, which I would have configured in my git hooks, wouldn't the same problem I reported be repeated, since this code attempts to execute a php interpreter vs running something like exec('docker' call?

veewee commented 4 years ago

Your docker container has to contain php with phar enabled. In that case the git hook runs something like docker ... grumphp. This will open up your docker container and run the grumphp command inside that container. But since this package is PHP, a valid php runtime is required inside the container.

ryantfowler commented 4 years ago

That makes sense. My intentions were to attempt to use this shim in a similar manner that I use the pure phpro/grumphp package, which would be like this: https://github.com/phpro/grumphp/pull/618#issuecomment-483001498 . The link I shared allows for me to run git operations on my host machine or within my docker container, and in both cases, grumphp will execute beautifully. At the moment, it doesn't seem like this is possible with the shim. Maybe the link I shared helps to clarify what I meant when I opened this issue?

veewee commented 4 years ago

I don't get why this wouldn't be possible with the shim package? The git hook should still contain the configured EXEC_GRUMPHP_COMMAND. The git hook behaves in a similar way as the original package. The only difference is that the code is in a phar file. (The original package contains a grumphp executable which is PHP as well, this package contains a grumphp executable that is a wrapper around the phar)

Can you elaborate on what is failing on your end?

ryantfowler commented 4 years ago

I think I understand now. I think part of my error was how I had configured the git hooks with this shim. Let me give it a shot again, and I'll get back to you. Following your code changes, I haven't tested it out, so it's just been theoretical. I'll get back to you. Thanks