Skip to content

Commit bfdf909

Browse files
sebastianfeldmannsebastianfeldmann
committed
Optimize docker command
Make sure that docker exec is run interactively, forward the tty if necessary and will forward git env vars.
1 parent 8417dbe commit bfdf909

File tree

3 files changed

+83
-10
lines changed

3 files changed

+83
-10
lines changed

‎src/Hook/Template/Docker.php

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,7 @@
1616
use CaptainHook\App\CH;
1717
use CaptainHook\App\Config;
1818
use CaptainHook\App\Hook\Template;
19-
use CaptainHook\App\Hook\Template\Docker\RunConfig as DockerConfig;
20-
use SebastianFeldmann\Camino\Path\Directory;
21-
use SebastianFeldmann\Camino\Path\File;
19+
use CaptainHook\App\Hooks;
2220

2321
/**
2422
* Docker class
@@ -78,13 +76,14 @@ public function getCode(string $hook): string
7876
$path2Config = $this->pathInfo->getConfigPath();
7977
$config = $path2Config !== CH::CONFIG ? ' --configuration=' . escapeshellarg($path2Config) : '';
8078
$bootstrap = !empty($this->config->getBootstrap()) ? ' --bootstrap=' . $this->config->getBootstrap() : '';
79+
$tty = Hooks::allowsUserInput($hook) ? 'exec < /dev/tty' : '';
8180

8281
$lines = [
8382
'#!/bin/sh',
84-
'',
83+
$tty,
8584
'# installed by CaptainHook ' . CH::VERSION,
8685
'',
87-
$this->config->getRunExec() . ' '
86+
$this->getOptimizeDockerCommand($hook) . ' '
8887
. $this->binaryPath . ' hook:' . $hook
8988
. $config
9089
. $bootstrap
@@ -93,6 +92,37 @@ public function getCode(string $hook): string
9392
return implode(PHP_EOL, $lines) . PHP_EOL;
9493
}
9594

95+
96+
/**
97+
* Returns the optimized docker exec command
98+
*
99+
* This tries to optimize the `docker exec` commands. Docker exec should always run in --interactive mode.
100+
* During hooks that could need user input it should use --tty.
101+
* In case of `commit -a` we have to pass the GIT_INDEX_FILE env variable so `git` inside the container
102+
* can recognize the temp index.
103+
*
104+
* @param string $hook
105+
* @return string
106+
*/
107+
private function getOptimizeDockerCommand(string $hook): string
108+
{
109+
$command = $this->config->getRunExec();
110+
$position = strpos($command, 'docker exec');
111+
// add interactive and tty flags if docker exec is used
112+
if ($position !== false) {
113+
$endExec = $position + 11;
114+
$executable = substr($command, 0, $endExec);
115+
$options = substr($command, $endExec);
116+
$tty = Hooks::allowsUserInput($hook) ? ' -t' : '';
117+
118+
$interactive = !preg_match('# -[a-z]*i| --interactive#', $options) ? ' -i' : '';
119+
$useTTY = !preg_match('# -[a-z]*t| --tty#', $options) ? $tty : '';
120+
$env = !preg_match('# (-[a-z]*e|--env)[= ]+GIT_INDEX_FILE#', $options) ? ' -e GIT_INDEX_FILE' : '';
121+
$command = trim($executable) . $interactive . $useTTY . $env . ' ' . trim($options);
122+
}
123+
return $command;
124+
}
125+
96126
/**
97127
* Resolves the path to the captainhook binary and returns it
98128
*

‎tests/unit/Hook/Template/BuilderTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public function testBuildDockerTemplate(): void
5555

5656
$code = $template->getCode('pre-commit');
5757
$this->assertStringContainsString('pre-commit', $code);
58-
$this->assertStringContainsString('docker exec captain-container', $code);
58+
$this->assertStringContainsString('docker exec -i -e GIT_INDEX_FILE captain-container', $code);
5959
$this->assertStringContainsString('vendor/bin/captainhook', $code);
6060
}
6161

@@ -88,7 +88,7 @@ public function testBuildDockerTemplateWithBinaryOutsideRepo(): void
8888

8989
$this->assertInstanceOf(Docker::class, $template);
9090
$this->assertStringContainsString('pre-commit', $code);
91-
$this->assertStringContainsString('docker exec captain-container', $code);
91+
$this->assertStringContainsString('docker exec -i -e GIT_INDEX_FILE captain-container', $code);
9292
$this->assertStringContainsString($executable, $code);
9393
}
9494

‎tests/unit/Hook/Template/DockerTest.php

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public function testTemplateCaptainHookDevelopment(): void
3636
$code = $template->getCode('commit-msg');
3737

3838
$this->assertStringContainsString('#!/bin/sh', $code);
39-
$this->assertStringContainsString('docker exec cap-container', $code);
39+
$this->assertStringContainsString('docker exec -i -e GIT_INDEX_FILE cap-container', $code);
4040
$this->assertStringContainsString('./bin/captainhook', $code);
4141
}
4242

@@ -58,7 +58,7 @@ public function testTemplateCaptainHookAsLibrary(): void
5858
$code = $template->getCode('commit-msg');
5959

6060
$this->assertStringContainsString('#!/bin/sh', $code);
61-
$this->assertStringContainsString('docker exec cap-container', $code);
61+
$this->assertStringContainsString('docker exec -i -e GIT_INDEX_FILE cap-container', $code);
6262
$this->assertStringContainsString('./vendor/bin/captainhook', $code);
6363
}
6464

@@ -81,8 +81,51 @@ public function testTemplateCustomPath(): void
8181
$code = $template->getCode('commit-msg');
8282

8383
$this->assertStringContainsString('#!/bin/sh', $code);
84-
$this->assertStringContainsString('docker exec cap-container', $code);
84+
$this->assertStringContainsString('docker exec -i -e GIT_INDEX_FILE cap-container', $code);
8585
$this->assertStringContainsString('./foo/captainhook', $code);
8686
$this->assertStringContainsString('bootstrap=vendor/autoload.php', $code);
8787
}
88+
89+
/**
90+
* Tests Docker::getCode
91+
*
92+
* @dataProvider replacementPossibilities
93+
*/
94+
public function testDockerCommandOptimization(string $exec, string $expected, string $msg): void
95+
{
96+
$pathInfo = $this->createMock(PathInfo::class);
97+
$pathInfo->method('getExecutablePath')->willReturn('./vendor/bin/captainhook');
98+
$pathInfo->method('getConfigPath')->willReturn('captainhook.json');
99+
100+
$configMock = $this->createConfigMock(false, 'captainhook.json');
101+
$configMock->method('getBootstrap')->willReturn('');
102+
$configMock->method('getRunExec')->willReturn('docker exec ' . $exec);
103+
$configMock->method('getRunPath')->willReturn('/usr/local/bin/captainhook');
104+
105+
$template = new Docker($pathInfo, $configMock);
106+
$code = $template->getCode('prepare-commit-msg');
107+
108+
$this->assertStringContainsString('docker exec ' . $expected, $code, $msg);
109+
$this->assertStringContainsString('/usr/local/bin/captainhook', $code);
110+
}
111+
112+
/**
113+
* The testDockerCommandOptimization data provider
114+
*
115+
* @return array
116+
*/
117+
public function replacementPossibilities(): array
118+
{
119+
return [
120+
['cap-container', '-i -t -e GIT_INDEX_FILE cap-container', 'none'],
121+
['-it cap-container', '-e GIT_INDEX_FILE -it cap-container', '-it'],
122+
['-ti cap-container', '-e GIT_INDEX_FILE -ti cap-container', '-ti'],
123+
['--interactive --tty cap-container', '-e GIT_INDEX_FILE --interactive --tty cap-container', 'long it'],
124+
['--tty --interactive cap-container', '-e GIT_INDEX_FILE --tty --interactive cap-container', 'long ti'],
125+
['--tty cap-container', '-i -e GIT_INDEX_FILE --tty cap-container', 'no i'],
126+
['-xit cap-container', '-e GIT_INDEX_FILE -xit cap-container', 'prefixed i'],
127+
['-xite=GIT_INDEX_FILE cap-container', '-xite=GIT_INDEX_FILE cap-container', 'prefixed e'],
128+
['--env=GIT_INDEX_FILE cap-container', '-i -t --env=GIT_INDEX_FILE cap-container', 'long e'],
129+
];
130+
}
88131
}

0 commit comments

Comments
 (0)