$STAGED_FILES is empty during ´git commit -a´ in Docker mode about captainhook

[sebastianfeldmann]

Testing unstaged files would be a bad idea, because if the user is not committing the files why check them.

You are right in local run mode the Cap'n detects the changed files and $STAGED_FILES is not empty after calling git commit -a. And that is expected since -a should change the index before executing the commit. It seems the docker version is running into another git issue that gets ignored. That would explain the empty STAGED_FILES.

What you could do to debug this locally is to remove the try catch block in vendor/sebastianfeldmann/git/src/Operator/Index.php:274

private function isHeadValid(): bool
{
    try {
        $cmd    = new GetCommitHash($this->repo->getRoot());
        $result = $this->runner->run($cmd);
        return $result->isSuccessful();
    } catch (RuntimeException $e) {
        // if we do not have a permission error the current head is just invalid
        if ($e->getCode() !== 128) {
            return false;
        }
        throw $e;
    }
}

private function isHeadValid(): bool
{
    $cmd    = new GetCommitHash($this->repo->getRoot());
    $result = $this->runner->run($cmd);
    return $result->isSuccessful();
}

Then run the `git commit -a -m "foo" again and see if you get any errors

[ktomk]

thats the original reason why i switched from local to docker-mode....

This is pretty straight and I get that angle now. I always forget about symfony console as I for myself drop it where I can as it creates so many implications with PHP tooling, the symfony projects don't have good portability. Anyway, it makes the docker mode mandatory.

It's only happening in docker mode

The GIT_INDEX_FILE environment parameter also needs to be passed into the docker container (-e GIT_INDEX_FILE) and the contents must map with the container file-system (/E: IIRC the suggestion by torek was with the working directory, so it should be relatively easy to do that: -w ...)

[ktomk]

@sebastianfeldmann can you please change the issue title and add " in docker mode" at the end to have a better description?

[sebastianfeldmann]

This is now fixed with version 5.18.0

If the run-git option is set. The Captain can use the provided path to the.git directory to map the path to the index file.
So für prepare-commit-msg and pre-commit hooks the env var is now added automatically to the hook script.

Big thanks to @renky for the support and the GIT_INDEX_FILE insight.

[renky]

@sebastianfeldmann unfortunatelly there isn't an exception - dump of $result gives the following including return-code 0:

SebastianFeldmann\Cli\Command\Runner\Result^ {#201
  -cmdResult: SebastianFeldmann\Cli\Command\Result^ {#200
    -cmd: "git rev-parse --verify HEAD"
    -code: 0
    -validExitCodes: array:1 [
      0 => 0
    ]
    -buffer: null
    -stdOut: "44566ae9326971104d5b46d61c21ef24f002be2a\n"
    -stdErr: ""
    -redirectPath: ""
  }
  -formatted: []
}

edit:
what I could imagine: sometimes - and I'm not that proficient in docker - I have the impression that file-system-changes of host-system sometimes needs a bit of time until it is also available in the container - I can imagine, that the docker-version still doesn't see the implicit add of the files immediatelly... But honestly I have no idea how this can be solved... I already tried different settings for the volume (including 'cached')

But then I made another test - running git commit directly in the container

docker exec -it phpcontainer git commit -a -m "test"

In this case I had to switch back to "local" setting, because otherwise the hook would try to call docker in docker...
after switching to local and running git commit -a in the docker-container, everything works as expected...

so I guess the docker-filesystem doesn't get the changes... I came up with several test with sleeps at different code-posisions but nothing worked...

Then I endet up in Google :-) and found this:
https://stackoverflow.com/questions/47775628/git-pre-commit-hook-docker-different-git-status

the solution with forwarding git-lock-file doesn't seem so great to me - or do you see a good way of using this indexfile in captainhook in docker?

Finally I tried to add "git status" drictly into the hook-script and there I can see my changed file in the list of files to be commited... So might it be a possibility to get the list of files in the pre-commit-hook-script instead of reading it via php?
And forward the output to captainhook as parameter? This could work on host and in docker...

If I add

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

right before the docker-call in the script I get the correct output with my changed files...

the output of this command could be added to an environment-variable and could be forwarded to captain-hook... Captain-hook then could check if the variable is set... if it is not set, captn could do what he always does - and if set, captn could omit the git calls and directly use the environment-varialbe... doesn't neet do check head-valid or running any other git-commands - only would have to format the output and filter the files...

[ktomk]

@renky: which operating system are you using if I may ask?

[renky]

@ktomk Im using Windows 11 with Ubuntu20.04 on WSL2 with Docker Desktop installed - the git commands are all calles in Ubuntu WSL - so no git installed on Windows level.

[ktomk]

@renky, is that Docker Desktop with WSL 2 backend?

[renky]

@ktomk yes it is...

[ktomk]

After re-reading, I think the SO answer points to the right cause. Not very proficient with Cptn' thought, but what _torek_writes more to the end about properly staging the commit into a build system (which would be here the docker container but it is already enough to have a different build root) requires to transport the (partial) added changes to the index as that is what is being committed. It seems on how this is handled for "docker mode" this is not taken care of (creating a temporary staging area to operate from/in).

My second guess would be that it would also not be compatible with git-worktree. Regarding this later one, I have a similar "problem" with the pipelines utility as I just mount the project folder (I don't have to make an assumption about git thought) and for "normal" git this works just out of the box/by accident, but it's just not handling any details.

That pipelines utility also has another benefit in this regard (from the implementation side), that it only builds the tree as-is. So far it does not need to care about actual revisions, nor staged files, partially staged files etc. so it normally works with git in the container if the whole commit is done with git in the container.

I strongly assume that Cptn Hook has a split-mode here not easy to overcome. And in case of your WSL situation, why not run a shell within the WSL and do the git/cptn hook interaction within the linux shell instead of docker mode? I may mistake some things here, so just correct me, but shouldn't that be the solution (local mode within linux shell)?

[renky]

Don't get stuck into WSL-Topic - the SO-Answer from torek - as far as I understood it - will be the same in a Linux-Native System - if you need it I can check it on an Ubuntu-Server and try the same there... I'm quite sure that it will happen identically...

The docker-environment makes sense on all systems - in our team we are developing on different systems - I'm on windows 11 with WSL, another guy works with a Linux Mint, and two more guys with MacOS - we all have to develop the same PHP-Software that needs a special Database and a Redis-Service - For local development and testing docker-compose is used and it is the only way that works on all systems without beeing dependent from the software on the host-system. For different reasons I'm not able to install PHP8.1 on my host-system, but the Software we develop needs PHP8.1.

And now we are back in the original reason, why I switched from local mode to docker mode: since I use PHP8.0 (and sorry - it could also be 7.4 or even NO php) on the host-system, but the composer-setup says we need PHP8.1 - we are not able to call captainhook from the host-system - ether because there is no PHP or because composer tells the version is missmatching - so I MUST call captain in the docker-container...

And imho thats the main reason why there IS a docker-mode - otherwise it would not be neede I guess...

And now back to the problem: how do we get the really "to be commited files" to docker-version of captain hook? I think the solution of torek is much to over-engineered.... since captainhook is installing the git hook shell-scripts itself, it could also change them - so why not (only for docker-mode) call the git diff already in the hook shell script? there it sees the correct files!! then writing the output to an environment variable and forwarding it to docker-captain? and then captain can check in general if the environment variable is set - if yes, only formatting and filtering the content of the varialbe; if no, do all the stuff that captain does currently and call git diff itself...

[ktomk]

Don't get stuck into WSL-Topic - the SO-Answer from torek - as far as I understood it - will be the same in a Linux-Native System - if you need it I can check it on an Ubuntu-Server and try the same there... I'm quite sure that it will happen identically...

Yes, please do that (I think just doing within WSL should suffice, just only use the local mode, not Docker).

The docker-environment makes sense on all systems - in our team we are developing on different systems - I'm on windows 11 with WSL, another guy works with a Linux Mint, and two more guys with MacOS - we all have to develop the same PHP-Software that needs a special Database and a Redis-Service - For local development and testing docker-compose is used and it is the only way that works on all systems without beeing dependent from the software on the host-system. For different reasons I'm not able to install PHP8.1 on my host-system, but the Software we develop needs PHP8.1.

As far as I understood it, the issues context is in a pre-commit situation. The docker images you should use with a clean checkout of a concrete revision - so this is after Captain Hook (in my understanding at least, don't want to create the impression you can't do things differently, so just FYI). And in that sense I won't see any blocker to not use docker for executing a build artifact of the project you're talking about, but I'm not sure you really need Docker to run Captain Hook.

And just to confirm: What torek wrote about is the same on any system, even Windows, because that is how git works. The gap is merely when mounting a project directory into a docker container, the git index / cache for the prepared (and if only for -a of git-commit(1)) - as you also reminded - won't transparently jump over the gap.

And now we are back in the original reason, why I switched from local mode to docker mode: since I use PHP8.0 (and sorry - it could also be 7.4 or even NO php) on the host-system, but the composer-setup says we need PHP8.1 - we are not able to call captainhook from the host-system - ether because there is no PHP or because composer tells the version is mismatching - so I MUST call captain in the docker-container...

I'm feeling sorry if my comment created the impression as if Docker is the culprit or any of your practice would be questionable. I was solely trying to discuss the technical issue even with my limited knowledge of Captain Hook and WSL nor even being remotely able to do manual testing with a WSL setup.

However I was under the impression that you'd be able to call Captain Hook within WSL and I'd hoped that it could produce what you're looking for.

And imho thats the main reason why there IS a docker-mode - otherwise it would not be neede I guess...

Oh well, the main reason there is a Docker mode I'm pretty sure is, because we can do it. At the end of the day, who needs Docker? Yes, I know, you need, but - I don't want to sound snarky - on my box, Captain Hook runs fine without. BTW, I'd welcome if there is more respect to actually have something like $STAGED_FILES in a variable, just speaking for myself, I've not yet found one working solution for that my own just with git(1) and only on Linux just for any kind of project managed with git. Just saying, I did a couple of implementations, but it never works for all cases (and just the remark: this is not even remotely speaking about Docker which I also support heavily in one of my tools). Maybe you got the wrong impression this is all about your issue, hopefully not.

And now back to the problem: how do we get the really "to be commited files" to docker-version of captain hook? I think the solution of torek is much to over-engineered.... since captainhook is installing the git hook shell-scripts itself, it could also change them - so why not (only for docker-mode) call the git diff already in the hook shell script? there it sees the correct files!! then writing the output to an environment variable and forwarding it to docker-captain? and then captain can check in general if the environment variable is set - if yes, only formatting and filtering the content of the varialbe; if no, do all the stuff that captain does currently and call git diff itself...

Good idea! Perhaps you can create a branch with a regression test and then provide a rough implementation as a draft? From my experience, the most far I got with for my original problem which I think is here at place (an guess torek relates to - mind: this is native, no Docker), I came most far with having multiple stage modes, one including to not only copy over the worktree but also applying what has been staged (e.g. partial staged files for the commit) to an isolated worktree (under same projects git) to perform hook operations. This is years ago and more up-to date I know you have to take care as well of a file named .git at your projects root and resolve it's content gitdir: <path> if you even want to remotely support containers you mount the project root into with git vcs support. The devil is just in the details. If you think you've found a solution for the problem you face, I'm the last person stopping you file a draft PR.

And if I follow your suggestion correctly, it should also be possible to use a file that is mounted into the container containing the path-names of the changed files instead of using an environment variable for that, as you can NUL-terminate each path inside such a file which should make it more portable. Just my 2 cents.

[ktomk]

And just a second remark on

And now we are back in the original reason, why I switched from local mode to docker mode: since I use PHP8.0 (and sorry - it could also be 7.4 or even NO php) on the host-system, but the composer-setup says we need PHP8.1 - we are not able to call captainhook from the host-system - ether because there is no PHP or because composer tells the version is mismatching - so I MUST call captain in the docker-container...

configure the platform php version in your composer project. composer-install(1) then infers it even if the PHP version you're using to execute composer(1) is not your projects PHP version. This also have the benefit that you document inside your project what the production requirement is. Compare Composer Config Platform, perhaps you can solve it by configuration. Just a guess.

[renky]

First of all: I tested the same situation on a Linux native System (Ubuntu 20.04) and the behaviour is exactly the same.
One File is changed, but not added and the pre-commit-hook skips all hooks via git commit -a

#git status
On branch master
Your branch is up to date with 'origin/master'.

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

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


#git commit -a -m "test"
pre-commit:
 - ./vendor/bin/phpmd {$STAGED_FILES|of-type:php|separated-by:,} ... : skipped
 - ./vendor/bin/phpcs {$STAGED_FILES|of-type:php|separated-by: }     : skipped

so it doesn't depend on wsl - it just depends on the docker-mode.
so conclusion: docker-mode with the current way of handling Staged-Files works great if the files are staged manually, but not with a git commit -a.

For my concrete situation, unfortunatelly platform config doesn't help.

"platform": {
      "php": "8.0"
}

this limits the php version to 8.0 - no higher verison is allowed. Since the developed software is using PHP8.1, this config will not install anything in the target and unfortunatelly not work.

the other idea was

"platform-check": false,

and yes - that was my solution until a few days ago and it worked great... in the Docker-Container PHP8.1 was running, the PHP8.1 code could work, and in the same time the same vendor-Folder could be used by the host-system with PHP8.0. With platform-check false the composer runs up captainhook without any error messages.

But in meanwhile (I used an older version of captainhook until now) one of the captainhook-requirements (symfony/console 6.x) requires PHP8.1 - so if the system runs PHP8.1, an installation of captainhook will automatically select symfony/console 6.x instead of one of the older ones on such a target. And as soon as this is installed, the captain throws the following on the host-system:

./vendor/bin/captainhook install -f -s
PHP Parse error:  syntax error, unexpected token ")" in /home/..../vendor/symfony/console/SignalRegistry/SignalRegistry.php on line 37

thats the original reason why i switched from local to docker-mode....

I already tried to downgrade some components but in the end one of the hard requirement is Laravel 9 in this project - and that also requires symfony/console 6... I still work on a different project with laravel 8 but also php 8.1 and there I can use captainhook in localmode until today wihtout problems.

So now back to solution idea:
I already did some local experiments. I added STAGED_FILES environment-Variable to the hook itself - I extended the docker installer for it:

/**
     * Return the code for the git hook scripts
     *
     * @param  string $hook Name of the hook to generate the sourcecode for
     * @return string
     */
    public function getCode(string $hook): string
    {
        $path2Config = $this->config->getRelativePathFrom($this->repository);
        $config      = $path2Config !== CH::CONFIG ? ' --configuration=' . escapeshellarg($path2Config) : '';
        $envVarLines = [];
        $envVarOpt   = '';

        if ($hook === 'pre-commit') {
            $envVarLines = [
                'STAGED_FILES=`git diff-index --diff-algorithm=myers --no-ext-diff --cached --name-status HEAD`',
                '',
            ];
            $envVarOpt = ' "$STAGED_FILES"';
        }

        $lines = [
            '#!/bin/sh',
            '',
            '# installed by CaptainHook ' . CH::VERSION,
            ...$envVarLines,
            $this->dockerConfig->getDockerCommand() . ' ' . $this->binaryPath . ' hook:' . $hook . $config . $envVarOpt . ' "$@"'
        ];
        return implode(PHP_EOL, $lines) . PHP_EOL;
    }

this results in a pre-commit-hook like that:

#!/bin/sh

# installed by CaptainHook 5.10.11
STAGED_FILES=`git diff-index --diff-algorithm=myers --no-ext-diff --cached --name-status HEAD`

docker exec -t -e XDEBUG_MODE=coverage container-php ./vendor/bin/captainhook hook:pre-commit "$STAGED_FILES" "$@"

additionally I made the hook aware for this new argument:

   /**
     * Configure the command
     *
     * @return void
     */
    protected function configure(): void
    {
        parent::configure();

        $this->addArgument('staged-files', InputArgument::OPTIONAL, 'Staged Files.');
    }

and then I saved this in the hook-PreCommit-runner

/**
     * Fetch the original hook arguments and message related config settings
     *
     * @return void
     */
    public function beforeHook(): void
    {
        $this->staged_files = $this->io->getArgument('staged-files');

        parent::beforeHook();
    }

until here everything works as expected, but now I reached the end of capabilities, since the OfType-Condition and the StagedFiles Command all works directly on a Git-Repository-Object...

How to go on now... if the changes shall all be resided in captainhook (I'd prefer that), then this change seems to get huge now, because somewhere we would have to do the conditional switch - if staged files is set... I think the last place where I have access to it is the HookRunner executeCliAction... or would it make sense to split up and create a new ActionType? or am I on the wrong way with the CliAction and would it better be a PHPAction? Or is it even time for a third type of action? I'm honest I'm still not that familiar with captainhooks source code that I really understand how this all works together...

Somehow Placeholder StagedFiles must receive the staged-files variable, and then there could be a local operation that doesn't need repository... but how to forward the variable from the hook runner to the Condition and Placeholder... ?!

[sebastianfeldmann]

First up thanks for all your debugging and all the effort.

I think before we discuss solutions we have to understand the error a bit better.
So if I get all that correctly this is what happens.

If you run this directly in the git hook (bash script) it works and delivers all files added by git commit -a -m"test" from the temp index.

#!/bin/sh

# installed by CaptainHook 5.10.11
STAGED_FILES=`git diff-index --diff-algorithm=myers --no-ext-diff --cached --name-status HEAD`
...

Can you confirm that STAGED_FILES has the right files in it?

If so, things get pretty confusing, because the Cap'n executes the same command to find all staged files.
That would mean, that executing the same command from the bash script and from PHP concludes in different results.

I think we should figure out why that's case and check if there is a better way to fix the problem and make the result the Cap'n generates more reliable.

The environment variable GIT_INDEX_FILE sounds promising, because it would make sense, if PHP uses the actual index because it does not have access to the environment variable, the result would be empty.
But the bash script somehow has access to the environment variable, so it checks the right index and delivers the right result.

If you could verify that, that would be awesome.
Maybe print the environment variable from the bash script and PHP to check if the results are different.

[sebastianfeldmann]

Maybe looking for a GIT_INDEX_FILE environment variable and adding it to the command would already solve the problem.

$cmd = 'GIT_INDEX_FILE=' . $envGitIndexFile . ' ' . $cmd;

If so I would solve the problem in two steps.
Patch sebastianfeldmann/git so you can provide a GIT_INDEX_FILE value to Git\Repositoryand use it where necessary. And then pass the actualGIT_INDEX_FILEvalue from the Cap'n to theRepository`

