Skip to content

Commit d1aedfb

Browse files
sebastianfeldmannsebastianfeldmann
authored
Merge pull request #226 from feature/docker-tty
Add some necessary onions to docker exec
2 parents 467cfe7 + 9d4dd18 commit d1aedfb

File tree

6 files changed

+117
-22
lines changed

6 files changed

+117
-22
lines changed

‎src/Hook/Template/Docker.php

Lines changed: 39 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,41 @@ 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+
117+
// Because this currently breaks working with Jetbrains IDEs I will deactivate it for now
118+
//
119+
// $tty = Hooks::allowsUserInput($hook) ? ' -t' : '';
120+
// $useTTY = !preg_match('# -[a-z]*t| --tty#', $options) ? $tty : '';
121+
122+
$useTTY = '';
123+
$interactive = !preg_match('# -[a-z]*i| --interactive#', $options) ? ' -i' : '';
124+
$env = !preg_match('# (-[a-z]*e|--env)[= ]+GIT_INDEX_FILE#', $options) ? ' -e GIT_INDEX_FILE' : '';
125+
$command = trim($executable) . $interactive . $useTTY . $env . ' ' . trim($options);
126+
}
127+
return $command;
128+
}
129+
96130
/**
97131
* Resolves the path to the captainhook binary and returns it
98132
*

‎src/Hook/Template/Local/Shell.php

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,13 @@
1515

1616
use CaptainHook\App\CH;
1717
use CaptainHook\App\Hook\Template;
18+
use CaptainHook\App\Hooks;
1819
use SebastianFeldmann\Camino\Path;
1920
use SebastianFeldmann\Camino\Path\Directory;
2021
use Symfony\Component\Process\PhpExecutableFinder;
2122

2223
/**
23-
* Local class
24+
* Shell class
2425
*
2526
* Generates the sourcecode for the php hook scripts in .git/hooks/*.
2627
*
@@ -31,15 +32,6 @@
3132
*/
3233
class Shell extends Template\Local
3334
{
34-
/**
35-
* Does the hook allow user input
36-
*
37-
* @var bool[]
38-
*/
39-
private array $allowUserInput = [
40-
'prepare-commit-msg' => true
41-
];
42-
4335
/**
4436
* Returns lines of code for the local src installation
4537
*
@@ -51,7 +43,7 @@ protected function getHookLines(string $hook): array
5143
$useStdIn = ' <&0';
5244
$useTTY = [];
5345

54-
if (isset($this->allowUserInput[$hook])) {
46+
if (Hooks::allowsUserInput($hook)) {
5547
$useStdIn = '';
5648
$useTTY = [
5749
'if [ -t 1 ]; then',

‎src/Hooks.php

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,21 @@ final class Hooks
5151
*
5252
* @var string[]
5353
*/
54-
private static $virtualHookTriggers = [
54+
private static array $virtualHookTriggers = [
5555
self::POST_CHECKOUT => self::POST_CHANGE,
5656
self::POST_MERGE => self::POST_CHANGE,
5757
self::POST_REWRITE => self::POST_CHANGE,
5858
];
5959

60+
/**
61+
* Is it necessary to give the Captain access to user input
62+
*
63+
* @var array<string, bool>
64+
*/
65+
private static array $hooksAllowingUserInput = [
66+
self::PREPARE_COMMIT_MSG => true,
67+
];
68+
6069
/**
6170
* Returns the list of valid hooks
6271
*
@@ -137,6 +146,17 @@ public static function getOriginalHookArguments(string $hook): string
137146
return $arguments[$hook];
138147
}
139148

149+
/**
150+
* Does a given hook allow for user input to be used
151+
*
152+
* @param string $hook
153+
* @return bool
154+
*/
155+
public static function allowsUserInput(string $hook): bool
156+
{
157+
return self::$hooksAllowingUserInput[$hook] ?? false;
158+
}
159+
140160
/**
141161
* Tell if the given hook should trigger a virtual hook
142162
*

‎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 -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 --env=GIT_INDEX_FILE cap-container', 'long e'],
129+
];
130+
}
88131
}

‎tests/unit/HooksTest.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,4 +63,10 @@ public function testGetVirtualHookFail(): void
6363
$this->expectException(Exception::class);
6464
Hooks::getVirtualHook('pre-commit');
6565
}
66+
67+
public function testAllowsUserInput(): void
68+
{
69+
$this->assertTrue(Hooks::allowsUserInput(Hooks::PREPARE_COMMIT_MSG));
70+
$this->assertFalse(Hooks::allowsUserInput(Hooks::PRE_COMMIT));
71+
}
6672
}

0 commit comments

Comments
 (0)