{$STAGED_FILES} empty in docker

[potsky]

Hello !

When using Cap'n in Laravel Sail (a simple wrapper for docker-compose), {$STAGED_FILES} is empty.

Here is a configuration sample:

{
    "config": {
        "run-mode": "docker",
        "run-exec": "docker-compose exec -T -u sail -e \"APP_URL=http://laravel.test\" -e \"DUSK_DRIVER_URL=http://selenium:4444/wd/hub\" laravel.test",
        "verbosity": "verbose"
    },
    "pre-commit": {
        "enabled": true,
        "actions": [
            {
                "action": "echo {$STAGED_FILES} > .commit && echo '----' >> .commit",
                "options": [],
                "conditions": []
            },
            {
                "action": "git diff-index --name-status HEAD >> .commit",
                "options": [],
                "conditions": []
            }
        ]
    }
}

Here is the output of the .commit file:

----
M   captainhook.json

If I remove the run-* directives and reinstall the hooks, the output of the .commit file is :

captainhook.json
----
M   captainhook.json

as expected.

[potsky]

poke @rooselle

[sebastianfeldmann]

Are you sure you container has access to a git binary and the repository / .git directory?

[potsky]

Yes 👍

[renky]

Hi there

I can confirm this issue with a docker-compose setup. And I also found a solution but maybe this can be solved somehow by captain hook

My docker-container didn't have an installed git. So I added it, and tried to call git status what resolved in this error-message:

fatal: detected dubious ownership in repository at '/var/www'
To add an exception for this directory, call:

        git config --global --add safe.directory /var/www

In this situation I found out the method isHeadValid in ...\Operator\Index.php returns false, since runner throws a RuntimeException.

If you run git config --global --add safe.directory /var/www once in your container, then this error is gone and Captainhook is working properly...

But can this be done automatically? I believe - especially in Laravel Environments - this setup isn't unusual. There are a view docker-containers running (php, nginx, mariadb...) for local development and for convenience, this makes the development independent from the PHP-Installation on the host... in my special case on my host-system I'm running php8.0 and in my current projects I'm using php8.1 - until few weeks ago, I worked with the local installation of captain hook - vendor/bin/captainhook was called on the host directly and worked properly... since a few versions some symfony-components have been updated to PHP8.1, and now this doesn't work any more because captainhook checks the platform and stopps if composer requires PHP8.1, but system has only PHP8.0 - in composer.json I was able to disable platform_checks, but this only works until the PHP8.1-Components of symfony are installed - if php8.1-code is there, it runs in Syntax-Errors with installed php8.0.

But thats only an explanation why this setup is used. Whats the best way to go on now? add an information that in docker-mode git must be installed and additionally working-directory must be a safedir? This will resolve the problem immediatelly for everybody who runs into this problem.

what if captain hook would do this on installation? Running a git config --global --add safe.directory "*" once? It might also ask the user if this shall be done... It also could do that on every run with a git config-call and remove it again afterwards...

[sebastianfeldmann]

Maybe I should not just ignore the error :/

[sebastianfeldmann]

I just released CaptainHook 5.10.11

From now on all permission related issues are not ignored anymore. Sadly I can't "fix" the issue from the Cap'n side because I do not want to change permissions or any security settings without the user knowing. But at least now the user knows what's going wrong.

[sebastianfeldmann]

Regarding your installation issue @renky : The Cap'n only requires php > 7.2 and I run test on 7.3, 7.4, 8.0 and 8.1 and everything seems to work ok on all versions without any ignore platform hacks, but it's always a clean install, maybe that's the issue on your end?

Never the less, big thank you for investigating the docker compose issue that helped a ton.

[ktomk]

@sebastianfeldmann Got noticed about the new release but I only see a GPG fingerprint and a change in the version number/date. Is there any related change (as what was talked about in this issue)? I might have mistaken something here ... but nevertheless was not shy to ask. Thanks a lot for any insights!

[sebastianfeldmann]

Yes the real fix is in a dependency sebastianfeldmann/git I bumped the version there as well so a composer update or if you are using the PHAR version a phive update should do the trick.

The related change can be found here https://github.com/sebastianfeldmann/git/commit/853066c3841faf96d3f0ea00822eeb3b6cd8d995

[ktomk]

thanks! @sebastianfeldmann

[renky]

@sebastianfeldmann: yes always clean installation. This issue happens, if you run e.g. PHP8.0 on the host and use composer only in a docker-container that uses PHP8.1 - if you then install any composer-package that really requires PHP8.1 then captainhook cannot be used any more in the localmode (platform-check fails). "Your composer.lock requires php8.1 but 8.0 is installed"... If also in the docker container only packages are installed that requires php8.0 you can turn off platform-check on host (composer.json -> platform-check: false) and it will work - but if check is disabled and real php8.1 code is executed, it failes with syntax-errors... so I had two options: install php8.1 on host, or use captainhook inside docker...