[renky]

Can you confirm that STAGED_FILES has the right files in it?

yes -> I also added an echo $STAGED_FILES for test and it contained the right files.

That would mean, that executing the same command from the bash script and from PHP concludes in different results.

No. Bash and PHP in fact show the same content.
But Bash/PHP and "Bash/PHP in Docker" show diffrent content. It's only happening in docker mode
If you'd do the following in a hook (call the same command, once on host, once in docker, but on the same repo and in the hook-script:

#!/bin/sh

git diff-index --diff-algorithm=myers --no-ext-diff --cached --name-status HEAD
docker exec -t -e container-php git diff-index --diff-algorithm=myers --no-ext-diff --cached --name-status HEAD

it would show different content in case of a git commit -a

and the reason why this happens is explained in the SO answer from torek

about Git index file - I don't have a clue how to test this... But if it works like you mentioned, then it seems to be a very straight forward solution for the problem.

[sebastianfeldmann]

Could you try to add some debug output that echos the GIT_INDEX_FILE environment variable?

[renky]

Could you try to add some debug output that echos the GIT_INDEX_FILE environment variable?

I added echo $STAGED_FILES and $GIT_INDEX_FILE to the hook-script and get the following (one file staged, one file not):

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

Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
        modified:   app/Http/Controllers/Controller.php

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:   composer.lock

