Skip to content

Commit 8185b46

Browse files
committed
Print install download progress and handle indentation inside installer plugin
Instead of having our caller re-write the "nice output" file descriptor to achieve the desired indent level, we print it with the correct indent in the first place. With this, we can also add a progress meter that is useful e.g. in local CNB installs over slow network connections, as the downloads take longer than the installs, meaning a build might hang for quite some time without and progress output, until, suddenly, the names of the packages installed are printed in fairly quick succession. This progress meter is disabled in the classic buildpack, because Heroku Dashboard currently trips over the ANSI cursor control codes and ends up swallowing quite a lot of lines. GUS-W-18506658
1 parent 3c8ee49 commit 8185b46

7 files changed

+166
-36
lines changed

bin/compile

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -429,12 +429,14 @@ status "Installing platform packages..."
429429
export export_file_path=$bp_dir/export
430430
export profile_dir_path=$build_dir/.profile.d
431431
export providedextensionslog_file_path=$(mktemp -t "provided-extensions.log.XXXXXX" -u)
432-
# we make a new file descriptor for the installer plugin to log "human readable" display output to, duplicated to stdout (via indent)
433-
exec {PHP_PLATFORM_INSTALLER_DISPLAY_OUTPUT_FDNO}> >(indent)
432+
# we make a new file descriptor for the installer plugin to log "human readable" display output to, duplicated to stdout
433+
exec {PHP_PLATFORM_INSTALLER_DISPLAY_OUTPUT_FDNO}>&1
434434
# the installer picks up the FD number in this env var, and writes a "display output" log to it
435435
# meanwhile, the "raw" output of 'composer install' goes to install.log in case we need it later
436436
export PHP_PLATFORM_INSTALLER_DISPLAY_OUTPUT_FDNO
437-
if composer install -d "$build_dir/.heroku/php" ${HEROKU_PHP_INSTALL_DEV-"--no-dev"} > $build_dir/.heroku/php/install.log 2>&1; then
437+
export PHP_PLATFORM_INSTALLER_DISPLAY_OUTPUT_INDENT=7
438+
# NO_COLOR=1 suppresses the download progress meter whose ANSI cursor codes trip up output in Heroku Dashboard
439+
if NO_COLOR=1 composer install -d "$build_dir/.heroku/php" ${HEROKU_PHP_INSTALL_DEV-"--no-dev"} > $build_dir/.heroku/php/install.log 2>&1; then
438440
:
439441
else
440442
code=$?
@@ -528,18 +530,16 @@ if [[ -s "$providedextensionslog_file_path" ]]; then
528530
for extension in "${provides[@]:1}"; do # remaining words in line are native extensions it declares "provide"d
529531
ext_package=$(cut -d":" -f1 <<< "$extension") # extract name from "ext-foo:1.2.3" string
530532
ext_name=${ext_package#"heroku-sys/"} # no "heroku-sys/" prefix for human output
531-
echo -n "- ${ext_name}... " | indent
532533
# run a `composer require`, confined to the package we're attempting, allowing any version
533534
# we're appending to install.log using tee, and using another new "display output" FD to redirect to a new single-step log (tee truncates for us)
534535
# this log is checked later to see if a package install actually happened, or if it was already enabled (since both exit status 0)
535-
# we're letting the indents begin the line with a \r character, so that the above `echo -n` gets overwritten
536-
exec {PHP_PLATFORM_INSTALLER_DISPLAY_OUTPUT_FDNO}> >(tee "$current_install_log" | indent "" $'\r ')
537-
if composer require -d "$build_dir/.heroku/php" "${ext_package}.native:*" >> $build_dir/.heroku/php/install.log 2>&1; then
536+
exec {PHP_PLATFORM_INSTALLER_DISPLAY_OUTPUT_FDNO}> >(tee "$current_install_log")
537+
# NO_COLOR=1 suppresses the download progress meter whose ANSI cursor codes trip up output in Heroku Dashboard
538+
if NO_COLOR=1 composer require -d "$build_dir/.heroku/php" "${ext_package}.native:*" >> $build_dir/.heroku/php/install.log 2>&1; then
538539
# package added successfully
539-
[[ -s "$current_install_log" ]] || { echo -e "- ${ext_name} (already enabled)" | indent "" $'\r '; } # if nothing was actually installed above, that means the dependency was already satisfied (i.e. extension is statically built into PHP); we need to echo something to that effect
540+
[[ -s "$current_install_log" ]] || { echo "- ${ext_name} (already enabled)" | indent; } # if nothing was actually installed above, that means the dependency was already satisfied (i.e. extension is statically built into PHP); we need to echo something to that effect
540541
else
541542
# composer did not succeed; this means no package that matches all existing package's requirements was found
542-
echo -n -e "\r" # reset the line
543543
notice_inline "no suitable native version of ${ext_name} available"
544544
fi
545545
exec {PHP_PLATFORM_INSTALLER_DISPLAY_OUTPUT_FDNO}>&-
@@ -549,7 +549,7 @@ if [[ -s "$providedextensionslog_file_path" ]]; then
549549
exec {fd_num}>&-
550550
fi
551551

552-
unset PHP_PLATFORM_INSTALLER_DISPLAY_OUTPUT_FDNO
552+
unset PHP_PLATFORM_INSTALLER_DISPLAY_OUTPUT_FDNO PHP_PLATFORM_INSTALLER_DISPLAY_OUTPUT_INDENT
553553

554554
# log number of installed platform packages
555555
mmeasure "platform.count" $(composer show -d "$build_dir/.heroku/php" --installed "heroku-sys/*" 2> /dev/null | wc -l)

support/installer/src/ComposerInstallerPlugin.php

Lines changed: 80 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,12 @@
55
use Composer\Composer;
66
use Composer\Factory;
77
use Composer\IO\{IOInterface, ConsoleIO, NullIO};
8-
use Symfony\Component\Console\Formatter\OutputFormatter;
9-
use Symfony\Component\Console\Helper\HelperSet;
8+
use Symfony\Component\Console\Helper\{HelperSet,ProgressBar};
109
use Symfony\Component\Console\Input\ArrayInput;
1110
use Symfony\Component\Console\Output\StreamOutput;
12-
use Composer\Plugin\PluginInterface;
11+
use Composer\Plugin\{PluginEvents,PluginInterface,PreFileDownloadEvent,PostFileDownloadEvent};
1312
use Composer\EventDispatcher\EventSubscriberInterface;
14-
use Composer\Installer\PackageEvent;
15-
use Composer\Installer\PackageEvents;
13+
use Composer\Installer\{PackageEvent,PackageEvents};
1614
use Composer\Package\PackageInterface;
1715
use Composer\Util\Filesystem;
1816

@@ -35,17 +33,36 @@ class ComposerInstallerPlugin implements PluginInterface, EventSubscriberInterfa
3533

3634
protected $allPlatformRequirements = null;
3735

36+
protected $progressBar;
37+
3838
public function activate(Composer $composer, IOInterface $io)
3939
{
4040
$this->composer = $composer;
4141
$this->io = $io;
4242

43+
// we were supplied with a file descriptor to write "display output" to
44+
// this can be used by a calling buildpack to get a clean progress bar for downloads, followed by a list of package installs as they happen
45+
// for this, we make a ConsoleIO instance to be passed to the downloaders for install() output, and a progress bar for our download event listeners
4346
if($fdno = getenv("PHP_PLATFORM_INSTALLER_DISPLAY_OUTPUT_FDNO")) {
44-
$styles = Factory::createAdditionalStyles();
45-
$formatter = new OutputFormatter(false, $styles);
47+
// a new <indent> tag that can be used in output to prefix a line using the specified indentation
48+
// this way the progress bar, downloaders, etc, do not have to handle the indentation each
49+
$styles = [
50+
'indent' => new IndentedOutputFormatterStyle(intval(getenv('PHP_PLATFORM_INSTALLER_DISPLAY_OUTPUT_INDENT')))
51+
];
52+
// special formatter that ignores colors if false is passed as first arg, we want that initially
53+
$formatter = new NoColorsOutputFormatter(false, $styles);
4654
$input = new ArrayInput([]);
4755
$input->setInteractive(false);
48-
$output = new StreamOutput(fopen("php://fd/{$fdno}", "w"), StreamOutput::VERBOSITY_NORMAL, null, $formatter);
56+
// obey NO_COLOR to control whether or not we want a progress bar
57+
// (unfortunately, we cannot get e.g. the --no-progress or --no-ansi options from the Composer command invocation)
58+
$output = new StreamOutput(fopen("php://fd/{$fdno}", "w"), StreamOutput::VERBOSITY_NORMAL, !getenv('NO_COLOR'), $formatter);
59+
if($output->isDecorated()) {
60+
$this->progressBar = new ProgressBar($output);
61+
$progressBarFormat = ProgressBar::getFormatDefinition('normal');
62+
$this->progressBar->setFormat(sprintf("<indent>Downloaded%s</indent>", $progressBarFormat));
63+
}
64+
// we force ANSI output to on here for the indentation output formatter style to work
65+
$output->setDecorated(true);
4966
$this->displayIo = new ConsoleIO($input, $output, new HelperSet());
5067
} else {
5168
$this->displayIo = new NullIO();
@@ -102,7 +119,61 @@ public function uninstall(Composer $composer, IOInterface $io)
102119

103120
public static function getSubscribedEvents()
104121
{
105-
return [PackageEvents::POST_PACKAGE_INSTALL => 'onPostPackageInstall'];
122+
return [
123+
PluginEvents::PRE_FILE_DOWNLOAD => 'onPreFileDownload',
124+
PluginEvents::POST_FILE_DOWNLOAD => 'onPostFileDownload',
125+
PackageEvents::PRE_PACKAGE_INSTALL => 'onPrePackageInstall',
126+
PackageEvents::POST_PACKAGE_INSTALL => 'onPostPackageInstall',
127+
];
128+
}
129+
130+
// this fires when a download starts
131+
public function onPreFileDownload(PreFileDownloadEvent $event)
132+
{
133+
$package = $event->getContext();
134+
if($event->getType() != 'package' || $package->getDistType() != 'heroku-sys-tar') {
135+
return;
136+
}
137+
// downloads happen in parallel, so marking progress here already would be useless
138+
// but we can set the number of expected downloads on the progress bar, and start it
139+
if($this->progressBar && !$this->progressBar->getMaxSteps()) {
140+
// we can get the number of total downloads from the current event
141+
// it's the package that fired this event, and it has a LockedArrayRepository of all the packages that will be installed
142+
// from that, we go find all the packages that have a dist type of "heroku-sys-tar" and count them
143+
$totalDownloadCount = array_reduce($package->getRepository()->getPackages(), function($carry, $item) { return $carry + intval($item->getDistType() == 'heroku-sys-tar'); }, 0);
144+
$this->progressBar->clear(); // in case our caller printed something on the same line, before the install
145+
$this->progressBar->start($totalDownloadCount);
146+
}
147+
}
148+
149+
// this fires when a download finishes, so we can output progress info
150+
// (Downloader::download returns a promise for parallel downloads, so would be as useless as onPreFileDownload above)
151+
public function onPostFileDownload(PostFileDownloadEvent $event)
152+
{
153+
if($event->getType() != 'package' || $event->getContext()->getDistType() != 'heroku-sys-tar') {
154+
return;
155+
}
156+
if($this->progressBar) {
157+
$this->progressBar->advance(); // this will re-draw for us
158+
}
159+
}
160+
161+
// this fires for every package install
162+
// nothing to do for us here except to clear a progress bar if it exists
163+
public function onPrePackageInstall(PackageEvent $event)
164+
{
165+
// clear our progress bar, since we're done with downloads
166+
// the actual package installs are printed via the Downloaders, just like Composer does it
167+
if($this->progressBar) {
168+
if($this->displayIo->isDecorated()) {
169+
// display output is ANSI capable, we can clear the progress bar
170+
$this->progressBar->clear();
171+
} else {
172+
// display output is not ANSI capable, we need a line break after the progress bar
173+
$this->displayIo->write("");
174+
}
175+
$this->progressBar = null;
176+
}
106177
}
107178

108179
public function onPostPackageInstall(PackageEvent $event)

support/installer/src/Downloader.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@ public function install(PackageInterface $package, string $path, bool $output =
5959
$fn = str_replace('/', '$', $package->getPrettyName());
6060
$marker = "$path/$fn.extracted";
6161
$this->addCleanupPath($package, $marker);
62-
$this->displayIo->write(sprintf('- <info>%s</info> (<comment>%s</comment>)', ComposerInstaller::formatHerokuSysName($package->getPrettyName()), $package->getFullPrettyVersion()));
62+
# our indent style can't be nested together with other styling tags
63+
$this->displayIo->write(sprintf("<indent>-</indent> <info>%s</info> (<comment>%s</comment>)", ComposerInstaller::formatHerokuSysName($package->getPrettyName()), $package->getFullPrettyVersion()));
6364
return parent::install($package, $path, $output);
6465
}
6566
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php
2+
3+
namespace Heroku\Buildpack\PHP;
4+
5+
use Symfony\Component\Console\Formatter\OutputFormatterStyle;
6+
7+
class IndentedOutputFormatterStyle extends OutputFormatterStyle
8+
{
9+
private $prefix;
10+
11+
public function __construct(int $indent = 0, ?string $foreground = null, ?string $background = null, array $options = [])
12+
{
13+
$this->prefix = str_repeat(" ", $indent);
14+
parent::__construct($foreground, $background, $options);
15+
}
16+
17+
public function apply(string $text)
18+
{
19+
return sprintf("%s%s", $this->prefix, $text);
20+
}
21+
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
3+
namespace Heroku\Buildpack\PHP;
4+
5+
use \Symfony\Component\Console\Formatter\{NullOutputFormatterStyle,OutputFormatter,OutputFormatterStyle,OutputFormatterStyleStack};
6+
7+
class NoColorsOutputFormatter extends OutputFormatter
8+
{
9+
public function __construct(bool $decorated = false, array $styles = [])
10+
{
11+
// this will set up the default styles below
12+
parent::__construct($decorated, $styles);
13+
14+
if(!$decorated) {
15+
// no "decoration", i.e. no ANSI stuff, so we reset the defaults
16+
$this->setStyle('error', new NullOutputFormatterStyle());
17+
$this->setStyle('info', new NullOutputFormatterStyle());
18+
$this->setStyle('comment', new NullOutputFormatterStyle());
19+
$this->setStyle('question', new NullOutputFormatterStyle());
20+
}
21+
}
22+
}

support/installer/src/NoopDownloader.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,8 @@ public function prepare(string $type, PackageInterface $package, string $path, ?
4040

4141
public function install(PackageInterface $package, string $path): PromiseInterface
4242
{
43-
$this->displayIo->write(sprintf("- %s", ($this->humanMessageFormatter)($package, $path)));
43+
# our indent style can't be nested together with other styling tags
44+
$this->displayIo->write(sprintf("<indent>-</indent> %s", ($this->humanMessageFormatter)($package, $path)));
4445
$this->io->writeError(sprintf(" - %s", ($this->installMessageFormatter)($package, $path)));
4546
return \React\Promise\resolve(null);
4647
}

test/spec/platform_spec.rb

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,15 @@
9393
@profiled_tmpdir = Dir.mktmpdir("profile.d")
9494
FileUtils.cp("#{generator_fixtures_subdir}/base/expected_platform_composer.json", "#{@install_tmpdir}/composer.json")
9595
Dir.chdir(@install_tmpdir) do
96-
cmd = "exec {PHP_PLATFORM_INSTALLER_DISPLAY_OUTPUT_FDNO}> #{@humanlog_tmpfile.path}; export PHP_PLATFORM_INSTALLER_DISPLAY_OUTPUT_FDNO; export_file_path=#{@export_tmpfile.path} profile_dir_path=#{@profiled_tmpdir} composer install --no-dev"
96+
cmd = <<~EOF
97+
exec {PHP_PLATFORM_INSTALLER_DISPLAY_OUTPUT_FDNO}> #{Shellwords.escape(@humanlog_tmpfile.path)}
98+
export PHP_PLATFORM_INSTALLER_DISPLAY_OUTPUT_FDNO
99+
export PHP_PLATFORM_INSTALLER_DISPLAY_OUTPUT_INDENT=4
100+
export export_file_path=#{Shellwords.escape(@export_tmpfile.path)}
101+
export profile_dir_path=#{Shellwords.escape(@profiled_tmpdir)}
102+
composer install --no-dev
103+
EOF
104+
97105
@stdout, @stderr, @status = Open3.capture3("bash -c #{Shellwords.escape(cmd)}")
98106
end
99107
end
@@ -144,23 +152,29 @@
144152
expect(Dir.entries("#{@install_tmpdir}/etc/php/conf.d").any? {|f| f.include?("ext-mbstring.ini")}).to eq(true)
145153
end
146154

147-
it "writes a human-readable log to a given file descriptor" do
155+
it "writes a human-readable log (with the expected indentation) to a given file descriptor" do
148156
version_triple = /\(\d+\.\d+\.\d+(\+[^)]+)?\)/ # 1.2.3 or 1.2.3+build2
149157
bundled = /\(bundled with php\)/
158+
# the download progress indicator is written using ANSI cursors:
159+
# " Downloaded 0/8 [>---------------------------] 0%\e[1G\e[2K Downloaded 1/8 [===>------------------------] 12%\e[1G\e[2K Downloaded 2/8 [=======>--------------------] 25%\e[1G\e[2K Downloaded 4/8 [==============>-------------] 50%\e[1G\e[2K Downloaded 5/8 [=================>----------] 62%\e[1G\e[2K Downloaded 6/8 [=====================>------] 75%\e[1G\e[2K Downloaded 7/8 [========================>---] 87%\e[1G\e[2K Downloaded 8/8 [============================] 100%\e[1G\e[2K - php (8.4.6)\n - ext-sqlite3 (bundled with php)\n..."
160+
# we want to check that the download progress is actually printed, and then removed using ANSI codes
161+
# so we just match on the last progress report (since we're not always guaranteed to get one for every step depending on speed)
150162
expect(@humanlog_tmpfile.read).to match Regexp.new(<<~EOF)
151-
^- php #{version_triple.source}$
152-
^- ext-sqlite3 #{bundled.source}$
153-
^- ext-redis #{version_triple.source}$
154-
^- ext-mbstring #{bundled.source}$
155-
^- ext-ldap #{bundled.source}$
156-
^- ext-intl #{bundled.source}$
157-
^- ext-imap #{version_triple.source}$
158-
^- ext-gmp #{bundled.source}$
159-
^- blackfire #{version_triple.source}$
160-
^- ext-blackfire #{version_triple.source}$
161-
^- apache #{version_triple.source}$
162-
^- composer #{version_triple.source}$
163-
^- nginx #{version_triple.source}$
163+
Downloaded 8/8 \\[============================\\] 100%\
164+
\\e\\[1G\\e\\[2K\
165+
- php #{version_triple.source}
166+
- ext-sqlite3 #{bundled.source}
167+
- ext-redis #{version_triple.source}
168+
- ext-mbstring #{bundled.source}
169+
- ext-ldap #{bundled.source}
170+
- ext-intl #{bundled.source}
171+
- ext-imap #{version_triple.source}
172+
- ext-gmp #{bundled.source}
173+
- blackfire #{version_triple.source}
174+
- ext-blackfire #{version_triple.source}
175+
- apache #{version_triple.source}
176+
- composer #{version_triple.source}
177+
- nginx #{version_triple.source}
164178
EOF
165179

166180
end

0 commit comments

Comments
 (0)