captainhook
captainhook copied to clipboard
[Question] Hooks not working properly
In the recent updated, the hook "Exists" was introduced which from looking at the comment in the hook itself, it checks if given files are included/exist. In our tests, we were unable to run this hook successfully.
We wanted to test if a README.md
file exits in our src
folder. The path to the folder is: src/*/*/README.md
.
The hook we setup in the captainhook.json file is:
"action": "\\CaptainHook\\App\\Hook\\File\\Action\\Exists",
"options": {
"files": [
"src/*/*/README.md"
]
},
"conditions": []
In multiple directories under src
, we removed the README.md
files to check if the hook will work.
The hook was executed with no errors.
Is our configuration of the hook wrong?
Can we have some documentation or comments explaining the proper way of configuring this hook?
We also tried IsEmpty
but got the same results.
In a recent discussion https://github.com/CaptainHookPhp/captainhook/issues/101, we also discussed checking for empty folders alongside checking for empty files. Will this be implemented in the future ?
You can test the hook by running
git ls-tree --name-only -r HEAD src/*/*/README.md
As long as you do not commit the removal. The file will still be in the repository. Meaning the hook checks the repository not the working directory.
If you remove files locally they are still "in the" repository.
This is meant to work as a pre-push
hook so you are still able to commit without having any of the configured files available, but you are not able to push unless you have committed them to the repository.
PS: Documentation is coming soonish :)
While testing this, we were unable to have a successful hook. In our scenarios, we were able to push regardless if the README.md file was present or not.
Steps to reproduce the issue:
Create an empty directory 'Test'
under src/*/*
Add a test.txt
file to Test
Add the following action to the pre-push
hook :
"action": "\\CaptainHook\\App\\Hook\\File\\Action\\Exists",
"options": {
"files": [
"src/*/*/README.md"
]
},
"conditions": []
Add the test.txt file to git, commit it and push. This is our scenario works.The hook did not fail.
Now add a README.md
file, and another test2.txt
file. Add test2.txt
to git, commit and push. Even tho we have a README.md
file present and not added to the repository, the push was still executed.
In both cases we were not able to have the hook catch the error/file.
Are you able to reproduce this? Maybe we have something misconfigured?
So I added this to a pre-commit
hook.
{
"action": "\\CaptainHook\\App\\Hook\\File\\Action\\Exists",
"options": {
"files": [
"for/bar.xml"
]
}
},
- :rotating_light:
foo/bar.xml
does not exist, commit fails. - :rotating_light:
foo/bar.xml
exists but is not committed, commit fails - ✅
foo/bar.xml
exists and is already committed to the repository, commit works
Using it with wildcards
{
"action": "\\CaptainHook\\App\\Hook\\File\\Action\\Exists",
"options": {
"files": [
"test/*/*/*Test.php"
]
}
},
- :rotating_light: No file in the
test
directory, commit fails - :rotating_light: File
test/foo/Test.php
exists and is committed to the repository, commit fails (as expected because/*/*/
) - ✅ File
test/foo/bar/FizTest.php
exists and is committed to the repository, commit works (as expected)
:warning: From now on every commit will work because there is ONE file that matches the pattern
Are you sure there is NO file matching your wildcard already in the repository?
Adding an explicit path like for/bar.xml
without having bar.xml
in the repository works.The commit the fails and an error is diplay. This is all fine.
But as we have multiple folders under src/*/*
and each of these folders have a README.md
file, and when running git ls-tree --name-only -r HEAD src/*/*/README.md
all the files are display in the repository.
Our aim is when we have a new folder under src/*/*
we check in that new folder if README.md
exists. My current assumption is that because the files we search for already exist in the repository, the hook does not fail.
The hook should fail if in one of the folders under src/*/*
there is a missing file. In the end the hook should loop over each folder and check if the specified files are present.
Yes I see.
I think what you are trying to archive is something like PairExists
as soon as src/*/*/*.php
exists src/*/*/README.md
should exist as well. I think that's a slightly different use case than a simple Exists
🤔
The main goal is that regardless of the fact that the searched file already exists in the repository, if a new one is added under the same path (src/*/*
) then the hook should catch it and return an error saying that this file should be included in the repository. So if we have 2 README.md
files in the repository, and add a third one that is not present in the repository, then the hook should warn us that this file should be included in the repository.
@sebastianfeldmann any updates on this issue/feature request?
@sebastianfeldmann could you give me an update on this issue/feature request?
Hi @BorisovskiP sorry I totally missed this one.
Like I said, I think it is not a good idea to change the behavior of Exists
. So let me check if I got it right so I can create a custom action for this.
If I understood it correctly this should be valid
- foo
- Fiz.php
- README.md
- bar
- Baz.php
- README.md
- stuff
- stuff.xml
- stuff.json
And this should not be valid
- foo
- Fiz.php
- bar
- Baz.php
- README.md
So if you would configure it like
"exists": "*/*.php",
"requires": "*/README.md"
What the action should do is, check all directories that contain PHP files and make sure a README.md exists in that directory as well. Is that right?
Hi @sebastianfeldmann , the custom action should check if in a given directory, the specified file extension is present. For example:
- foo
- Fiz.php
- README.md
In captainhook.json we should be able to specify in the custom action that inside the "foo" directory we want to check for either a "README.md" file or any file that ends with ".md" . Of course, this should not be limited to the .md extension.
About the configuration it should look more like this:
"exists": "*/*",
"requires": "README.md"
An invalid example of this would be:
- foo
- Fiz.php
This is invalid because the "requested" file in the configuration is not present in the 'foo' directory (in this case a 'README.md' file).
An valid example would be:
- bar
- Baz.php
- README.md
This is valid because the "requested" file in the configuration is present in the 'bar' directory (in this case a 'README.md' file).
Also it would be nice to have another custom action that also check if a given file is not empty. If we take the example from above, we should check if "README.md" is not an empty file.
In my eyes it makes sense to check that because if the hook that check for an existing file (README.md), and that file is present but its also empty, that the hook will pass, thus we are able to commit an empty file.
So in the end we indeed check for that specified file, but it wont make sense to have it there if that file is empty.
What do you think about this? Does that make sense?
Ok I think I got it ;)
@sebastianfeldmann do you have an update on this?
@sebastianfeldmann could you give us an update on this issues?
@sebastianfeldmann , is there anything we could do to help you here, so that this check actually works?
The idea would be to write a new check and find a proper name for it :)
Closing due to inactivity.