#git commit -a -m "test"
STAGED_FILES:
M       app/Http/Controllers/Controller.php
M       composer.lock

GIT_INDEX_FILE:
/home/..../.git/index.lock
pre-commit: 
...

so it only contains the name of the file - if this is forwarded to the docker container the path would have to be translated somehow... nevertheless for just testing if it works, this file (./.git/index.lock) might be used in the docker-container?

[renky]

@sebastianfeldmann does my last comment help somehow?

[ktomk]

@renky it sounds as-if it could already work by just mounting the file as the containers working directory is the project directory already.

[renky]

Maybe I didn't get the point...

I tried the following now:
captainhook.json

"config": {
        "verbosity": "verbose",
        "run-mode": "docker",
        "run-exec": "docker exec -t -w $GIT_INDEX_FILE container-php"
    },

in my hook script it looks good:

#!/bin/sh
  
# installed by CaptainHook 5.10.11

docker exec -t -w $GIT_INDEX_FILE container-php ./vendor/bin/captainhook hook:pre-commit "$@"

but when I commit now I get an error:

#git commit -a -m "test"
OCI runtime exec failed: exec failed: unable to start container process: chdir to cwd ("/home/user/project/.git/index.lock") set in config.json failed: no such file or directory: unknown

Did I miss anything?

