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

Git hook - Reading from standard input does not work. #3165

Closed
stacyharper opened this issue Oct 21, 2017 · 9 comments
Closed

Git hook - Reading from standard input does not work. #3165

stacyharper opened this issue Oct 21, 2017 · 9 comments

Comments

@stacyharper
Copy link

Hello everyone,
I'm facing a big problem. I'd like to create a good git hook to automaticaly fix my php files before commiting. My goal is to question me if a change has been done by php-cs-fixer

Here is my script:

#!/bin/bash -u

executable="$1"
rules="$2"
path="$3"

if [ -d "$path" -o -f "$path" ];
then
	if [ -f "$executable"  ];
	then
		needInteraction=0
		for file in $(git diff-index --cached --name-status HEAD -- | cut -c 3- | grep .php$);
		do
			git --no-pager show :"$file" | "$executable" fix --dry-run --quiet --rules="$rules" -
			if [ "$?" -eq "8" ]
			then
				needInteraction=1
				"$executable" fix --quiet --cache-file .git/.php_cs.cache --rules="$rules" "$file"
				echo "$file has been fixed by php-cs-fixer."
			fi
		done
		if [ "$needInteraction" -eq "1" ]
		then
			exec < /dev/tty
			read -p "Do you still want to commit ? [y/n]"
			if [[ ! "$REPLY" =~ ^[Yy]$ ]]
			then
				exit 1
			fi
		fi
	else
		echo "$executable not found"
		exit 1
	fi
fi

I want php-cs-fixer to test my staged files like they will be commited.

My problem is when I do this.

git --no-pager show :"$file" | "$executable" fix --dry-run --quiet --rules="$rules" -

The value of "$?" will always be "0", breaking all my logic...

When I try to execute this command outside of my script, replacing variables with true values, it seem thas php-cs-fixer does not read my file from standard input. It's the same thing if I try to cat a file then pipe the output to the php-cs-fixer with the "-" at the end...

And ofc, I tested my script with php files that contains multiple coding convention mistakes.

I think I probably do something badly, but the doc tell me that we can

$ cat foo.php | php php-cs-fixer fix --diff -

So where is my mistake ?

For information

php -v
PHP 7.1.10 (cli) (built: Sep 27 2017 17:33:37) ( NTS )
Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.1.0, Copyright (c) 1998-2017 Zend Technologies
lsb_release -a
LSB Version:    1.4
Distributor ID: Arch
Description:    Arch Linux
Release:        rolling
Codename:       n/a
uname -a
Linux IDCI-FlyingSquirrel 4.13.7-1-ARCH #1 SMP PREEMPT Sat Oct 14 20:13:26 CEST 2017 x86_64 GNU/Linux

Thank a lot in advance !

@stacyharper
Copy link
Author

Mhh, the $path variable is actually useless in the script, but this is definitly not the cause of my problem.

#!/bin/bash -u

executable="$1"
rules="$2"

if [ -f "$executable"  ];
then
	needInteraction=0
	for file in $(git diff-index --cached --name-status HEAD -- | cut -c 3- | grep .php$);
	do
		git --no-pager show :"$file" | "$executable" fix --dry-run --quiet --rules="$rules" -
		if [ "$?" -eq "8" ]
		then
			needInteraction=1
			"$executable" fix --quiet --cache-file .git/.php_cs.cache --rules="$rules" "$file"
			echo "$file has been fixed by php-cs-fixer."
		fi
	done
	if [ "$needInteraction" -eq "1" ]
	then
		exec < /dev/tty
		read -p "Do you still want to commit ? [y/n]"
		if [[ ! "$REPLY" =~ ^[Yy]$ ]]
		then
			exit 1
		fi
	fi
else
	echo "$executable not found"
	exit 1
fi

@stacyharper
Copy link
Author

Here is my temporary solution

git --no-pager show :"$file" > "/tmp/php-cs-fixer-checked-file-cache.php"
"$executable" fix --dry-run --quiet --using-cache=no --rules="$rules" "/tmp/php-cs-fixer-checked-file-cache.php"