[renky]

btw: I'll add

RUN git config --global --add safe.directory /var/www

to my dockerfile and with that all local setups are working now...

[renky]

@sebastianfeldmann Fully coincidentally: I just ran into the same issue with a total different error-reason

STAGED_FILES are empty in a 'commit -a -m' call if you didn't add anything before.... But nevertheless I'd like to check them in a 'commit -a -m' call

In my case I modified two files (js and php) and if I call: git diff-index --diff-algorithm=myers --no-ext-diff --cached --name-status HEAD i get no files

git diff-index --diff-algorithm=myers --no-ext-diff --name-status HEAD shows me the two files...

so the question is, why only look onto the really staged files? From my point of view git commit -a -m is an often used command...

here my output:

$ git status
On branch testbranch
Your branch is ahead of 'origin/testbranch' by 14 commits.
  (use "git push" to publish your local commits)

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   app/Http/Controllers/Controller.php
        modified:   resources/js/app.js

no changes added to commit (use "git add" and/or "git commit -a")

now run

$git commit -a -m "testmessage"

all hooks are skipped due to empty STAGED_FILES

What could be the solution - remove --cached option to get all files? Only add files that would be commited with commit -a -> this means only formatter M - so git diff-index would have to be called twice - first time with --cached and all formatters, and second time without --cached but only formatter M

If captain hook would have the possibility to get the commit-attributes, maybe it even could check if commit was called with the -a option and add unstaged modified files in this case?

Another option would also be to add a new Variable - UNSTAGED_FILES additionally to STAGED_FILES - then everybody could decide on his own if he wants to search in both...

EDIT: I made additional tests. I modified a php-File without calling git add. captainhook is running in local run-mode

"config": {
        "verbosity": "verbose"
    },

I added a dump in git/src/Operator/Index.php in Line 205 and dump the result:

$result    = $this->runner->run($cmd, $formatter);
dd($result);
$files     = $result->getFormattedOutput();

When calling git commit -a -m "test" I get the following result:

SebastianFeldmann\Cli\Command\Runner\Result^ {#203
  -cmdResult: SebastianFeldmann\Cli\Command\Result^ {#200
    -cmd: "git diff-index --diff-algorithm=myers --no-ext-diff --cached --name-status HEAD"
    -code: 0
    -validExitCodes: array:1 [
      0 => 0
    ]
    -buffer: array:1 [
      0 => "M\tapp/Models/Model.php"
    ]
    -stdOut: "M\tapp/Models/Model.php\n"
    -stdErr: ""
    -redirectPath: ""
  }
  -formatted: array:1 [
    0 => "app/Models/Model.php"
  ]
}

So it really finds the modified file, although it is not staged already! thats the behaviour I'd expect.

Now I switch to docker run-mode - without changing any other file - still only modified and not staged.:

  "config": {
      "verbosity": "verbose",
      "run-mode": "docker",
      "run-exec": "docker exec -t phpcontainer"
  },

and my dump gives me this:

SebastianFeldmann\Cli\Command\Runner\Result^ {#202
  -cmdResult: SebastianFeldmann\Cli\Command\Result^ {#199
    -cmd: "git diff-index --diff-algorithm=myers --no-ext-diff --cached --name-status HEAD"
    -code: 0
    -validExitCodes: array:1 [
      0 => 0
    ]
    -buffer: []
    -stdOut: ""
    -stdErr: ""
    -redirectPath: ""
  }
  -formatted: []
}

so the same situation, one time local, one time docker gives different result. What I don't understand: If I call the command

git diff-index --diff-algorithm=myers --no-ext-diff --cached --name-status HEAD

directly on the bash it ALLWAYS doesn't give me any result - so I don't understandy why in the first case (local) it really finds any file...

[renky]

@sebastianfeldmann : just for the case, that my post was overseen - could you please reopen the issue, since it is still there with another reason - see analysis in last post.... Thanks

[ktomk]

@renky: I think it would be more friendly to open a new issue as a regression against this one here with a clear path to reproduce. and if it is only for the reason that there was a version bump already. it would also have the benefit to make your report more visible as what it is. and perhaps would be helpful to maintain this. like you wrote yourself:

I just ran into the same issue with a total different error-reason

Different reason sounds like a good measurement to make it more visible in a new issue that also has the benefit to start from scratch (but can easily give reference to existing material, e.g. the number or hypertext).

just my 2 cents.

/E: feel free to @ping me in a first comment on the new issue, thanks!

[sebastianfeldmann]

I opened a new issue for the commit -a problem

https://github.com/captainhookphp/captainhook/issues/178