The error message seems logically to me, since there is no place inside the container like this /home/user/project/.git/index.lock

[renky]

ooooookkkkkkkk.....

after again reading toreks answer on SO and reading your answers here, I got the point...

Finally I dont think that a docker run -v {workingdir}:{workingdir} is very convenient, since you always try to use a docker-compose file and nginx-config, that works for everybody in your project - and I guess everybody uses different working-dirs...

so only forwarding GIT_INDEX_FILE to docker requires the same directory structure in the container...

that's why I now tried this hook:

#!/bin/sh
  
# installed by CaptainHook 5.10.11

docker exec -t -e GIT_INDEX_FILE=/var/www/.git/index.lock container-php ./vendor/bin/captainhook hook:pre-commit "$@"

and voila - it detects all files correctly with commit -a

unfortunatelly this doesn't work any more with normal git commit - so after git add, a git commit doesn't see this file - because index.lock doesn't exist... totally clear...

so my idea is a replacement now...

docker exec -t -e GIT_INDEX_FILE="${GIT_INDEX_FILE/${PWD}/\/var\/www}" container-php ./vendor/bin/captainhook hook:pre-commit "$@"

and this works fine now with git commit -a and git commit
and it even works with git commit --only

so this is the solution that works for me now - unfortunatelly the substitution above needs bash - so I had to edit the hook manually.... so it is not the final solution at all :)

the big question is now - how to deal with that? Is it really the right way to let the user of captainhook with this alone? figuring out this issue here and find my solution now?

I'd prefer a better way, if captainhook can do this for us in docker-mode...
a suggestion would be that we extend the docker-mode-settings
unfortunatelly until now the run-exec looks something like this:
docker exec ...... container
and we need to add this -e somewhere inbetween.
so would it be more convenient to open up a config-sub-value like "docker"?

like this:

"config": {
        "verbosity": "verbose",
        "run-mode": "docker",
        "docker": {
            "container": "php-container",
            "workdir": "/var/www",
            "attributes": "" / "-e myothervar=somvalue -otherAttributesForDocker"
        }
    },

if run-exec is set, run exec wins, but if not, docker-config is taken and this leads to the hook I wrote:

#!/bin/bash <---------- important  - or if there is a sh-expert out there - how to substitute in sh?

# installed by CaptainHook 5.10.11

docker exec -t [attributes] -e GIT_INDEX_FILE="${GIT_INDEX_FILE/${PWD}/[workdir]}" [container] ./vendor/bin/captainhook hook:pre-commit "$@"

[ktomk]

@renky for your previous comment already:

This looks wrong to me: -w $GIT_INDEX_FILE: it sets the working directory. we want just the environment parameter (IIRC the options is -e GIT_INDEX_FILE, or more explicit -e "GIT_INDEX_FILE=$GIT_INDEX_FILE").