Yeah ugly... But it works !

@stacyharper
Copy link
Author

stacyharper commented Oct 21, 2017

Basicaly

// test.php
<?php

class Test {
        private $foo;

    public function __consruct( ){
        $this->foo =  "bar";
    }
}
$ cat test.php |php-cs-fixer fix --diff -
Loaded config default.
$ cat test.php |php-cs-fixer fix --quiet --dry-run -                                                                
$ echo $?
0
$ php-cs-fixer -v
PHP CS Fixer 2.7.0 Sandy Pool by Fabien Potencier and Dariusz Ruminski (e4e93a1)

@SpacePossum
Copy link
Contributor

Thanks @Eluminae for sharing this 👍
Do you think we could use any of your experience on the integration samples we've in the project or maybe in the documentation somewhere?

@stacyharper
Copy link
Author

@SpacePossum Yeah ofc ! I think it could be usefull for a lot. There is only this little problem which is breaking the script.

@keradus
Copy link
Member

keradus commented Oct 23, 2017

why not ask ppl to use grumphp (or anything else) for such kind of local-git-level integration ?

@stacyharper
Copy link
Author

Mh I didn't know about grumphp but it's seems to be a very good tool ! I'll probably take a deeper look.
But my problem is not so about git hooks but the "read file from stdinput" feature which is just unworking at all on my machine.

@keradus
Copy link
Member

keradus commented Oct 23, 2017

there are few githook managers (written in any lang, not only PHP).
sorry, I was confused by title of this issue - "git hook" :D
let us investigate stdin issue for sure!

orgads added a commit to orgads/git that referenced this issue Nov 17, 2020
Let hooks receive user input if applicable.

Closing stdin originates in f5bbc32 (Port git commit to C,
2007). Looks like the original shell implementation did have
stdin open. Not clear why the author chose to close it on
the C port (maybe copy&paste).

Some references of users requesting this feature. Some of
them use acrobatics to gain access to stdin:
[1] https://stackoverflow.com/q/1067874/764870
[2] https://stackoverflow.com/q/47477766/764870
[3] https://stackoverflow.com/q/3417896/764870
[4] PHP-CS-Fixer/PHP-CS-Fixer#3165
[5] typicode/husky#442
orgads added a commit to orgads/git that referenced this issue Nov 17, 2020
Let hooks receive user input if applicable.

Closing stdin originates in f5bbc32 (Port git commit to C,
2007). Looks like the original shell implementation did have
stdin open. Not clear why the author chose to close it on
the C port (maybe copy&paste).

The only hook that passes internal information to the hook
via stdin is pre-push, which has its own logic.

Some references of users requesting this feature. Some of
them use acrobatics to gain access to stdin:
[1] https://stackoverflow.com/q/1067874/764870
[2] https://stackoverflow.com/q/47477766/764870
[3] https://stackoverflow.com/q/3417896/764870
[4] PHP-CS-Fixer/PHP-CS-Fixer#3165
[5] typicode/husky#442
orgads added a commit to orgads/git that referenced this issue Nov 17, 2020
Let hooks receive user input if applicable.

Closing stdin originates in f5bbc32 (Port git commit to C,
2007). Looks like the original shell implementation did have
stdin open. Not clear why the author chose to close it on
the C port (maybe copy&paste).

The only hook that passes internal information to the hook
via stdin is pre-push, which has its own logic.

Some references of users requesting this feature. Some of
them use acrobatics to gain access to stdin:
[1] https://stackoverflow.com/q/1067874/764870
[2] https://stackoverflow.com/q/47477766/764870
[3] https://stackoverflow.com/q/3417896/764870
[4] PHP-CS-Fixer/PHP-CS-Fixer#3165
[5] typicode/husky#442

Signed-off-by: Orgad Shaneh <orgads@gmail.com>
@Wirone
Copy link
Member

Wirone commented Mar 10, 2023

Personally I can recommend Captain Hook.

@Wirone Wirone closed this as completed Mar 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants