[Question] Hooks not working properly

[BorisovskiP]

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 ?

[sebastianfeldmann]

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 :)

[BorisovskiP]

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?

[sebastianfeldmann]

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?

[BorisovskiP]

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.

[sebastianfeldmann]

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 🤔

[BorisovskiP]

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.

[BorisovskiP]

@sebastianfeldmann any updates on this issue/feature request?

[BorisovskiP]

@sebastianfeldmann could you give me an update on this issue/feature request?

[sebastianfeldmann]

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?

[BorisovskiP]

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?

[sebastianfeldmann]

Ok I think I got it ;)

[BorisovskiP]

@sebastianfeldmann do you have an update on this?

[BorisovskiP]

@sebastianfeldmann could you give us an update on this issues?

[norgeindian]

@sebastianfeldmann , is there anything we could do to help you here, so that this check actually works?

[sebastianfeldmann]

The idea would be to write a new check and find a proper name for it :)

[BorisovskiP]

Closing due to inactivity.