Nevertheless the error message shows another problem:

"/home/user/project/.git/index.lock"

So the wrong option (-w) revealed that GIT_INDEX_FILE contains an absolute path. I'm somewhat sure this would not work this way. Instead:

"run-exec": "docker exec -t -e "GIT_INDEX_FILE=/docker/.git/index.lock" container-php"

Give it a try.

/edit: some remarks:

1. replace /docker/ with the actual project root path inside the container (e.g. if you changed it, /docker/ has been taken from the projects' dockerfiles).
2. the project incl. the work-tree and the .git folder need to be there, too, inside the container. I was under the impression this is already the case, but perhaps it is not (different configuration or me just wrong).

[ktomk]

#!/bin/bash <---------- important - or if there is a sh-expert out there - how to substitute in sh?

#! /bin/bash-> #! /bin/sh then check the script is using only portable syntax (e.g. no bashism).

[renky]

ok, - there it is:

#!/bin/sh

# installed by CaptainHook 5.10.11

docker exec -t -e GIT_INDEX_FILE="/var/www/.git/$(basename $GIT_INDEX_FILE)" container-php ./vendor/bin/captainhook hook:pre-commit "$@"

nevertheless - it would be more convenient if captainhook gives a hint of doing that, or even adds the
-e GIT_INDEX_FILE="/var/www/.git/$(basename $GIT_INDEX_FILE)" on its own... with /var/www dynamically created by e.g. config...

[ktomk]

Sure, it is always more convenient when problems are already solved - so they won't surface ;) - but for that the problems need to be known, and see your code - what if $GIT_INDEX_FILE ain't set etc.: it needs to settle a bit before good instructions can be given.

But indeed it would make sense if this is a reliable way to support GIT_INDEX_FILE to add it to the configuration.

Can you draft a PR?

[renky]

as far as I understood torek, GIT_INDEX_FILE is always set on commit - but I can be wrong...
I understood that GIT_INDEX_FILE might be also set by the user before and then this is used - in all other cases git always uses .git/index as default - and also sets this variable for following git commands...!? i added an echo to the hook and the variable had always a content... nevertheless i also tryed a bad value (for coincidence for sure :) ) because my substituion didn't work and it figured out that git seems to look for the file in GIT_INDEX_FILE and if this is set but not there it uses .git/index

so I mean - it should work in every case...

[ktomk]

@renky nice tests. but nevertheless it is possible to add some precautions (for which I can offer help when drafting a PR and in review). Do you think it is possible to bring this into the project in some form? Do you see any opportunity to draft a PR? Do you need any kind of support or feedback for that? Please share your suggestions on what you think would be good to go on.

[sebastianfeldmann]

I'm not sure this is something that can be handled by the Cap'n
Because of path mappings between the host and the container.
There is no way for the Cap'n to know the directory structure within the container.
One way would be to force users to do it in a very specific way, but I'm not comfortable with such an approach.

Since the Cap'n allows to configure the whole docker command the only way I see to provide any help is to parse the command if -e ... is present and issue a warning of some kind that if you do not specify the GIT_INDEX_FILE env var certain git commands like git commit -a might fail.

Further more the documentation should make very clear that you have to do this yourself and offer some best practices on how to solve the issue.

[renky]

Two totally different approaches... What @sebastianfeldmann says is totally valid - it can be handled by documentation.

But I also think the way to force users when captain would solve the problem, might not be sooooo very specific... Since docker commands to call the captain inside, aren't rocket-science in my opinion... in the end it will allways be a "docker exec" Command - this will imho also use often or even almost the -t Attribute - and I also think never the -i attribute since the idea is to call everything automatically. In the end we also need always a container name and we all know, that captain adds the captain-executable to the end of this command - so at this time it's already not completeley configurable...

The only reason why we cannot add the -e GIT_INDEX_FILE already now automatically is beacuse it has to be added in front of the containername, which is currently (imho) always the last word in the docker-command in run-exec...

so I think the "flexibility" of the "full" docker-command isn't given really already now.

From my point of view in almost all cases where I use the docker command right now, it would be totally fine if captain would only allow one single parameter called "container-name" and would setup the full command itself... there are a few situations where I want to add more environment-variables... but this can also be handled by config.

So I really think it is an option to add docker-specific config-parameters...

suggestion:

{
  "config": {
    "run-mode": "docker",
    "docker-container": "CONTAINER_NAME",
    "docker-attributes": "-t -e my docker attributes", (<-- could also default to -t)
  }
}

or second option

{
  "config": {
    "run-mode": "docker",
    "docker": {
        "container": "CONTAINER_NAME",
        "attributes": "-t -e my docker attributes", (<-- could also default to -t)
    }
  }
}

in both cases (the latter suits better to me) there must be a fallback to "run-exec" that will be still available and would be there for fallback reasons.

If docker-sub-config is given, it will result in the above case in a command like this:
docker exec -t -e my docker attributes -e GIT_INDEX_FILE="/var/www/.git/$(basename $GIT_INDEX_FILE)" CONTAINER_NAME ....

@ktomk yes I can prepare a PR - since it isn't very urgent any more, I'll do that in a more smooth situation as now...